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.

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

Separate OmniAuth controller from Account controller #16552

Merged
merged 6 commits into from
Sep 20, 2024

Conversation

oliverguenther
Copy link
Member

@oliverguenther oliverguenther commented Aug 29, 2024

What are you trying to accomplish?

In preparation to add more functionality to the omniauth routes, specifically a setup route, I wanted to separate out the omniauth specific functionality from the concern mounted to AccountController into its own controller.

https://community.openproject.org/work_packages/57951

@oliverguenther oliverguenther marked this pull request as draft August 29, 2024 12:00
@oliverguenther oliverguenther force-pushed the chore/omniauth-setup-controller branch 4 times, most recently from e23570c to 1997173 Compare September 17, 2024 12:52
@oliverguenther oliverguenther force-pushed the chore/omniauth-setup-controller branch from 1997173 to e96b53a Compare September 18, 2024 06:18
@oliverguenther oliverguenther marked this pull request as ready for review September 18, 2024 11:02
@machisuji machisuji self-assigned this Sep 18, 2024
Copy link
Member

@machisuji machisuji left a comment

Choose a reason for hiding this comment

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

Looks good to me in general. There is just a single remark about the omniauth failure error message display. Could you please have a look?

@oliverguenther oliverguenther force-pushed the chore/omniauth-setup-controller branch from e96b53a to 982ce8d Compare September 20, 2024 08:25
@oliverguenther oliverguenther merged commit dea38d4 into dev Sep 20, 2024
11 checks passed
@oliverguenther oliverguenther deleted the chore/omniauth-setup-controller branch September 20, 2024 09:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants