-
Notifications
You must be signed in to change notification settings - Fork 9
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
Remove redundant Pinterest API error handlers #1087
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… Having 403 as the reason of disconnection was causing Pinterest account to disconnect due to account status change and feed actions returning 403 http responses.
…tion handler. Action scheduler uses Pinterest APIs exceptions from which are handled by APIV5 class, no need in doubling the handling effort, making multiple disconnects per a single API failure.
…ethods which do not throw anything. - Updating the repeating error message making it more clear on what is going on and what was the previous exception. - Refactoring Pinterest API Exception supported data types.
…st API response error code.
…admin notices for the Pinterest API errors which may appear during feed registration.
…o account error. - Hooking into admin exception admin notice and disconnect actions within feed generation action to show and hide account failure notices.
… exceptions while running it.
…l information about the failure. Returning boolean false, in this case, will mark action as failed w/o the exception.
… add/notices-for-important-api-exceptions
rawdreeg
approved these changes
Nov 25, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @message-dimke. The changes test well. The interest connection is not removed on failed pinterest-for-woocommerce-handle-feed-registration action.
…pi-exceptions Add notices for important api exceptions
…-exceptions Update Action Scheduler actions to fail with exceptions.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes proposed in this Pull Request:
Removing the Action Scheduler (AS) action failure hook to disconnect the extension on the Pinterest API call failures from inside the AS actions.
Customers complain about this issue when saying that after a while of the successful connection, the extension gets disconnected.
This is a requested feature that did not work out. Users prefer to stay connected even if the application is not partially working.
Detailed test instructions:
pinterest-for-woocommerce/src/API/APIV5.php
Lines 524 to 530 in 1552a0f
add exactly before the
return
statement:pinterest-for-woocommerce-handle-feed-registration
action.Changelog entry