site-kit-wp icon indicating copy to clipboard operation
site-kit-wp copied to clipboard

Ensure that audiences are listed in the correct order.

Open techanvil opened this issue 1 year ago • 2 comments

Feature Description

As discussed on Figma, we should ensure that audiences are displayed in the correct order, both on the Selection Panel and in the list of Audience Tiles.


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

Acceptance criteria

  • Within the Selection Panel, audiences should be listed in the following order:
    • Primary order: User defined audiences, Site Kit-created audiences, default audiences.
    • Secondary order: The order the audiences are listed in the Analytics UI/API.
    • Note that this ordering is applicable to the lists of audiences as they sit within the "current selection" and "available groups" groups. In other words this top-level grouping is effectively the true primary order when there is a current selection.
  • The list of Audience Tiles should be ordered by the same criteria, i.e. the Audience Tiles order should match that of the "current selection" group in the Selection Panel.

Implementation Brief

  • [ ] Add a new constant to assets/js/modules/analytics-4/constants.js to define the order weights of each audience type:
export const AUDIENCE_TYPE_ORDER_WEIGHTS = {
	USER_AUDIENCE: 0,
	SITE_KIT_AUDIENCE: 1,
	DEFAULT_AUDIENCE: 2,
};
  • [ ] Update the getAudiences selector to sort the audience provided by the state in the following way:
return state.audiences.sort( ( a, b ) => {
	const aWeight = AUDIENCE_TYPE_ORDER_WEIGHTS[ a.audienceType ] || 0;
	const bWeight = AUDIENCE_TYPE_ORDER_WEIGHTS[ b.audienceType ] || 0;

	return aWeight - bWeight;
} );

TODO: complete this IB when #8157 is implemented, and address the comments below.

Test Coverage

  • Add tests to the getAudiences selector to confirm it returns audiences in the expected order.

QA Brief

Changelog entry

techanvil avatar Apr 10 '24 16:04 techanvil

Hey @benbowler, thanks for drafting this IB. A few points:

  • As per the dependency #8487, getAudiences() will be replaced with getAvailableAudiences() by the time this issue is implemented, so it should accordingly refer to getAvailableAudiences().
  • Even still, I'm not sure the proposed change will cover the sorting in all cases. We need to ensure the selected list of audiences is sorted (both in the Selection Panel and the list in AudienceTiles), as well as the remaining available audiences. So it might be the case that we need to apply the sort in getConfiguredAudiences() as well. To be honest, though, it's not clear if the Selection Panel will need any additional modification to ensure the audiences are sorted properly considering how the audiences are grouped within the panel. It might actually be worth holding off on drafting this IB until the dependency #8157 is implemented.
  • Lastly, it's worth mentioning we do try to avoid explicit code blocks in IBs, I think this one could be written to avoid the code blocks.

techanvil avatar Apr 16 '24 12:04 techanvil

As this IB is blocked, and I am away for a couple of weeks, I will un-assign myself but happy to pick it up again if it's not beed addressed when I'm back.

I added a note to the IB section to make it clear the IB is not ready for review.

benbowler avatar Apr 17 '24 09:04 benbowler

IB :white_check_mark:

techanvil avatar Jun 12 '24 09:06 techanvil

Hi @jimmymadon, thanks for drafting the IB here which LGTM.

However, while we do need to allow a good amount of time for testing - the estimate still seems a bit on the high side. Did you mean to increase it from an 11 to a 15?

techanvil avatar Jun 12 '24 09:06 techanvil

@techanvil Yeah was just worried about some uncertainty in it all "coming together" when we test the order out in the widget area and the side panel. But 11 should be fine.

jimmymadon avatar Jun 14 '24 13:06 jimmymadon

QA Update ✅

  • When SK just creates the 'New visitors' and Returning visitors', they are both appearing in the same order as in the Analytics page. ie. New first and then returning. The order matches the tiles and also the selection panel. In the latter, the default 'All visitors' appears at the very bottom.

    Screenshot 2024-06-25 at 14 51 23
  • After creating new audiences in analytics, these won't appear straightaway. After disconnecting and reconnecting the analytics module, the new audiences appeared and they are following the order:

    • User defined audiences
    • Site Kit-created audiences
    • Default audiences
    Screenshot 2024-06-26 at 15 33 39
  • The secondary order is respected as per listed in the Analytics UI/API.

    Screenshot 2024-06-26 at 15 42 26 Screenshot 2024-06-26 at 15 23 14
  • Did different permutations and combinations at the 'Current selection' and at the 'Additional groups' and every time, the primary and secondary order would the one defined in the AC. Also, the tiles would also follow the order as per the current selection.

This is all looking good. Moving this ticket to approval.

kelvinballoo avatar Jun 26 '24 11:06 kelvinballoo

Note that I made a small amendment to the AC to reflect the change introduced in https://github.com/google/site-kit-wp/issues/9168 which ensures the Site Kit-created audiences are always listed in the order "new visitors", "returning visitors". This ensures the AC for this issue can be used as a source of truth for the audience ordering.

techanvil avatar Sep 18 '24 10:09 techanvil