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] Handle exception when deactivating the plugin and the WPCOM token does not exist. #2449

Conversation

jorgemd24
Copy link
Contributor

@jorgemd24 jorgemd24 commented Jul 3, 2024

Changes proposed in this Pull Request:

Part of #2146 .

Currently, we throw an exception if the request to revoke the token fails, such as when the WPCOM token is not found and therefore can't be deleted. This can lead to a fatal error when deactivating the plugin, preventing its deactivation. Since not all merchants will opt into the new mechanism, the scenario where the WPCOM token is missing is possible. To prevent this fatal error, I think it's better to catch the exception and log the error.

Additionally, I've removed the "Revoke WPCOM Access token" option from the Connection Test page. We now have a similar option, "WPCOM REST API Status" -> "Disconnect," which has the same purpose, making the option redundant.

image

Screenshots:

Detailed test instructions:

  1. Revoke any WPCOM token, you can use the connection test page if you already have one WPCOM token.
  2. Deactivate the plugin.
  3. See that we prevent the fatal error and log the error.

Additional details:

Changelog entry

@jorgemd24 jorgemd24 self-assigned this Jul 3, 2024
@github-actions github-actions bot added the changelog: tweak Small change, that isn't actually very important. label Jul 3, 2024
Copy link

codecov bot commented Jul 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 64.7%. Comparing base (7ff8a9e) to head (dc34f56).
Report is 1 commits behind head on feature/google-api-project.

Additional details and impacted files

Impacted file tree graph

@@                      Coverage Diff                       @@
##             feature/google-api-project   #2449     +/-   ##
==============================================================
+ Coverage                          64.6%   64.7%   +0.1%     
+ Complexity                         4552    4550      -2     
==============================================================
  Files                               473     473             
  Lines                             17757   17747     -10     
==============================================================
+ Hits                              11478   11483      +5     
+ Misses                             6279    6264     -15     
Flag Coverage Δ
php-unit-tests 64.7% <100.0%> (+0.1%) ⬆️

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

Files Coverage Δ
src/API/WP/OAuthService.php 96.4% <100.0%> (+0.4%) ⬆️
src/ConnectionTest.php 0.0% <ø> (ø)

@jorgemd24 jorgemd24 marked this pull request as ready for review July 3, 2024 09:30
@jorgemd24 jorgemd24 changed the title Handle exception when deactivating the plugin and the WPCOM token does not exist. [API Pull] Handle exception when deactivating the plugin and the WPCOM token does not exist. Jul 3, 2024
@eason9487 eason9487 requested a review from a team July 4, 2024 07:42
Copy link
Contributor

@puntope puntope left a comment

Choose a reason for hiding this comment

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

✅ LGTM

@jorgemd24 jorgemd24 merged commit 13b47f0 into feature/google-api-project Jul 4, 2024
12 checks passed
@jorgemd24 jorgemd24 deleted the tweak/handle-wp-error-revoke-token-deactivating-plugin branch July 4, 2024 09:44
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