-
Notifications
You must be signed in to change notification settings - Fork 8
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
feat(API): add Custom Metadata endpoints #474
Conversation
@@ -10,7 +10,7 @@ tags: | |||
parameters: | |||
- "$ref": "../../parameters.yaml#/X-PhraseApp-OTP" | |||
- "$ref": "../../parameters.yaml#/account_id" | |||
- "$ref": "../../parameters.yaml#/data_type" | |||
- "$ref": "../../parameters.yaml#/custom_metadata_data_type" |
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'm curious, why is there a data_type parameter? is it for filtering properties only for the given type? what was the use case for adding this?
in phrase code, I can see there's also q
parameter (which makes more sense IMO), why has it been left out?
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.
is it for filtering properties only for the given type?
yep, believe so
what was the use case for adding this?
perhaps a convenience method of getting all date-related properties for mass update?
…CM schema in translation key response (#480) * add CM to create/update keys * Add custom metadata object on CM get key * Add more description for update --------- Co-authored-by: Ildar Safin <knock@ildarsafin.tech>
parameters.yaml
Outdated
@@ -35,9 +35,18 @@ custom_metadata_data_type: | |||
in: query | |||
name: data_type | |||
description: Data Type of Custom Metadata Property | |||
required: false | |||
required: true |
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.
by doing this, it is also mandatory in the index
call, right? I'm pretty sure we don't want that.
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.
this is actually confusing since it's rendered also in create/update CM pages and it's shown as not required which is not true
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.
yeah, I'm afraid it can't be reused as a single reference then.
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.
wait, perhaps it can be specified using required
array
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.
which one?
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.
nah, forget it. you can't reuse the same reference.
Screen.Recording.2023-11-30.at.15.02.48.mov
DONT MERGE
DONT MERGE UNTIL FEATURE TOGGLE IS REMOVED!
Ticket
https://phrase.atlassian.net/browse/TSS-2511