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.
Created attachment 162793 [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.
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>
What is the reason for the new step?
(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.
(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.
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?
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
(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