-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
Make ChangePassword a real Controller #868
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
Conversation
|
@rullzer, thanks for your PR! By analyzing the annotation information on this pull request, we identified @Kondou-ger, @schiessle and @LukasReschke to be potential reviewers |
| /** @var IAppManager */ | ||
| private $appManager; | ||
|
|
||
| public function __construct($appName, |
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.
PHPDoc? ;)
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.
Done
|
Mind adding some tests for the stuff that's testable? (e.g. changePersonalPassword) |
f17928e to
5c7b188
Compare
|
Added some tests |
|
LGTM |
| ], | ||
| ]); | ||
| } | ||
| } No newline at end of file |
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.
Fix your IDE finally
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.
fixed
|
Changing still works as user, admin and subadmin 👍 |
Please rebase while fixing the typo from above |
* Still no full DI because of encryption fu * Remove old "Controller"
5c7b188 to
789082e
Compare
|
And rebased |
Make ChangePassword a real Controller
|
There was a regression: #1634 fixed it ;) |
Less depreacted calls 🎉
This will get us a step closer to properly changepassword stuff.
CC: @LukasReschke @nickvergessen @MorrisJobke @icewind1991