Bug 7651 - Add separate permission for managing currencies and exchange rates
Summary: Add separate permission for managing currencies and exchange rates
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Katrin Fischer
QA Contact:
URL:
Keywords:
: 14499 (view as bug list)
Depends on: 3849
Blocks: 14391
  Show dependency treegraph
 
Reported: 2012-03-06 08:48 UTC by Katrin Fischer
Modified: 2020-11-12 15:04 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 7651: Add a new permission for managing currencies and exchange rates (7.84 KB, patch)
2018-05-20 21:58 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 7651: Add a new permission for managing currencies and exchange rates (7.59 KB, patch)
2018-05-20 22:19 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 7651: (follow-up) Correct visibility on admin sidebar (3.64 KB, patch)
2018-05-24 06:19 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 7651: Add a new permission for managing currencies and exchange rates (7.66 KB, patch)
2018-05-25 21:53 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 7651: (follow-up) Correct visibility on admin sidebar (3.70 KB, patch)
2018-05-25 21:53 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 7651: (QA follow-up) Fix warning about undefined active currency (1.23 KB, patch)
2018-06-04 19:20 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 7651: Add a new permission for managing currencies and exchange rates (7.72 KB, patch)
2018-06-05 17:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 7651: (follow-up) Correct visibility on admin sidebar (3.78 KB, patch)
2018-06-05 17:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 7651: (QA follow-up) Fix warning about undefined active currency (1.31 KB, patch)
2018-06-05 17:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 7651: put TT directive on a single line to help translation (1.33 KB, patch)
2018-06-05 17:55 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2012-03-06 08:48:56 UTC
Currently it's not possible to update exchange rates in acquisitions without giving the user 'paramters' permission, which means access to a lot of administration parameters. 

I suggest following change:

Parameters:
- change active currency
- add new currencies (?)

New permission: acquisition > manage_currencies 
- update exchange rates for existing currencies
- add new currencies (?)

Not sure about adding new currencies. Any opinions on this? Is it a good idea? Should it work differently?
Comment 1 Katrin Fischer 2014-09-19 14:05:52 UTC
This is still a problem currently, as you need full administration access in order to update currency exchange rates.
Comment 2 Katrin Fischer 2015-05-25 10:37:24 UTC
Hm, I'd like to work on this, but not sure about 2 things:

- Do we really need to separate adding and editing?
- Should the new permission(s) be under acquisitions or under administration?
Comment 3 Jonathan Druart 2015-05-26 09:13:43 UTC
(In reply to Katrin Fischer from comment #2)
> Hm, I'd like to work on this, but not sure about 2 things:
> 
> - Do we really need to separate adding and editing?

I would say no.

> - Should the new permission(s) be under acquisitions or under administration?

Heh :)
Since budget permissions are listed under acq, I would say acq.
Comment 4 Katrin Fischer 2015-07-07 12:02:20 UTC
*** Bug 14499 has been marked as a duplicate of this bug. ***
Comment 5 Katrin Fischer 2018-05-20 21:58:37 UTC Comment hidden (obsolete)
Comment 6 Katrin Fischer 2018-05-20 22:19:17 UTC
Created attachment 75452 [details] [review]
Bug 7651: Add a new permission for managing currencies and exchange rates

At the moment staff users need parameters or parameters_remaining_permissions
in order to be able to change exchange rates for acquisition orders.

This patch adds a new separate permission currencies_manage and
updates staff users currently having those permissions to get the
new permission as well.

To test:
- Create some staff users with different permission sets
  1) superlibrarian
  2) parameters
  3) parameters_remaining_permissions
  4) manage_circ_rules, but not parmeters_remaining_permissions
  5) all acquisition permissions
- Apply patch and run database update
- Verify new permission has been added and staff users updated
  1) remains the same
  2) + 3) will have currencies_manage
  4) remains unchanged, doesn't have new permission
  5) remains the same, will have access now because of having
     the top level acquisition permission
- Verify the changed pages work correctly:
  - navigation on admin home page
    NOTE: the acquisition parameters section will now honor all
    different related permissions (edi_manage, budget_manage,...)
  - navigation on acquisition home page
  - try to access currencies page directly

https://bugs.koha-community.org/show_bug.cgi?id=3849
Comment 7 Owen Leonard 2018-05-22 15:53:58 UTC
>   2) parameters
>   3) parameters_remaining_permissions
>   4) manage_circ_rules, but not parmeters_remaining_permissions

Testing these different users, I don't see the acquisitions-related links on the Adminstration home page but I do see them in the sidebar when I view an administration page which I have permission to view.

(Not related to this patch, but I see that with only manage_circ_rules set, the user sees the same Administration home page links as the user above even though they only have access to circ rules).

>   5) all acquisition permissions

Testing with this user has a similar result: The administration sidebar menu shows links I don't have access to. On the other hand, I'm prevented from seeing the Administration home page altogether even though I should be able to reach Acquisitions preferences from there.
Comment 8 Katrin Fischer 2018-05-22 16:07:16 UTC
Ah yes, I will make administration start page and administration links in acq consistent! Thx for testing!
Comment 9 Katrin Fischer 2018-05-24 06:19:26 UTC
Created attachment 75517 [details] [review]
Bug 7651: (follow-up) Correct visibility on admin sidebar

This patch corrects visibility of links to the different
acquisition pages from the admin sidebar.

It also makes a correction to the links on the acq start
page:
- Budgets = period_manage
- Funds = budget_manage

