Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Enhancement/6623 ga4 widgets real data #6790
Enhancement/6623 ga4 widgets real data #6790
Changes from 9 commits
f18e950
e349669
0bed654
797578c
4c06396
6ca7774
680a723
2a2d97e
97e835a
e130cc9
eb5e884
4aa8fec
182b2fe
3b5a4c8
d71278a
fd91b5a
b458c4f
1dc3c19
84cb586
73384fa
43f6d6e
ebe895f
78387da
fb98422
53f76e1
1557f86
c2ec3cf
9ae6e2b
da96ecf
054afa9
ac2339a
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic here needs to be updated based on how many date ranges there are. This is correct for the case where there is a single date range, but when there are multiple date ranges, we need to add a row for each date range, with a 2nd dimension value which is
date_range_X
, where X is the zero-based index of the date range.You can see this in action by requesting a multi-date-range report. For illustration here's one of our fixtures which shows the same thing:
site-kit-wp/assets/js/modules/analytics-4/utils/__fixtures__/mocked-report-multiple-distinct-date-ranges.json
Lines 8 to 36 in 97e835a
Furthermore - the logic also needs changing to ensure the rows are inserted in the right order, when
date
is provided as an order-by dimension. As GA4 reports no longer return rows ordered by default, we now need to explicitly specify for them to be ordered bydate
for our charts. As things stand this code simply appends the dates to the end of$rows
which can result in an out of order sequence when there are some rows returned in the response.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, added the date range index and updated the logic to ensure that rows are sorted correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, as mentioned on a separate comment though, the logic still needs a bit of an update. Details are on the other comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the update, but this still needs another tweak, with details on a separate comment.