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

Stats Traffic: Increase number of weeks to fetch for monthly chart #22826

Merged

Conversation

staskus
Copy link
Contributor

@staskus staskus commented Mar 13, 2024

More context: p1709642938683799/1709576037.356079-slack-C06BR07TJHK

Long months, such as October 2023 have 6 weeks that fall into one month's range. We should fetch 6 weeks to make sure they are included in the charts. View Model makes sure that the weeks that don't belong to a given month are not shown in a chart.

To test:

  1. Enable Stats Traffic feature flag
  2. Open Stats, Traffic
  3. Select Month
  4. Go through months and confirm that we show all (no more and less) weeks that fall into a provided month's range.

Regression Notes

  1. Potential unintended areas of impact

Couldn't identify any

  1. What I did to test those areas of impact (or what existing automated tests I relied on)

  2. What automated tests I added (or what prevented me from doing so)

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding unit tests for my changes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

Testing checklist:

  • WordPress.com sites and self-hosted Jetpack sites.
  • Portrait and landscape orientations.
  • Light and dark modes.
  • Fonts: Larger, smaller and bold text.
  • High contrast.
  • VoiceOver.
  • Languages with large words or with letters/accents not frequently used in English.
  • Right-to-left languages. (Even if translation isn’t complete, formatting should still respect the right-to-left layout)
  • iPhone and iPad.
  • Multi-tasking: Split view and Slide over. (iPad)

@staskus staskus added this to the 24.5 milestone Mar 13, 2024
@staskus staskus requested a review from guarani March 13, 2024 16:39
@wpmobilebot
Copy link
Contributor

WordPress Alpha📲 You can test the changes from this Pull Request in WordPress Alpha by scanning the QR code below to install the corresponding build.
App NameWordPress Alpha WordPress Alpha
ConfigurationRelease-Alpha
Build Numberpr22826-1210657
Version24.4
Bundle IDorg.wordpress.alpha
Commit1210657
App Center BuildWPiOS - One-Offs #9145
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@wpmobilebot
Copy link
Contributor

Jetpack Alpha📲 You can test the changes from this Pull Request in Jetpack Alpha by scanning the QR code below to install the corresponding build.
App NameJetpack Alpha Jetpack Alpha
ConfigurationRelease-Alpha
Build Numberpr22826-1210657
Version24.4
Bundle IDcom.jetpack.alpha
Commit1210657
App Center Buildjetpack-installable-builds #8189
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@staskus staskus modified the milestones: 24.5, Pending Mar 15, 2024
Copy link
Contributor

@guarani guarani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay reviewing this, @staskus.

I ran into some bugs which I'll detail below.

An error occurred when viewing certain weeks

I thought it might be related to Oct having six calendar weeks, but it looks to be related to DST kicking in in my timezone on Oct 1st.

On the team P2 with the week of "Sep 25 - Oct 1, 2023" selected, the screen doesn't load and `An error occurred" is shown:

See screenshot

self.periodDataQueryDateFormatter.date(from: "2023-10-01") returns nil for me, which ends up being due to clocking moving forward one hour at that time. Adding df.timeZone = TimeZone(secondsFromGMT: 0) seems to fix the issue.

This is unrelated to this PR, so not a blocker.

Zero counts display for month of Oct 2023

Oct 2023 also shows zeros for Views/Visitors/Likes/Comments for me:

See screenshot

I didn't have time to finish looking into this unfortunately, but it's reproducible without this change, so again not a blocker. I think it's likely the same issue we say in March 2024 (the one you raised in p1709296218622579-slack-C82FZ5T4G).


I think this PR is fine. I see the six bars in Oct 2023 now and the counts look correct. I think the above are existing, unrelated issues.

@staskus
Copy link
Contributor Author

staskus commented Mar 19, 2024

@guarani

self.periodDataQueryDateFormatter.date(from: "2023-10-01") returns nil for me, which ends up being due to clocking moving forward one hour at that time. Adding df.timeZone = TimeZone(secondsFromGMT: 0) seems to fix the issue.

Certainly this is a bug that needs to be fixed. I wonder if it also happens to users that see these "An error occurred errors"

I didn't have time to finish looking into this unfortunately, but it's reproducible without this change, so again not a blocker. I think it's likely the same issue we say in March 2024 (the one you raised in p1709296218622579-slack-C82FZ5T4G).

This workaround should've resolved issues such as these. #22750 Although that workaround by itself needs to be improved #22750 (comment) which I haven't done.

@staskus staskus modified the milestones: Pending, 24.6 Mar 19, 2024
@staskus staskus merged commit 4632edf into trunk Mar 19, 2024
25 of 30 checks passed
@staskus staskus deleted the task/stats-traffic-increase-maximum-quantity-for-weekly-data branch March 19, 2024 12:08
@staskus
Copy link
Contributor Author

staskus commented Mar 19, 2024

@guarani
Copy link
Contributor

guarani commented Mar 19, 2024

Certainly this is a bug that needs to be fixed. I wonder if it also happens to users that see these "An error occurred errors"

I don't think this is related to the "An error occurred" errors I saw in #12945 (comment), which was for this month (not Oct 2023).

This workaround should've resolved issues such as these. #22750 Although that workaround by itself needs to be improved #22750 (comment) which I haven't done.

I think it might be best to start looking into the backend. I didn't follow up on this over the last few weeks because it seemed low-priority, but it's happened in Oct 2023 and Feb 2024 IIRC.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants