Bug 36237

Summary: Improve set-library UI after 34478
Product: Koha Reporter: Lucas Gass (lukeg) <lucas>
Component: Staff interfaceAssignee: Bugs List <koha-bugs>
Status: CLOSED INVALID QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: david, gmcharlt, jonathan.druart
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36480
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
This improves the user interface when setting or editing the library, desk, and cash register (Circulation > Set library and desk): - Adds white background for the update successful page - Improves the messages about the updates made
Version(s) released in:
Circulation function:
Bug Depends on: 34478    
Bug Blocks:    
Attachments: Bug 36237: UI improvments for set-library page
Bug 36237: UI improvments for set-library page

Description Lucas Gass (lukeg) 2024-03-05 21:58:18 UTC
Bug 34478 added a new step when setting a library, desk, or register. I could use a few UI improvements. 

To recreate:
1. Enable UseCirculationDesks, make some desks via UseCashRegisters > Circulation desks
2. Enable UseCashRegisters, make some registers via UseCashRegisters > Registers
3. Go to set library page. Try changing the branch, desk, or register.
4. See that UI could use some improvements.
Comment 1 Lucas Gass (lukeg) 2024-03-05 22:00:09 UTC Comment hidden (obsolete)
Comment 2 David Nind 2024-03-09 23:11:39 UTC
Created attachment 163013 [details] [review]
Bug 36237: UI improvments for set-library page

To test:
1. Enable UseCirculationDesks, make some desks via UseCashRegisters > Circulation desks
2. Enable UseCashRegisters, make some registers via UseCashRegisters > Registers
3. Go to set library page. Try changing the branch, desk, or register.
4. See that UI could use some improvements.
5. APPLY patch
6. Try 3 again, making sure everything works right.

Signed-off-by: David Nind <david@davidnind.com>
Comment 3 Owen Leonard 2024-04-02 12:28:21 UTC
What is the reason for the new step?
Comment 4 Lucas Gass (lukeg) 2024-04-02 14:44:15 UTC
(In reply to Owen Leonard from comment #3)
> What is the reason for the new step?

It is related to Bug 34478 and CSRF protection, I believe. Im CC'ing Jonathan here as he will have a much better answer to that question.
Comment 5 Jonathan Druart 2024-04-05 07:24:12 UTC
(In reply to Lucas Gass from comment #4)
> (In reply to Owen Leonard from comment #3)
> > What is the reason for the new step?
> 
> It is related to Bug 34478 and CSRF protection, I believe. Im CC'ing
> Jonathan here as he will have a much better answer to that question.

I don't remember to be honest.

If there is a behaviour change it's coming from

It's coming from
  Bug 34478: Fix circ/set-library
Where I asked "Why do not we simply redirect instead of POSTing?".

There can be a problem if you are redirecting with op starting with cud-, but otoh it can be a problem if you POST op not starting with cud-!

I would open a separate bug.
Comment 6 Owen Leonard 2024-04-05 11:35:42 UTC
In my test it's working to change library but not working to change desk and register. Maybe I rebased wrong? Could someone else take a look?
Comment 7 David Nind 2024-04-08 06:58:57 UTC
Went to retest for Owen, but patch no longer applies:

git bz apply 36237

Bug 36237 - Improve set-library UI after 34478

163013 - Bug 36237: UI improvments for set-library page

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 36237: UI improvments for set-library page
Using index info to reconstruct a base tree...
M	circ/set-library.pl
Falling back to patching base and 3-way merge...
Auto-merging circ/set-library.pl
CONFLICT (content): Merge conflict in circ/set-library.pl
error: Failed to merge in the changes.
Patch failed at 0001 Bug 36237: UI improvments for set-library page
Comment 8 Lucas Gass (lukeg) 2024-04-11 09:03:47 UTC
(In reply to Owen Leonard from comment #3)
> What is the reason for the new step?

Some changes in 34478 brought some much older code back to life. It seems like the 'Updated' step was always there but we were never hitting the condition. 

To preserve existing behavior and clean up the set library page I filed Bug 36572. 

I think this one can now be set to RESOLVED - INVALID