Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ANW-1474 refactor agent required field customization form #2703

Merged
merged 2 commits into from May 20, 2022

Conversation

quoideneuf
Copy link
Collaborator

Rework the agents/agent_type/required form:

  • change the semantics of the required_field blob so that presence of jsonmodel_type indicates sub record is required; replace 'REQ' with the JSONModel type of the subrecord being required.
  • add tests
  • stop using the FormContext template definitions and emitters, as these were adding unneeded templates all over the places.
  • etc.

Related JIRA Ticket or GitHub Issue

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have read the CONTRIBUTING document.
  • I have authority to submit this code.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@mark-cooper
Copy link
Member

On the technical side everything LGTM. This is a nice refactor!

I found a couple of minor UI "issues" (which can probably be ignored for now):

  1. Where there's a mandatory subrecord the checkbox for "Require Subrecord" should be checked and unmodifiable? (example: Name Form)
  2. When creating a record with required subrecords the required subrecords should have the required icon (asterisk) like fields do? (example: when Genders is a required subrecord it isn't visually clear that you need to select from the dropdown, until saving and getting an error. Translation missing: validation_errors.database_integrity_constraint_conflict__java__javasql__sqlexception__field__gender_id__doesn_t_have_a_default_value)
  3. Genders is in the create Required Fields form (agent person) but not in the sidebar.

In the interests of moving this forward for testing should I merge now and let the issues be resolved later if necessary / relevant?

@mark-cooper mark-cooper merged commit 1f94640 into master May 20, 2022
@mark-cooper mark-cooper deleted the ANW-1474-refactor branch May 20, 2022 23:05
@cdibella
Copy link
Contributor

Thanks for doing this @quoideneuf and for reviewing/merging it @mark-cooper ! When this is returned to in order to clean up the issues noted above, the WAVE tool flags a number of things, mostly missing or orphaned form labels within the main panel.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants