Bug 29496

Summary: Can't save an order with mandatory items subfields
Product: Koha Reporter: Koha Team University Lyon 3 <koha>
Component: AcquisitionsAssignee: Jonathan Druart <jonathan.druart+koha>
Status: RESOLVED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: blocker    
Priority: P5 - low CC: jean-manuel.broust, jonathan.druart+koha, kyle, m.de.rooy, marion.durand.lelion
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Text to go in the release notes:
Version(s) released in:
21.11.00
Bug Depends on: 27526    
Bug Blocks:    
Attachments: Bug 29496: (bug 27526 follow-up) Fix item form validation
Bug 29496: CheckMandatorySubfields don't work properly with select field in serials-edit.tt
Bug 29496: (bug 27526 follow-up) Fix item form validation
Bug 29496: (bug 27526 follow-up) Fix item form validation
Bug 29496: CheckMandatorySubfields don't work properly with select field in serials-edit.tt
Bug 29496: (bug 27526 follow-up) Fix item form validation

Description Koha Team University Lyon 3 2021-11-16 17:27:13 UTC
Here is the plan to reproduce it :
- Unimarc flavour
- Items in 995 field whith 3 mandatory /
  > Tab:10, | Koha field: items.homebranch, Not repeatable, Mandatory, Not Important, | Auth value:branches,
  > Tab:10, | Koha field: items.holdingbranch, Not repeatable, Mandatory, Not important, | Auth value:branches,
  > Tab:10, | Koha field: items.location, Not repeatable, Mandatory, Not important, | Auth value:LOC,
 
- AcqCreateItem="placing an order"

When adding an order from a new empty record,I filled
- the title 
- the 3 mandatory items subfields (and clicked on add item)
-  the fund
- the currency
- the vendor price
- the tax rate
- the discount
- the retail price
- the replacement cost
- the budgeted cost
- the total

When saving, I have the message :
"Form not submitted because of the > following problem(s): 
 - 3 item mandatory fields empty".

You can try here : http://staff-master.sandboxes.ptfs-europe.co.uk/cgi-bin/koha/acqui/basket.pl?basketno=2

Sonia
Comment 1 Jonathan Druart 2021-11-17 14:34:39 UTC
Caused by
  commit 6ed29bccef590a1bc5e4cea175b41b8024483320
  Bug 27526: Fix mandatory and important checks
Comment 2 Jonathan Druart 2021-11-17 14:51:55 UTC
Created attachment 127733 [details] [review]
Bug 29496: (bug 27526 follow-up) Fix item form validation

The validation of the forms were blocked with "X item mandatory fields
empty" when at least one dropdown list subfield was marked as mandatory.

We need to add the 'input_marceditor' class to the select (does it
actually make sense? select vs input...)

Caused by
  commit 6ed29bccef590a1bc5e4cea175b41b8024483320
  Bug 27526: Fix mandatory and important checks
Which lamentably failed as it was stating:
"Using .input_marceditor let us fix the additem.tt form and prevent to break the other ones"
Comment 3 Katrin Fischer 2021-11-18 09:47:31 UTC
Hi, is this related to bug 29499?
Comment 4 Jonathan Druart 2021-11-18 10:18:05 UTC
*** Bug 29499 has been marked as a duplicate of this bug. ***
Comment 5 Jonathan Druart 2021-11-18 10:18:21 UTC
Created attachment 127780 [details] [review]
Bug 29496: CheckMandatorySubfields don't work properly with select field in serials-edit.tt

Since one of the patches of BZ 27526 (Bug 27526: Fix mandatory and
important checks), CheckMandatorySubfields use the class
"input_marceditor" but in file serials-edit.tt this class is not set for
all field (it is present on text input but not on select input) 5
9- Check that no error appear and that your item has been created.

In consequence if a select field is set as mandatory, it is detected as
missing even if it is filed and so you can't submit the form and receive
the new serial.

Test plan:
0- Be sure to be in a version of koha where the patch that introduces
   the bug is present (it is present in master since Jul 8 2021 (it is
   present in 21.06.00.046) and will be pushed in 21.11.00)
1- Create (or find) a subscription for a biblio record and select the
   option "Create an item record when receiving this serial"
2- Be sure to have at least one mandatory subfield that is filed with a
   select input in the framework used by the biblio record. (ex: 995$b,
   995$c or 995$e in unimarc; 952$a, 952$b or 952$c in marc21)
3- From the subscription-detail page click on "Receive"
4- Change the status to "Arrived" and fill the item form that appears.
5- Click on "Save"
6- Check that an error box appear with the message " Form not submitted
   because of the following problem(s) 1 mandatory fields empty
   (highlighted)" (the number can be different according to the number of
   concerned subfields)
7- Apply the patch
8- Repeat step 3 to 5
9- Check that no error appear and that your item has been created

JD amended patch: remove comma to separate classes
Comment 6 Jonathan Druart 2021-11-18 10:18:24 UTC
Created attachment 127781 [details] [review]
Bug 29496: (bug 27526 follow-up) Fix item form validation

The validation of the forms were blocked with "X item mandatory fields
empty" when at least one dropdown list subfield was marked as mandatory.

We need to add the 'input_marceditor' class to the select (does it
actually make sense? select vs input...)

Caused by
  commit 6ed29bccef590a1bc5e4cea175b41b8024483320
  Bug 27526: Fix mandatory and important checks
Which lamentably failed as it was stating:
"Using .input_marceditor let us fix the additem.tt form and prevent to break the other ones"
Comment 7 Marion Durand 2021-11-19 09:01:51 UTC
Thank you Jonathan for moving my patch here !

(When I created Bug 29499 I couldn't find this one, sorry for the duplicated bug.)

Everything seems good to me. Since I have made one of the patches, should I signoff the other one or not ?
Comment 8 Jonathan Druart 2021-11-19 09:34:30 UTC
(In reply to marion.durand from comment #7)
> Thank you Jonathan for moving my patch here !
> 
> (When I created Bug 29499 I couldn't find this one, sorry for the duplicated
> bug.)
> 
> Everything seems good to me. Since I have made one of the patches, should I
> signoff the other one or not ?

Yes, please :)
Comment 9 Marion Durand 2021-11-19 10:07:44 UTC
Created attachment 127853 [details] [review]
Bug 29496: (bug 27526 follow-up) Fix item form validation

The validation of the forms were blocked with "X item mandatory fields
empty" when at least one dropdown list subfield was marked as mandatory.

We need to add the 'input_marceditor' class to the select (does it
actually make sense? select vs input...)

Caused by
  commit 6ed29bccef590a1bc5e4cea175b41b8024483320
  Bug 27526: Fix mandatory and important checks
Which lamentably failed as it was stating:
"Using .input_marceditor let us fix the additem.tt form and prevent to break the other ones"

Signed-off-by: Marion Durand <marion.durand@biblibre.com>
Comment 10 Marcel de Rooy 2021-11-19 10:17:48 UTC
(In reply to Jonathan Druart from comment #6)
> Created attachment 127781 [details] [review] [review]
> Bug 29496: (bug 27526 follow-up) Fix item form validation
> 
> The validation of the forms were blocked with "X item mandatory fields
> empty" when at least one dropdown list subfield was marked as mandatory.
> 
> We need to add the 'input_marceditor' class to the select (does it
> actually make sense? select vs input...)
> 
> Caused by
>   commit 6ed29bccef590a1bc5e4cea175b41b8024483320
>   Bug 27526: Fix mandatory and important checks
> Which lamentably failed as it was stating:
> "Using .input_marceditor let us fix the additem.tt form and prevent to break
> the other ones"

No test plan?

Please explain how the addorderiso2709 change is supposed to work? This template does not use cataloging.js. I do not see the connection (yet)?
Comment 11 Marcel de Rooy 2021-11-19 10:20:08 UTC
(In reply to marion.durand from comment #9)
> Signed-off-by: Marion Durand <marion.durand@biblibre.com>

What did you test, Marion ?
Comment 12 Marion Durand 2021-11-19 10:27:58 UTC
(In reply to Marcel de Rooy from comment #11)
> (In reply to marion.durand from comment #9)
> > Signed-off-by: Marion Durand <marion.durand@biblibre.com>
> 
> What did you test, Marion ?

I tested test plan given by the first patch and test plan given by Koha Team University Lyon 3 (fist comment)
Comment 13 Marcel de Rooy 2021-11-19 10:35:07 UTC
From IRC:
[11:28] <@Joubu> marcelr: it matches the pattern, and the pattern is: select with name="field_value" has input_marceditor class
[11:29] <@Joubu> if we don't add on addorderiso, it will be the only place where input_marceditor is not set to selects with name="field_value"

That makes sense :)
Comment 14 Marcel de Rooy 2021-11-19 10:35:49 UTC
(In reply to marion.durand from comment #12)
> (In reply to Marcel de Rooy from comment #11)
> > (In reply to marion.durand from comment #9)
> > > Signed-off-by: Marion Durand <marion.durand@biblibre.com>
> > 
> > What did you test, Marion ?
> 
> I tested test plan given by the first patch and test plan given by Koha Team
> University Lyon 3 (fist comment)

Thanks for your fast response.
Comment 15 Marcel de Rooy 2021-11-19 10:37:26 UTC
Created attachment 127854 [details] [review]
Bug 29496: CheckMandatorySubfields don't work properly with select field in serials-edit.tt

Since one of the patches of BZ 27526 (Bug 27526: Fix mandatory and
important checks), CheckMandatorySubfields use the class
"input_marceditor" but in file serials-edit.tt this class is not set for
all field (it is present on text input but not on select input) 5
9- Check that no error appear and that your item has been created.

In consequence if a select field is set as mandatory, it is detected as
missing even if it is filed and so you can't submit the form and receive
the new serial.

Test plan:
0- Be sure to be in a version of koha where the patch that introduces
   the bug is present (it is present in master since Jul 8 2021 (it is
   present in 21.06.00.046) and will be pushed in 21.11.00)
1- Create (or find) a subscription for a biblio record and select the
   option "Create an item record when receiving this serial"
2- Be sure to have at least one mandatory subfield that is filed with a
   select input in the framework used by the biblio record. (ex: 995$b,
   995$c or 995$e in unimarc; 952$a, 952$b or 952$c in marc21)
3- From the subscription-detail page click on "Receive"
4- Change the status to "Arrived" and fill the item form that appears.
5- Click on "Save"
6- Check that an error box appear with the message " Form not submitted
   because of the following problem(s) 1 mandatory fields empty
   (highlighted)" (the number can be different according to the number of
   concerned subfields)
7- Apply the patch
8- Repeat step 3 to 5
9- Check that no error appear and that your item has been created

JD amended patch: remove comma to separate classes

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 16 Marcel de Rooy 2021-11-19 10:37:29 UTC
Created attachment 127855 [details] [review]
Bug 29496: (bug 27526 follow-up) Fix item form validation

The validation of the forms were blocked with "X item mandatory fields
empty" when at least one dropdown list subfield was marked as mandatory.

We need to add the 'input_marceditor' class to the select (does it
actually make sense? select vs input...)

Caused by
  commit 6ed29bccef590a1bc5e4cea175b41b8024483320
  Bug 27526: Fix mandatory and important checks
Which lamentably failed as it was stating:
"Using .input_marceditor let us fix the additem.tt form and prevent to break the other ones"

Signed-off-by: Marion Durand <marion.durand@biblibre.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 17 Jonathan Druart 2021-11-19 14:16:02 UTC
Pushed to master for 21.11, thanks to everybody involved!
Comment 18 Jonathan Druart 2021-11-19 14:24:56 UTC
Congratulations for your first patch pushed, Marion!
Comment 19 Marion Durand 2021-11-19 14:25:59 UTC
(In reply to Jonathan Druart from comment #18)
> Congratulations for your first patch pushed, Marion!

Thank you :)