-
Notifications
You must be signed in to change notification settings - Fork 17
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
[OP#49621] fix app password form becomes inactive #456
[OP#49621] fix app password form becomes inactive #456
Conversation
568db26
to
a5666f2
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.
Is is possible to cover that with unit tests?
Also we can add API test for it as well. @SwikritiT |
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.
I checked it manually and seems to be working more fine now.
Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com>
5d23965
to
d29b991
Compare
Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com>
d29b991
to
5137f8f
Compare
Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com>
2613180
to
dea267c
Compare
@SagarGi @individual-it I added the tests can you review it again please |
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.
LGTM 👍
@@ -1739,6 +1744,36 @@ public function testIsSystemReadyForGroupFolderSetUpUserOrGroupExistsException( | |||
$service->isSystemReadyForProjectFolderSetUp(); | |||
} | |||
|
|||
public function testProjectFolderHasAppPassword(): void { |
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.
lets add also a test where multiple tokens are returned but only one is an OpenProject token.
and what happens if multiple tokens have the name OpenProject
?
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.
If there are multiple OpenProject tokens then hasAppPassword
will return true
but while deleting it will delete all the app passwords before creating a new one. Is that okay or do we want to check if there's only one token named OpenProject
and if there are multiple then send an error message?
Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com>
0e6406b
to
bb7f8f7
Compare
Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com>
bb7f8f7
to
7c44315
Compare
JS Code CoverageCoverage after merging bug/49621-openproject-form-becomes-inactive into release/2.4 will be
Coverage Report
|
* Check and remove the app password with name OpenProject Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * add phpunit test Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * add api test to cover the bug Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * add unit test for multiple token Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * add unit test for delete token Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> --------- Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com>
* Check and remove the app password with name OpenProject Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * add phpunit test Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * add api test to cover the bug Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * add unit test for multiple token Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * add unit test for delete token Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> --------- Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com>
* [OP#48106] use same component as project tab for searching in smart picker (#448) * reuse component Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> reuse Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> emit event Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> add op icon vue Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * fix failing tests Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * check for access token Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * add unit test Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * address review Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> --------- Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * Fix the lable of managed project folder (#452) Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * [OP#49182] Fix the form heading index colour in different themes (#453) * Fix the colour of the indexes in different themes Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * update snapshots Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * fix style lint Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> --------- Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * [OP#49621] fix app password form becomes inactive (#456) * Check and remove the app password with name OpenProject Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * add phpunit test Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * add api test to cover the bug Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * add unit test for multiple token Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * add unit test for delete token Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> --------- Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * [OP#49435] Nextcloud oauth section should not be disabled when the user deletes the oauth credentials (#457) * [#49435] Nextcloud oauth section should not be disabled when the user deletes the oauth credentials https://community.openproject.org/work_packages/49435 Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * fix project folder form Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * fix and add unit tests Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * fix the feature Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> --------- Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * Fix link previewd Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * fix link on front end and fix test Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * fix failing php unit test and add more patterns' Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * add php test Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * ignore path Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * Support global work package URL without split view Signed-off-by: Wieland Lindenthal <w.lindenthal@forkmerge.com> --------- Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> Signed-off-by: Wieland Lindenthal <w.lindenthal@forkmerge.com> Co-authored-by: Wieland Lindenthal <w.lindenthal@forkmerge.com>
* [OP#48106] use same component as project tab for searching in smart picker (#448) * reuse component Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> reuse Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> emit event Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> add op icon vue Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * fix failing tests Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * check for access token Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * add unit test Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * address review Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> --------- Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * Fix the lable of managed project folder (#452) Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * [OP#49182] Fix the form heading index colour in different themes (#453) * Fix the colour of the indexes in different themes Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * update snapshots Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * fix style lint Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> --------- Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * [OP#49621] fix app password form becomes inactive (#456) * Check and remove the app password with name OpenProject Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * add phpunit test Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * add api test to cover the bug Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * add unit test for multiple token Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * add unit test for delete token Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> --------- Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * [OP#49435] Nextcloud oauth section should not be disabled when the user deletes the oauth credentials (#457) * [#49435] Nextcloud oauth section should not be disabled when the user deletes the oauth credentials https://community.openproject.org/work_packages/49435 Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * fix project folder form Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * fix and add unit tests Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * fix the feature Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> --------- Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * Fix link previewd Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * fix link on front end and fix test Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * fix failing php unit test and add more patterns' Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * add php test Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * ignore path Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> * Support global work package URL without split view Signed-off-by: Wieland Lindenthal <w.lindenthal@forkmerge.com> --------- Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com> Signed-off-by: Wieland Lindenthal <w.lindenthal@forkmerge.com> Co-authored-by: Wieland Lindenthal <w.lindenthal@forkmerge.com>
Fixes: https://community.openproject.org/projects/nextcloud-integration/work_packages/49621/
Our previous check for
hasAppPassword
expects the user "OpenProject" to have only one app password. But in cases where the user "OpenProject" has multiple app passwords for instance if the user is logged in somewhere it will create a session app password, this check will returnfalse
which will result in the project folder section in the administrative form being disabled.So in this PR we loop through all the tokens and only return true if the token with the name "OpenProject" exists and also while deleting, delete all the tokens with the name "OpenProject". This fixes the issue we are facing