Skip to content

Conversation

Bibo-Joshi
Copy link
Member

adds nome convenience functionality around chat invite links. nothing urgent.

Checklist for PRs

  • Added .. versionadded:: version, .. versionchanged:: version or .. deprecated:: version to the docstrings for user facing changes (for methods/class descriptions, arguments and attributes)
  • Created new or adapted existing unit tests
  • Added myself alphabetically to AUTHORS.rst (optional)
  • Added new classes & modules to the docs

@Bibo-Joshi Bibo-Joshi added this to the v14 milestone Nov 20, 2021
@Bibo-Joshi Bibo-Joshi requested a review from Poolitzer November 20, 2021 10:54
@Bibo-Joshi Bibo-Joshi added the 📋 pending-review work status: pending-review label Nov 21, 2021
@Bibo-Joshi Bibo-Joshi force-pushed the invite-link-convenience branch from d4cf543 to 3164c0b Compare November 21, 2021 12:28
Copy link
Member

@harshil21 harshil21 left a comment

Choose a reason for hiding this comment

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

nice feature, but is there a reason why we don't use the other parameters of ChatInviteLink here too? Similar to how we do in send/edit_location

@Bibo-Joshi
Copy link
Member Author

nice feature, but is there a reason why we don't use the other parameters of ChatInviteLink here too? Similar to how we do in send/edit_location

TBH, I hadn't really thought about it, but interesting idea! you'd do something like e.g.

if isinstance(invite_link, ChatInviteLink):
    expire_date = expire_date or  invite_link.expire_date

? That could work. If we want to allow users to e.g. delete the expire_data, we'd have to maybe do something like

def edit_chat_link(…, expire_date = DEFAULT_NONE):
    if isinstance(invite_link, ChatInviteLink):
         expire_date = expire_date or invite_link.expire_date
    expire_data = DefaultValue.get_value(expire_date)

such that edit_chat_link(chat_link_instance, expire_date=None) allows to explicitly set expire_date to None.
And we'd have to mind that one could want to override member_limit with creates_join_request and vice versa.

At first glance, I'm not sure if the added convenience is worth the added complexity of the method. What do you think?

@harshil21
Copy link
Member

ugh, in that case probably best to leave it like this. Not to mention documentation will also increase in complexity.

Copy link
Member

@Poolitzer Poolitzer left a comment

Choose a reason for hiding this comment

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

Changes look good.

@Bibo-Joshi Bibo-Joshi merged commit c9be970 into v14 Nov 29, 2021
@Bibo-Joshi Bibo-Joshi deleted the invite-link-convenience branch November 29, 2021 19:32
@github-actions github-actions bot locked and limited conversation to collaborators Dec 7, 2021
@harshil21 harshil21 removed the 📋 pending-review work status: pending-review label Apr 18, 2022
@Bibo-Joshi Bibo-Joshi added 🔌 enhancement pr description: enhancement and removed enhancement labels Nov 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🔌 enhancement pr description: enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants