Bug 22444 - currencies_manage permission doesn't provide link to manage currencies when selected alone
Summary: currencies_manage permission doesn't provide link to manage currencies when s...
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Owen Leonard
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-03-01 21:00 UTC by Andrew Fuerste-Henry
Modified: 2019-06-06 17:54 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
19.05.00, 18.11.05


Attachments
Bug 22444: currencies_manage permission doesn't provide link to manage currencies when selected alone (2.28 KB, patch)
2019-03-06 19:13 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 22444: currencies_manage permission doesn't provide link to manage currencies when selected alone (2.34 KB, patch)
2019-04-04 09:41 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 22444: currencies_manage permission doesn't provide link to manage currencies when selected alone (2.41 KB, patch)
2019-04-09 08:09 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Fuerste-Henry 2019-03-01 21:00:58 UTC
Bug 7651 added the permission currencies_manage within the acquisitions permission subgroup. Giving a borrower just this permission and catalogue does make them technically able to load /currency.pl, but it doesn't provide them any link to get there.

To Test:
- create a patron
- give them currencies_manage and catalogue permissions
- log in as this patron
- observe that there is no Currencies link out of Acquisitions and Administration doesn't appear on the home page or the More menu
- give the patron any other admin permission
- observe that the Currencies link now shows in Acquisitions
- take away admin permissions and give the patron full acquisitions permissions
- observe that a link to Currencies still appears in Acquisitions
Comment 1 Jonathan Druart 2019-03-04 00:41:14 UTC
I guess this apply for other permissions as well. It seems tedious to handle that case.
Comment 2 Owen Leonard 2019-03-06 19:13:15 UTC
Created attachment 86212 [details] [review]
Bug 22444: currencies_manage permission doesn't provide link to manage currencies when selected alone

This patch modifies the acquisitions sidebar menu so that it checks
properly for the currencies_manage permission.

This patch also removes the menu link directly to the administration
home page. Since we're removing the check on the general
"CAN_user_parameters" permission it doesn't make sense to assume the
page is accessible.

To test, apply the patch and modifiy a staff user so that they have no
"parameters" permissions but do have "currencies_manage" permission.

Go to the Acquisitions home page. There should be a link to currencies
management in the sidebar menu.

Perform the same test with a user who has "parameters" permissions but
not "currencies_manage" permission. There should be no link to
currencies in the acquisitions sidebar menu. The user should have access
to the administrative home page.
Comment 3 Owen Leonard 2019-03-06 19:17:11 UTC
(In reply to Andrew from comment #0)

> Administration doesn't appear on the home page or the More menu

Since all acquisitions-related management pages can be accessed from the acquisitions home page I don't think it's necessary to be concerned about the lack of a link to the administration home page.
Comment 4 Katrin Fischer 2019-04-04 09:41:36 UTC
Created attachment 87404 [details] [review]
Bug 22444: currencies_manage permission doesn't provide link to manage currencies when selected alone

This patch modifies the acquisitions sidebar menu so that it checks
properly for the currencies_manage permission.

This patch also removes the menu link directly to the administration
home page. Since we're removing the check on the general
"CAN_user_parameters" permission it doesn't make sense to assume the
page is accessible.

To test, apply the patch and modifiy a staff user so that they have no
"parameters" permissions but do have "currencies_manage" permission.

Go to the Acquisitions home page. There should be a link to currencies
management in the sidebar menu.

Perform the same test with a user who has "parameters" permissions but
not "currencies_manage" permission. There should be no link to
currencies in the acquisitions sidebar menu. The user should have access
to the administrative home page.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 5 Katrin Fischer 2019-04-04 09:42:38 UTC
Thx Owen!
Comment 6 Martin Renvoize 2019-04-09 08:09:29 UTC
Created attachment 87583 [details] [review]
Bug 22444: currencies_manage permission doesn't provide link to manage currencies when selected alone

This patch modifies the acquisitions sidebar menu so that it checks
properly for the currencies_manage permission.

This patch also removes the menu link directly to the administration
home page. Since we're removing the check on the general
"CAN_user_parameters" permission it doesn't make sense to assume the
page is accessible.

To test, apply the patch and modifiy a staff user so that they have no
"parameters" permissions but do have "currencies_manage" permission.

Go to the Acquisitions home page. There should be a link to currencies
management in the sidebar menu.

Perform the same test with a user who has "parameters" permissions but
not "currencies_manage" permission. There should be no link to
currencies in the acquisitions sidebar menu. The user should have access
to the administrative home page.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 7 Martin Renvoize 2019-04-09 08:09:57 UTC
No regressions found and the change makes sense to me... Passing QA
Comment 8 Nick Clemens 2019-04-11 13:57:07 UTC
Awesome work all!

Pushed to master for 19.05
Comment 9 Martin Renvoize 2019-04-16 13:02:05 UTC
Pushed to 18.11.x for 18.11.05
Comment 10 Lucas Gass 2019-04-19 16:40:15 UTC
backported to 18.05.x for 18.05.12