[Bug 30123] New: On set library page, desk always defaults to last in list instead of desk user is signed in at
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30123 Bug ID: 30123 Summary: On set library page, desk always defaults to last in list instead of desk user is signed in at Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: minor Priority: P5 - low Component: Staff Client Assignee: koha-bugs@lists.koha-community.org Reporter: lucas@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com TO recreate: -Turn on UseCirculationDesks -Create several desks for the library you are logged in as -Set your desk to one you just created -Reload set-library.pl and notice that no matter what desk you are set as the dropdown defaults to the last one in the list It should default to your currently logged in desk -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30123 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com Depends on| |24201 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24201 [Bug 24201] Attach desk to intranet session -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30123 Nicolas Legrand <nicolas.legrand@bulac.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nicolas.legrand@bulac.fr -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30123 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |blawlor@clamsnet.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30123 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch Status|NEW |Needs Signoff -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30123 --- Comment #1 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 164389 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164389&action=edit Bug 30123: Set desk default to logged-in-desk-id on set library page This patch refactors the javascript that runs on the set library page. If the logged in user branch code is the same as the seleceted library, default the desk selection to the current logged in desk id. Otherwise set the default desk selection to the first option for the current branch. Test plan: 1. Turn on UseCirculationDesks 2. Create a few desks with unique names for a few different libraries 3. Set your library and desk 4. Reload set-library.pl and notice that the desk always defaults to the last option for the library selected 5. Apply patch and restart_all 6. Reload set-library.pl and notice that the desk selection defaults to your current logged in desk 7. Try changing the library selection to libraries with and without desks 8. Notice that the desk selection defaults to the first option available for the selected library 9. Notice that the desk selection defaults to the --- no desk for libraries without desks 10. Notice the desk selection defaults to the currenty logged in desk if you select the library you are currently logged into -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30123 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |blawlor@clamsnet.org |ity.org | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30123 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #164389|0 |1 is obsolete| | --- Comment #2 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 164390 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164390&action=edit Bug 30123: Set desk default to logged-in-desk-id on set library page This patch refactors the javascript that runs on the set library page. If the logged in user branch code is the same as the seleceted library, default the desk selection to the current logged in desk id. Otherwise set the default desk selection to the first option for the current branch. Test plan: 1. Turn on UseCirculationDesks 2. Create a few desks with unique names for a few different libraries 3. Set your library and desk 4. Reload set-library.pl and notice that the desk always defaults to the last option for the library selected 5. Apply patch and restart_all 6. Reload set-library.pl and notice that the desk selection defaults to your current logged in desk 7. Try changing the library selection to libraries with and without desks 8. Notice that the desk selection defaults to the first option available for the selected library 9. Notice that the desk selection defaults to the --- no desk for libraries without desks 10. Notice the desk selection defaults to the currenty logged in desk if you select the library you are currently logged into -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30123 --- Comment #3 from Brendan Lawlor <blawlor@clamsnet.org> --- For some reason when I made this commit the code got autoformatted in a way that I don't really like. The code still works, but could be nicer to read if the whitespace is fixed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30123 --- Comment #4 from Lucas Gass <lucas@bywatersolutions.com> --- Thanks for your work here Brendan! I have two questions: 1. Do we need all this JavaScript in desk_selection.js? Can't we just use what is already in set-library.tt? [% PROCESS options_for_desks desks => Desks.all(), selected => desk_id %] 2. Are there any circumstances where a user should be able to unset their desk? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30123 --- Comment #5 from Brendan Lawlor <blawlor@clamsnet.org> --- (In reply to Lucas Gass from comment #4) These are some heavy questions so I'm in a little over my head but I'll try.
1. Do we need all this JavaScript in desk_selection.js? Can't we just use what is already in set-library.tt?
[% PROCESS options_for_desks desks => Desks.all(), selected => desk_id %]
Maybe the javascript could be reduced, but as is I think it's required for this behavior. The template renders an option element for each desk and just sets properties to selected, hidden and disabled. The javascript listens for the user to change the set library selection and then updates hidden and disabled properties on the options to basically filter the list. You can see how PROCESS options_for_desks creates all the options in html_helpers.inc I guess it might be possible to reduce the javascript here if set-library.pl was reloaded every time the set library selection is changed. I'm not sure how Koha usually handles this sort of thing, but I'm interested in learning.
2. Are there any circumstances where a user should be able to unset their desk?
This is a quirk that may also need to be addressed. The current behavior is that once a desk is set, it can't be unset to No desk. The javascript currently hides/disables the no desk <option id="nodesk" value="">---</option> when a library has desks, so it forces the user to select a desk. But if you change the javascript and submit the form with the desk value '' the page just reloads but nothing happens. I think there needs to be an update to set-library.pl to handle that no desk value as an update. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30123 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |pedro.amorim@ptfs-europe.co | |m -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30123 --- Comment #6 from Lucas Gass <lucas@bywatersolutions.com> --- (In reply to Brendan Lawlor from comment #5)
(In reply to Lucas Gass from comment #4)
These are some heavy questions so I'm in a little over my head but I'll try.
1. Do we need all this JavaScript in desk_selection.js? Can't we just use what is already in set-library.tt?
[% PROCESS options_for_desks desks => Desks.all(), selected => desk_id %]
Maybe the javascript could be reduced, but as is I think it's required for this behavior. The template renders an option element for each desk and just sets properties to selected, hidden and disabled. The javascript listens for the user to change the set library selection and then updates hidden and disabled properties on the options to basically filter the list. You can see how PROCESS options_for_desks creates all the options in html_helpers.inc
I guess it might be possible to reduce the javascript here if set-library.pl was reloaded every time the set library selection is changed. I'm not sure how Koha usually handles this sort of thing, but I'm interested in learning.
Thanks, for the detailed answer. I see that we need this JS. :)
2. Are there any circumstances where a user should be able to unset their desk?
This is a quirk that may also need to be addressed. The current behavior is that once a desk is set, it can't be unset to No desk. The javascript currently hides/disables the no desk <option id="nodesk" value="">---</option> when a library has desks, so it forces the user to select a desk. But if you change the javascript and submit the form with the desk value '' the page just reloads but nothing happens. I think there needs to be an update to set-library.pl to handle that no desk value as an update.
We should deal with this in a follow-up bug, I think. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30123 Andrew Auld <andrew.auld@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andrew.auld@ptfs-europe.com Status|Needs Signoff |Failed QA --- Comment #7 from Andrew Auld <andrew.auld@ptfs-europe.com> --- I applied this patch in a sandbox. In testing I noticed the following: I had applied the patch on a sandbox on which I had previously applied patch 36096. I don't know if this is relevant but thought I would mention. Once the patch was applied, I created the following desks: desk1 desk2 desk3 desk4 The system would allow me to set the desk on login, but not change it on the 'setdesk.pl' screen. Only by logging off and setting it again at login could I change the desk. On 'setdesk.pl' it did default to the currently set desk, rather than the last one in the list. So that bit seems to work. Thinking that it might be a conflict with the other bug I had applied to the sandbox, I loaded a new sandbox just with this patch applied. This time I created desks as before. Before I started testing, I inadvertently created a duplicate desk (desk1) on the same library branch. I tried to delete it, but it created an error and couldn't delete the desk. The confirmation of deletion screen didn't have any data under desk ID, desk and library, so this is probably why. Again, it would only allow me to set the desk on logout, login. Not on the setdesk.pl screen. Hope this is useful. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30123 --- Comment #8 from Lucas Gass <lucas@bywatersolutions.com> --- Thanks Andrew! I think if we apply this one on top of Bug 36572 it will work? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30123 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |36572 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36572 [Bug 36572] Cleanup the set library page and avoid extra confirmation step -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30123 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |36582 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36582 [Bug 36582] Add option to set library, desk, and register from user menu -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30123 --- Comment #9 from Owen Leonard <oleonard@myacpl.org> --- Why trigger change() after the on("change") event? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30123 --- Comment #10 from Brendan Lawlor <blawlor@clamsnet.org> --- (In reply to Owen Leonard from comment #9)
Why trigger change() after the on("change") event?
The javascript currently has a bit of logic that is repeated twice, because it has to run once when the page loads and then again whenever the library select is changed. I wanted to make it DRY, so change() triggers the on change event when the page first loads. Maybe it would be more readable if it was instead written as a function that gets called twice? I wrote it this way so that when I was testing I only had to edit the logic in one place. Maybe I should submit another patch that just updates the minimal number of lines that need to be changed? Instead of changing the whole file so that the change is easier to see. It's mostly just adding a conditional to set the current desk if it needs to be set. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30123 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #11 from Owen Leonard <oleonard@myacpl.org> --- (In reply to Brendan Lawlor from comment #10)
(In reply to Owen Leonard from comment #9)
Why trigger change() after the on("change") event? so change() triggers the on change event when the page first loads
Ah, that makes sense. IMO you don't need to redo the patch again to change the number of updated lines. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30123 --- Comment #12 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 164846 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164846&action=edit Screenshot showing missing option This is working except for one weird issue: If I try to switch to a library which has more than one desk, only one desk appears in the filtered dropdown. I'm attaching this screenshot which includes the DOM inspector because you can see that both options *should* be visible. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30123 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #13 from Owen Leonard <oleonard@myacpl.org> --- In my test, changing the markup in html_helpers.inc fixed the issue: - <option class="[% d.branchcode | html %]" value="[% d.desk_id | html %]" disabled hidden>[% d.desk_name | html %]</option> + <option class="[% d.branchcode | html %]" value="[% d.desk_id | html %]" disabled style="display: none">[% d.desk_name | html %]</option> jQuery's show and hide don't play nice with the "hidden" attribute? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30123 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com --- Comment #14 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I've seen that jQuery fun before.. though I can't remember where or why it happens.. I'm not averse to the proposed change from Owen. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30123 --- Comment #15 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 165331 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=165331&action=edit Bug 30123: Set desk default to logged-in-desk-id on sety library page This patch refactors the javascript that runs on the set library page. If the logged in user branch code is the same as the seleceted library, default the desk selection to the current logged in desk id. Otherwise set the default desk selection to the first option for the current branch. Test plan: 1. Turn on UseCirculationDesks 2. Create a few desks with unique names for a few different libraries 3. Set your library and desk 4. Reload set-library.pl and notice that the desk always defaults to the last option for the library selected 5. Apply patch and restart_all 6. Reload set-library.pl and notice that the desk selection defaults to your current logged in desk 7. Try changing the library selection to libraries with and without desks 8. Notice that the desk selection defaults to the first option available for the selected library 9. Notice that the desk selection defaults to the --- no desk for libraries without desks 10. Notice the desk selection defaults to the currenty logged in desk if you select the library you are currently logged into -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30123 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #164390|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30123 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30123 --- Comment #16 from Brendan Lawlor <blawlor@clamsnet.org> --- This new patch changes the fewest number of lines and doesn't have the weird auto formatting issue so it's more clear to see what it does. It also includes the markup fix that Owen suggested. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30123 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30123 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #165331|0 |1 is obsolete| | --- Comment #17 from Lucas Gass <lucas@bywatersolutions.com> --- Created attachment 165339 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=165339&action=edit Bug 30123: Set desk default to logged-in-desk-id on sety library page This patch refactors the javascript that runs on the set library page. If the logged in user branch code is the same as the seleceted library, default the desk selection to the current logged in desk id. Otherwise set the default desk selection to the first option for the current branch. Test plan: 1. Turn on UseCirculationDesks 2. Create a few desks with unique names for a few different libraries 3. Set your library and desk 4. Reload set-library.pl and notice that the desk always defaults to the last option for the library selected 5. Apply patch and restart_all 6. Reload set-library.pl and notice that the desk selection defaults to your current logged in desk 7. Try changing the library selection to libraries with and without desks 8. Notice that the desk selection defaults to the first option available for the selected library 9. Notice that the desk selection defaults to the --- no desk for libraries without desks 10. Notice the desk selection defaults to the currenty logged in desk if you select the library you are currently logged into Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30123 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #165339|0 |1 is obsolete| | --- Comment #18 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 165358 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=165358&action=edit Bug 30123: Set desk default to logged-in-desk-id on sety library page This patch refactors the javascript that runs on the set library page. If the logged in user branch code is the same as the seleceted library, default the desk selection to the current logged in desk id. Otherwise set the default desk selection to the first option for the current branch. Test plan: 1. Turn on UseCirculationDesks 2. Create a few desks with unique names for a few different libraries 3. Set your library and desk 4. Reload set-library.pl and notice that the desk always defaults to the last option for the library selected 5. Apply patch and restart_all 6. Reload set-library.pl and notice that the desk selection defaults to your current logged in desk 7. Try changing the library selection to libraries with and without desks 8. Notice that the desk selection defaults to the first option available for the selected library 9. Notice that the desk selection defaults to the --- no desk for libraries without desks 10. Notice the desk selection defaults to the currenty logged in desk if you select the library you are currently logged into Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30123 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |martin.renvoize@ptfs-europe |y.org |.com Status|Signed Off |Passed QA --- Comment #19 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Thanks for your work here. The patch is clean and clear, the code works nicely and QA scripts are happy. Passing QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30123 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |24.05.00 released in| | Status|Passed QA |Pushed to main -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30123 --- Comment #20 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 24.05! Well done everyone, thank you! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30123 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m Status|Pushed to main |Needs documenting --- Comment #21 from Fridolin Somers <fridolin.somers@biblibre.com> --- Depends on Bug 36572 not in 23.11.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30123 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|Needs documenting |RESOLVED CC| |david@davidnind.com --- Comment #22 from David Nind <david@davidnind.com> --- Fixes an error in the circulation desk selection. No changes to the manual required (checked Administration > Circulation desks). -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org