Adam/222 oauth flow

[AdamGrzybkowski]

Description

I am opening this as a Draft to discuss the API and the structure in general. The code is not final yet.

This is the module structure for now:

• oauth package will hold all the OAuth related code (UI and network)
• editor will contain what's related to the Editor including the modal bottom sheet wrapper and some extra extensions
• GravatarQuickEditor is the only file in the top package. This one will serve as an easy entry point for the integration. For now, it has two .show() functions to start the quick editor from the fragment or activity but it will also contain other functions like `.logout().

Integration

Jetpack Compose

GravatarQuickEditorBottomSheet(
            gravatarQuickEditorParams = GravatarQuickEditorParams {
                appName = "yourAppName"
                oAuthParams = OAuthParams {
                    clientId = "clientId
                    clientSecret = "clientSecret"
                    redirectUri = "redirectUri"
                }
                scope = GravatarQuickEditorScope.AVATARS
            },
            onDismiss = {            },
   )

Activity/Fragment

GravatarQuickEditor.show(
                activity = this,
                gravatarQuickEditorParams = GravatarQuickEditorParams {
                    appName = "yourAppName"
                    oAuthParams = OAuthParams {
                        clientId = "clientId"
                        clientSecret = "clientSecret"
                        redirectUri = "redirectUri"
                    }
                    scope = GravatarQuickEditorScope.AVATARS
                },
                onDismiss = {                },
            )
)

For the ...Params classes I've used the same patterns as we use for the classes generated by OpenAPI - meaning the builder for Java and DSL for Kotlin.

About the scope = GravatarQuickEditorScope.AVATARS - I've followed the web here - they offer different scopes to edit by the user. Not sure if we are planning to have more than just Avatars so we could drop it for now.

I will continue with cleaning up and extracting the oauth package. We need ViewModel and separate services but let me know what you think so far.

Testing Steps

Add these to your local.properties

demo-app.wordpress.oauth.clientId=yourId
demo-app.wordpress.oauth.clientSecret=yourSecret
demo-app.wordpress.oauth.redirectUri=yourURI

and play with the demo app.

[AdamGrzybkowski]

What other callback we might want to provide here?
We will likely need something like onProfileUpdated. I wonder if we want to communicate the OAuth errors here as well or do we want to keep it within the SDK and handle ourselves fully?

[hamorillo]

We will likely need something like onProfileUpdated

Yeah. Do we want to be less generic for the moment? For example, onAvatarUpdated, and if we later add the possibility of modifying the profile, we'll add a different callback.

I wonder if we want to communicate the OAuth errors here as well or do we want to keep it within the SDK and handle ourselves fully?

Good question. I'm trying to use the third-party developer hat, and I guess it could be useful to know, at least, that maybe I decide to show something in the UI? However, I wouldn't be too specific with the error details.

[AdamGrzybkowski]

Yeah. Do we want to be less generic for the moment? For example, onAvatarUpdated, and if we later add the possibility of modifying the profile, we'll add a different callback.

If we decide to go for the scopes I was thinking about something like onProfileUpdated(ProfilePayload) -> The ProfilePayload would then correspond to the scope selected.

Having a simple onProfileUpdated and returning a full profile would be simpler for third-party devs to handle I guess, but we won't have that. We might not even have access to the full profile.

Good question. I'm trying to use the third-party developer hat, and I guess it could be useful to know, at least, that maybe I decide to show something in the UI? However, I wouldn't be too specific with the error details.

Yeah, we probably need at least one error to propagate when the OAuth fails - as this could be a config issue.

[hamorillo]

Great job @AdamGrzybkowski 🎉!

I like the overall structure. It's easy to understand at first sight.

About the scope = GravatarQuickEditorScope.AVATARS - I've followed the web here - they offer different scopes to edit by the user. Not sure if we are planning to have more than just Avatars so we could drop it for now.

We can ask around, and if we think we'll add more scopes, I'm not against adding it right now. If we add it now, we won't have changes in the public API later, and it will be easier to handle. However, if we have doubts about whether we'll add more scopes, I will drop it, and we'll handle it later.

[hamorillo]

We will likely need something like onProfileUpdated

Yeah. Do we want to be less generic for the moment? For example, onAvatarUpdated, and if we later add the possibility of modifying the profile, we'll add a different callback.

I wonder if we want to communicate the OAuth errors here as well or do we want to keep it within the SDK and handle ourselves fully?

Good question. I'm trying to use the third-party developer hat, and I guess it could be useful to know, at least, that maybe I decide to show something in the UI? However, I wouldn't be too specific with the error details.

[hamorillo]

🤔 I think we shouldn't include the OAuthParams inside GravatarQuickEditorParams. In my mind, being totally strict, the QuickEditor doesn't need the OAuthParams. It needs a token to connect with the backend. For that reason, I would consider keeping it outside this class.

That way, we can have two GravatarQuickEditorPage composables. One when we already have the token, for example, if we finally decide to save it locally or the app has already the token:

@Composable
internal fun GravatarQuickEditorPage(
    gravatarQuickEditorParams: GravatarQuickEditorParams,
    oauthToken: String
    onAuthError: () -> Unit = {},
) {

And another option if want to go through the OAuth flow:

@Composable
internal fun GravatarQuickEditorPage(
    gravatarQuickEditorParams: GravatarQuickEditorParams,
    oAuthParams: OAuthParams,
    onAuthError: () -> Unit = {},
) {

[AdamGrzybkowski]

Yeah, I can extract it. The main idea was to have fewer params (with callbacks this will grow quite a bit) and to have one object for the quick editor configuration.

One when we already have the token, for example, if we finally decide to save it locally or the app has already the token:

Whether we store it or not the token might still expire, and in such a case, we won't be able to handle this. As long as we are handling the token ourselves (which is what we will do I believe) we need that OAuth config.

I guess one specific case is the Jetpack app, this one will have the token already, right?