Bug 22734 - Fund not marked as mandatory when ordering from a staged file
Summary: Fund not marked as mandatory when ordering from a staged file
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Owen Leonard
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on:
Blocks: 22802 25473
  Show dependency treegraph
 
Reported: 2019-04-18 08:26 UTC by Séverine Queune
Modified: 2020-05-14 11:52 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
19.05.00


Attachments
'Fund' should be in red (31.03 KB, image/png)
2019-04-18 08:26 UTC, Séverine Queune
Details
Bug 22734: Fund not marked as mandatory when ordering from a staged file (15.07 KB, patch)
2019-04-18 14:34 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 22734: Fund not marked as mandatory when ordering from a staged file (15.16 KB, patch)
2019-04-18 15:52 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 22734: Fund not marked as mandatory when ordering from a staged file (15.22 KB, patch)
2019-04-25 07:22 UTC, Martin Renvoize
Details | Diff | Splinter Review
Fund defined for all item(s) shoudln't need for order (73.75 KB, image/png)
2019-04-25 15:50 UTC, Nick Clemens
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Séverine Queune 2019-04-18 08:26:12 UTC
Created attachment 88267 [details]
'Fund' should be in red

When adding a new order from a staged file, "Fund" label should be in red as it's a mandatory field. As you can see on attachment, not selecting a fund is correctly blocked.
Should the word 'Required' be added at the right of the 2 fields 'Quantity' and 'Fund' to have the same display as we have adding an order directly in a basket ?
Comment 1 Owen Leonard 2019-04-18 14:34:11 UTC Comment hidden (obsolete)
Comment 2 Biblibre Sandboxes 2019-04-18 15:52:10 UTC
Patch tested with a sandbox, by Séverine QUEUNE <severine.queune@bulac.fr>
Comment 3 Biblibre Sandboxes 2019-04-18 15:52:33 UTC Comment hidden (obsolete)
Comment 4 Séverine Queune 2019-04-18 15:53:43 UTC
Looked good to me !
Thank you Owen
Comment 5 Martin Renvoize 2019-04-25 07:22:24 UTC
Created attachment 88697 [details] [review]
Bug 22734: Fund not marked as mandatory when ordering from a staged file

This patch does some refactoring of JavaScript to make handling of
required fields more robust. It also moves some template JavaScript to
the footer and some to a separate file.

A fallback "pattern" attribute is added to the quantity field, requiring
0-9+ values.

The "required" property of the funds dropdown is now dependent on the
value set under the "Default accounting details" tab. If a default is
selected, the individual item funds are preselected and not marked
required.

To test, apply the patch and begin the process of ordering from a staged
file.

 - Select all items to import.
   - Under each item, the fund should be marked mandatory.
   - The quantity field should accept only numbers.
   - Select the "Default accounting details" tab and select a default
     fund.
     - Return to the "Select to import" tab. Your selected default fund
       should now be preselected under each item. The fund should no
       longer be marked required.

       Un-selecting a default fund should result in each item fund
       select becoming required.

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 6 Martin Renvoize 2019-04-25 07:23:46 UTC
When I came to this bug I wasn't expecting to find such a large patch.. refactoring seems to be solid though and I can't find any regressions, well done Owen.

I'd love to see some selenium regression tests here but I'm not going to fail QA for that.

Passed.
Comment 7 Nick Clemens 2019-04-25 11:58:42 UTC
Awesome work all!

Pushed to master for 19.05
Comment 8 Nick Clemens 2019-04-25 15:49:31 UTC
Erk, I missed an issue here:
If the funds are defined per item we shouldn't have to supply a fund for the order as a whole.

This will need a followup to take that into account or cannot be in the release
Comment 9 Nick Clemens 2019-04-25 15:50:53 UTC
Created attachment 88791 [details]
Fund defined for all item(s) shoudln't need for order
Comment 10 Martin Renvoize 2019-04-26 15:29:49 UTC
Enhancement will not be backported to 18.11.x series.
Comment 11 Martin Renvoize 2019-04-26 15:30:23 UTC
Do we need a new bug opening for comment #8
Comment 12 Owen Leonard 2019-04-29 11:58:35 UTC
(In reply to Martin Renvoize from comment #11)
> Do we need a new bug opening for comment #8

It sounds like we do, but I'm unable to reproduce the problem. I think I just don't understand what the right circumstances are to trigger it.
Comment 13 Nick Clemens 2019-04-29 12:35:20 UTC
(In reply to Owen Leonard from comment #12)
> (In reply to Martin Renvoize from comment #11)
> > Do we need a new bug opening for comment #8
> 
> It sounds like we do, but I'm unable to reproduce the problem. I think I
> just don't understand what the right circumstances are to trigger it.

bug 22802 opened