|
Description
Katrin Fischer
2015-08-07 13:32:47 UTC
We keep running into this. As it is now, 'mandatory' makes no sense for fields with a pull down list as it will never trigger the message and it's easier to spot items where you forgot to set the item type than items where the first entry of the list was saved accidentally. This causes quite a bit of trouble - upping severity a bit. Example: Item type Koha is unhappy if no item type is set for an item, so it makes a lot of sense to set this field to mandatory. Setting it to mandatory, will result in the first item of the list to be preselected and often left unchanged. There is no way to find those, that have been incorrectly set. We have a "default" option in the frameworks, this should be used when you want to preselect, mandatory should just enforce that the value is set before saving. This is a gigantic pain and as Katrin mentions, difficult to identify after the fact. Just to clarify how I would expect this to work, Katrin's combined descriptions are correct: * If there is a default value: dropdown should be preselected to the default * If no default value: dropdown should be preselected to blank * If field is compulsory and no default value: Koha should show an error complaining that there is no value assigned to the field in question. Currently making something mandatory preselects the first value in the dropdown and doesn't give an error or warning. This is very rarely what users would want and never what administrators would expect. Hi Hugh, thx for your comment - I was starting to think it was only me (note the dates on the comments!) :) Summarizing with a test plan and description of expected behaviour: To test: - Mark collection, itemtype and damaged as mandatory in your MARC framework - Add a new item - Verify that instead of the fields being empty, the first value is selected If there is an itemtype on bilbio level, this itemtype should be selected. Expected behaviour: - All pull downs should offer an empty entry - Empty should be preselected, if there is no - existing value - default in the framework - Only for itemtypes: value in 942$c - If one of those is set, this value should be preselected. Created attachment 77419 [details] [review] Bug 14662: Add empty entries to pull downs on item form for mandatory subfields The code assumed that if a subfield is marked as mandatory, there should be no empty entry in the pull downs. This assumption is not correct, as it leads to the first entry of the pull down being preselected if there is no default set. Which means you will never be alerted of any cataloguing errors and errors will be very hard to find later on. Correct behaviour would be to preselect the empty value when there is no default. This means on saving the item an error message is triggered and the cataloger is forced to set the value. To test: - Adapt your frameworks: - Make 942$c non-mandatory - In 952 make itemtype, classification source and some other pull downs like location or collection mandatory - Add a new item - Verify that the first value of each pull down is preselected, there is no way to trigger the 'required' error - Apply patch - Add a new item - Verify that classification source is preselected according to the DefaultClassificationSource system preference - Verify that the itemtype is preselected according to 942$c in the bibliographic record - Verify all mandatory fields can be set to empty - Verify that you can't save before correctly setting them - Change the 942$c in the record to empty - Add another item - Verify the itemtype is now empty - Change your frameworks and set a default for itemtype (Ex: BK) - Repeat default check with another pull down like collection or location Having a go at this :) Created attachment 77443 [details] [review] Bug 14662: Add empty entries to pull downs on item form for mandatory subfields The code assumed that if a subfield is marked as mandatory, there should be no empty entry in the pull downs. This assumption is not correct, as it leads to the first entry of the pull down being preselected if there is no default set. Which means you will never be alerted of any cataloguing errors and errors will be very hard to find later on. Correct behaviour would be to preselect the empty value when there is no default. This means on saving the item an error message is triggered and the cataloger is forced to set the value. To test: - Adapt your frameworks: - Make 942$c non-mandatory - In 952 make itemtype, classification source and some other pull downs like location or collection mandatory - Add a new item - Verify that the first value of each pull down is preselected, there is no way to trigger the 'required' error - Apply patch - Add a new item - Verify that classification source is preselected according to the DefaultClassificationSource system preference - Verify that the itemtype is preselected according to 942$c in the bibliographic record - Verify all mandatory fields can be set to empty - Verify that you can't save before correctly setting them - Change the 942$c in the record to empty - Add another item - Verify the itemtype is now empty - Change your frameworks and set a default for itemtype (Ex: BK) - Repeat default check with another pull down like collection or location Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 77444 [details] [review] Bug 14662: Add empty entries to pull downs in cataloguing form for mandatory subfields First patch deals with the problem on the item form, this patch fixes the same problem on catalouging form for the bibliographic record. To test: - Adapt your frameworks: - Make sure 942$c is mandatory - Link another field to an authorised value, for example: 942$n to YESNO - Add a new record - Verify that the first value of each pull down is preselected, there is no way to trigger the 'required' error - Apply patch - Add a record - Verify that classification source is preselected according to the DefaultClassificationSource system preference - Verify that 942$c now defaults to empty - Verify all your other mandatory subfields can be set to empty - Verify that you can't save before correctly setting them - Chane your frameworks - Add a default value for 942$c, for example: CF - Add another record - Verify the itemtype is now preset to your default itemtype - Edit an existing record, verify all values show correctly Created attachment 77445 [details] [review] Bug 14662: Add empty entries to pull downs in cataloguing form for mandatory subfields First patch deals with the problem on the item form, this patch fixes the same problem on catalouging form for the bibliographic record. To test: - Adapt your frameworks: - Make sure 942$c is mandatory - Link another field to an authorised value, for example: 942$n to YESNO - Add a new record - Verify that the first value of each pull down is preselected, there is no way to trigger the 'required' error - Apply patch - Add a record - Verify that classification source is preselected according to the DefaultClassificationSource system preference - Verify that 942$c now defaults to empty - Verify all your other mandatory subfields can be set to empty - Verify that you can't save before correctly setting them - Chane your frameworks - Add a default value for 942$c, for example: CF - Add another record - Verify the itemtype is now preset to your default itemtype - Edit an existing record, verify all values show correctly Signed-off-by: Owen Leonard <oleonard@myacpl.org> It does not work for me when adding a new item. (In reply to Jonathan Druart from comment #11) > It does not work for me when adding a new item. Hum wait, let me follow the test plan without skipping lines :) Created attachment 77490 [details] [review] Bug 14662: Add empty entries to pull downs on item form for mandatory subfields The code assumed that if a subfield is marked as mandatory, there should be no empty entry in the pull downs. This assumption is not correct, as it leads to the first entry of the pull down being preselected if there is no default set. Which means you will never be alerted of any cataloguing errors and errors will be very hard to find later on. Correct behaviour would be to preselect the empty value when there is no default. This means on saving the item an error message is triggered and the cataloger is forced to set the value. To test: - Adapt your frameworks: - Make 942$c non-mandatory - In 952 make itemtype, classification source and some other pull downs like location or collection mandatory - Add a new item - Verify that the first value of each pull down is preselected, there is no way to trigger the 'required' error - Apply patch - Add a new item - Verify that classification source is preselected according to the DefaultClassificationSource system preference - Verify that the itemtype is preselected according to 942$c in the bibliographic record - Verify all mandatory fields can be set to empty - Verify that you can't save before correctly setting them - Change the 942$c in the record to empty - Add another item - Verify the itemtype is now empty - Change your frameworks and set a default for itemtype (Ex: BK) - Repeat default check with another pull down like collection or location Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> We do not want empty values for branches (holdingbranch and homebranch must be mandatory, see bug 21011) Created attachment 77491 [details] [review] Bug 14662: Add empty entries to pull downs in cataloguing form for mandatory subfields First patch deals with the problem on the item form, this patch fixes the same problem on catalouging form for the bibliographic record. To test: - Adapt your frameworks: - Make sure 942$c is mandatory - Link another field to an authorised value, for example: 942$n to YESNO - Add a new record - Verify that the first value of each pull down is preselected, there is no way to trigger the 'required' error - Apply patch - Add a record - Verify that classification source is preselected according to the DefaultClassificationSource system preference - Verify that 942$c now defaults to empty - Verify all your other mandatory subfields can be set to empty - Verify that you can't save before correctly setting them - Chane your frameworks - Add a default value for 942$c, for example: CF - Add another record - Verify the itemtype is now preset to your default itemtype - Edit an existing record, verify all values show correctly Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Awesome work all! Pushed to master for 18.11 Pushed to 18.05.x for 18.05.03 Pushed to 17.11.x for 17.11.09 |