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

resource-config: set collection-name on annotated field #397

Merged
merged 6 commits into from
Dec 5, 2022

Conversation

mdibaiee
Copy link
Member

@mdibaiee mdibaiee commented Dec 2, 2022

Changes

  • Automatically fill in field marked with x-collection-name with last piece of collection name

Tests

Tested locally

Issues

#311

Content

N/A

Screenshots

image

@mdibaiee mdibaiee requested a review from a team as a code owner December 2, 2022 15:21
@mdibaiee mdibaiee force-pushed the mahdi/collection-name-annotation branch from def27eb to 757c50e Compare December 2, 2022 15:25
@mdibaiee mdibaiee force-pushed the mahdi/collection-name-annotation branch from 757c50e to 055e116 Compare December 2, 2022 15:26
Copy link
Member

@travjenkins travjenkins left a comment

Choose a reason for hiding this comment

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

This creates an infinite loop. Do not merge this. I'll take a look at it.

Copy link
Member

@travjenkins travjenkins left a comment

Choose a reason for hiding this comment

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

Switched the approach to setting the default property in the schema. This allows ajv to set the value once it makes it into jsonForms.

Please test and make sure this is still working as expected for you.

Also... I am assuming we're taking the approach that only a single field should ever have this annotation.

Comment on lines +54 to +68
const collectionNameFieldKey = useMemo(() => {
if (resourceSchema.properties) {
// Find the field with the collection name annotation
const collectionNameField =
Object.entries(resourceSchema.properties).find(
([_, value]) =>
value && Annotations.defaultResourceConfigName in value
) ?? [];

// Try to fetch the key
return collectionNameField[0];
}

return null;
}, [resourceSchema.properties]);
Copy link
Member

Choose a reason for hiding this comment

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

This chunk isn't too heavy but since we don't need to recalculate it often decided to move this into a stand alone memo.

Comment on lines 71 to 81
const preparedResourceSchema = useMemo(() => {
if (collectionNameFieldKey) {
// Add a default property set to the stripped collection name
const response = cloneDeep(resourceSchema);
response.properties[collectionNameFieldKey].default =
stripPathing(collectionName);
return response;
}

return resourceSchema.properties;
}, [collectionName, collectionNameFieldKey, resourceSchema]);
Copy link
Member

Choose a reason for hiding this comment

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

Make a memo of the schema which will either be the normal schema or one with the injected default value.

Comment on lines +28 to +30
// Setup immer
enableMapSet();
setAutoFreeze(false);
Copy link
Member

Choose a reason for hiding this comment

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

This allows AJV to more easily manipulate the data being passed to it. Related to this change but one I have been eyeing for a while so made it here.

@@ -30,6 +32,7 @@ export const addKeywords = (ajv: any) => {
ajv.addKeyword('advanced'); // Should be collapsed by default (over ridden if section contains required fields)
ajv.addKeyword('order'); // Used to order the fields in the UI
ajv.addKeyword('x-oauth2-provider'); // Used to display OAuth
ajv.addKeyword('x-collection-name'); // Used to default name in resource configs
Copy link
Member

Choose a reason for hiding this comment

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

We keep all custom annotations in here so we can keep them all documented together.

@travjenkins travjenkins merged commit dc3e237 into main Dec 5, 2022
@travjenkins travjenkins deleted the mahdi/collection-name-annotation branch December 5, 2022 21:52
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.

3 participants