Bug 16754 - Use validation plugin in budgets, planning, and contracts
Summary: Use validation plugin in budgets, planning, and contracts
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Target Milestone: ---
Assignee: Owen Leonard
QA Contact: Testopia
URL: /cgi-bin/koha/admin/aqbudgetperiods.pl
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-06-16 13:22 UTC by Owen Leonard
Modified: 2018-11-28 15:34 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 16754 - Use validation plugin when adding or editing budgets (9.03 KB, patch)
2016-06-16 13:40 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 16754 - Use validation plugin in budgets, planning, and contracts (22.14 KB, patch)
2016-06-17 11:57 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 16754 - Use validation plugin in budgets, planning, and contracts (22.24 KB, patch)
2016-08-05 17:07 UTC, Frédéric Demians
Details | Diff | Splinter Review
Bug 16754: Use validation plugin in budgets, planning, and contracts (22.89 KB, patch)
2018-10-03 14:53 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 16754: Use validation plugin in budgets, planning, and contracts (22.94 KB, patch)
2018-10-04 08:52 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 16754: Use validation plugin in budgets, planning, and contracts (23.00 KB, patch)
2018-10-06 20:45 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Owen Leonard 2016-06-16 13:22:09 UTC
There is a lot of custom form validation JavaScript in the budget editing template which can be handled instead by the jQuery validation plugin.
Comment 1 Owen Leonard 2016-06-16 13:40:22 UTC Comment hidden (obsolete)
Comment 2 Owen Leonard 2016-06-16 13:47:32 UTC
I see that the date comparison operation is handled better in other templates. I'm going to revise this patch to try to incorporate a working version.
Comment 3 Owen Leonard 2016-06-17 11:57:04 UTC Comment hidden (obsolete)
Comment 4 Frédéric Demians 2016-08-01 17:29:03 UTC
There is something wrong (strange) in budget creation:

- Create a new budget

- Click on Save

- A warning is displayed after Start date input box: This field is required.
  And the date picker is displayed

- Pick a date. => The date is copied to the input box, but (1) it is red, and
  (2) the message 'This field is required' is still displayed.
Comment 5 Owen Leonard 2016-08-05 13:47:04 UTC
(In reply to Frédéric Demians from comment #4)

> - Pick a date. => The date is copied to the input box, but (1) it is red, and
>   (2) the message 'This field is required' is still displayed.

This is a problem not with this patch but with the way the validation plugin works together with the datepicker widget. Now that I know of the problem I can find it elsewhere too, for instance aqcontract.pl.

I can try to find a solution, but I would like to do so as a separate bug if that's okay with everyone.
Comment 6 Frédéric Demians 2016-08-05 17:07:18 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2016-08-12 11:05:49 UTC
I'm not sure how this is valid but, if you enter start date == end date, the validator tell you that it is not allowed.
It's a change with before this patch. We could imagine a budget for only 1 day, which was allowed before, but not anymore.
Feel free to contradict :)
Comment 8 Owen Leonard 2016-08-26 14:07:28 UTC
(In reply to Jonathan Druart from comment #7)

> We could imagine a budget for only 1
> day, which was allowed before, but not anymore.

That seems very unlikely. It wouldn't be difficult to add a new validation setting to allow the two dates to match, but I'm not sure it's worth it just for this form and without an indication that it's an important feature. I think this form should match others in Koha where the end date in a date span is required to be after the start.
Comment 9 Owen Leonard 2018-10-03 14:53:11 UTC
Created attachment 79896 [details] [review]
Bug 16754: Use validation plugin in budgets, planning, and contracts

This patch removes the custom javascript form validation from three
templates: Budgets, budget planning, and contracts. Validation via the
validation plugin is done instead.

This patch also removes use of event attributes like onclick and
onchange. Also changed: I added the "Required" text next to required
fields as necessary.

To test, apply the patch and go to Administration -> Budgets.

- Test the budget creation process for adding, editing, and duplicating
  budgets. In all cases:
  - The start date and end date should be required
  - You should be warned if you enter an end date which falls before the
    start date.
  - The description should be required
  - The budget amount should be required to be a number

To test the budget planning page:

Note that much of the JS removed from this template was unused. It
looked for inputs by the name 'est_total' which don't exist.

- When viewing the table of funds for a budget, select a choice from the
  'planning' toolbar menu.
  - Verify that after entering a numeric value in any cell, the
    'remaning' value at the end of the row is correctly updated.
  - Verify that after entering non-numeric data an error is displayed.
  - Verify that clicking the 'auto-fill' button at the end of the row
    works to fill any empty or zero-value fields with the remaning funds
    available.

To test the vendor contracts page:

- Go to Acquisitions -> Vendor -> Contracts
- Add or edit a contract
  - Confirm that name, start date, and end date are required.
  - Confirm that you are warned if you enter an end date which falls
    before the start date.
Comment 10 Michal Denar 2018-10-04 08:52:02 UTC
Created attachment 79940 [details] [review]
Bug 16754: Use validation plugin in budgets, planning, and contracts

This patch removes the custom javascript form validation from three
templates: Budgets, budget planning, and contracts. Validation via the
validation plugin is done instead.

This patch also removes use of event attributes like onclick and
onchange. Also changed: I added the "Required" text next to required
fields as necessary.

To test, apply the patch and go to Administration -> Budgets.

- Test the budget creation process for adding, editing, and duplicating
  budgets. In all cases:
  - The start date and end date should be required
  - You should be warned if you enter an end date which falls before the
    start date.
  - The description should be required
  - The budget amount should be required to be a number

To test the budget planning page:

Note that much of the JS removed from this template was unused. It
looked for inputs by the name 'est_total' which don't exist.

- When viewing the table of funds for a budget, select a choice from the
  'planning' toolbar menu.
  - Verify that after entering a numeric value in any cell, the
    'remaning' value at the end of the row is correctly updated.
  - Verify that after entering non-numeric data an error is displayed.
  - Verify that clicking the 'auto-fill' button at the end of the row
    works to fill any empty or zero-value fields with the remaning funds
    available.

To test the vendor contracts page:

- Go to Acquisitions -> Vendor -> Contracts
- Add or edit a contract
  - Confirm that name, start date, and end date are required.
  - Confirm that you are warned if you enter an end date which falls
    before the start date.

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 11 Katrin Fischer 2018-10-06 20:45:25 UTC
Created attachment 80169 [details] [review]
Bug 16754: Use validation plugin in budgets, planning, and contracts

This patch removes the custom javascript form validation from three
templates: Budgets, budget planning, and contracts. Validation via the
validation plugin is done instead.

This patch also removes use of event attributes like onclick and
onchange. Also changed: I added the "Required" text next to required
fields as necessary.

To test, apply the patch and go to Administration -> Budgets.

- Test the budget creation process for adding, editing, and duplicating
  budgets. In all cases:
  - The start date and end date should be required
  - You should be warned if you enter an end date which falls before the
    start date.
  - The description should be required
  - The budget amount should be required to be a number

To test the budget planning page:

Note that much of the JS removed from this template was unused. It
looked for inputs by the name 'est_total' which don't exist.

- When viewing the table of funds for a budget, select a choice from the
  'planning' toolbar menu.
  - Verify that after entering a numeric value in any cell, the
    'remaning' value at the end of the row is correctly updated.
  - Verify that after entering non-numeric data an error is displayed.
  - Verify that clicking the 'auto-fill' button at the end of the row
    works to fill any empty or zero-value fields with the remaning funds
    available.

To test the vendor contracts page:

- Go to Acquisitions -> Vendor -> Contracts
- Add or edit a contract
  - Confirm that name, start date, and end date are required.
  - Confirm that you are warned if you enter an end date which falls
    before the start date.

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 12 Nick Clemens 2018-10-17 19:21:14 UTC
Awesome work all!

Pushed to master for 18.11
Comment 13 Martin Renvoize 2018-10-26 12:19:49 UTC
Nice bit of code tidying.

Pushed to 18.05.x for 18.05.06
Comment 14 Fridolin SOMERS 2018-11-09 14:38:02 UTC
I prefer not to push to oldstable 17.11.x to avoid behavior and display changes