Skip to content

Update to resolve Issue with outdated _convert_entities function in copy_message method #2240

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.

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

Merged
merged 2 commits into from
Apr 27, 2024

Conversation

YouKnow-sys
Copy link
Contributor

Description

I identified an issue with the outdated function _convert_entities used in both the sync and async versions of the copy_message method. This function was not functioning correctly, resulting in invalid entities. While all other methods were using a more updated method for converting entities, this particular method was still relying on _convert_entities.

To address this issue, I have removed _convert_entities and replaced it with more better json.dumps(types.MessageEntity.to_list_of_dicts(...)). This change ensures that the entities are properly converted and are valid

Python version:
3.12

OS:
windows 10

Checklist:

  • I added/edited example on new feature/change (if exists)
  • My changes won't break backward compatibility
  • I made changes both for sync and async

…ssageEntity.to_list_of_dicts(...))` method for caption entities
@YouKnow-sys YouKnow-sys changed the title Removed the _convert_entites function and used `json.dumps(types.Me… Removed the _convert_entites function usage from copy_message Apr 24, 2024
@YouKnow-sys YouKnow-sys changed the title Removed the _convert_entites function usage from copy_message Update to resolve Issue with outdated _convert_entities function in copy_message method Apr 24, 2024
@Badiboy
Copy link
Collaborator

Badiboy commented Apr 25, 2024

Looks reasonable, thank you.

@Badiboy
Copy link
Collaborator

Badiboy commented Apr 25, 2024

image

Could you please enchance MessageEntity.to_list_of_dicts with _convert_entities checks?

  1. None or empty => None
  2. If not serializable => entities
  3. Else - make list.

@YouKnow-sys
Copy link
Contributor Author

image

Could you please enchance MessageEntity.to_list_of_dicts with _convert_entities checks?

  1. None or empty => None
  2. If not serializable => entities
  3. Else - make list.

Ok so I updated the MessageEntity.to_list_of_dicts with a few changes, instead of checking if the object is instance of JsonSerializable I check if they are instance of MessageEntity, because this is more accurate.
although checking the instance for just the first element isn't that accurate, user can still pass other types in the list, but I didn't want to sacrifice more performance to check every element...

@Badiboy
Copy link
Collaborator

Badiboy commented Apr 27, 2024

LGTM. Thank you.

@Badiboy Badiboy merged commit 9ff09f2 into eternnoir:master Apr 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants