-
-
Notifications
You must be signed in to change notification settings - Fork 205
feat(subsequences): add OpenAPI schema for advanded features DEV-1230 #6547
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
base: refactor-subsequences-2025
Are you sure you want to change the base?
feat(subsequences): add OpenAPI schema for advanded features DEV-1230 #6547
Conversation
… dev-1230-openapi-schema-for-advanded-features
Signed-off-by: Guillermo <[email protected]>
| open_api_201_created_response, | ||
| ) | ||
| from kpi.utils.viewset_mixins import AssetNestedObjectViewsetMixin | ||
| from kpi.versioning import APIV2Versioning |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's specify the custom lookup_field, i.e.:
lookup_field = 'uid_advanced_feature'
| parameters=[ | ||
| OpenApiParameter( | ||
| name='uid', | ||
| name='id', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use the name custom lookup_field uid_advanced_feature instead.
| type=str, | ||
| location=OpenApiParameter.PATH, | ||
| required=True, | ||
| description='UID of the action', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe action is not really accurate in this context?
| parameters=[ | ||
| OpenApiParameter( | ||
| name='uid', | ||
| name='id', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please update the name
| }, | ||
| 'options': {'type': 'object'}, | ||
| }, | ||
| 'required': ['uuid', 'type', 'labels'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should enforce that choices should be present only when type is qualSelectMultiple or qualSectionOne.
| require_auth=False, | ||
| raise_access_forbidden=False, | ||
| ), | ||
| examples=get_advanced_features_create_examples(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The examples of the response seems wrong. I assume they should match the list.
| raise_access_forbidden=False, | ||
| validate_payload=False, | ||
| ), | ||
| examples=get_advanced_features_list_examples(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some examples are missing (manual transcription, automatic google translation).
If you did it in purpose, let's add a description with your example and make the example more generic.
See Delete NLP action for supplement details "
Moreover, the examples are showing a double nested list.
[
[
{
"action": "manual_translation",
"question_xpath": "q1",
"params": [
{
"language": "es"
}
],
"uid": "qa123456789AbCdEfGhIjklm"
}
]
]| ) | ||
| @extend_schema_view( | ||
| create=extend_schema( | ||
| description=read_md('subsequences', 'subsequences/create.md'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please update the markdowns. They could be simplified thanks to the examples you have added to this PR. (e.g. no need to explain what params should look like anymore in the markdown, the examples do the trick).
| ], | ||
| examples=get_advanced_features_update_examples(), | ||
| ), | ||
| retrieve=extend_schema( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| examples=get_advanced_features_list_examples(), | ||
| ), | ||
| partial_update=extend_schema( | ||
| description=read_md('subsequences', 'subsequences/update.md'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as for "retrieve" .
Moreover, I'm not sure to understand the labels of the examples. Many are missing too.
| def _get_action_refs(cls, registry): | ||
| nlp_component = ResolvedComponent( | ||
| name='NLPActionParams', | ||
| schema=cls._get_nlp_params_schema(), | ||
| type=ResolvedComponent.SCHEMA, | ||
| object=dict, | ||
| ) | ||
| registry.register_on_missing(nlp_component) | ||
| qual_component = ResolvedComponent( | ||
| name='QualActionParams', | ||
| schema=cls._get_qual_params_schema(), | ||
| type=ResolvedComponent.SCHEMA, | ||
| object=dict, | ||
| ) | ||
| registry.register_on_missing(qual_component) | ||
|
|
||
| return [ | ||
| nlp_component.ref, | ||
| qual_component.ref, | ||
| ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I created a mixin in #6535 for that purpose. Let's refactor the code when both branches are merged
| name='NLPActionParams', | ||
| schema=cls._get_nlp_params_schema(), | ||
| type=ResolvedComponent.SCHEMA, | ||
| object=dict, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it's expect dict here. If you look at drf-spectular code, it's used to detect collision based on object.__class__ , so it returns type all the time for dict.
Not a big deal, though!
What do you think?
(I personaly used the instance of the OpenApiSerializerExtension class).
…a-for-advanded-features # Conflicts: # jsapp/js/api/models/advancedFeaturePostRequestParamsItem.ts # jsapp/js/api/models/advancedFeatureResponseParamsItem.ts # jsapp/js/api/models/assetAdvancedSubmissionSchema.ts # jsapp/js/api/models/assetAnalysisFormJsonEngines.ts # jsapp/js/api/models/dataSupplementResponse.ts # jsapp/js/api/models/nLPActionParamsItem.ts # jsapp/js/api/models/patchedAdvancedFeaturePatchRequestParamsItem.ts # jsapp/js/api/models/patchedDataSupplementPayload.ts # jsapp/js/api/models/patchedDataSupplementPayloadOneOfOneOfQualOneOfAllOf.ts # jsapp/js/api/models/qualActionParamsItemChoicesItemLabels.ts # static/openapi/schema_v2.json # static/openapi/schema_v2.yaml

🗒️ Checklist
#Support Docs Updates, if any<type>(<scope>)<!>: <title> DEV-1234Front endand/orBack endorworkflow📣 Summary
Add OpenAPI schema for the
/api/v2/assets/{uid_asset}/advanced-features/endpoint.📖 Description
The API schema output files and the generated Orval types have been updated with the schema details for the action parameters in the
QuestionAdvancedFeaturemodel.