Closed kelvinballoo closed 1 month ago
IB β
Hey @nfmohit, following @hussain-t's question on Slack I've realised a couple of additional related fixes should be addressed here, so I've updated the AC and IB accordingly.
Please can you give the IB another review?
Thank you @techanvil! Updated IB π β
Hi @hussain-t, the PR looks good, but the QAB could be a bit confusing as the steps to reproduce are really quite specific to the first AC point; the aspects relating to archiving audiences are irrelevant to the additional AC points (and indeed, logging in as a second user has no bearing on the first additional sub point).
Please can you amend the QAB to help clarify this?
Thanks @hussain-t, I see your thumbs up on the above comment. I've merged the PR and assigned this to you in QA to make the update to the QAB.
Thanks, @techanvil. I've updated the QAB. Please let me know if it looks good.
Thanks @hussain-t! That LGTM, cheers.
For the primary admin, having dismissed the Setup CTA on the dashboard, the Enable groups CTA is still present in the Settings section.
For a secondary admin, if the user lands on the Settings screen before having visited the dashboard to allow their audiences to be auto-selected, the Enable groups CTA is not present, and the Display visitor groups in dashboard toggle appears instead
Bug Description
This was raised while testing https://github.com/google/site-kit-wp/issues/8577
Steps to reproduce
Screenshots
https://github.com/user-attachments/assets/3b6b7d84-2c20-4af4-bcb9-9fd5975c610f
Additional Context
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
Implementation Brief
SettingsCardVisitorGroups
:audienceSegmentationSetupCompletedBy
and returnnull
while it isundefined
.SetupCTA
is rendered whenconfiguredAudiences
is truthy andaudienceSegmentationSetupCompletedBy
isnull
, rather than testingconfiguredAudiences.length
. I.e. the condition should be! configuredAudiences && audienceSegmentationSetupCompletedBy === null
. Render theSetupSuccess
andSwitch
fragment when the inverse is true. https://github.com/google/site-kit-wp/blob/40f5363b0a49a3c24126d76b44e20cd1c22e919f/assets/js/modules/analytics-4/components/audience-segmentation/settings/SettingsCardVisitorGroups/index.js#L74-L75SettingsCardVisitorGroups/SetupCTA
, remove the conditional rendering ofnull
when theAUDIENCE_SEGMENTATION_SETUP_CTA_NOTIFICATION
item is dismissed: https://github.com/google/site-kit-wp/blob/c8f046917554e9883108545cb864ea20647ab9ba/assets/js/modules/analytics-4/components/audience-segmentation/settings/SettingsCardVisitorGroups/SetupCTA.js#L46-L54Test Coverage
QA Brief
Changelog entry