-
Notifications
You must be signed in to change notification settings - Fork 286
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
Handle the special case for “new visitors” and “returning visitors” to avoid the “partial data” state in the Audience Tile #8144
Comments
Note that I've moved this to the Backlog as we have an open discussion on the "collecting data" terminology in the design doc. |
The aforementioned discussion has been resolved, and this is ready for AC. |
Hi @ankitrox, thanks for drafting this IB. You raise an interesting concern as to how this issue relates to #8726. On balance, I think it would in fact be preferable to wait for #8726 to be implemented before tackling this issue, as #8726 will result in some significant refactoring of the It's also the case that if we implement this issue before #8726, then #8726 itself will be impacted and require additional work that we haven't accounted for in its IB. In terms of execution efficiency I think the refactor specced in #8726 would be better done first because it will simplify the data extraction, and lead to a fairly straightforward implementation for this issue, whereas if we implement this issue first we'll add complexity to the data extraction side of things which will be a bit harder to subsequently refactor in #8726. So, as a result I've added #8726 as a blocker to this issue, and would suggest revisiting the IB here with this in mind. Here are a few additional points on the current IB:
|
Thank you @techanvil . As we have pretty much implemented #8484 and #8726 is also in CR, I've written the IB for this one. Requesting you to kindly review the same. Thanks |
Thanks @ankitrox! The IB is most of the way there. A couple of smallish points:
|
Thank you @techanvil . I have updated the IB for the points mentioned. Assigning to you for review. |
Hussain flagged this will go over the estimate a bit due to increased complexity. |
…-returning-visitors Enhancement/#8144 - Handle “new visitors” and “returning visitors” to avoid the “partial data” state
…ructure-follow-up Fix array destructuring with fallback to prevent crashing when `getAudienceUserCountReportErrors` returns `undefined`
QA Update: ✅Verified:
|
Feature Description
Implement the special case for “new visitors” and “returning visitors” on the Audience Tile whereby their metrics are retrieved using the
newVsReturning
dimension instead of via the audience until the corresponding audiences are out of the "partial data" state.See special case to avoid "partial data" state in the design doc.
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
newVsReturning
dimension rather thanaudienceResourceName
.Implementation Brief
Inside
AudienceTilesWidget
componentassets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget.js
.getConfiguredSiteKitAndOtherAudiences
selector added in Handle error of user count retrieval for audiences in partial data state #8923 which should return[ otherAudiences, siteKitAudiences ]
in response.siteKitAudiences
are in partial data state usinghasAudiencePartialData( audiences )
selector added in Handle error of user count retrieval for audiences in partial data state #8923. Assign the result to a constantisSiteKitAudiencePartialData
.isSiteKitAudiencePartialData
is true:siteKitAudiencesReport
which should be fetched with samereportOptions
, but using thenewVsReturning
dimension instead ofaudienceResourceName
, and filtering for the values'new'
or'returning'
for this dimension (i.e. to exclude'(not set)'
).siteKitAudienceResourceNames
separately withdimensionFilters
set tonewVsReturning
.getAudienceTileMetrics
insideAudienceTilesWidget
which should be called when we are iterating over thevisibleAudiences
to returnAudienceTile
component. It should return these metrics so that those can be passed toAudienceTile
component.audienceResourceName
as argument. It should check ifisSiteKitAudiencePartialData
is true.isSiteKitAudiencePartialData
istrue
and theaudienceResourceName
belongs to a Site Kit audience (use theisSiteKitAudience
selector), return the data from the report fetched for Site Kit audiences which usednewVsReturning
dimension.audienceResourceName
dimension.AudienceTile
component insideassets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTile/index.js
isAudiencePartialData
, use theisSiteKitAudience
selector to check if theaudienceResourceName
Site Kit audience.false
.true
.Test Coverage
QA Brief
Prerequisites
audienceSegmentation
feature flag.Data Verification
newVsReturning
dimension rather than theaudienceResourceName
dimension.Testing the Error State
Changelog entry
The text was updated successfully, but these errors were encountered: