Summary: | ILL "Edit request" action is broken | ||
---|---|---|---|
Product: | Koha | Reporter: | Pedro Amorim <pedro.amorim> |
Component: | ILL | Assignee: | Pedro Amorim <pedro.amorim> |
Status: | CLOSED FIXED | QA Contact: | Martin Renvoize (ashimema) <martin.renvoize> |
Severity: | critical | ||
Priority: | P5 - low | CC: | david, fridolin.somers, martin.renvoize, nugged, pedro.amorim, tomascohen, victor |
Version: | Main | Keywords: | regression |
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | Trivial patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: |
24.05.00
|
|
Circulation function: | |||
Bug Depends on: | 34478 | ||
Bug Blocks: | 36273, 36249 | ||
Attachments: |
Bug 36243: Remove cud- from op
Bug 36243: Update edit_action to cud-edit_action Bug 36243: Remove cud- from op Bug 36243: Update edit_action to cud-edit_action |
Description
Pedro Amorim
2024-03-06 12:35:26 UTC
Created attachment 162832 [details] [review] Bug 36243: Remove cud- from op 'cud-' needs to be removed here because it's showing the form, not handling its POST submission Created attachment 162833 [details] [review] Bug 36243: Update edit_action to cud-edit_action 1) Install FreeForm, enable ILL module, run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.sh) 2) You'll have to switch the FreeForm branch to the one compatible with this work, i.e.: cd /kohadevbox/koha/Koha/Illbackends/FreeForm git checkout b_36243 3) Restart plack: koha-plack --restart kohadev 4) Create a new FreeForm request, visit: <staff_url>/cgi-bin/koha/ill/ill-requests.pl?method=create&backend=FreeForm 5) Set 'type' enter '42' in cardnumber and pick a library. Click 'Create'. 6) Edit that request, click "manage request" or if #1 visit: <staff_url>/cgi-bin/koha/ill/ill-requests.pl?op=illview&illrequest_id=1 7) Click "Edit request". Click "Submit". Notice it errors "No CSRF token passed for POST" 9) Apply patches. Restart plack. 10) Repeat 7) Created attachment 162869 [details] [review] Bug 36243: Remove cud- from op 'cud-' needs to be removed here because it's showing the form, not handling its POST submission Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 162870 [details] [review] Bug 36243: Update edit_action to cud-edit_action 1) Install FreeForm, enable ILL module, run: bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.sh) 2) You'll have to switch the FreeForm branch to the one compatible with this work, i.e.: cd /kohadevbox/koha/Koha/Illbackends/FreeForm git checkout b_36243 3) Restart plack: koha-plack --restart kohadev 4) Create a new FreeForm request, visit: <staff_url>/cgi-bin/koha/ill/ill-requests.pl?method=create&backend=FreeForm 5) Set 'type' enter '42' in cardnumber and pick a library. Click 'Create'. 6) Edit that request, click "manage request" or if #1 visit: <staff_url>/cgi-bin/koha/ill/ill-requests.pl?op=illview&illrequest_id=1 7) Click "Edit request". Click "Submit". Notice it errors "No CSRF token passed for POST" 9) Apply patches. Restart plack. 10) Repeat 7) Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Follow-up fix for regression introduced. All working as designed now and code is reasonable to follow. Going straight for a QA and passing. Can we add the _form suffix to the op parameter? (In reply to Tomás Cohen Arazi from comment #6) > Can we add the _form suffix to the op parameter? Yes, we can. And we should. Though it'll require a rewrite on how $op is handled in ill-requests.pl and ill-requests.tt. The focus of this bug is to fix the regression. ill-requests.pl handles an edit_action "form" and an edit_action "save" in the same $op eq 'edit_action', which means these would have to be decoupled to cud-edit_action and edit_action_form. The same would have to be made for cud-create, cud-generic_confirm, cud-delete (?) and possibly others. We'd also have to account for the fact that existing backends may have not yet adopted these new $op names (See get_op_param_deprecation in Illrequest.pm) In short, my suggestion would be to push this refactor/clean-up work to its own bug, but I'm obviously happy to discuss if you or others disagree. (In reply to Pedro Amorim from comment #7) > (In reply to Tomás Cohen Arazi from comment #6) > > Can we add the _form suffix to the op parameter? > > Yes, we can. And we should. Though it'll require a rewrite on how $op is > handled in ill-requests.pl and ill-requests.tt. > The focus of this bug is to fix the regression. FAIR. Put some FIXME inline so there are no oversights on this later. I've added bug 36273. Please take a look to see if what I wrote there makes sense. Pushed for 24.05! Well done everyone, thank you! Depends on Bug 34478 not in 23.11.x |