-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Refactor complex LoginController logic into a login chain of responsibility #15365
Conversation
f782905
to
17d6864
Compare
17d6864
to
c868ce2
Compare
From what I can tell CI failures do not look related. But tell me if I'm wrong. |
-> restarted |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just great. So much cleaner. And tests 🚀
I didn't test in debt but most stuff didn't blow up with my smoke testing.
} | ||
|
||
public function testNotTwoFactorAuthenticated() { | ||
$this->fail('todo'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
guess what … CI fails for a reason
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NO WAY!
Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
c868ce2
to
170582d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested and works 👍
This method has been on my list of code to refactor for a long time. As I now have to touch it for https://github.com/orgs/nextcloud/projects/29, I decided to break the code into smaller, maintainable and testable chunks. The code now follows a more strict single responsibility principle, while implementing the chain of responsibility pattern.