Closed fariassdev closed 1 year ago
Hi Fernando,
Thanks for a great PR!
Apple's decision to effectively make data
optional is inherently a breaking change, so I think it's reasonable to allow breaking changes to this library as well. The solution you've come up with looks good to me.
Could you add another type guard to check if the payload is a summary notification (i.e. isDecodedNotificationSummaryPayload
) and format the files (there's a prettier config in package.json)?
Hey August!
Thanks for the great feedback! I've made the changes as you suggested.
I totally agree that Apple's decision to make data
optional is a breaking change. So, updating the types to match the state of the Apple API is definitely the way to go.
Take a look at the PR whenever you get a chance. If everything looks good to you, feel free to merge it.
Thanks again for your input!
Released in v0.10.0. Thank you for your contribution! 🎊
This pull request addresses issue #24.
Problem
During testing, we encountered an issue where the![Screenshot 2023-08-01 at 11 45 32](https://github.com/agisboye/app-store-server-api/assets/19715763/a405deba-e28f-44c7-a63a-45acdc42338e)
DecodedNotificationPayload
type cannot be used effectively to generate notification mocks for testing purposes. This limitation hinders our ability to create accurate test scenarios for handling Apple notifications.To solve that, we have to do some typing trick like this:
Proposed Solution
I propose a solution to improve the
DecodedNotificationPayload
type, enabling seamless utilization of the newsummary
field and facilitating the convenient generation of notification mocks for testing purposes.Changes Made
Updated Typed Payload: I have improved the
DecodedNotificationPayload
type to better represent the Apple notification data payload according to the official documentation.Type Guard Function: I have introduced a type guard function called
isDecodedNotificationDataPayload
. This function can be used to mitigate potential breaking changes and allow users to safely access thedata
(isDecodedNotificationDataPayload
) andsummary
(!isDecodedNotificationDataPayload
) properties.Additional Notes
I understand that there may be concerns about the placement of the type guard function in
Models.ts
. If you believe that a different file would be more suitable, please feel free to move it accordingly or revert the commit if it is deemed unnecessary.Thank you for your time, we find the library very useful ❤️. Looking forward to your review and feedback on this pull request.