variations update should now properly accept feature keys as an arg#199
Conversation
| selectedVariation = await fetchVariationByKey(this.authToken, this.projectKey, featureKey, args.key) | ||
| } | ||
|
|
||
| const feature = await fetchFeatureByKey(this.authToken, this.projectKey, featureKey) |
There was a problem hiding this comment.
could we just do this if we're not using the feature prompt? otherwise we're fetching the feature from the api twice
There was a problem hiding this comment.
I still need to fetch even if were not using the feature prompt, since the feature prompt doesnt return the full feature. I made a ticket to refactor that because the prompt is being used in several different places
There was a problem hiding this comment.
Won't the request to fetch the variables associated with the feature already return an error like this if the key is invalid?
There was a problem hiding this comment.
yeah I think youre right. Pretty sure I got baited by typescript here
elliotCamblor
force-pushed
the
DVC-8074-variations-update-feature-key
branch
from
98aabfc to
9c6a7cd
Compare
| this.authToken, | ||
| this.projectKey, | ||
| featureKey, | ||
| feature._id, |
There was a problem hiding this comment.
this seems like it would be an API bug, all the endpoints should accept either a key or an ID 🤔
There was a problem hiding this comment.
ohhh, I see the error is coming from the variables request, not the variations request
There was a problem hiding this comment.
Even for query params? This field in the query params class has a decorater called isObjectIDorFalse which makes it seem intentional
There was a problem hiding this comment.
Im just not sure why 😅
There was a problem hiding this comment.
Ideally all values could be accepted as keys, I'm assuming we made this choice to avoid querying the feature by key just to build the variable query params. We could consider updating the API, but that's out of scope of this work 😛
elliotCamblor
force-pushed
the
DVC-8074-variations-update-feature-key
branch
from
9c6a7cd to
20bcb5e
Compare
- postcss <8.5.10 -> ^8.5.10 (resolves to 8.5.14) (medium, alert #199)
2 participants
No description provided.