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.
Jonathan, what's correct here?
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.
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.
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>
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).
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>
Pushed to master for 23.05. Nice work everyone, thanks!
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
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>
Follow-up pushed to master.
You broke the tests.
(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 | );
Nice work everyone! Pushed to stable for 22.11.x
Not needed in 22.05.x, no backport