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] Disable create and update notifications for variations #2464

Merged

Conversation

puntope
Copy link
Contributor

@puntope puntope commented Jul 9, 2024

Changes proposed in this Pull Request:

  • Disable Create notification if the product is a variation
  • Disable Update notification if the product is a variation
  • Schedules delete notifications when the parent product is (DONT_SYNC_AND_SHOW, unpublished etc...)
  • Tweak tests

Slack ref: p1720458122308169/1719561677.366209-slack-C02BB3F30TG

Detailed test instructions:

  1. Create a variable product
  2. Set the variables for the product and prices
  3. Publish the product
  4. See the Notification Create Job is scheduled for the parent. Run it
  5. Update the product description
  6. See the Notification Update Job is scheduled for the parent. Run it
  7. Update a variation price
  8. See the Notification Update Job is scheduled for the parent. Run it
  9. Delete a variation
  10. See the Notification Update Job is scheduled for the parent. Run it
  11. See in the logs the delete notification has been sent for the variable
  12. Set the parent product as "DONT SYNC AND SHOW"
  13. See the Notification Delete Job is scheduled for the parent and variations

Additional details:

Changelog entry

@puntope puntope requested a review from a team July 9, 2024 13:05
@puntope puntope self-assigned this Jul 9, 2024
@github-actions github-actions bot added the changelog: tweak Small change, that isn't actually very important. label Jul 9, 2024
@puntope puntope marked this pull request as ready for review July 9, 2024 13:06
Copy link

codecov bot commented Jul 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 64.8%. Comparing base (e7be1ca) to head (0159630).
Report is 47 commits behind head on feature/google-api-project.

Additional details and impacted files

Impacted file tree graph

@@                     Coverage Diff                      @@
##             feature/google-api-project   #2464   +/-   ##
============================================================
  Coverage                          64.8%   64.8%           
- Complexity                         4561    4564    +3     
============================================================
  Files                               473     473           
  Lines                             17799   17801    +2     
============================================================
+ Hits                              11533   11535    +2     
  Misses                             6266    6266           
Flag Coverage Δ
php-unit-tests 64.8% <100.0%> (+<0.1%) ⬆️

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

Files Coverage Δ
src/Product/ProductHelper.php 92.2% <100.0%> (+0.1%) ⬆️
src/Product/SyncerHooks.php 90.0% <100.0%> (ø)

... and 6 files with indirect coverage changes

Copy link
Contributor

@jorgemd24 jorgemd24 left a comment

Choose a reason for hiding this comment

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

@puntope, thanks for the adjustments, LGTM. I tested the following scenarios:

  • Update variation -> Creates a notification for the parent.
  • Create variation -> Creates a notification for the parent.
  • Delete variation -> Creates an update notification for the parent + sends a request with a delete notification for the variation, sending the offer_id. (It is not scheduled the notification)
  • Delete variable product -> sends the delete notification for parent and variants without scheduling the job.

@puntope puntope merged commit 48be943 into feature/google-api-project Jul 10, 2024
14 checks passed
@puntope puntope deleted the tweak/disable-notifications-variations branch July 10, 2024 13:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: tweak Small change, that isn't actually very important.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants