Enabling Unsupported Block Editor for Jetpack sites with SSO enabled. by marecar3 · Pull Request #12606 · wordpress-mobile/WordPress-Android

marecar3 · 2020-08-05T20:56:06Z

Details can be found on this PR: WordPress/gutenberg#24476

PR submission checklist:

I have considered adding unit tests where possible.
I have considered adding accessibility improvements for my changes.
I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

peril-wordpress-mobile · 2020-08-05T20:56:11Z

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

peril-wordpress-mobile · 2020-08-05T21:03:21Z

You can test the changes on this Pull Request by downloading the APK here.

marecar3 · 2020-08-06T15:27:57Z

This PR is currently not ready for review as both test cases are not working.
We need to add some extra steps (from the user side) after which they will work.

…k-sso-enabled # Conflicts: # WordPress/src/main/java/org/wordpress/android/ui/posts/EditPostActivity.java

loremattei · 2020-08-10T14:10:13Z

Hey! I'm moving this to 15.6 since 15.5 has been cut. I see this is marked as Not Ready for review, so I guess it's fine. Otherwise, please feel free to ping me to get it added to 15.5

This reverts commit b356af7.

…k-sso-enabled

…k-sso-enabled # Conflicts: # libs/gutenberg-mobile

guarani

I had an issue with self-hosted: WordPress/gutenberg#24476 (review)

…k-sso-enabled # Conflicts: # libs/gutenberg-mobile

etoledom

I have tested on a Jurassic Ninja instance connected via Jetpack and an Atomic site.
In both cases this feature is working as expected! 🎉

For the Jetpack case, I needed to change the web-editor preference manually. This is because of an issue that is external to us (so is the best we can do at this moment).

cc @guarani

Could someone check out the code side of the PR?
@mchowning or @cameronvoell perhaps?

…k-sso-enabled

marecar3 · 2020-09-04T22:48:50Z

I have tested on a Jurassic Ninja instance connected via Jetpack and an Atomic site.
In both cases this feature is working as expected! 🎉

For the Jetpack case, I needed to change the web-editor preference manually. This is because of an issue that is external to us (so is the best we can do at this moment).

cc @guarani

Could someone check out the code side of the PR?
@mchowning or @cameronvoell perhaps?

Thanks, Eduardo!

guarani

I tried again with a new Jurassic Ninja test site using build 71274 and things worked fine. FWIW, the only difference here was that the WordPress.com account I connected with the site was not an Automattic account.

For the Jetpack case, I needed to change the web-editor preference manually. This is because of an issue that is external to us (so is the best we can do at this moment).

I didn't need to change any web-editor preference, it just worked 🤷‍♂️.

I re-tested Atomic and it's also working well.

etoledom · 2020-09-08T15:01:59Z

Thanks for testing @guarani !

I connected with the site was not an Automattic account.
I didn't need to change any web-editor preference, it just worked 🤷‍♂️.

Nice to know that it works with a different account.

Is this account newer than December 2018? (Is your a8c account older than December 2018 anyway?)
AFAIK those older accounts are the ones that had this problem.

…k-sso-enabled

etoledom · 2020-09-09T14:48:17Z

Is the code side of this PR reviewed?
cc @marecar3 @mchowning

mchowning

Just left a couple of small-ish code comments. Otherwise, the code looks good to me.

mchowning · 2020-09-09T14:58:20Z

...r/WordPressEditor/src/main/java/org/wordpress/android/editor/WPGutenbergWebViewActivity.java

-            String token = bundle.getString(ARG_AUTHENTICATION_TOKEN);
-            boolean isSitePrivate = bundle.getBoolean(ARG_IS_SITE_PRIVATE, false);
+
+            boolean isSitePrivate = !gutenbergWebViewAuthorizationData.isWPCom();


Is isSitePrivate an accurate name for this variable? Would something like isSelfHosted or !isWpCom be more precise?

Thanks for addressing this one. You are right. isSitePrivate isn't an accurate name.
Changed to isSelfHosted .

mchowning · 2020-09-09T15:12:28Z

...ressEditor/src/main/java/org/wordpress/android/editor/gutenberg/GutenbergEditorFragment.java

        Bundle arguments = getArguments();
-        boolean supportStockPhotos = arguments != null && arguments.getBoolean(ARG_SITE_USING_WPCOM_REST_API);
+        GutenbergWebViewAuthorizationData gutenbergWebViewAuthorizationData =
+                arguments.getParcelable(ARG_GUTENBERG_WEB_VIEW_AUTH_DATA);


Should we check for arguments being null here? The next two lines that use the arguments do that with arguments != null && .... Obviously we can't do that here since it's not a boolean, but I think we could achieve that effect if we inline this variable where it is used on line 409, since that is a boolean and has the null check.

There are actually a few places where we don't null check getArguments() in this class. I actually don't feel strongly about null-checking getArguments() because they should never be null as long as they are set (which we do), but I do think we should be consistent--either we always check them or we never do. Having a mix and sometimes checking them seems confusing. We seem closer to always checking them, and that's obviously a bit safer, so that would be my preference but, like I said, I don't feel particularly strongly (particularly if there is no reasonable way to handle a particular case where getArguments() is null). WDYT?

Thanks, @mchowning! Good point here. We should pick a direction and probably we are closer to check getArguments() to not be null.

Fixed.

…k-sso-enabled

marecar3 · 2020-09-10T17:55:10Z

Hey @mchowning 👋
I have addressed all comments, you can do another iteration of review! Thanks!

mchowning

LGTM! Nice work @marecar3 !

marecar3 · 2020-09-10T19:35:33Z

LGTM! Nice work @marecar3!

Thanks, @mchowning!

cc @etoledom this one is ready for merge when the time comes.

…k-sso-enabled

Enabling Unsupported Block Editor for Jetpack sites with SSO enabled.

ac3e043

marecar3 added the gutenberg-mobile label Aug 5, 2020

marecar3 added this to the 15.5 milestone Aug 5, 2020

marecar3 requested a review from guarani August 5, 2020 20:56

marecar3 added 2 commits August 5, 2020 17:18

Fix lint error

48f9ecc

Fix lint errors

07e3c59

marecar3 added the [Status] Not Ready for Review label Aug 6, 2020

marecar3 added 7 commits August 7, 2020 23:58

Send update capabilities events from Android native to JS

705e574

Merge branch 'develop' into gutenberg/unsupported-block-editor-jetpac…

e8c87ba

…k-sso-enabled # Conflicts: # WordPress/src/main/java/org/wordpress/android/ui/posts/EditPostActivity.java

Small fix

66fa9f9

Remove empty lines

b87dffd

Fix lint errors

cdf8899

Update gb-mobile reference

8678b8d

Update gb-mobile reference

ee3a0e7

loremattei modified the milestones: 15.5, 15.6 Aug 10, 2020

marecar3 added 5 commits August 10, 2020 22:48

Update gb-mobile reference

b227a08

Merge

b356af7

Revert "Merge"

9fa7b6e

This reverts commit b356af7.

Update gb-mobile reference

0c9b6ec

update gb-mobile reference

b846c13

marecar3 mentioned this pull request Aug 11, 2020

Rnmobile/capabilities refresh WordPress/gutenberg#24476

Merged

6 tasks

marecar3 added 4 commits August 13, 2020 12:58

Merge branch 'develop' into gutenberg/unsupported-block-editor-jetpac…

7bab8c1

…k-sso-enabled

Merge

5c95d1f

Merge branch 'develop' into gutenberg/unsupported-block-editor-jetpac…

eb57550

…k-sso-enabled # Conflicts: # libs/gutenberg-mobile

Update gb-mobile submodule

a970616

guarani reviewed Sep 1, 2020

View reviewed changes

marecar3 added 3 commits September 1, 2020 17:30

Remove unused line of code

4598b61

Update gb mobile reference

251f348

Merge branch 'develop' into gutenberg/unsupported-block-editor-jetpac…

5755582

…k-sso-enabled # Conflicts: # libs/gutenberg-mobile

etoledom approved these changes Sep 4, 2020

View reviewed changes

marecar3 added 2 commits September 4, 2020 18:30

Update gb-mobile ref

254d962

Merge branch 'develop' into gutenberg/unsupported-block-editor-jetpac…

96672fc

…k-sso-enabled

marecar3 modified the milestones: 15.7, 15.8 Sep 4, 2020

guarani self-requested a review September 8, 2020 12:20

guarani approved these changes Sep 8, 2020

View reviewed changes

marecar3 added 2 commits September 8, 2020 23:57

Update gb-mobile reference

a32721b

Merge branch 'develop' into gutenberg/unsupported-block-editor-jetpac…

4de9f08

…k-sso-enabled

mchowning reviewed Sep 9, 2020

View reviewed changes

marecar3 added 4 commits September 10, 2020 13:04

Merge branch 'develop' into gutenberg/unsupported-block-editor-jetpac…

7c4de6b

…k-sso-enabled

Update gb-mobile reference

020c882

Rename isSitePrivate to isSelfHosted

9269fd5

Check in one place if arguments are not null

a63df79

mchowning approved these changes Sep 10, 2020

View reviewed changes

marecar3 added 3 commits September 11, 2020 19:38

Update gb-mobile reference

86206be

Merge branch 'develop' into gutenberg/unsupported-block-editor-jetpac…

6c6e037

…k-sso-enabled

Update gb-mobile reference to point to develop

9128215

marecar3 merged commit 29903f8 into develop Sep 12, 2020

marecar3 deleted the gutenberg/unsupported-block-editor-jetpack-sso-enabled branch September 12, 2020 00:33

designsimply mentioned this pull request Oct 5, 2020

Release 1.38.0 wordpress-mobile/gutenberg-mobile#2677

Merged

5 tasks

Conversation

marecar3 commented Aug 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

peril-wordpress-mobile bot commented Aug 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

peril-wordpress-mobile bot commented Aug 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

marecar3 commented Aug 6, 2020

Uh oh!

loremattei commented Aug 10, 2020

Uh oh!

guarani left a comment

Choose a reason for hiding this comment

Uh oh!

etoledom left a comment

Choose a reason for hiding this comment

Uh oh!

marecar3 commented Sep 4, 2020

Uh oh!

guarani left a comment

Choose a reason for hiding this comment

Uh oh!

etoledom commented Sep 8, 2020

Uh oh!

etoledom commented Sep 9, 2020

Uh oh!

mchowning left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mchowning Sep 9, 2020

Choose a reason for hiding this comment

Uh oh!

marecar3 Sep 10, 2020

Choose a reason for hiding this comment

Uh oh!

mchowning Sep 9, 2020

Choose a reason for hiding this comment

Uh oh!

marecar3 Sep 10, 2020

Choose a reason for hiding this comment

Uh oh!

marecar3 commented Sep 10, 2020

Uh oh!

mchowning left a comment

Choose a reason for hiding this comment

Uh oh!

marecar3 commented Sep 10, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

marecar3 commented Aug 5, 2020 •

edited

Loading

peril-wordpress-mobile bot commented Aug 5, 2020 •

edited

Loading

peril-wordpress-mobile bot commented Aug 5, 2020 •

edited

Loading

mchowning left a comment •

edited

Loading