Skip to content
This repository was archived by the owner on Feb 6, 2025. It is now read-only.

Conversation

@AmandaRiu
Copy link
Contributor

This PR merges the woocommerce-android changes to the login library from this PR - which includes detailed documentation on what changed in the Woo login flow, including screenshots.

Every effort was made to not effect WPAndroid's use of the login library during this project, but there were a handful of new interface methods and strings added. I've opened a draft PR on WordPress-Android that merges the changes from this branch, as well as implements the interface methods so WPAndroid can be fully tested before this branch is merged, and includes recommended testing scenarios.

aforcier and others added 24 commits April 17, 2019 16:07
The new login flow for Woo has the login by site screen leading to
this view so it doesn't make any sense having the option available as
it would essentially be routing the user back to the previous screen.
This reverts commit d8a0f33d. Turns out this part is not yet
feasible so we'll skip it for now.
This new mode is for the Woo Android app.
…in-lib

# Conflicts:
#	libs/login/WordPressLoginFlow/build.gradle
#	libs/login/WordPressLoginFlow/src/main/java/org/wordpress/android/login/LoginUsernamePasswordFragment.java
#	libs/login/gradle.properties-example
@malinajirka
Copy link
Contributor

Thanks @AmandaRiu! I've tested the changes in WPAndroid and it works as expected.

@aforcier I'm not sure what to look out for in these "subtree" pull/push PRs. Any tips? Eg. Can I somehow double-check it wasn't pulled/pushed using the buggy version of git?

@aforcier
Copy link
Contributor

aforcier commented Jul 9, 2019

Thanks @AmandaRiu & @malinajirka!

@malinajirka generally the big thing is to look for client-specific things that maybe don't need to be exposed to the library. For instance, I'm not sure the flow needs to be called WOO_LOGIN_MODE, a more generic name would probably be suitable. But, it's pretty minor + this is just a step in a series of iterations. But maybe something to note for the next iteration.

Other than that, making sure it builds independently (we have CI for that 👍), and checking the integration with the rest of the client apps (you've both done that) are the main things.

Can I somehow double-check it wasn't pulled/pushed using the buggy version of git?
Absolutely.

First, note that the buggy version of git fails when pushing, but not when pulling. So if the changes have been pushed to the login library, they're probably okay. We should check anyway, but just an FYI of what behavior to expect while this is still a problem with git 😔

Regardless, you can test it by then pulling these changes from a client app:

  1. First, make sure you're not running a buggy version of git.
  2. a. subtree pull the changes in this branch to a test branch of WPAndroid
    b. Or, if a branch already exists (in this case there's already Merge login lib changes and implement new interface methods  WordPress-Android#10150), branch off that one and subtree pull again
  3. In either 2a or 2b, if you have a safe git version and the changes were pushed/pulled using a broken version, the subtree pull will completely fail:
git subtree pull --prefix=libs/login loginlib develop --squash
remote: Enumerating objects: 1, done.
remote: Total 1 (delta 0), reused 0 (delta 0), pack-reused 1
Unpacking objects: 100% (1/1), done.
From github.com:wordpress-mobile/WordPress-Login-Flow-Android
 * branch                  develop    -> FETCH_HEAD
   78c5307f7a..6d486bbf6b  develop    -> loginlib/develop
fatal: ambiguous argument '979ddf4aec664cf9afafa1cdf7d99599488f1eee^0': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'
could not rev-parse split hash 979ddf4aec664cf9afafa1cdf7d99599488f1eee from commit 43aef9534664821893f4a65ac7fb52b2e8c01eee
Can't squash-merge: 'libs/login' was never added.

Otherwise, you should be okay.

(You can see where I did that to test an earlier PR that did use a bad git version: wordpress-mobile/WordPress-Android#10149 (comment))

Copy link
Contributor

@malinajirka malinajirka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the detailed instructions @aforcier ! ;)

All seems good to me, thanks @AmandaRiu ;)! 🚢

@malinajirka malinajirka merged commit 43ac7e9 into develop Jul 10, 2019
@malinajirka malinajirka deleted the merge-wca branch July 10, 2019 05:58
wzieba pushed a commit that referenced this pull request Oct 15, 2024
Merge Woo Signin M1 login flow changes
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants