xt/find-missing-op-in-forms.t is missing some occurrences.
Created attachment 170701 [details] [review] Bug 37728: Adjust test to catch more missing 'op' in POSTed forms The test is not catching missing op in forms that are not the first POST form of the template. Found when fixing the test for bug 37309. It was not catching the missing op in the second form.
So this is certainly hiding... more than 20 bugs. not ok 1 - The <form> in the following files are missing it's corresponding op parameter, or op does not start with 'cud-' (see bug 34478) # Failed test 'The <form> in the following files are missing it's corresponding op parameter, or op does not start with 'cud-' (see bug 34478)' # at xt/find-missing-op-in-forms.t line 43. # got: '20' # expected: '0' # $VAR1 = 'koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-illrequests.tt:260,293'; # $VAR2 = 'koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-passwd.tt:110'; # $VAR3 = 'koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-password-recovery.tt:152'; # $VAR4 = 'koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-shelves.tt:808'; # $VAR5 = 'koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgetperiods.tt:327'; # $VAR6 = 'koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqcontract.tt:150,182'; # $VAR7 = 'koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqplan.tt:209,270'; # $VAR8 = 'koha-tmpl/intranet-tmpl/prog/en/modules/admin/auth_tag_structure.tt:206,236'; # $VAR9 = 'koha-tmpl/intranet-tmpl/prog/en/modules/admin/currency.tt:183,205'; # $VAR10 = 'koha-tmpl/intranet-tmpl/prog/en/modules/admin/itemtypes.tt:382'; # $VAR11 = 'koha-tmpl/intranet-tmpl/prog/en/modules/admin/marc_subfields_structure.tt:333'; # $VAR12 = 'koha-tmpl/intranet-tmpl/prog/en/modules/admin/marctagstructure.tt:217'; # $VAR13 = 'koha-tmpl/intranet-tmpl/prog/en/modules/admin/systempreferences.tt:292,302'; # $VAR14 = 'koha-tmpl/intranet-tmpl/prog/en/modules/circ/pendingreserves.tt:290'; # $VAR15 = 'koha-tmpl/intranet-tmpl/prog/en/modules/installer/step3.tt:294'; # $VAR16 = 'koha-tmpl/intranet-tmpl/prog/en/modules/members/boraccount.tt:191'; # $VAR17 = 'koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt:1443'; # $VAR18 = 'koha-tmpl/intranet-tmpl/prog/en/modules/serials/serials-collection.tt:152'; # $VAR19 = 'koha-tmpl/intranet-tmpl/prog/en/modules/tags/review.tt:248,274'; # $VAR20 = 'koha-tmpl/intranet-tmpl/prog/en/modules/tools/letter.tt:547,589';
Not good, but good that we caught them now. Who can work on resolving these?
I can help, but not let alone.
I can't do things like opac-illrequests.tt which would require that I spend several hours of my employer's money learning how to write a test plan for something we don't use, but I can do some of them.
Wish I would have realized how many of these were "a cancel deletion button which should be a GET, and an OK button in a deletion confirmed page which currently doesn't even show but should be a GET" before I filed them as separate bugs.
Thanks, Phil. Much appreciated! You've been killing it lately :D.
(In reply to Katrin Fischer from comment #3) > Who can work on resolving these? I'll help out as well. I'm a bit worried about the stability of 24.05 and the upcoming 24.11, so I the present is a great time to get as much done as we can.
Not quite sure what to do with koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt:1443 - JS inserts the op in the form in line 1859, and the form won't work without JS since it's also inserting the reserve_id, and without a reserve_id you aren't going to cancel. Do we just stick the op in normally instead of inserting it, to make the test happy? Or is there a reason I'm not seeing that it needs to be inserted by JS?
And although I was thinking of sticking it in normally right below the CSRF token, it would actually be much funnier to stick it in the empty <div id="inputs"> in the template, which would make the test happy, and then the very first thing the JS does is to empty() that div, so it can stay happy about putting in the op itself.
Asked that question in the form of a Needs Signoff in bug 37783.
Current status after applying the patch and following dependencies (minus the one that's filed but not patched): # $VAR1 = 'koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-illrequests.tt:260,293'; # $VAR2 = 'koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-passwd.tt:110'; # $VAR3 = 'koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-password-recovery.tt:152'; # $VAR4 = 'koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-shelves.tt:808'; # $VAR5 = 'koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgetperiods.tt:327'; # $VAR6 = 'koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqcontract.tt:150,182'; # $VAR7 = 'koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqplan.tt:209,270'; # $VAR8 = 'koha-tmpl/intranet-tmpl/prog/en/modules/installer/step3.tt:294'; # $VAR10 = 'koha-tmpl/intranet-tmpl/prog/en/modules/serials/serials-collection.tt:152'; I don't have any current plans to touch those, since they are in areas where I don't know either the code or the UI, so they are up for grabs.
Thanks Phil, awesome job!
I can grab the acquisitions ones. I'll see if I have time to SO/QA some of yours after (the day got away from me... D:) Thanks for jumping on so many, Phil!
Shame I can't write a test plan for the opac-illrequests.tt one, because it looks like the patch would be an amusing [%# This will get its name="op" value="cud-" from the loop over whole.keys below %] to fake out the test.
Should this be NSO?
I could trivially sign off that it catches more missing 'op' in POSTed forms, but I can't sign off that it passes when there aren't any, since I haven't seen that. # $VAR1 = 'koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-illrequests.tt:260,293'; # $VAR2 = 'koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-password-recovery.tt:152'; # $VAR3 = 'koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqcontract.tt:167'; # $VAR4 = 'koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqplan.tt:209,270'; # $VAR5 = 'koha-tmpl/intranet-tmpl/prog/en/modules/installer/step3.tt:294'; # $VAR6 = 'koha-tmpl/intranet-tmpl/prog/en/modules/serials/serials-collection.tt:158';
Seriously, I'm done. Somebody take these. # $VAR1 = 'koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-illrequests.tt:260,293'; Requires having an ILL backend that does availability and knowing how to get to typedisclaimer and how to write a test plan so someone else can. # $VAR2 = 'koha-tmpl/intranet-tmpl/prog/en/modules/installer/step3.tt:294'; Requires knowing how to write an installer test plan, and knowing what on earth it's using to keep track of substeps. # $VAR3 = 'koha-tmpl/intranet-tmpl/prog/en/modules/serials/serials-collection.tt:158'; Should be easy for someone who uses serials-collection, and thus knows whether it needs to keep POSTing because sometimes you do stuff to so many things at once that you exceed Apache's URL maxlength if it's a GET, or not.
Thanks a lot for your work, Phil!