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 ?
Created attachment 88299 [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.
Patch tested with a sandbox, by Séverine QUEUNE <severine.queune@bulac.fr>
Created attachment 88322 [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>
Looked good to me ! Thank you Owen
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>
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.
Awesome work all! Pushed to master for 19.05
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
Created attachment 88791 [details] Fund defined for all item(s) shoudln't need for order
Enhancement will not be backported to 18.11.x series.
Do we need a new bug opening for comment #8
(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.
(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