Bug 32495

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