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

API PULL - Add a filter for enabling the feature #2340

Merged
merged 1 commit into from
Apr 2, 2024

Conversation

puntope
Copy link
Contributor

@puntope puntope commented Mar 28, 2024

Changes proposed in this Pull Request:

This PR enables a filter for enabling the full Notifications feature:

If the woocommerce_gla_notifications_enabled is false

  • The enable feature banner and the info in the settings page cards won't show in the UI
  • wpcom_rest_api_status will return disabled -- Even setting the gla_wpcom_rest_api_status Option as approved, error or disapproved
  • The Notification service won't trigger notifications (This will be done when API PULL - Validate MC connection in notifications #2336 gets merged)
  • The Syncer Hooks won't schedule any notification job
  • MC will sync products and coupons as usual. (old strategy)

The PR does some naming changes

NotificationsService::is_enabled() -> Just checks the filter.
Options::notifications_enabled() -> Renamed to Options::is_wpcom_api_authorized()
NotificationsService::is_ready() -> Check if NotificationsService::is_enabled() and if Options::is_wpcom_api_authorized()

Detailed test instructions:

  1. Checkout this PR ( woocommerce_gla_notifications_enabled is false by default)
  2. Complete the onboarding
  3. Go to settings, no banner is shown
  4. Set gla_wpcom_rest_api_status as approved
  5. No info in GMC Card is shown
  6. Set gla_wpcom_rest_api_status as error
  7. No info in GMC Card is shown
  8. Change some product
  9. See the notification jobs are not scheduled
  10. Set woocommerce_gla_notifications_enabled filter as true
  11. Delete gla_wpcom_rest_api_status option
  12. Banner is shown
  13. Set gla_wpcom_rest_api_status as approved
  14. Info is shown in the card
  15. Change some product
  16. See the notification jobs is scheduled

@github-actions github-actions bot added changelog: add A new feature, function, or functionality was added. type: enhancement The issue is a request for an enhancement. labels Mar 28, 2024
Copy link

codecov bot commented Mar 28, 2024

Codecov Report

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

Project coverage is 62.5%. Comparing base (f19b5f7) to head (e8efb1b).
Report is 3 commits behind head on add/grant-access-to-google-wpcom-app-in-setting-page.

Additional details and impacted files

Impacted file tree graph

@@                                  Coverage Diff                                   @@
##             add/grant-access-to-google-wpcom-app-in-setting-page   #2340   +/-   ##
======================================================================================
  Coverage                                                    62.5%   62.5%           
- Complexity                                                   4461    4469    +8     
======================================================================================
  Files                                                         764     764           
  Lines                                                       22560   22585   +25     
  Branches                                                      543     543           
======================================================================================
+ Hits                                                        14104   14123   +19     
- Misses                                                       7999    8005    +6     
  Partials                                                      457     457           
Flag Coverage Δ
js-unit-tests 55.3% <ø> (ø)
php-unit-tests 64.0% <72.2%> (+<0.1%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
js/src/constants.js 100.0% <ø> (ø)
src/Coupon/SyncerHooks.php 74.5% <100.0%> (ø)
src/Jobs/Notifications/AbstractNotificationJob.php 100.0% <100.0%> (ø)
src/MerchantCenter/AccountService.php 96.2% <100.0%> (+<0.1%) ⬆️
src/MerchantCenter/MerchantCenterService.php 94.2% <100.0%> (+<0.1%) ⬆️
src/Product/SyncerHooks.php 83.1% <100.0%> (ø)
src/Settings/SyncerHooks.php 100.0% <100.0%> (ø)
src/Shipping/SyncerHooks.php 100.0% <100.0%> (ø)
src/Options/Options.php 62.3% <0.0%> (ø)
src/API/WP/NotificationsService.php 86.0% <0.0%> (-4.2%) ⬇️

... and 4 files with indirect coverage changes

@puntope puntope requested a review from a team March 28, 2024 10:39
@puntope puntope self-assigned this Mar 28, 2024
@puntope puntope marked this pull request as ready for review March 28, 2024 10:47
Copy link
Member

@ianlin ianlin left a comment

Choose a reason for hiding this comment

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

Thanks for adding the filter to enable/disable the notification service. Tested locally and it worked as expected. 👍

Base automatically changed from add/grant-access-to-google-wpcom-app-in-setting-page to feature/google-api-project April 2, 2024 08:47
@puntope puntope merged commit dd09487 into feature/google-api-project Apr 2, 2024
17 checks passed
@puntope puntope deleted the add/filter-for-enabling-banner branch April 2, 2024 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: add A new feature, function, or functionality was added. type: enhancement The issue is a request for an enhancement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants