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 remove today card #20242

Merged
merged 6 commits into from
Feb 22, 2024
Merged

Conversation

ravishanker
Copy link
Contributor

@ravishanker ravishanker commented Feb 22, 2024

Fixes #20241


To Test:

Test 1:

  • Enable stats_traffic_tab feature flag (Me -> Debug settings - Remote Features)
  • Restart the app
  • Go to Stats
  • Verify only Traffic tab, and Insights tabs are shown
  • Switch to Insights tab if not on it already
  • Tap on Insights settings (⚙️)
  • Verify there's no Today Stats option under General
  • Verify that Today card is not shown

Test 2:

  • Disable stats_traffic_tab feature flag
  • Restart the app
  • Go to Stats
  • Switch to Insights tab if not on it already
  • Tap on Insights settings (⚙️)
  • Verify there's Today Stats option under General
  • Enable Today Stats by tapping on it, and press Save
  • Return to Insights
  • Verify that Today card is shown

Regression Notes

  1. Potential unintended areas of impact

    • N/A
  2. What I did to test those areas of impact (or what existing automated tests I relied on)

    • Checked other places like My Site, Traffic, and Widget where Today card is shown
  3. What automated tests I added (or what prevented me from doing so)

    • Existing unit tests

PR Submission Checklist:

  • I have completed the Regression Notes.
  • 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.

Remove Today Stats from Insights when TrafficTab is enabled
Add Today Stats only when Traffic Tab is not enabled
@wpmobilebot
Copy link
Contributor

wpmobilebot commented Feb 22, 2024

Jetpack📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
App NameJetpack Jetpack
FlavorJalapeno
Build TypeDebug
Versionpr20242-cc8d3ca
Commitcc8d3ca
Direct Downloadjetpack-prototype-build-pr20242-cc8d3ca.apk
Note: Google Login is not supported on these builds.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Feb 22, 2024

WordPress📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
App NameWordPress WordPress
FlavorJalapeno
Build TypeDebug
Versionpr20242-cc8d3ca
Commitcc8d3ca
Direct Downloadwordpress-prototype-build-pr20242-cc8d3ca.apk
Note: Google Login is not supported on these builds.

@ravishanker ravishanker force-pushed the Stats-Traffic-Remove-Today-card branch from 7da16f8 to 88734d0 Compare February 22, 2024 05:05
Copy link

codecov bot commented Feb 22, 2024

Codecov Report

Attention: Patch coverage is 85.71429% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 40.32%. Comparing base (2440e07) to head (cc8d3ca).
Report is 10 commits behind head on trunk.

Files Patch % Lines
...ns/insights/management/InsightsManagementMapper.kt 85.71% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##            trunk   #20242   +/-   ##
=======================================
  Coverage   40.31%   40.32%           
=======================================
  Files        1469     1469           
  Lines       67691    67698    +7     
  Branches    11209    11212    +3     
=======================================
+ Hits        27293    27302    +9     
+ Misses      37904    37901    -3     
- Partials     2494     2495    +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@irfano irfano left a comment

Choose a reason for hiding this comment

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

LGTM! 👍🏻 I've added a minor comment.
Today card is not being removed without restarting the app. But I think it's fine because we fetch new configs when only starting the app.

Remove IS_JETPACK_APP check since Stats only on Jetpack app now
fix detekt warning about imports
@ravishanker ravishanker merged commit efa163c into trunk Feb 22, 2024
19 checks passed
@ravishanker ravishanker deleted the Stats-Traffic-Remove-Today-card branch February 22, 2024 23:35
@irfano irfano added the Stats label Mar 21, 2024
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.

Remove Today card from insights
3 participants