-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Facebook Publicize disable selection of profiles #8115
Conversation
warning about the end of publicize on August 1
This reverts commit 3834d18.
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 @kwonye , I left a comment and a question, will start testing now
private String getName(PublicizeConnection connection) { | ||
String name = connection.getExternalDisplayName(); | ||
|
||
if (name.isEmpty()) { |
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.
Let's use TextUtils.isEmpty(name)
here so we also have null
as a possibility
} | ||
} | ||
|
||
return totalAccounts > 0; | ||
if (PublicizeTable.onlyExternalConnections(serviceId)) { | ||
return totalAccounts > 0; |
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.
shouldn't this be return totalExternalAccounts > 0
?
I've tried upgrading from a fresh install of tag
|
@@ -170,6 +171,9 @@ public WordPressDB(Context ctx) { | |||
case 64: | |||
// add site icon | |||
mDb.execSQL(SiteSettingsModel.ADD_SITE_ICON); | |||
case 65: | |||
// add external users only to publicize services table | |||
mDb.execSQL(PublicizeTable.ADD_EXTERNAL_USERS_ONLY); |
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.
This crashes when there is no pre-existent PublicizeTable
. Apparently, it is only created when PublicizeListActivity
is first visited here . We should add a check that the table exists before creating it, or force creating it before this step execution by calling PublicizeTable.createTables(mDb);
right above this ALTER TABLE
statement.
As discussed over Slack, feel free to merge after the crash is taken care of @kwonye 👍 |
…ading from Cursor
…dpress-mobile/WordPress-Android into fix/facebook-publicize-aug-1
conditionally if it’s not empty
Tested! LGTM |
Fixes #7591 & #8026
We're no longer allowing users to add their profiles to publicize. Also, this adds pages to the selection list where people can connect to.
To test:
@loremattei we're going to make a 10.5.1 to release this by August 1. Feel free to ship 10.5 without this tomorrow, thanks!