-
Notifications
You must be signed in to change notification settings - Fork 120
[Just In Time Messages] Dismiss JITM #8001
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
You can test the changes from this Pull Request by:
|
|
@rachelmcr FYI this implements dismissal on the announcements. Adding you as an optional reviewer because it'll affect the product onboarding too. This change, to nil the announcement view model when the x is tapped, will make the announcements disappear. I'll animate it a bit later. See the implementation in |
pachlava
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for such detailed testing instructions! It all worked for me exactly as you describe, and I noticed nothing suspicious. I've tested this with iPhone 14 Sim, both successful dismissal and making it to fail (via breakpoint and via breaking the Internet connection).
Having the JITM dismissal fail resulted in JITM banner be shown right after relaunch, which I suppose is the correct behaviour.
| let action = JustInTimeMessageAction.dismissMessage(justInTimeMessage, | ||
| siteID: siteID, | ||
| completion: { result in | ||
| // We deliberately strongly capture self here: the owning reference to the VM may have been |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good addition of this explanatory comment 👍
|
Code looks great! |
Generated by 🚫 dangerJS |
Thanks for the heads up! The current products onboarding experience doesn't include a dismiss button (ref: pe5pgL-11B-p2#comment-1004) so for now the only way to dismiss it is by tapping the CTA/adding a product. So we're good in terms of dismissing announcements. 👍 We'll keep this in mind in case we need to add back that button, though! |
Closes: #7854
Description
We've added the ability to show a Just In Time Message at the top of the dashboard in #7853. Just In Time Messages can be dismissed; when they are, that should be communicated back to Jetpack via the REST API, and the dismissal is noted there.
After a message has been dismissed, the rules for whether it should be displayed will take that in to account. The message can have a number of dismissals defined, and a duration between those dismissals. By default, after being dismissed a message will be shown again six weeks later, and if it's dismissed a second time, it won't be shown again.
This PR implements the dismiss behaviour and analytics on our JITM.
Testing instructions
N.B. see pdfdoF-1uc-p2#comment-2581 for details of setting up your store to be eligible for the test JITM
The test JITM uses unrealistic dismissal rules to make testing easier. There are 1000 dismissals possible, and the dismissal only lasts 30 seconds. With this JITM it's not really possible to check that the JITM isn't shown again after the dismissal limit is reached, but that isn't the app's responsibility. We can test this once we make the JITM production-ready and set the dismissal limit to a sensible value.
With a US store that is eligible for the test JITM, on a debug/alpha build of the app
Dismissal request is sent
xDismissal is effective
Analytics
woocommerceios_jitm_dismiss_successandwoocommerceios_jitm_dismissedevents are tracked with the following properties:*_jitm_dismissedevent with propsjitm_group: woomobile_ipp,jitm_id: woomobile_ipp_barcode_users,source: my_store*_jitm_dismiss_successevent with propsjitm_group: woomobile_ipp,jitm_id: woomobile_ipp_barcode_users,source: my_storehttps://public-api.wordpress.com/rest/v1.1/jetpack-blogs/{siteid}/rest-api/, for POST requests only.*_jitm_dismiss_failureevent with propsjitm_group: woomobile_ipp,jitm_id: woomobile_ipp_barcode_users,source: my_storeand error propsScreenshots
dismiss-jitm.mp4
RELEASE-NOTES.txtif necessary.