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

Adds tracking to site visibility editing #13223

Open
wants to merge 7 commits into
base: issue/13213-disable-notifications-api-request
Choose a base branch
from

Conversation

JorgeMucientes
Copy link
Contributor

@JorgeMucientes JorgeMucientes commented Dec 30, 2024

⚠️ Do not merge until base branch is trunk

Closes: #13222

Description

Adds tracking to edit site's visibility feature.

Steps to reproduce

Testing information

  1. Open site picker screen and check the following log is shown once site's are loaded: 🔵 Tracked: woocommerceandroid_site_picker_edit_button_shown
  2. Tap on Edit Stores cta an verify the following is shown 🔵 Tracked: woocommerceandroid_site_picker_edit_button_tapped,
  3. Edit the selected sites and click Save. Check the following is tracked: 🔵 Tracked: woocommerceandroid_site_picker_list_save_button_tapped, Properties: {"hidden_site_count":2

Error tracking

  1. Go to settings > Developer options > Api faker
  2. Add the following endpoint and enable API faker:
  1. Check the following event is tracked: 🔵 Tracked: woocommerceandroid_site_picker_list_saving_failure, Properties: {"error":"ApiError(message=)"

The tests that have been performed

The above.

Images/gif

  • I have considered if this change warrants release notes and have added them to RELEASE-NOTES.txt if necessary. Use the "[Internal]" label for non-user-facing changes.

Reviewer (or Author, in the case of optional code reviews):

Please make sure these conditions are met before approving the PR, or request changes if the PR needs improvement:

  • The PR is small and has a clear, single focus, or a valid explanation is provided in the description. If needed, please request to split it into smaller PRs.
  • Ensure Adequate Unit Test Coverage: The changes are reasonably covered by unit tests or an explanation is provided in the PR description.
  • Manual Testing: The author listed all the tests they ran, including smoke tests when needed (e.g., for refactorings). The reviewer confirmed that the PR works as expected on big (tablet) and small (phone) in case of UI changes, and no regressions are added.

@JorgeMucientes JorgeMucientes added this to the 21.4 milestone Dec 30, 2024
@JorgeMucientes JorgeMucientes added the feature: site picker Any task related to site picker screen label Dec 30, 2024
@JorgeMucientes JorgeMucientes changed the base branch from trunk to issue/13213-disable-notifications-api-request December 30, 2024 18:05
@dangermattic
Copy link
Collaborator

dangermattic commented Dec 30, 2024

1 Message
📖

This PR contains changes to Tracks-related logic. Please ensure (author and reviewer) the following are completed:

  • The tracks events must be validated in the Tracks system.
  • Verify the internal Tracks spreadsheet has also been updated.
  • Please consider registering any new events.
  • The PR must be assigned the category: tracks label.

Generated by 🚫 Danger

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Dec 30, 2024

📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.
App Name WooCommerce-Wear Android
Platform⌚️ Wear OS
FlavorJalapeno
Build TypeDebug
Commit74bcad7
Direct Downloadwoocommerce-wear-prototype-build-pr13223-74bcad7.apk

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Dec 30, 2024

📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.

App Name WooCommerce Android
Platform📱 Mobile
FlavorJalapeno
Build TypeDebug
Commit74bcad7
Direct Downloadwoocommerce-prototype-build-pr13223-74bcad7.apk

@JorgeMucientes JorgeMucientes marked this pull request as ready for review December 31, 2024 09:05
@JorgeMucientes JorgeMucientes added the category: tracks Related to analytics, including Tracks Events. label Dec 31, 2024
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 89.47368% with 2 lines in your changes missing coverage. Please review.

Project coverage is 40.67%. Comparing base (36c229b) to head (74bcad7).

Files with missing lines Patch % Lines
...merce/android/ui/sitepicker/SitePickerViewModel.kt 75.00% 0 Missing and 1 partial ⚠️
...cker/sitevisibility/WooSitesVisibilityViewModel.kt 90.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@                                 Coverage Diff                                 @@
##             issue/13213-disable-notifications-api-request   #13223      +/-   ##
===================================================================================
+ Coverage                                            40.65%   40.67%   +0.01%     
- Complexity                                            6386     6390       +4     
===================================================================================
  Files                                                 1351     1351              
  Lines                                                77474    77492      +18     
  Branches                                             10653    10657       +4     
===================================================================================
+ Hits                                                 31500    31518      +18     
  Misses                                               43192    43192              
  Partials                                              2782     2782              

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

@hafizrahman hafizrahman self-assigned this Jan 2, 2025
Copy link
Contributor

@hafizrahman hafizrahman left a comment

Choose a reason for hiding this comment

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

Nice work, all good in my tests with Simulator API 34. 🚢

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: tracks Related to analytics, including Tracks Events. feature: site picker Any task related to site picker screen
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adds tracking to site visibility editing
5 participants