Bug 32495

Summary: Required fields in API and UI form don't match
Product: Koha Reporter: Matt Blenkinsop <matt.blenkinsop>
Component: ERMAssignee: Matt Blenkinsop <matt.blenkinsop>
Status: CLOSED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: normal    
Priority: P5 - low CC: david, jonathan.druart, jonathan.field, lucas, m.de.rooy, martin.renvoize, pedro.amorim, tomascohen
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
This enhancement changes the new agreement form so that the description field is no longer required (to match with the API).
Version(s) released in:
23.05.00,22.11.04
Bug Depends on:    
Bug Blocks: 33235    
Attachments: Bug 32495: Mis-matched required fields in UI and api
Bug 32495: Mis-matched required fields in UI and api
Bug 32495: Mis-matched required fields in UI and api
Bug 32495: (follow-up) Remove required property
Bug 32495: (follow-up) Remove required property

Description Matt Blenkinsop 2022-12-19 17:42:46 UTC
The required fields in the API for adding a new agreement and the form on the UI to add it don't match.

The API endpoint for POST "/api/v1/erm/agreements" requires "name, status, is_perpetual"
The form on the UI requires the fields above (is_perpetual is a checkbox with a default value selected) but also requires "Description".

Description should either be added to the API or the flag on the UI should be removed.
Comment 1 Jonathan Druart 2023-01-04 15:31:33 UTC
Jonathan, what's correct here?
Comment 2 Jonathan Field 2023-01-04 15:44:17 UTC
I would say that the Description could definitely be an optional field (in UI and therefore API).
In fact, is_perpetual could also be deemed not required. It is in the UI right now as it's a Yes/No option.
Comment 3 Matt Blenkinsop 2023-01-09 15:15:54 UTC
Created attachment 145149 [details] [review]
Bug 32495: Mis-matched required fields in UI and api

The "description" field is marked as required in the UI when creating a new Agreement. This flag has been removed to match the API where it is not a required field. Is Perpetual has not been marked as required as it is a checkbox with a default selection so will always provide a value.

Test plan:
1) In the ERM module, add a new agreement and observe that the Description field is required
2) Apply patch
3) Repeat step 1 but this time the Required flag will not be visible.
Comment 4 David Nind 2023-01-10 22:17:29 UTC
Created attachment 145205 [details] [review]
Bug 32495: Mis-matched required fields in UI and api

The "description" field is marked as required in the UI when creating a new Agreement. This flag has been removed to match the API where it is not a required field. Is Perpetual has not been marked as required as it is a checkbox with a default selection so will always provide a value.

Test plan:
1) In the ERM module, add a new agreement and observe that the Description field is required
2) Apply patch
3) Repeat step 1 but this time the Required flag will not be visible.

Signed-off-by: David Nind <david@davidnind.com>
Comment 5 David Nind 2023-01-10 22:49:30 UTC
Testing notes (using KTD):

1. After step 2 (apply the patch), you have to yarn build_js

2. Then I flushed the cached (flush_memcached) and restarted everything (restart_all).
Comment 6 Marcel de Rooy 2023-02-24 10:46:59 UTC
Created attachment 147290 [details] [review]
Bug 32495: Mis-matched required fields in UI and api

The "description" field is marked as required in the UI when creating a new Agreement. This flag has been removed to match the API where it is not a required field. Is Perpetual has not been marked as required as it is a checkbox with a default selection so will always provide a value.

Test plan:
1) In the ERM module, add a new agreement and observe that the Description field is required
2) Apply patch
3) Repeat step 1 but this time the Required flag will not be visible.

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 7 Tomás Cohen Arazi 2023-02-24 19:56:29 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 8 Matt Blenkinsop 2023-03-01 10:48:35 UTC
Created attachment 147555 [details] [review]
Bug 32495: (follow-up) Remove required property

This follow-up removes the required property from the text area as this
was missed previously.

Test plan:
1) In the ERM module, add a new agreement and observe that the Description field is required
2) Apply patch
3) Repeat step 1 but this time the Required flag will not be visible.
4) Submit the form with the Description field blank - the submission
   should work
Comment 9 Pedro Amorim 2023-03-01 11:50:42 UTC
Created attachment 147558 [details] [review]
Bug 32495: (follow-up) Remove required property

This follow-up removes the required property from the text area as this
was missed previously.

Test plan:
1) In the ERM module, add a new agreement and observe that the Description field is required
2) Apply patch
3) Repeat step 1 but this time the Required flag will not be visible.
4) Submit the form with the Description field blank - the submission
   should work

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 10 Tomás Cohen Arazi 2023-03-02 11:42:34 UTC
Follow-up pushed to master.
Comment 11 Jonathan Druart 2023-03-02 14:45:50 UTC
You broke the tests.
Comment 12 Jonathan Druart 2023-03-02 14:46:46 UTC
(In reply to Jonathan Druart from comment #11)
> You broke the tests.

Timed out retrying after 4000ms: Not enough elements found. Found '2', expected '3'.
t/cypress/integration/Agreements_spec.ts:188:65
  186 | 
  187 |         cy.get("#agreements_add").contains("Submit").click();
> 188 |         cy.get("input:invalid,textarea:invalid,select:invalid").should(
      |                                                                 ^
  189 |             "have.length",
  190 |             3
  191 |         );
Comment 13 Matt Blenkinsop 2023-03-17 17:20:50 UTC
Nice work everyone!

Pushed to stable for 22.11.x
Comment 14 Lucas Gass 2023-04-21 16:50:54 UTC
Not needed in 22.05.x, no backport