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.

Sign up for GitHub

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

Jump to bottom

Account for localization.isrequired with field.isRequired #4721

Draft
wants to merge 2 commits into
base: production
Choose a base branch
from
Draft

Account for localization.isrequired with field.isRequired #4721

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1fd8bdc
Account for localization.isrequired with field.isRequired
melton-jason Apr 2, 2024
94174d2
Merge branch 'production' into issue-4720
melton-jason May 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion specifyweb/frontend/js_src/lib/components/DataModel/businessRules.ts
View file
Open in desktop
Expand Up @@ -342,7 +342,7 @@ export class BusinessRuleManager<SCHEMA extends AnySchema> {
return true;
if (
field !== undefined &&
!(field.isRequired || field.localization.isrequired) &&
!field.isRequired &&
(value === undefined || value === null)
) {
return false;
Expand Down
8 changes: 4 additions & 4 deletions specifyweb/frontend/js_src/lib/components/DataModel/specifyField.ts
View file
Open in desktop
Expand Up @@ -131,19 +131,19 @@ export abstract class FieldBase {
this.isReadOnly =
globalFieldOverride === 'readOnly' || fieldDefinition.readOnly === true;

this.localization =
Copy link
Member

@maxpatiiuk maxpatiiuk Apr 3, 2024 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you please add the test case that ensures schema config setting impacts the isRequired prop?

specify7/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/specifyField.test.ts

Lines 48 to 61 in 0257f82

describe('isRequired', () => {
test('not-required field', () =>
expect(tables.CollectionObject.getField('catalogNumber')?.isRequired).toBe(
false
));
test('field required by schema', () =>
expect(tables.Accession.getField('accessionNumber')?.isRequired).toBe(
true
));
test('field required by front-end override', () =>
expect(tables.Taxon.getField('parent')?.isRequired).toBe(true));
test('field made optional by front-end override', () =>
expect(tables.Attachment.getField('tableID')?.isRequired).toBe(false));
});

this.table.localization.items[this.name.toLowerCase()] ?? {};

this.isRequired =
globalFieldOverride === 'required'
? true
: globalFieldOverride === 'optional'
? false
: fieldDefinition.required;
: fieldDefinition.required || (this.localization?.isrequired ?? false);
this.type = fieldDefinition.type;
this.length = fieldDefinition.length;
this.databaseColumn = fieldDefinition.column;

this.localization =
this.table.localization.items[this.name.toLowerCase()] ?? {};

this.label =
typeof this.localization.name === 'string' &&
this.localization.name.length > 0
Expand Down