-
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
Add support for multiple Twitter Accounts #238
Conversation
This reverts commit 6b331dd.
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 thanks for this amazing work! I've requested for few changes, let me know if you'd like to discuss it over a call.
Thanks a lot for the review and feedback @Sidsector9. I have addressed all feedback in 5a24c7d. Could you please help to re-review and confirm once? Thanks. |
includes/admin/post-transition.php
Outdated
@@ -92,7 +94,7 @@ function( $post ) { | |||
* @param int $post_id The current post ID. | |||
* @param bool $force Publish tweet regardless of autoshare enabled or disabled on post. | |||
* | |||
* @return object | |||
* @return bool|void |
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 think we should strictly set it to bool
.
In case this function returns void
which will evaluate to null
, that null
will be set here:
$message['is_retweeted'] = $is_retweeted; |
In JS, we strictly check it against false
here:
( ( response.success && response.data.message ) || ( false === response.success && false === response.data.is_retweeted) ) |
What do you think?
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.
Yeah, not related to this PR but important enough to fix it here. Updated. Thanks
Description of the Change
Added support for connecting multiple Twitter Accounts and sending tweets to those accounts.
PR also, Fixes E2E tests failure by skip Twitter API calls.
Block Editor:
Classic Editor:
Closes #230
Closes #237
How to test the Change
Backward compatibility
New features
Changelog Entry
Credits
Props @iamdharmesh @jeffpaul
Checklist: