Skip to content

Conversation

@staskus
Copy link
Contributor

@staskus staskus commented Jan 3, 2023

Description

"Comments" card should not appear after Stats Revamp is enabled. Filter it out from visible insights.

Testing instructions

Case 1:

  1. Disable "New Appearance for Stats" and "New Cards for Stats Insights" feature flags
  2. Open Stats
  3. Add "Comments" card
  4. Enable "New Appearance for Stats" and "New Cards for Stats Insights" feature flags
  5. Come back to Stats
  6. "Comments" card should not be visible
  7. Click gear icon on top right
  8. "Comments" row should not be visible as well

Regression Notes

  1. Potential unintended areas of impact

Make sure "Comments" continues to be visible when feature flag is disabled, make sure other cards appear when Stats Revamp is enabled

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

Manual testing

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

None

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.

Images & Videos

Before the fix

Top.Commenters.-.Before.the.fix.mov

After the fix

Top.Commenters.-.After.the.fix.mp4

@wpmobilebot
Copy link
Contributor

You can test the changes in Jetpack from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr19830-8ab1e6e on your iPhone

If you need access to App Center, please ask a maintainer to add you.

@wpmobilebot
Copy link
Contributor

You can test the changes in WordPress from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr19830-8ab1e6e on your iPhone

If you need access to App Center, please ask a maintainer to add you.

@staskus staskus modified the milestones: 21.5, 21.6 Jan 6, 2023
@staskus staskus requested review from guarani and sla8c January 6, 2023 12:04
@staskus staskus self-assigned this Jan 9, 2023
@salimbraksa salimbraksa self-requested a review January 10, 2023 17:14
@sla8c
Copy link
Contributor

sla8c commented Jan 11, 2023

@staskus It took me sometime to sketch out what has changed here so I thought it was best to diagram. On testing, I haven't run into any issues with removing .insightsFollowersWordPress but I just wanted to check whether this was intentional to remove it but specifically keep .insightsFollowersEmail?

image

@staskus
Copy link
Contributor Author

staskus commented Jan 11, 2023

@staskus It took me sometime to sketch out what has changed here so I thought it was best to diagram. On testing, I haven't run into any issues with removing .insightsFollowersWordPress but I just wanted to check whether this was intentional to remove it but specifically keep .insightsFollowersEmail?

image

@sla8c, thanks for carefully taking a look.

For both older and revamped stats, we only show .insightsFollowersEmail which displays both WordPressand Emailfollowers. The same is for insightsCommentsPostsin the older stats where we allow to switch between authorsand posts (See InsightsManagementViewController.insights)

I didn't come up with this list myself, the changes you are looking at are copy-pasted from InsightsManagementViewController so we wouldn't have 2 places defining new set of stats.

image

@staskus staskus merged commit a6fb81b into trunk Jan 11, 2023
@staskus staskus deleted the fix/stats-revamp-remove-top-commenters-card branch January 11, 2023 13:26
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.

5 participants