Bug 36507 - Cannot set desk or register as non superlibrarian
Summary: Cannot set desk or register as non superlibrarian
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff interface (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Lucas Gass
QA Contact: Testopia
URL:
Keywords: additional_work_needed
Depends on:
Blocks: 36572
  Show dependency treegraph
 
Reported: 2024-04-03 20:30 UTC by Lucas Gass
Modified: 2024-04-11 08:54 UTC (History)
2 users (show)

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


Attachments
Bug 36507: Allow non-superlibrarians to set desk and register (4.02 KB, patch)
2024-04-03 21:49 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 36507: Allow non-superlibrarians to set desk and register (4.07 KB, patch)
2024-04-03 23:15 UTC, David Nind
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Lucas Gass 2024-04-03 20:30:14 UTC
To recreate:

1. Turn on UseCirculationDesks
2. Create some desks
3. Login as a non-superlibrarian
4. Try to switch desks 


Undefined subroutine &C4::Context::IsSuperLibraian called at /kohadevbox/koha/circ/set-library.pl line 54
Comment 1 Lucas Gass 2024-04-03 20:34:13 UTC
Typo: IsSuperLibraian/IsSuperLibrarian
Comment 2 Lucas Gass 2024-04-03 20:36:24 UTC
N(In reply to Lucas Gass from comment #1)
> Typo: IsSuperLibraian/IsSuperLibrarian

Not just a typo though, with that fixed I am still unable to set a desk as a non-superlibrarian
Comment 3 Lucas Gass 2024-04-03 21:49:32 UTC
Created attachment 164395 [details] [review]
Bug 36507: Allow non-superlibrarians to set desk and register

To test:

1. Turn on UseCirculationDesks
2. Create some desks
3. Login as a non-superlibrarian
4. Try to switch desks, get an error

5. Turn on UseCashRegisters
6. Create some registers
7. As a non-superlibrarian try to change your register, get an error.

8. APPLY PATCH and restart_all
9. Try changing your desk and register again, it should now work.
Comment 4 David Nind 2024-04-03 23:15:16 UTC
Created attachment 164396 [details] [review]
Bug 36507: Allow non-superlibrarians to set desk and register

To test:

1. Turn on UseCirculationDesks
2. Create some desks
3. Login as a non-superlibrarian
4. Try to switch desks, get an error

5. Turn on UseCashRegisters
6. Create some registers
7. As a non-superlibrarian try to change your register, get an error.

8. APPLY PATCH and restart_all
9. Try changing your desk and register again, it should now work.

Signed-off-by: David Nind <david@davidnind.com>
Comment 5 Pedro Amorim 2024-04-04 09:52:49 UTC
Existing patch is good and fixes what's supposed to fix. But I think additional work is required here:

When you set a desk and save, if you go back to the set desk screen, you'll notice the pre-selected value is not the one you just saved to.

I believe this is caused by something wrong happening in desk_selection.js. If you apply the below changes, the issue I mention above gets fixed (but I'm unsure what it breaks - if anything):

+++ b/koha-tmpl/intranet-tmpl/prog/js/desk_selection.js
@@ -2,7 +2,7 @@ $(document).ready(function() {
     $("#desk_id").children().each(function() {
         var selectedBranch = $("#branch"). children("option:selected"). val();
         if ($(this).attr('id') === "nodesk") { //set no desk by default, should be first element
-            $(this).prop("selected", true);
+            // $(this).prop("selected", true);
             $(this).prop("disabled", false);
             $(this).show();
         }
@@ -11,7 +11,7 @@ $(document).ready(function() {
             $('#nodesk').hide();
             $(this).prop("disabled", false);
             $(this).show();
-            $(this).prop("selected", true)
+            // $(this).prop("selected", true)
         } else {
         
         
---- FURTHER TESTING ----

Delete a desk:
/cgi-bin/koha/admin/desks.pl?op=delete_confirm&desk_id=1

Confirmation screen has all blank data

----

Following the above, click confirm delete, you get:
An error occurred when deleting this desk. Check the logs for details.

----

Edit an existing desk:
/cgi-bin/koha/admin/desks.pl?op=add_form&desk_id=1

You're unable to change library (unsure if this is intended or not)

-----

Some of the above may be considered their own separate bugs (the delete and edit(?) issues), but I feel like at least the first issue (wrong pre-selected desk) should be fixed here. Let me know if you disagree, and we can turn this to PQA and fix the others in their own bugs.
Comment 6 Pedro Amorim 2024-04-04 10:58:27 UTC
Another note: circulate permission is not required for the staff member to set a desk/cash register, but upon successful update the patron is redirected to /cgi-bin/koha/circ/circulation.pl and that page does require the circulate permission, i.e. patron is presented with a 403 screen after successfully setting a desk/cash_register in this specific use case.
Comment 7 Lucas Gass 2024-04-04 14:44:16 UTC
(In reply to Pedro Amorim from comment #5)
> Existing patch is good and fixes what's supposed to fix. But I think
> additional work is required here:
> 
> When you set a desk and save, if you go back to the set desk screen, you'll
> notice the pre-selected value is not the one you just saved to.
> 
> I believe this is caused by something wrong happening in desk_selection.js.
> If you apply the below changes, the issue I mention above gets fixed (but
> I'm unsure what it breaks - if anything):

Pedro, have a look at Bug 30123, there is work been done to fix this particular issue there:

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30123
Comment 8 Pedro Amorim 2024-04-04 15:01:09 UTC
(In reply to Lucas Gass from comment #7)
> (In reply to Pedro Amorim from comment #5)
> > Existing patch is good and fixes what's supposed to fix. But I think
> > additional work is required here:
> > 
> > When you set a desk and save, if you go back to the set desk screen, you'll
> > notice the pre-selected value is not the one you just saved to.
> > 
> > I believe this is caused by something wrong happening in desk_selection.js.
> > If you apply the below changes, the issue I mention above gets fixed (but
> > I'm unsure what it breaks - if anything):
> 
> Pedro, have a look at Bug 30123, there is work been done to fix this
> particular issue there:
> 
> https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30123

Hey, cool! Thanks.
Should we PQA this one (36507) then and address the other issues in different bugs? i.e. "cannot delete a desk" and/or the others if they are indeed bugs?
Comment 9 Katrin Fischer 2024-04-05 09:09:01 UTC
I feel like this bug survived way too long undetected.
Comment 10 Katrin Fischer 2024-04-05 09:11:44 UTC
Please file bugs for the other issues detected!
Comment 11 Katrin Fischer 2024-04-05 09:53:22 UTC
Pushed for 24.05!

Well done everyone, thank you!