Bug 36246

Summary: Have a centralized method for submitting a form via a link
Product: Koha Reporter: Nick Clemens (kidclamp) <nick>
Component: Architecture, internals, and plumbingAssignee: Nick Clemens (kidclamp) <nick>
Status: RESOLVED FIXED QA Contact: Martin Renvoize (ashimema) <martin.renvoize>
Severity: enhancement    
Priority: P5 - low CC: blawlor, caroline.cyr-la-rose, dcook, fridolin.somers, jonathan.druart, phil
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37111
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.05.00
Circulation function:
Bug Depends on:    
Bug Blocks: 31632, 36192, 38155, 36735, 37111, 37190, 37267, 38020    
Attachments: Bug 36246: Add JS function/include for submitting forms from link data
Bug 36246: Add JS function/include for submitting forms from link data
Bug 36246: Add the confirmation text directly to the attribute
Bug 36246: Add JS function/include for submitting forms from link data
Bug 36246: Add the confirmation text directly to the attribute
Bug 36246: (follow-up) Fix confirm
Bug 36246: Add JS function/include for submitting forms from link data
Bug 36246: Add the confirmation text directly to the attribute
Bug 36246: (follow-up) Fix confirm
Bug 36246: (QA follow-up) New JS should start tidy
Bug 36246: (QA follow-up) Add missing filter

Description Nick Clemens (kidclamp) 2024-03-06 13:13:59 UTC
With new CSRF protection we have a fair number places where action buttons have become full forms, or links need to use JS to submit a form.

We have generally agreed that the form should be in the page, and the js can add the necessary values for POSTing

I think we should go a step further and have a class that can be used to submit a form via a link with data elements, i.e.

<a class="btn btn-default btn-xs submit-form-link" href="#" data-adjustment_id="[% adjustment.adjustment_id | html %]" data-invoiceid="[% invoiceid | html %]" data-action="invoice.pl" data-method="post" data-op="cud-del_adj" data-confirmation="confirm_delete_adjustment">
Comment 1 Nick Clemens (kidclamp) 2024-03-06 13:33:10 UTC
Created attachment 162835 [details] [review]
Bug 36246: Add JS function/include for submitting forms from link data

This patch adds a new js include file form-submit.js which will read data elements from a link
and use those to submit a form

This patch fixes forms on acqui/invoice.pl as an example

To test:
1 - Add some invoices for a vendor
2 - Got to Acquisitions->Invoices
3 - Actions -> Close - confirm it works
4 - Got to 'Closed invoices' - reopen
5 - Go to Details on the invoice
6 - Add an adjustment
7 - Delete the adjustment
8 - Confirm it works
Comment 2 David Nind 2024-03-12 19:15:48 UTC
Created attachment 163098 [details] [review]
Bug 36246: Add JS function/include for submitting forms from link data

This patch adds a new js include file form-submit.js which will read data elements from a link
and use those to submit a form

This patch fixes forms on acqui/invoice.pl as an example

To test:
1 - Add some invoices for a vendor
2 - Got to Acquisitions->Invoices
3 - Actions -> Close - confirm it works
4 - Got to 'Closed invoices' - reopen
5 - Go to Details on the invoice
6 - Add an adjustment
7 - Delete the adjustment
8 - Confirm it works

Signed-off-by: David Nind <david@davidnind.com>
Comment 3 Jonathan Druart 2024-03-20 09:52:31 UTC
If you cancel then click again you won't get the confirmation and the form will be submitted.
Comment 4 Jonathan Druart 2024-03-20 09:54:21 UTC
Created attachment 163517 [details] [review]
Bug 36246: Add the confirmation text directly to the attribute

To avoid the eval and have all the values together.

For discussion.
Comment 5 Nick Clemens (kidclamp) 2024-04-12 11:51:48 UTC
Created attachment 164830 [details] [review]
Bug 36246: Add JS function/include for submitting forms from link data

This patch adds a new js include file form-submit.js which will read data elements from a link
and use those to submit a form

