-
Notifications
You must be signed in to change notification settings - Fork 19
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
Migrate to Twitter API v2 #229
Conversation
@iamdharmesh For some reason, E2E tests are failing. This pull request does not change anything related to UI/UX. Do you know if these tests are falling correctly? |
@ravinderk I have reverted app setup instructions as we discussed internally on call, Also tests seems failing due to credentials currently setup in repo secret is not migrated to Twitter API V2 yet. Thanks |
@iamdharmesh I added the |
Let's update the sidebar error from:
to:
...also, if the Twitter error is the "When authenticating requests to the Twitter API v2 endpoints, you must use keys and tokens from a Twitter developer App that is attached to a Project. You can create a project via the developer portal." (hopefully there's an error code we can target in case they change that copy) then let's please add after the Twitter error message "Learn more here." and link to https://developer.twitter.com/en/docs/twitter-api/migrate/ready-to-migrate. ...which means in this case that would be like:
|
On the Settings page, update all references existing to match spelling/case of:
...this way we match the exact same spelling/case/wording used on https://developer.twitter.com to avoid user confusion on configuring the plugin. |
Let's update the admin notice to: |
@iamdharmesh the above couple updates should bring this PR to a good point for final review/merge... thanks! |
@jeffpaul Currently we are also showing an error code in this message. for eg: |
If there's no code then let's not display the "Error: ." bit, if there is a code then we can keep that wrapping text. |
It failed due to the API limit was exceeded for 24 hours. |
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 for this PR! I've requested some minor changes, after that it's good to go 🎉
autoshare-for-twitter.php
Outdated
__FILE__, | ||
function () { | ||
// Don't need to show migration notice to new users. | ||
update_option( 'migrate_to_twitter_v2_api_notice_dismissed', true ); |
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.
A nitpick here - will be good to prefix the option with autoshare_
@@ -317,7 +317,7 @@ function get_tweet_status_message( $post ) { | |||
|
|||
case 'error': | |||
$response_array[] = [ | |||
'message' => __( 'Failed to tweet: ', 'autoshare-for-twitter' ) . $tweet_meta['message'], | |||
'message' => __( 'Failed to tweet; ', 'autoshare-for-twitter' ) . $tweet_meta['message'], |
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.
Is this intentional? Colon :
would be appropriate here.
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.
@Sidsector9 it is updated as per the request here #229 (comment)
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.
@iamdharmesh I left a few questions.
Description of the Change
PR migrates the current Twitter API implementation to use API v2. Majorly PR has made the below changes
Closes #173
How to test the Change
Follow the steps mentioned on the
Autoshare for Twitter Settings
admin page and set up the Twitter app. You should be able to set up the plugin without struggle. After saving the plugin setting with the Twitter app details, you should be able to tweet from WordPress from the post edit page.Changelog Entry
Credits
Props @iamdharmesh, @qasumitbagthariya, @ravinderk, @jeffpaul.
Noticeable changes
7.4
in composer.jsonChecklist: