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

Add support for program types when requesting a review #2344

Merged
merged 2 commits into from
Apr 9, 2024

Conversation

puntope
Copy link
Contributor

@puntope puntope commented Apr 1, 2024

Changes proposed in this Pull Request:

Currently, when requesting a review, we merged all the program types (free listings and shopping ads) into one single region. Then we call WCS for requesting the review for both programs Free Listings and Shopping Ads.

This PR adds a "types" param in the request review request with the programs required to review per each region. That way we can request a review in WCS only to the required program types. If both programs are requested, only Free Listings one is processed in WCS

Screenshots

Screen.Recording.2024-04-03.at.14.35.24.mov

Related PR

https://github.com/Automattic/woocommerce-connect-server/pull/2404

Detailed test instructions:

Since we are testing a full review Request in https://github.com/Automattic/woocommerce-connect-server/pull/2404 in this PR I advise to alter the types param to have only 'shoppingads' type.

  1. Setup WCS and checkout https://github.com/Automattic/woocommerce-connect-server/pull/2404
  2. Alter https://github.com/woocommerce/google-listings-and-ads/pull/2344/files#diff-cc9faaa2c65642115b9c462704152e93ac01760cd0b6338a2e80fb5846382b76R584 to force sending the types as [ 'shoppingadsprogram' ]
  3. Send a request review for an account ready to review
  4. See the request is successful

Additional details:

Changelog entry

Fix - Exception in request review

@github-actions github-actions bot added changelog: fix Took care of something that wasn't working. type: bug The issue is a confirmed bug. labels Apr 1, 2024
Copy link

codecov bot commented Apr 1, 2024

Codecov Report

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

Project coverage is 64.5%. Comparing base (cbf7d23) to head (cb628f5).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##             develop   #2344   +/-   ##
=========================================
  Coverage       64.5%   64.5%           
- Complexity      4252    4253    +1     
=========================================
  Files            458     458           
  Lines          18041   18046    +5     
=========================================
+ Hits           11634   11638    +4     
- Misses          6407    6408    +1     
Flag Coverage Δ
php-unit-tests 64.5% <80.0%> (+<0.1%) ⬆️

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

Files Coverage Δ
src/Google/RequestReviewStatuses.php 100.0% <100.0%> (ø)
src/API/Google/Middleware.php 86.5% <0.0%> (-0.3%) ⬇️

@puntope puntope self-assigned this Apr 3, 2024
@puntope puntope marked this pull request as ready for review April 3, 2024 12:53
@puntope puntope requested review from mikkamp and a team April 3, 2024 12:53
Copy link
Contributor

@martynmjones martynmjones left a comment

Choose a reason for hiding this comment

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

Hey @puntope, thanks for adding support for program types reviews.

Tested locally with an instance of the connect server running https://github.com/Automattic/woocommerce-connect-server/pull/2404 and was able to successfully request a review with a specified program type. So LGTM ✅

@puntope puntope merged commit 1c6e04d into develop Apr 9, 2024
15 checks passed
@puntope puntope deleted the fix/request-review branch April 9, 2024 17:33
@ianlin ianlin mentioned this pull request Apr 16, 2024
21 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: fix Took care of something that wasn't working. type: bug The issue is a confirmed bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants