Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Social signup #208

Merged
merged 10 commits into from
Feb 12, 2024
Merged

Social signup #208

merged 10 commits into from
Feb 12, 2024

Conversation

k1rill
Copy link
Contributor

@k1rill k1rill commented Jan 24, 2024

Added social auth buttons to the registration screen.

Social auth should be configured in the config.yaml before.

5296575775623890254.mp4

k1rill added 6 commits January 24, 2024 10:15
@k1rill
feat: social auth buttons on sign up
3bbde83
@k1rill
refactor: decrease subscriptions count in signup
6aa48ba
@k1rill
feat: social sign up
30f27da
@k1rill
Merge branch 'develop' into feature/social_signup
81935c2 
# Conflicts:
#	app/src/main/java/org/openedx/app/di/AppModule.kt
#	auth/src/main/java/org/openedx/auth/presentation/signin/compose/SignInView.kt
#	auth/src/main/java/org/openedx/auth/presentation/signup/SignUpFragment.kt
@k1rill
refactor: after merge
1225969
@k1rill
fix: tests
c3fc46f
@farhan-arshad-dev
Copy link
Contributor

Starting review.

@farhan-arshad-dev
Copy link
Contributor

@k1rill test cases are still failing.

@k1rill
Copy link
Contributor Author

k1rill commented Jan 26, 2024

@k1rill test cases are still failing.

@farhan-arshad-dev thank you for report. I just updated PR with tests fix and resolving conflicts

@volodymyr-chekyrta
Copy link
Contributor

I've started review

farhan-arshad-dev
farhan-arshad-dev requested changes Jan 30, 2024
View reviewed changes
Copy link
Contributor

@farhan-arshad-dev farhan-arshad-dev left a comment

Choose a reason for hiding this comment

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

Just some minor improvements. btw thank you for the well-written code. It was a pleasure to review. 🔥

auth/src/main/java/org/openedx/auth/presentation/signin/compose/SignInView.kt Show resolved Hide resolved
auth/src/main/java/org/openedx/auth/presentation/signup/SignUpViewModel.kt Outdated Show resolved Hide resolved
auth/src/main/java/org/openedx/auth/presentation/signup/SignUpViewModel.kt Outdated Show resolved Hide resolved
auth/src/main/java/org/openedx/auth/presentation/signup/SignUpViewModel.kt Show resolved Hide resolved
auth/src/main/java/org/openedx/auth/presentation/sso/GoogleAuthHelper.kt Show resolved Hide resolved
auth/src/test/java/org/openedx/auth/presentation/signup/SignUpViewModelTest.kt Outdated Show resolved Hide resolved
core/src/main/java/org/openedx/core/extension/ContinuationExt.kt Outdated Show resolved Hide resolved
auth/src/main/java/org/openedx/auth/presentation/signup/SignUpFragment.kt Outdated Show resolved Hide resolved
auth/src/main/java/org/openedx/auth/presentation/sso/OAuthHelper.kt Show resolved Hide resolved
auth/src/main/java/org/openedx/auth/presentation/signup/compose/SocialSignedView.kt Show resolved Hide resolved
@hassan6190
Copy link

hassan6190 commented Feb 9, 2024 •
edited
Loading

@k1rill @farhan-arshad-dev, I have tested out the social logins and they are working as expected apart from some of the following issues below

  • While login with a new user (Gmail), the user gets an error message “Something went wrong“ instead of “this user is not linked with any edX account“
  • When a user login or register through Facebook it shows 2 loaders at the same time

Note: Please see the attached video for the issues

Screen_Recording_20240209-173139_edX.mp4

@k1rill
Copy link
Contributor Author

k1rill commented Feb 11, 2024

@k1rill @farhan-arshad-dev, I have tested out the social logins and they are working as expected apart from some of the following issues below

  • While login with a new user (Gmail), the user gets an error message “Something went wrong“ instead of “this user is not linked with any edX account“
  • When a user login or register through Facebook it shows 2 loaders at the same time

Note: Please see the attached video for the issues

Screen_Recording_20240209-173139_edX.mp4

thank you @hassan6190, can you please open issues for this, this PR is for Sign Up pages only, sign in was merged in #89

@k1rill k1rill force-pushed the feature/social_signup branch from 7b1a6d7 to 76318e3 Compare February 12, 2024 07:58
@volodymyr-chekyrta
Copy link
Contributor

@k1rill @farhan-arshad-dev, I have tested out the social logins and they are working as expected apart from some of the following issues below

  • While login with a new user (Gmail), the user gets an error message “Something went wrong“ instead of “this user is not linked with any edX account“
  • When a user login or register through Facebook it shows 2 loaders at the same time

Note: Please see the attached video for the issues

Screen_Recording_20240209-173139_edX.mp4

It would be nice to hide the whole button section on data loading.
But this can be addressed separately.

@volodymyr-chekyrta volodymyr-chekyrta merged commit e6876be into openedx:develop Feb 12, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bilalawan321 bilalawan321 bilalawan321 left review comments

@miankhalid miankhalid miankhalid left review comments

@volodymyr-chekyrta volodymyr-chekyrta volodymyr-chekyrta approved these changes

@farhan-arshad-dev farhan-arshad-dev farhan-arshad-dev approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@k1rill @farhan-arshad-dev @volodymyr-chekyrta @hassan6190 @miankhalid @bilalawan321