Add invoice_payload filtering#4005
Conversation
Add class `filters.SuccessfulPayment` with support for `invoice_payload` filtering.
|
Thanks for the PR! I have two general comments that I'd like you to address before I review in details:
|
|
Thanks for the feedback.
Aha got it, I did not understand why it was there
Ok. I thought it was needed because of this #3773 (comment)
On it. |
|
I created some tests. I see some checks are still failing. Doc build is failing, although it built fine locally. |
Fix an indentation error that was preventing sphinx-build from exiting with code 0.
invoice_payload filtering in (new) filters.SuccessfulPayment and (existing) PreCheckoutQueryHandlerinvoice_payload filtering
|
Couldn't figure out why pytest (3.8, macos-latest) is still failing. But I think its ready for review. @Bibo-Joshi |
Bibo-Joshi
left a comment
There was a problem hiding this comment.
Thanks for the nice PR! I left a few smaller comments below :) the tests on macOS recently tend to fail due to flakyness in time-based jobs …
| def __init__( | ||
| self, | ||
| callback: HandlerCallback[Update, CCT, RT], | ||
| pattern: Optional[Union[str, Pattern[str]]] = None, |
There was a problem hiding this comment.
please move the argument to the end of the list. Otherwise this would be a breaking change as PCQH(callback, False) would no longer work as expected. Please also move the docstring entries for argument and attribute above.
| if isinstance(pattern, str): | ||
| pattern = re.compile(pattern) | ||
|
|
||
| self.pattern: Optional[Union[str, Pattern[str]]] = pattern |
There was a problem hiding this comment.
| if isinstance(pattern, str): | |
| pattern = re.compile(pattern) | |
| self.pattern: Optional[Union[str, Pattern[str]]] = pattern | |
| self.pattern: Optional[Pattern[str]] = re.compile(pattern) if pattern is not None else None |
Just a bit shorter and also gives the attribute a more narrow type.
This way we can also use self.pattern.match(…) in check_update below :)
| ) | ||
|
|
||
| def test_with_pattern(self, pre_checkout_query): | ||
| handler = PreCheckoutQueryHandler(self.callback, pattern=".*voice.*") |
There was a problem hiding this comment.
please also test passing a compiled pattern :)
|
Applied changes, ready for more |
Bibo-Joshi
left a comment
There was a problem hiding this comment.
Thanks for the updates and sorry for the late review. LGTM now :) Let me check if one of the other team members can also have a second look.
| callback: HandlerCallback[Update, CCT, RT], | ||
| pattern: Optional[Union[str, Pattern[str]]] = None, | ||
| block: DVType[bool] = DEFAULT_TRUE, | ||
| pattern: Optional[Pattern[str]] = None, |
There was a problem hiding this comment.
| pattern: Optional[Pattern[str]] = None, | |
| pattern: Optional[Union[str, Pattern[str]]] = None, |
accepting a string as input is okay, my point in #4005 (comment) was just that we can convert strings to patterns on init already :) sorry if that was unclear.
|
Thanks very much for the nice contribution :) |
|
Ha! No really, thank you. I learnt something new. |
Closes #3752
.. versionadded:: NEXT.VERSION,.. versionchanged:: NEXT.VERSIONor.. deprecated:: NEXT.VERSIONto the docstrings for user facing changes (for methods/class descriptions, arguments and attributes)CSI standard <https://standards.mousepawmedia.com/en/stable/csi.html>__Added myself alphabetically toAUTHORS.rst(optional)__all__sStability Policy <https://docs.python-telegram-bot.org/stability_policy.html>_ in case of deprecations or changes to documented behaviorwhy not add
collect_additional_contextinPreCheckoutQueryHandlerto store the match since we're adding apatternargument. i can do it if you approve.