wordpress-mobile / WordPress-Login-Flow-Android

Pluggable WordPress login flow for Android
GNU General Public License v2.0
14 stars 3 forks source link

Add redirect url support to gotConnectedSiteInfo #23

Closed AmandaRiu closed 4 years ago

AmandaRiu commented 4 years ago

This PR merges the changes in this WCAndroid PR: https://github.com/woocommerce/woocommerce-android/pull/1224 to support passing the redirectUrl back to the LoginListener. See the original PR for more information.

To Test

I've created a new branch in WPAndroid that includes the changes in this branch, along with any tweaks necessary to support these changes: amanda/login-lib-redirect-support, and opened a draft PR for testing those changes: https://github.com/wordpress-mobile/WordPress-Android/pull/10232

AmandaRiu commented 4 years ago

@malinajirka That's interesting. What branch were you using to pull in the develop branch of the login library into WPAndroid? I thought that step wouldn't be done until this PR had been approved and merged? I created a test branch in WP Android and a draft PR that I mentioned in the description of this PR - you can find it here.

AmandaRiu commented 4 years ago

FWIW I'm using git version 2.19.2 which should be okay.

Screen Shot 2019-07-22 at 11 21 32 AM

aforcier commented 4 years ago

@malinajirka @AmandaRiu yes that is a bad sign 🤔 I too would like to know which branch of WPAndroid you tried @malinajirka - I had tested this branch before and it seemed okay, and I just did these tests with no issue:

# On WPAndroid
git checkout develop
git pull
git checkout -b test-login-subtree-pull
git subtree pull --prefix=libs/login git@github.com:wordpress-mobile/WordPress-Login-Flow-Android.git develop --squash

that works fine. Also tried:

# On WPAndroid
git checkout amanda/login-lib-redirect-support
git pull
git checkout -b test-login-subtree-pull-amandas-branch
git subtree pull --prefix=libs/login git@github.com:wordpress-mobile/WordPress-Login-Flow-Android.git develop --squash

I also tried pulling in the branch of this PR instead of develop and that worked as well (though I see in your case Jirka you were pulling the login subtree's develop).

Incidentally which version of git are you using @malinajirka ?

malinajirka commented 4 years ago

Thanks for looking into this. I'm still not sure what is going on 😕.

I run the second set of steps yesterday. The first set works as expected.

git-error

I tried to clone the WPAndroid repo from scratch and I'm still getting the same error :(. I thought perhaps git has some kind of cache (which I don't think is true). However, I tried to run the same set of command on my old laptop (with git version 2.17.2) and I'm still getting the same error. I have never had the buggy version of git installed on the old laptop, so I'm positive it's not the issue.

It's really weird that it works for both of you and it doesn't work on neither of my laptops 😞.

I also tried the following

git checkout develop
git checkout -b create-new-branch-from-scratch
git subtree pull --prefix=libs/login git@github.com:wordpress-mobile/WordPress-Login-Flow-Android.git merge/woocommerce-android/1224-amanda-new --squash
git subtree pull --prefix=libs/login git@github.com:wordpress-mobile/WordPress-Login-Flow-Android.git develop --squash

And it worked without any issues 🤷‍♂ .

Any ideas?:)

malinajirka commented 4 years ago

Just for the record: I had to re-pull amanda's branch (merge/woocommerce-android/1224-amanda-new) from the subtree repo git subtree pull --prefix=libs/login git@github.com:wordpress-mobile/WordPress-Login-Flow-Android.git merge/woocommerce-android/1224-amanda-new --squash before pulling subtree's develop.

Here is the complete list of the commands I run in WPAndroid and it worked without any issues

git checkout amanda/login-lib-redirect-support
git pull
git checkout -b test-login-subtree-pull-amandas-branch

git subtree pull --prefix=libs/login git@github.com:wordpress-mobile/WordPress-Login-Flow-Android.git merge/woocommerce-android/1224-amanda-new --squash

git subtree pull --prefix=libs/login git@github.com:wordpress-mobile/WordPress-Login-Flow-Android.git develop --squash

So the mystery is resolved and this PR looks good to me. Sorry for the false alarm and thank you again for helping me out!