woocommerce-ios
woocommerce-ios copied to clipboard
Remove orders/totals request from notifications manager
Follow-up to: #7735
Description
This PR updates PushNotificationsManager to prevent triggering data reload notifications on each "app badge clear" action. Without it order list will trigger orders/totals requests on each viewWillAppear.
Same change was already implemented in https://github.com/woocommerce/woocommerce-ios/pull/7459, but Application Icon badge handling accidentally removed with it, so this PR now decouples one from another.
Note: Application icon badge != Orders tab badge. They have different logic and values (app icon can only be 1 or nil)
Testing instructions
- Allow notifications to display a badge on app icon
- Place an order to trigger app icon badge update (always "1" for any number of orders)
- Go to the orders tab and confirm that application badge is cleared, but
orders/totalsAPI request is not triggered.
- [x] I have considered if this change warrants user-facing release notes and have added them to
RELEASE-NOTES.txtif necessary.
You can test the changes from this Pull Request by:
- Clicking here or scanning the QR code below to access App Center
- Then installing the build number
pr7740-28627c1on your iPhone
Hey @ealeksandrov, thanks for tackling the https://github.com/woocommerce/woocommerce-ios/pull/7735 fix and adjusting the solution to avoid the problem again! Much appreciated, and sorry for introducing that bug 😅. Just one question to acquire some additional context: I noticed this PR hanging while I was AFK. Is the code here OK for review, or it's waiting for something?
@ThomazFB no worries, I'm pretty sure it's not a final performance improvement that we'll find! PR is ready for review, it wasn't urgent to prioritize earlier and you should have the best context awareness around it 🙂