To test:
- Follow the test plan of the first patch
Comment 10 Charles Farmer 2018-05-25 21:53:53 UTC
Created attachment 75573 [details] [review]
Bug 7651: Add a new permission for managing currencies and exchange rates

At the moment staff users need parameters or parameters_remaining_permissions
in order to be able to change exchange rates for acquisition orders.

This patch adds a new separate permission currencies_manage and
updates staff users currently having those permissions to get the
new permission as well.

To test:
- Create some staff users with different permission sets
  1) superlibrarian
  2) parameters
  3) parameters_remaining_permissions
  4) manage_circ_rules, but not parmeters_remaining_permissions
  5) all acquisition permissions
- Apply patch and run database update
- Verify new permission has been added and staff users updated
  1) remains the same
  2) + 3) will have currencies_manage
  4) remains unchanged, doesn't have new permission
  5) remains the same, will have access now because of having
     the top level acquisition permission
- Verify the changed pages work correctly:
  - navigation on admin home page
    NOTE: the acquisition parameters section will now honor all
    different related permissions (edi_manage, budget_manage,...)
  - navigation on acquisition home page
  - try to access currencies page directly

https://bugs.koha-community.org/show_bug.cgi?id=3849

Signed-off-by: Charles Farmer <charles.farmer@inLibro.com>
Comment 11 Charles Farmer 2018-05-25 21:53:58 UTC
Created attachment 75574 [details] [review]
Bug 7651: (follow-up) Correct visibility on admin sidebar

This patch corrects visibility of links to the different
acquisition pages from the admin sidebar.

It also makes a correction to the links on the acq start
page:
- Budgets = period_manage
- Funds = budget_manage

To test:
- Follow the test plan of the first patch

Signed-off-by: Charles Farmer <charles.farmer@inLibro.com>
Comment 12 Charles Farmer 2018-05-25 21:59:07 UTC
Went through the test plan with all 5 users and their respective permissions.

Only the fourth account, manage_circ_rules sans parmeters_remaining_permissions, couldn't access the 'currencies' page after update. Every other account gain access to 'Acquisitions' page, as described in the test plan.

With the newest follow-up patch, I only saw two hyperlinks, 'Acqui home' and 'Currencies', when using accounts with restrictive permission sets.

Signed Off
Comment 13 Jonathan Druart 2018-05-30 20:15:12 UTC
From koha-tmpl/intranet-tmpl/prog/en/includes/budgets-active-currency.inc:

  8         [% IF CAN_user_parameters_parameters_remaining_permissions %]
  9             <p><a href="/cgi-bin/koha/admin/currency.pl">Please specify an active currency.</a></p>
Comment 14 Katrin Fischer 2018-06-04 19:20:59 UTC
Created attachment 75788 [details] [review]
Bug 7651: (QA follow-up) Fix warning about undefined active currency

If no active currency is defined we show a warning. This fixes
the link shown below to check the correct permission.
Comment 15 Katrin Fischer 2018-06-04 19:21:35 UTC
Thx for catching this, Joubu!
Comment 16 Jonathan Druart 2018-06-05 17:55:21 UTC
Created attachment 75825 [details] [review]
Bug 7651: Add a new permission for managing currencies and exchange rates

At the moment staff users need parameters or parameters_remaining_permissions
in order to be able to change exchange rates for acquisition orders.

This patch adds a new separate permission currencies_manage and
updates staff users currently having those permissions to get the
new permission as well.

To test:
- Create some staff users with different permission sets
  1) superlibrarian
  2) parameters
  3) parameters_remaining_permissions
  4) manage_circ_rules, but not parmeters_remaining_permissions
  5) all acquisition permissions
- Apply patch and run database update
- Verify new permission has been added and staff users updated
  1) remains the same
  2) + 3) will have currencies_manage
  4) remains unchanged, doesn't have new permission
  5) remains the same, will have access now because of having
     the top level acquisition permission
- Verify the changed pages work correctly:
  - navigation on admin home page
    NOTE: the acquisition parameters section will now honor all
    different related permissions (edi_manage, budget_manage,...)
  - navigation on acquisition home page
  - try to access currencies page directly

Signed-off-by: Charles Farmer <charles.farmer@inLibro.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 17 Jonathan Druart 2018-06-05 17:55:27 UTC
Created attachment 75826 [details] [review]
Bug 7651: (follow-up) Correct visibility on admin sidebar

This patch corrects visibility of links to the different
acquisition pages from the admin sidebar.

It also makes a correction to the links on the acq start
page:
- Budgets = period_manage
- Funds = budget_manage

To test:
- Follow the test plan of the first patch

Signed-off-by: Charles Farmer <charles.farmer@inLibro.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 18 Jonathan Druart 2018-06-05 17:55:32 UTC
Created attachment 75827 [details] [review]
Bug 7651: (QA follow-up) Fix warning about undefined active currency

If no active currency is defined we show a warning. This fixes
the link shown below to check the correct permission.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 19 Jonathan Druart 2018-06-05 17:55:38 UTC
Created attachment 75828 [details] [review]
Bug 7651: put TT directive on a single line to help translation

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 20 Nick Clemens 2018-07-18 19:13:27 UTC
Awesome work all!

Pushed to master for 18.11
Comment 21 Martin Renvoize 2018-08-02 13:30:05 UTC
Enhancement, will not be backported to 18.05.x series.