This patch fixes forms on acqui/invoice.pl as an example

To test:
1 - Add some invoices for a vendor
2 - Got to Acquisitions->Invoices
3 - Actions -> Close - confirm it works
4 - Got to 'Closed invoices' - reopen
5 - Go to Details on the invoice
6 - Add an adjustment
7 - Delete the adjustment
8 - Confirm it works

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 6 Nick Clemens (kidclamp) 2024-04-12 11:51:51 UTC
Created attachment 164831 [details] [review]
Bug 36246: Add the confirmation text directly to the attribute

To avoid the eval and have all the values together.

For discussion.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 7 Nick Clemens (kidclamp) 2024-04-12 11:51:54 UTC
Created attachment 164832 [details] [review]
Bug 36246: (follow-up) Fix confirm

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 8 Nick Clemens (kidclamp) 2024-04-12 11:53:21 UTC
(In reply to Jonathan Druart from comment #3)
> If you cancel then click again you won't get the confirmation and the form
> will be submitted.

Fixed, we shouldn't delete the confirmation message (to avoid form population) unless the confirm happens

(In reply to Jonathan Druart from comment #4)
> Created attachment 163517 [details] [review] [review]
> Bug 36246: Add the confirmation text directly to the attribute
> 
> To avoid the eval and have all the values together.
> 
> For discussion.

I love this! I didn't know how to do this, but I think it is the proper way
Comment 9 Martin Renvoize (ashimema) 2024-05-01 13:22:54 UTC
Created attachment 165967 [details] [review]
Bug 36246: Add JS function/include for submitting forms from link data

This patch adds a new js include file form-submit.js which will read data elements from a link
and use those to submit a form

This patch fixes forms on acqui/invoice.pl as an example

To test:
1 - Add some invoices for a vendor
2 - Got to Acquisitions->Invoices
3 - Actions -> Close - confirm it works
4 - Got to 'Closed invoices' - reopen
5 - Go to Details on the invoice
6 - Add an adjustment
7 - Delete the adjustment
8 - Confirm it works

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 10 Martin Renvoize (ashimema) 2024-05-01 13:22:56 UTC
Created attachment 165968 [details] [review]
Bug 36246: Add the confirmation text directly to the attribute

To avoid the eval and have all the values together.

For discussion.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 11 Martin Renvoize (ashimema) 2024-05-01 13:22:59 UTC
Created attachment 165969 [details] [review]
Bug 36246: (follow-up) Fix confirm

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 12 Martin Renvoize (ashimema) 2024-05-01 13:23:01 UTC
Created attachment 165970 [details] [review]
Bug 36246: (QA follow-up) New JS should start tidy

Add the `/* keep tidy */` flag so the new file stays tidy

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 13 Martin Renvoize (ashimema) 2024-05-01 13:23:04 UTC
Created attachment 165971 [details] [review]
Bug 36246: (QA follow-up) Add missing filter

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 14 Martin Renvoize (ashimema) 2024-05-01 13:25:17 UTC
Nice re-usable component introduced here... QA scripts happy and new file is tidy now.. I added a couple of very minor QA follow-ups.

Passing QA

Question: Where can/should we document these sorts of little helpers.. we're really bad at getting them widely adopted/advertised for use.
Comment 15 Katrin Fischer 2024-05-03 16:02:52 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 16 Fridolin Somers 2024-05-22 12:47:02 UTC
Not backported to 23.11.x

Maybe needs a coding guideline.
Comment 17 Katrin Fischer 2024-05-23 15:15:18 UTC
(In reply to Fridolin Somers from comment #16)
> Not backported to 23.11.x
> 
> Maybe needs a coding guideline.

Added it to the next agenda:
https://wiki.koha-community.org/wiki/Development_IRC_meeting_29_May_2024
Comment 18 Caroline Cyr La Rose 2024-09-10 20:16:16 UTC
This doesn't seem to change anything in the manual. Please reopen with more detail if needed.