-
Notifications
You must be signed in to change notification settings - Fork 223
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
Preserve the box rocket_activation_notice with update #6120
Conversation
@engahmeds3ed Thanks for the PR. Please find exploratory test notes below: |
Good catch, based on the current code yes this is expected after checking this old code. |
@Tabrisrp do u remember why do we clear all rocket boxes with update? |
I do not, it's something that was there before my time. It probably made sense at some point, but not anymore. |
Thanks @Tabrisrp Summary: the used function we remove all dismissed notices for all users and only fire dismissing the following boxes with every update:
and the second box is the activation notice. Even if the user didn't dismiss both boxes, both will be dismissed based on the current code. So based on the above discussion we have three solutions here:
I believe we need the @wp-media/productrocket decision here. |
@engahmeds3ed @Tabrisrp Do you see any reason why the boxes might be cleared? It doesn't seem to be making sense currently |
Really I don't know. |
@engahmeds3ed Let's go with 2 then. |
Scenario 2 here #6120 (comment) is fixed, while 1 is still there |
Working as expected now both 1,2 are fine |
Description
Preserve the box rocket_activation_notice with update.
Fixes #5889
Type of change
Please delete options that are not relevant.
Is the solution different from the one proposed during the grooming?
No
Checklists
Generic development checklist
Test summary
If not, detail what you could not test.
Please describe any additional tests you performed.