Skip to content

variations update should now properly accept feature keys as an arg #199

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

Merged
elliotCamblor merged 2 commits into from
Jul 4, 2023
Merged

variations update should now properly accept feature keys as an arg #199

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e854e2c
variations update should now properly accept feature keys as an arg
elliotCamblor Jun 30, 2023
20bcb5e
fix tests
elliotCamblor Jul 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 16 additions & 3 deletions src/commands/variations/update.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,11 @@ describe('variations update', () => {
.reply(200, mockVariation)
)
.nock(BASE_URL, (api) => api
.get(`/v1/projects/${projectKey}/variables?feature=${featureKey}`)
.get(`/v1/projects/${projectKey}/features/${featureKey}`)
.reply(200, mockFeature)
)
.nock(BASE_URL, (api) => api
.get(`/v1/projects/${projectKey}/variables?feature=63b5eea3e6e91987bae47f3a`)
.reply(200, mockVariables)
)
.nock(BASE_URL, (api) => api
Expand Down Expand Up @@ -153,7 +157,11 @@ describe('variations update', () => {
.reply(200, mockVariation)
)
.nock(BASE_URL, (api) => api
.get(`/v1/projects/${projectKey}/variables?feature=${featureKey}`)
.get(`/v1/projects/${projectKey}/features/${featureKey}`)
.reply(200, mockFeature)
)
.nock(BASE_URL, (api) => api
.get(`/v1/projects/${projectKey}/variables?feature=63b5eea3e6e91987bae47f3a`)
.reply(200, mockVariables)
)
.nock(BASE_URL, (api) => api
Expand All @@ -180,13 +188,18 @@ describe('variations update', () => {
.reply(200, mockVariation)
)
.nock(BASE_URL, (api) => api
.get(`/v1/projects/${projectKey}/variables?feature=${featureKey}`)
.get(`/v1/projects/${projectKey}/features/${featureKey}`)
.reply(200, mockFeature)
)
.nock(BASE_URL, (api) => api
.get(`/v1/projects/${projectKey}/variables?feature=63b5eea3e6e91987bae47f3a`)
.reply(200, mockVariables)
)
.nock(BASE_URL, (api) => api
.patch(`/v1/projects/${projectKey}/features/${featureKey}/variations/${variationKey}`, requestBody)
.reply(200, mockFeature)
)

.stub(inquirer, 'registerPrompt', () => { return })
.stub(inquirer, 'prompt', () => {
return {
Expand Down
7 changes: 4 additions & 3 deletions src/commands/variations/update.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,15 @@ import inquirer from 'inquirer'
import UpdateCommand from '../updateCommand'
import { fetchVariationByKey, updateVariation, UpdateVariationParams } from '../../api/variations'
import { featurePrompt, keyPrompt, namePrompt } from '../../ui/prompts'
import { Variable } from '../../api/schemas'
import { Feature, Variable } from '../../api/schemas'

import {
getVariationVariablePrompt,
promptForVariationVariableValues,
variationPrompt,
} from '../../ui/prompts/variationPrompts'
import { Args, Flags } from '@oclif/core'
import { fetchFeatureByKey } from '../../api/features'

export default class UpdateVariation extends UpdateCommand {
static hidden = false
Expand Down Expand Up @@ -65,11 +66,11 @@ export default class UpdateVariation extends UpdateCommand {
} else {
selectedVariation = await fetchVariationByKey(this.authToken, this.projectKey, featureKey, args.key)
}

const feature = await fetchFeatureByKey(this.authToken, this.projectKey, featureKey) as Feature
this.prompts.push(await getVariationVariablePrompt(
this.authToken,
this.projectKey,
featureKey,
feature._id,
Copy link
Copy Markdown
Contributor

@laurawarr laurawarr Jul 4, 2023

Choose a reason for hiding this comment

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

this seems like it would be an API bug, all the endpoints should accept either a key or an ID 🤔

Copy link
Copy Markdown
Contributor

@laurawarr laurawarr Jul 4, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

ohhh, I see the error is coming from the variables request, not the variations request

Copy link
Copy Markdown
Contributor Author

@elliotCamblor elliotCamblor Jul 4, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Even for query params? This field in the query params class has a decorater called isObjectIDorFalse which makes it seem intentional

Copy link
Copy Markdown
Contributor Author

@elliotCamblor elliotCamblor Jul 4, 2023

Choose a reason for hiding this comment

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

Im just not sure why 😅

Copy link
Copy Markdown
Contributor

@laurawarr laurawarr Jul 4, 2023

Choose a reason for hiding this comment

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

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 😛

))

this.writer.blankLine()
Expand Down