[Bug 36735] New: Cannot revert the waiting status of a hold
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36735 Bug ID: 36735 Summary: Cannot revert the waiting status of a hold Change sponsored?: --- Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: critical Priority: P5 - low Component: Hold requests Assignee: koha-bugs@lists.koha-community.org Reporter: emily.lamancusa@montgomerycountymd.gov QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com Depends on: 36246 Blocks: 36192 To reproduce: 1. Place a hold on a biblio record 2. Check an item in and confirm the hold 3. On the holds tab for the biblio record, click the "Revert waiting status" button for that hold --> The page reloads but the hold is still waiting Looking at the code for the button, I'm pretty sure it just needs to be updated for CSRF. I think this is one of the use cases bug 36246 is for, if I'm understanding it correctly? Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36192 [Bug 36192] [OMNIBUS] CSRF Protection for Koha https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36246 [Bug 36246] Have a centralized method for submitting a form via a link -- 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=36735 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |emily.lamancusa@montgomeryc |ity.org |ountymd.gov Status|NEW |ASSIGNED -- 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=36735 --- Comment #1 from Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> --- Created attachment 165930 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=165930&action=edit Bug 36735: Fix revert hold button Update the revert hold button to use the new include for submitting forms from link data with a POST request To test: 1. Place a hold on a biblio record 2. Check an item in to fill the hold 3. On the holds tab for the biblio record, click the "Revert waiting status" button for that hold --> The page reloads but the hold is still waiting 4. Apply patchset 5. Click to another page and then return to the holds tab (we don't want to refresh the page and resend the request) 6. Click the "Revert waiting status" button for that hold --> The hold should be reverted to pending status -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36735 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36735 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36735 --- Comment #2 from Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> --- Submitting for feedback - this seems to work, but there are a couple of things I'm not sure about: - I kept the uri TT filters from the existing code. Should those be changed to html instead since they're not being passed in the URL anymore? - I feel like there has to be a better way to handle the dependency between the .inc file and the JS file, but including the JS asset directly in the .inc file breaks things (I think because the include is added in the middle of the page in request.tt, which violates the coding guideline of including JS assets at the end?) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36735 David Nind <david@davidnind.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=36735 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #165930|0 |1 is obsolete| | --- Comment #3 from David Nind <david@davidnind.com> --- Created attachment 165931 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=165931&action=edit Bug 36735: Fix revert hold button Update the revert hold button to use the new include for submitting forms from link data with a POST request To test: 1. Place a hold on a biblio record 2. Check an item in to fill the hold 3. On the holds tab for the biblio record, click the "Revert waiting status" button for that hold --> The page reloads but the hold is still waiting 4. Apply patchset 5. Click to another page and then return to the holds tab (we don't want to refresh the page and resend the request) 6. Click the "Revert waiting status" button for that hold --> The hold should be reverted to pending status Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36735 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #4 from David Nind <david@davidnind.com> --- (In reply to Emily Lamancusa from comment #2)
Submitting for feedback - this seems to work, but there are a couple of things I'm not sure about: - I kept the uri TT filters from the existing code. Should those be changed to html instead since they're not being passed in the URL anymore?
- I feel like there has to be a better way to handle the dependency between the .inc file and the JS file, but including the JS asset directly in the .inc file breaks things (I think because the include is added in the middle of the page in request.tt, which violates the coding guideline of including JS assets at the end?)
I don't have any feedback on these points, as I don't know enough to knowledgeably comment 8-) However, I have signed off the bug, as I can replicate the issue and it is fixed as per the test plan. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36735 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |RM_priority -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36735 --- Comment #5 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- (In reply to Emily Lamancusa from comment #2)
Submitting for feedback - this seems to work, but there are a couple of things I'm not sure about: - I kept the uri TT filters from the existing code. Should those be changed to html instead since they're not being passed in the URL anymore?
Correct, they should all be html filters now. In the end we are building a form and submitting, so they won't be in a URL
- I feel like there has to be a better way to handle the dependency between the .inc file and the JS file, but including the JS asset directly in the .inc file breaks things (I think because the include is added in the middle of the page in request.tt, which violates the coding guideline of including JS assets at the end?)
I think I understand the issue, it seems odd to have the JS asset in the script and not in the include directly? In this case, the include is only used on this page, but perhaps adding a comment to the include to specify that it needs the asset would be helpful? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36735 --- Comment #6 from Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> --- (In reply to Nick Clemens (kidclamp) from comment #5) Thanks for the feedback!
Correct, they should all be html filters now. In the end we are building a form and submitting, so they won't be in a URL
In retrospect this was a dumb question. ^^" Fixing...
I think I understand the issue, it seems odd to have the JS asset in the script and not in the include directly? In this case, the include is only used on this page, but perhaps adding a comment to the include to specify that it needs the asset would be helpful?
Basically, yes. Thanks, just wanted another pair of eyes in case there was a better solution that I wasn't finding. Keeping this as signed-off and attaching a follow-up since both of the above are trivial changes. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36735 --- Comment #7 from Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> --- Created attachment 166418 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=166418&action=edit Bug 36735: (follow-up) Fix TT filters and add a comment holds_table.inc depends on form-submit.js, but importing the asset directly breaks the script. Add a comment stating that any script that imports holds_table.inc must import form-submit.js too. Also change uri filters to html -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36735 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36735 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #165931|0 |1 is obsolete| | --- Comment #8 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Created attachment 166552 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=166552&action=edit Bug 36735: Fix revert hold button Update the revert hold button to use the new include for submitting forms from link data with a POST request To test: 1. Place a hold on a biblio record 2. Check an item in to fill the hold 3. On the holds tab for the biblio record, click the "Revert waiting status" button for that hold --> The page reloads but the hold is still waiting 4. Apply patchset 5. Click to another page and then return to the holds tab (we don't want to refresh the page and resend the request) 6. Click the "Revert waiting status" button for that hold --> The hold should be reverted to pending status Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36735 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #166418|0 |1 is obsolete| | --- Comment #9 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Created attachment 166553 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=166553&action=edit Bug 36735: (follow-up) Fix TT filters and add a comment holds_table.inc depends on form-submit.js, but importing the asset directly breaks the script. Add a comment stating that any script that imports holds_table.inc must import form-submit.js too. Also change uri filters to html Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36735 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to main Version(s)| |24.05.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36735 --- Comment #10 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=36735 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m Status|Pushed to main |RESOLVED Resolution|--- |FIXED --- Comment #11 from Fridolin Somers <fridolin.somers@biblibre.com> --- Depends on Bug 36246 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=36735 Bug 36735 depends on bug 36246, which changed state. Bug 36246 Summary: Have a centralized method for submitting a form via a link https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36246 What |Removed |Added ---------------------------------------------------------------------------- Status|Needs documenting |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36735 Cheryl <clackman@fargolibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |clackman@fargolibrary.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36735 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|RM_priority | -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org