Skip to content
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

The isAudienceSegmentationWidgetHidden flag should retain its value when changing the connected Analytics property #9432

Closed
3 tasks done
kelvinballoo opened this issue Sep 27, 2024 · 4 comments
Labels
Module: Analytics Google Analytics module related issues P0 High priority Team M Issues for Squad 2 Type: Bug Something isn't working

Comments

@kelvinballoo
Copy link
Collaborator

kelvinballoo commented Sep 27, 2024

Bug Description

For iwpc986_googlesitekit_audience_settings, whenever we disconnect or change analytics properly, the values would not only be cleared but the entired rows will be deleted.

Expectation: The rows should not be deleted; the configuredAudiences and didSetAudiences properties should have their values reset to the defaults, while the isAudienceSegmentationWidgetHidden property should retain its value.

Screenshot 2024-09-27 at 01 36 56

Steps to reproduce

  1. Spin up a site where you have DB editor access
  2. Set up SK with an Analytics property and also Audience segmentation tiles (enable feature flag if required)
  3. Make sure you configure audience tiles.
  4. Go to DB editor, under WP User meta table, look for 'googlesitekit_audience_settings' and take note of the row ID for it.
  5. Disconnect the Analytics property
  6. Refresh the DB editor and look for the previously noted row ID. You will notice that the row doesn't exist anymore.

Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

  • When the connected Analytics property is changed by selecting a new property via the Analytics settings:

See #8180 (comment) (Item 3) for additional context.

Implementation Brief

In includes/Core/User/Audience_Settings.php:

  • Update the Audience_Settings::merge() method to handle null values for configuredAudiences.

In includes/Modules/Analytics_4/Reset_Audiences.php:

  • Update the Reset_Audiences::reset_audience_data() method to use the Audience_Settings::merge() method to reset the configuredAudiences and didSetAudiences properties with null and false values, respectively.
  • Remove calling the Audience_Settings::delete() method to prevent deleting the entire row.

Test Coverage

  • Add test coverage for the Audience_Settings::merge() method.
  • Add test coverage for the Reset_Audiences::reset_audience_data() method.

QA Brief

  • Follow the steps to reproduce.
  • Verify that the row still exists after the property change, where the isAudienceSegmentationWidgetHidden property still retains its value, but the others have reset to their default values (null and false respectively).
  • Ensure that the ACs are met (add QA:Eng if necessary).

Changelog entry

  • Prevent audience settings from being deleted when changing or disconnecting the Analytics property. Reset configuredAudiences and didSetAudiences to default values while keeping isAudienceSegmentationWidgetHidden unchanged.
@techanvil techanvil changed the title WP usermeta row deleted for googlesitekit_audience_settings during Analytics reset The isAudienceSegmentationWidgetHidden flag should retain its value when changing the connected Analytics property Sep 30, 2024
@techanvil techanvil removed their assignment Sep 30, 2024
@techanvil techanvil added Type: Bug Something isn't working P1 Medium priority Module: Analytics Google Analytics module related issues labels Sep 30, 2024
@hussain-t hussain-t self-assigned this Oct 1, 2024
@ivonac4 ivonac4 added the Team M Issues for Squad 2 label Oct 4, 2024
@hussain-t hussain-t removed their assignment Oct 7, 2024
@nfmohit nfmohit self-assigned this Oct 10, 2024
@eclarke1 eclarke1 added P0 High priority and removed P1 Medium priority labels Oct 10, 2024
@nfmohit
Copy link
Collaborator

nfmohit commented Oct 10, 2024

IB ✅

@nfmohit nfmohit assigned nfmohit and unassigned nfmohit Oct 10, 2024
@nfmohit nfmohit removed their assignment Oct 14, 2024
@hussain-t hussain-t self-assigned this Oct 15, 2024
hussain-t added a commit that referenced this issue Oct 15, 2024
Retain choice to display visitor groups on reset
@hussain-t hussain-t removed their assignment Oct 15, 2024
@kelvinballoo kelvinballoo self-assigned this Oct 15, 2024
@kelvinballoo
Copy link
Collaborator Author

QA Update ✅

Verified the following:

  • Followed the steps to reproduce and verified that the row still exists after the property change/property disconnected.

    Before: ![Image](https://github.com/user-attachments/assets/3540ea8b-f002-4643-be72-cb4c158a46d2)

    After: Row 35 is NOT deleted
    Image

  • Verified that the isAudienceSegmentationWidgetHidden property still retains its value, but the others have reset to their default values (null and false respectively) when disconnecting or changing analytics property.

    googlesitekit_audience_settings before disconnecting/changing property: - configuredAudiences and didSetAudiences is reset - isAudienceSegmentationWidgetHidden is maintained

    a:3:{s:19:"configuredAudiences";a:2:{i:0;s:41:"properties/463402384/audiences/9830284984";i:1;s:41:"properties/463402384/audiences/9830265275";}s:34:"isAudienceSegmentationWidgetHidden";b:0;s:15:"didSetAudiences";b:1;}

    googlesitekit_audience_settings after disconnecting/changing property:
    a:3:{s:19:"configuredAudiences";N;s:34:"isAudienceSegmentationWidgetHidden";b:0;s:15:"didSetAudiences";b:0;}

  • The state of the "Display visitor groups in dashboard" toggle is not reset on the FE after enabling a different analytics property.

    ![Image](https://github.com/user-attachments/assets/110a9819-a57f-4ec0-bd46-43d85b43f52c)

@kelvinballoo kelvinballoo removed their assignment Oct 15, 2024
@tofumatt
Copy link
Collaborator

I wasn't sure the ACs here match up with the QA instructions. I think I was misunderstanding, as the ACs state:

When the connected Analytics property is changed by selecting a new property via the Analytics settings: The audience selection should be cleared for all users.

But the QA done by Asvin mentions a database field not being reset/deleted. I think this isn't exactly related to the QA Brief though, and the values returned by the selectors is the main result that matters here.

So moving to Approval based on my understanding.

@techanvil
Copy link
Collaborator

Thanks @tofumatt - to clarify, the AC point stating "The audience selection should be cleared for all users." relates to the underlying configuredAudiences setting which resides in the googlesitekit_audience_settings usermeta entry.

We do want to retain isAudienceSegmentationWidgetHidden, so the expected behaviour is for the googlesitekit_audience_settings usermeta entry to be retained but with configuredAudiences nulled (and didSetAudiences reset to its default of false), which is what Kelvin has verified for QA, so it's all looking good here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Module: Analytics Google Analytics module related issues P0 High priority Team M Issues for Squad 2 Type: Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

7 participants