Summary: | Bad redirect when adding a patron message from members/files.pl | ||
---|---|---|---|
Product: | Koha | Reporter: | Lucas Gass (lukeg) <lucas> |
Component: | Patrons | Assignee: | Eric Garcia <cubingguy714> |
Status: | Pushed to oldoldstable --- | QA Contact: | Aleisha Amohia <aleisha> |
Severity: | minor | ||
Priority: | P5 - low | CC: | aleisha, cubingguy714, david, fridolin.somers, gmcharlt, kyle.m.hall, m.de.rooy |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | Trivial patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: |
This fixes a redirect when adding a patron message straight after uploading a patron file (when EnableBorrowerFiles is enabled). Before this fix, an error message "Patron not found. Return to search" was displayed if you added a message straight after you finished uploading a file (the "Add message" option on other pages worked as expected).
|
Version(s) released in: |
24.11.00,24.05.06,23.11.11
|
Circulation function: | |||
Attachments: |
Bug 37365: Fixed redirect when adding a patron message from members/files.pl
Bug 37365: Fixed redirect when adding a patron message from members/files.pl Bug 37365: (QA follow-up) Add borrowernumber to referrer url Bug 37365: (QA follow-up) Add error checking around AddFile call Bug 37365: (QA follow-up) Add borrowernumber to referrer url Bug 37365: (QA follow-up) Add error checking around AddFile call |
Description
Lucas Gass (lukeg)
2024-07-15 15:27:35 UTC
Created attachment 169028 [details] [review] Bug 37365: Fixed redirect when adding a patron message from members/files.pl To test: 1. Set 'EnableBorrowerFiles' to 'Do' 2. Find a patron record 3. Upload a file 4. Once the uploaed completes 'Add message' and save 5. You are redirected to /cgi-bin/koha/members/files.pl with a message like: "Patron not found. Return to search" 6. Apply patch and restart_all 7. Repeat steps 1-4 and notice you are redirected to the correct page '/cgi-bin/koha/members/files.pl?borrowernumber=' Created attachment 169034 [details] [review] Bug 37365: Fixed redirect when adding a patron message from members/files.pl To test: 1. Set 'EnableBorrowerFiles' to 'Do' 2. Find a patron record 3. Upload a file 4. Once the uploaed completes 'Add message' and save 5. You are redirected to /cgi-bin/koha/members/files.pl with a message like: "Patron not found. Return to search" 6. Apply patch and restart_all 7. Repeat steps 1-4 and notice you are redirected to the correct page '/cgi-bin/koha/members/files.pl?borrowernumber=' Signed-off-by: David Nind <david@davidnind.com> I cannot reproduce the original problem. I do not see the "Patron not found" message. Please clarify. After a print redirect, I would expect an exit statement. Failed QA This is how to reproduce: 1. Set the EnableBorrowerFiles system preference to 'Do'. 2. Go to a patron record (I used Mary Burton). 3. Go to the 'Files' section for the patron (in the left hand-side navigation menu between statistics and purchase suggestions). 4. Upload any file: 4.1 Add a description 4.2 Choose a file 4.3 Click 'Upload file' Result ==> You are on a page that lists files in a table (Name, Type, Description, Uploaded > Action column with a Delete button) and the upload new file form below it. The URL is /cgi-bin/koha/members/files.pl 5. Without changing pages, click 'Add message': Result ==> The pop-up form appears for adding a message. 6. Enter a message and click 'Save' 7. You get an error message: "Patron not found. Return to search": 7.1 The URL is /cgi-bin/koha/members/files.pl 7.2 The breadcrumb is Home > Patrons > > Files 7.3 The browser title is "Files for > Patrons > Koha - [your browser name]" It took me a while to figure it out - the 'Add message' button appears to work correctly from all the other patron pages. It works OK when you first go to the Files page - it only generates the error after you click on 'Add message' and the 'Save' straight after you upload a file. (In reply to David Nind from comment #4) > This is how to reproduce: Thx David. I saw it now. Created attachment 172381 [details] [review] Bug 37365: (QA follow-up) Add borrowernumber to referrer url Since we post to add_message, we need to ensure that the borrowernumber is added to the referrer url. This 'tric' is already done there for another form. Test plan: See original plan. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Now lets pay attention to the first patch still. (In reply to Marcel de Rooy from comment #7) > Now lets pay attention to the first patch still. Actually we need to remove that redirect now. We could add some missing error checking around calling AddFile though. Created attachment 172382 [details] [review] Bug 37365: (QA follow-up) Add error checking around AddFile call Test plan: Add a return; in AddFile to simulate a failing db insert. Verify that an alert pops up. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> It looks like this bug is still assigned to Eric, but both patches are from Marcel and only signed by Marcel. What about the first patch that is now obsoleted? Can we get one extra set of eyes here? Eric - do you agree with the solution? (In reply to Katrin Fischer from comment #10) > It looks like this bug is still assigned to Eric, but both patches are from > Marcel and only signed by Marcel. What about the first patch that is now > obsoleted? Correct. We dont need it anymore. Please see above. > Can we get one extra set of eyes here? Eric - do you agree with the solution? Agreed. But FQA is the way to forget this one completely. Moving back to SO. It's a way to get the developer's attention to clarify :) SO is perfect thanks. (In reply to Katrin Fischer from comment #12) > It's a way to get the developer's attention to clarify :) SO is perfect > thanks. SO = Signed off Created attachment 174061 [details] [review] Bug 37365: (QA follow-up) Add borrowernumber to referrer url Since we post to add_message, we need to ensure that the borrowernumber is added to the referrer url. This 'tric' is already done there for another form. Test plan: See original plan. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com> Created attachment 174062 [details] [review] Bug 37365: (QA follow-up) Add error checking around AddFile call Test plan: Add a return; in AddFile to simulate a failing db insert. Verify that an alert pops up. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com> Pushed for 24.11! Well done everyone, thank you! Backported to 24.05.x for upcoming 24.05.06 Pushed to 23.11.x for 23.11.11 |