Skip to content

Invoice Payload Filtering#3773

Closed
thefunkycat wants to merge 7 commits intopython-telegram-bot:masterfrom
thefunkycat:3752-invoice-payload-filtering
Closed

Invoice Payload Filtering#3773
thefunkycat wants to merge 7 commits intopython-telegram-bot:masterfrom
thefunkycat:3752-invoice-payload-filtering

Conversation

@thefunkycat
Copy link
Contributor

@thefunkycat thefunkycat commented Jun 23, 2023

Closes #3752

I've made two commits:

  • one updates the successful payment message filter to also accept a list of strings to be able to filter
  • the other adds a pattern argument to the precheckoutqueryhandler and checks updates based on that.

Looking forward to a review, will gladly make any changes required.

@thefunkycat
Copy link
Contributor Author

pytest seems to be angry @harshil21

@harshil21
Copy link
Member

pytest seems to be angry @harshil21

Sorry, #3769 is fixing that

@thefunkycat
Copy link
Contributor Author

trusted Github's automatic conflict resolution for the merge conflict, never again. fixed manually now

@harshil21 harshil21 added the 📋 pending-review work status: pending-review label Jun 28, 2023
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.

Hi, thanks for the PR and sorry for the late review. It's mostly well done except you also need to write tests for this. See our testing guide and Checklist for PRs to know what to complete. Please also run the doc build locally to make sure it passes.

:class:`telegram.ext.ConversationHandler`.
pattern (:obj:`str` | :func:`re.Pattern <re.compile>` | :obj:`callable` | :obj:`type`, \
optional):
Pattern to test :attr:`telegram.Update.pre_checkout_query.invoice_payload` against. If
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Pattern to test :attr:`telegram.Update.pre_checkout_query.invoice_payload` against. If
Pattern to test :attr:`telegram.Update.pre_checkout_query.invoice_payload <telegram.PreCheckoutQuery.invoice_payload>` against. If

don't commit these directly from Github, since it will exceed the line limit. Do it in the IDE so you can correct it

optional):
Pattern to test :attr:`telegram.Update.pre_checkout_query.invoice_payload` against. If
a string or a regex pattern is passed, :func:`re.match` is used on
:attr:`telegram.Update.pre_checkout_query.invoice_payload` to determine if an update
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
:attr:`telegram.Update.pre_checkout_query.invoice_payload` to determine if an update
:attr:`telegram.Update.pre_checkout_query.invoice_payload <telegram.PreCheckoutQuery.invoice_payload>` to determine if an update

callback (:term:`coroutine function`): The callback function for this handler.
block (:obj:`bool`): Determines whether the callback will run in a blocking way..
pattern (:func:`re.Pattern <re.compile>` | :obj:`callable` | :obj:`type`): Optional.
Regex pattern to test :attr:`telegram.Update.pre_checkout_query.invoice_payload`
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Regex pattern to test :attr:`telegram.Update.pre_checkout_query.invoice_payload`
Regex pattern to test :attr:`telegram.Update.pre_checkout_query.invoice_payload <telegram.PreCheckoutQuery.invoice_payload>`

def __init__(
self,
callback: HandlerCallback[Update, CCT, RT],
pattern: Optional[Union[str, Pattern[str]]] = None,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
pattern: Optional[Union[str, Pattern[str]]] = None,
pattern: Optional[Union[str, Pattern[str], Callable[[str], Optional[bool]]] = None,

if isinstance(pattern, str):
pattern = re.compile(pattern)

self.pattern: Optional[Union[str, Pattern[str]]] = pattern
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
self.pattern: Optional[Union[str, Pattern[str]]] = pattern
self.pattern: Optional[Union[Pattern[str], Callable[[str], Optional[bool]]] = pattern

a string or a regex pattern is passed, :func:`re.match` is used on
:attr:`telegram.Update.pre_checkout_query.invoice_payload` to determine if an update
should be handled by this handler.

Copy link
Member

Choose a reason for hiding this comment

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

please add a .. versionadded:: note here. Search for it in the codebase if you want to know how to add them.

block (:obj:`bool`): Determines whether the callback will run in a blocking way..
pattern (:func:`re.Pattern <re.compile>` | :obj:`callable` | :obj:`type`): Optional.
Regex pattern to test :attr:`telegram.Update.pre_checkout_query.invoice_payload`
against.
Copy link
Member

Choose a reason for hiding this comment

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

same versionadded note here as well


class _SuccessfulPayment(MessageFilter):
__slots__ = ()
class SuccessfulPayment(MessageFilter):
Copy link
Member

Choose a reason for hiding this comment

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

please write a docstring for this class. Make sure to include the versionadded

class SuccessfulPayment(MessageFilter):
__slots__ = ("strings",)

def __init__(self, strings: Optional[Union[List[str], Tuple[str, ...]]] = None):
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
def __init__(self, strings: Optional[Union[List[str], Tuple[str, ...]]] = None):
def __init__(self, invoice_payloads: Optional[Union[List[str], Tuple[str, ...]]] = None):

explicit naming is better. Change it below also ofc

return bool(message.successful_payment)
if self.strings is None:
return bool(message.successful_payment)
return message.successful_payment in self.strings if message.successful_payment else False
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return message.successful_payment in self.strings if message.successful_payment else False
return message.successful_payment.invoice_payload in self.strings if message.successful_payment else False

@harshil21 harshil21 added 📋 pending-reply work status: pending-reply and removed 📋 pending-review work status: pending-review labels Jul 2, 2023
@Bibo-Joshi
Copy link
Member

@thefunkycat are you still interested in working on this pr?

@Bibo-Joshi
Copy link
Member

Closing due to inactivity

@Bibo-Joshi Bibo-Joshi closed this Aug 2, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Aug 10, 2023
@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 📋 pending-reply work status: pending-reply

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Filter support for invoice_payload in PreCheckoutQueryHandler and successful_payment message

3 participants