Bug 8637

Summary: Lack of control on mandatory in orderreveive.tt
Product: Koha Reporter: Christophe Croullebois <christophe.croullebois>
Component: AcquisitionsAssignee: Christophe Croullebois <christophe.croullebois>
Status: CLOSED FIXED QA Contact:
Severity: critical    
Priority: P1 - high CC: chris, jonathan.druart, katrin.fischer, paul.poulain
Version: 3.8   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: proposed patch
rewrited patch
Bug 8637: Lack of control on mandatory in orderreveive.tt
this patch is finalised
patch finalized
[SIGNED-OFF] Bug 8637: Lack of control on mandatory in orderreveive.tt
proposed patch
proposed patch
[SIGNED-OFF] Bug 8637: Lack of control on mandatory in orderreveive.tt

Description Christophe Croullebois 2012-08-14 08:37:58 UTC
Just add a check in Jscript when the form is submited.
Comment 1 Christophe Croullebois 2012-08-14 09:03:50 UTC Comment hidden (obsolete)
Comment 2 Owen Leonard 2012-08-14 17:04:00 UTC
This looks like it works correctly, but I think it would be a good idea to pull a couple of things from the additem.pl template:

<label class="required">[% ite.subfield %] - [% ite.marc_lib %]</label>

...and...

[% IF ( ite.mandatory ) %] <span class="required">Required</span>[% END %]

This would make the mandatory nature of the fields a little more obvious and make it consistent with other forms.

Looking at this makes me wonder... Why don't we use services/itemrecorddisplay.pl for additem.pl too?
Comment 3 Christophe Croullebois 2012-08-16 13:07:02 UTC Comment hidden (obsolete)
Comment 4 Chris Cormack 2012-08-18 02:08:49 UTC Comment hidden (obsolete)
Comment 5 Chris Cormack 2012-08-18 02:09:38 UTC
Works as described, signing off
Comment 6 Jonathan Druart 2012-08-20 11:27:05 UTC
QA Comments:

Please use an existing class for style.
There are .required and span.required in the staff-global.css file.

+ rename the alertString2 with alertString

Marked as Failed QA
Comment 7 Christophe Croullebois 2012-08-20 12:32:27 UTC Comment hidden (obsolete)
Comment 8 Christophe Croullebois 2012-08-20 12:38:17 UTC Comment hidden (obsolete)
Comment 9 Katrin Fischer 2012-08-22 06:02:43 UTC Comment hidden (obsolete)
Comment 10 Christophe Croullebois 2012-08-22 13:06:59 UTC Comment hidden (obsolete)
Comment 11 Katrin Fischer 2012-08-22 13:27:54 UTC
Hi Christophe - what is missing used for?
Comment 12 Christophe Croullebois 2012-08-22 13:41:53 UTC
hello Katrin,
it is Jonathan who prefers to have style in css files, I have just created this class to point the background style on it.
And I have done the some in additem.
But I failed with the last patch, I'll put the right one in a minute.
For testing, it changes nothing, just the patch must be ok as before.
Thx Katrin.
Comment 13 Christophe Croullebois 2012-08-22 13:53:07 UTC Comment hidden (obsolete)
Comment 14 Katrin Fischer 2012-08-22 20:17:18 UTC
Hi Cristophe,
I remember when I tested this morning, that the mandatory fields that were missing values got highlighted in yellow. This does not seem to work for me now. Sorry!
Comment 15 Katrin Fischer 2012-08-22 20:21:01 UTC
Forgot to reload the CSS... works fine now - signing off!
Comment 16 Katrin Fischer 2012-08-22 20:21:58 UTC
Created attachment 11755 [details] [review]
[SIGNED-OFF] Bug 8637: Lack of control on mandatory in orderreveive.tt

Just add a check in Jscript when the form is submited, the same as in additem.tt
On Owen's suggestion I have added the red color and 'Required', the same as in additem.
On Jonathan's suggestion I have used the CSS class for red and italic and I have changed a variable's name (alertString2).
Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Mandatory fields are now correctly checked on saving.

On other Jonathan's suggestion I have created a new class "missing" and I have added the background to staff-global.css.
The same for additem.tt.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Retested because of minor CSS change. Works nicely.
Comment 17 Jonathan Druart 2012-08-23 07:47:57 UTC
QA Comments:

Templates modifications only. Patch clean.
It refactores some css style into a new class "missing".

Marked as Passed QA.
Comment 18 Paul Poulain 2012-08-31 12:09:38 UTC
This patch fixes the problem (so i'll push), but it could be improved later: if you recieve more than one item, and click on "Add" before clicking on "Save", you'll be warned of a problem only when you click "Save". You should be warned on each "Add"
(if you add more than 1 recieve, and only 1 lack a mandatory information, you won't know/see which item is wrong !)
Comment 19 Chris Cormack 2012-09-01 02:55:06 UTC
Pushed to 3.8.x, will be in 3.8.5. I agree with Paul this could be improved, but it is a good start already. Kaizen! :)