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

feat: feature evaluation #394

Merged
merged 2 commits into from
Jun 6, 2023

Conversation

bimalgrg519
Copy link
Collaborator

No description provided.

@bimalgrg519 bimalgrg519 changed the title Feature evaluation feat: feature evaluation Jun 5, 2023
@bimalgrg519 bimalgrg519 force-pushed the feature-evaluation branch from 20527a0 to dc710a1 Compare June 5, 2023 12:32
@bimalgrg519 bimalgrg519 force-pushed the feature-evaluation branch from dc710a1 to be1fd33 Compare June 6, 2023 06:09
@@ -211,7 +211,12 @@ export const FeatureIndexPage: FC = memo(() => {
options && options.hasExperiment
? options.hasExperiment === 'true'
: null;
const tags = options && options.tagIds ? options.tagIds : [];

const tags = Array.isArray(options?.tagIds)
Copy link
Member

Choose a reason for hiding this comment

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

If the options can be null, it will crash.
Shouldn't this be as follows?

const tags = options && Array.isArray(options.tagIds)
  ? options.tagIds
  : [];

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@cre8ivejp The variable options.tagIds can be either a string or a array or strings. Api will accept only array of strings thats why we need to make array of string if there is only one tagId.

Copy link
Member

Choose a reason for hiding this comment

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

The API sends the tagIds as an array. I don't understand why it can be a string if the length of the array is 1.
Are you saying when there is only one element, the tagIds will be a string 'test' instead of an array ['test']?

Copy link
Collaborator Author

@bimalgrg519 bimalgrg519 Jun 6, 2023

Choose a reason for hiding this comment

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

Yes, when there is only one tag, options.tagsId gives us a string and more than one tag gives us an array of strings.

Copy link
Member

Choose a reason for hiding this comment

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

@bimalgrg519

It will be safer if we check it is a string and is not empty.

const tags = options && Array.isArray(options.tagIds)
  ? options.tagIds
  : typeof options.tagIds === 'string' && options.tagIds.length > 0
  ? [options.tagIds]
  : [];

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thank you @cre8ivejp .

@bimalgrg519 bimalgrg519 force-pushed the feature-evaluation branch from be1fd33 to a6f1b40 Compare June 6, 2023 13:45
Copy link
Member

@cre8ivejp cre8ivejp left a comment

Choose a reason for hiding this comment

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

Thanks!

@cre8ivejp cre8ivejp merged commit 9c7cb34 into bucketeer-io:main Jun 6, 2023
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.

2 participants