Clear the "temporarily hidden" state for an audience when it's removed from the audience selection
Feature Description
If an audience has zero data and has been temporarily hidden, and the audience is subsequently removed from the audience selection, the "temporarily hidden" state should be cleared.
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
- Given an audience has zero data and has been temporarily hidden:
- When the audience is removed from the audience selection via the Selection Panel and the selection is saved via clicking on the Save selection CTA, the "temporarily hidden" state of the audience should be cleared.
- This in turn means next time the audience is added to the selection its tile will be shown as usual.
Implementation Brief
Updates to Dismissed Item Infra
- [ ] Update
includes/Core/Dismissals/REST_Dismissals_Controller.php, create a new rest endpoint:- [ ] The endpoint should be
'core/user/data/dismiss-item' - [ ] The method should be
WP_REST_Server::DELETABLE - [ ] In the callback function, use the existing
removemethod onDismissed_Itemsto remove the dismissed item by slug. Use slug parameter validation and return the updated dismissed items (use the existing create route for reference).
- [ ] The endpoint should be
- [ ] Update
assets/js/googlesitekit/datastore/user/dismissed-items.js:- [ ] Create a new action called
removeDismissedItem, which takes a slug and resolves using a new fetch store, which usesAPI.deletewithin it'scontrollerCallbackto delete the requested dismissed item key.
- [ ] Create a new action called
Audience Selection Updates
- [ ] Update
assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceSelectionPanel/Footer.js, within thesaveSettingscallback- If there in not an error:
- [ ] Get all dismissed items with
getDismissedItemsselector. - [ ] Use
.filterto find any dismissed items where the slug starts withaudience-tile- - [ ] For each of these (if any are found), dispatch the new
removeDismissedItemaction.
Test Coverage
- Add new test coverage for the new remove functionality in both
tests/phpunit/integration/Core/Dismissals/REST_Dismissals_ControllerTest.phpandassets/js/googlesitekit/datastore/user/dismissed-items.test.js
QA Brief
Changelog entry
Hi @benbowler, thanks for drafting this IB. A couple of points:
- [ ] Create a new action called
removeDismissedItem, which takes a slug and resolves using a new fetch store, which usesAPI.deletewithin it'scontrollerCallbackto delete the requested dismissed item key.
We don't actually have an API.delete() function at present. We could add one, but the only current use of the HTTP DELETE method is implemented using API.set() as follows:
https://github.com/google/site-kit-wp/blob/b67e9a1ea124dc8d233def811d9adfb99c995544/assets/js/googlesitekit/modules/datastore/sharing-settings.js#L90-L98
- [ ] For each of these (if any are found), dispatch the new
removeDismissedItemaction.
This point makes me think that, seeing as we're adding a new REST endpoint for the deletion, we might as well allow that endpoint to handle multiple items so we can batch these into a single request. WDYT?
Thanks @techanvil, I've updated this to use API.set and to accept and array of slugs and delete them all to reduce the number of REST requests.
Thanks @benbowler! IB LGTM. I've bumped the estimate up a level to allow for extra time modelling the scenarios in testing, and because we've generally been underestimating Audience Segmentation issues.
IB :white_check_mark:
QA Update ⚠️
The tests on desktop are fine. I did one round of testing on mobile and there are 2 issues to flag:
ITEM 1:
After I hide the temporarily hide for one tile, all the tiles vanish or rather are collapsed I would say.
When I click on the group that is there, it will appear.
Question would then be, once I hide one of the tiles, should we auto-reselect the existing audience rather than having to click on it? ⚠️
https://github.com/user-attachments/assets/174fcedf-838d-4bfe-8650-267d4c97586d
ITEM 2:
I had 3 tiles setup and somehow hit an error on mobile when I clicked on one of the tabs. ❌
https://github.com/user-attachments/assets/4c9f0bf2-7cd8-4216-bf78-dc20ce0ccf6e
Other than that, I tested the happy and unhappy scenarios on desktop and it's working as expected. ✅
Happy path: The previously hidden audiences which were removed and re-added reappeared in the list of Audience Tiles.
https://github.com/user-attachments/assets/1e1aa34f-57b4-4df5-b5ce-e3e8e736d29a
Unhappy path: The error will be showing up when the tweak extension is activated and once disabled, it works as expected too.
Hey @kelvinballoo! Thanks for flagging these defects. They are both valid concerns, but are outside the scope of this issue. Please can you raise issues for them?
QA Update ✅
Thanks for checking @techanvil . New issue raised under : https://github.com/google/site-kit-wp/issues/9168
As for this issue, it's good to go to Approval. I tested the happy and unhappy scenarios on desktop and it's working as expected.
Happy path: The previously hidden audiences which were removed and re-added reappeared in the list of Audience Tiles. ✅
https://github.com/user-attachments/assets/1e1aa34f-57b4-4df5-b5ce-e3e8e736d29a
Unhappy path: The error will be showing up when the tweak extension is activated and once disabled, it works as expected too. ✅