Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Added fields serializer for LinkPreviewOptions #1483

Closed
wants to merge 9 commits into from
Closed

Added fields serializer for LinkPreviewOptions #1483

wants to merge 9 commits into from

Conversation

DanZ-ix
Copy link
Contributor

@DanZ-ix DanZ-ix commented May 8, 2024

Description

Added custom fields json serializer for LinkPreviewOptions, so model_dump_json() works with messages with links

Fixes #1450

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

[x] Send message to bot with link preview
Try to call model_dump_json method on it
Message serializes

Test Configuration:

  • Operating System: Windows 10
  • Python version: 3.10

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • New and existing unit tests pass locally with my changes

@github-actions github-actions bot added the 3.x Issue or PR for stable 3.x version label May 8, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented May 8, 2024

✔️ Changelog found.

Thank you for adding a description of the changes

@JrooTJunior JrooTJunior mentioned this pull request May 8, 2024
Open
2 tasks
JrooTJunior
JrooTJunior requested changes May 8, 2024
View reviewed changes
Copy link
Member

@JrooTJunior JrooTJunior left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fix is completely incorrect.

The specified fields are of type bool, but for some unknown reason you are converting it to strings like Default('prefer_small_media') therefore, the serialization result cannot be properly deserialized.

I described the workaround in the issue #1450 (comment).

But I need to research it more to find a better solution.

aiogram/types/link_preview_options.py Outdated Show resolved Hide resolved
aiogram/types/link_preview_options.py Outdated Show resolved Hide resolved
@DanZ-ix
Copy link
Contributor Author

DanZ-ix commented May 9, 2024

Ive wrote my thoughts about this problem in the issue topic, now the Message object can be serialized and deserialized properly

@DanZ-ix DanZ-ix requested a review from JrooTJunior May 9, 2024 17:53
@JrooTJunior
Copy link
Member

I'm sorry, but you keep making things worse and worse, so stop it.

As I wrote earlier, please wait, I need to analyze the situation and find a system solution, because the problem described is actually not only related to this object.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JrooTJunior JrooTJunior Awaiting requested review from JrooTJunior

Assignees
No one assigned
Labels
3.x Issue or PR for stable 3.x version
Projects
None yet
Milestone
No milestone
2 participants
@DanZ-ix @JrooTJunior