Skip to content

Fix: Feature selection display #29

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

Merged
merged 2 commits into from
Jun 16, 2025
Merged

Conversation

rickary
Copy link
Contributor

@rickary rickary commented Jun 9, 2025

https://github.com/Flagsmith/flagsmith-private/issues/30

The issue seems to be using index to display the data in the table. Rewrote to use id instead.

@rickary rickary changed the title #30 Feature data mismatch Fix: Feature data mismatch Jun 9, 2025
@rickary rickary changed the title Fix: Feature data mismatch Fix: Feature selection display Jun 9, 2025
@rickary rickary requested a review from matthewelwell June 9, 2025 12:34
@matthewelwell matthewelwell requested a review from Zaimwa9 June 11, 2025 10:39
Copy link
Collaborator

@riceyrice riceyrice left a comment

Choose a reason for hiding this comment

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

😊

Copy link

@Zaimwa9 Zaimwa9 left a comment

Choose a reason for hiding this comment

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

Tested, fixes the issue thanks 👍 . Minor comment for readability

Comment on lines 227 to 238
{issueFeatureIds.map((featureId) => {
const baseFeature = features.find((f) => String(f.id) === featureId);
if (!baseFeature) {
return null;
}

const envFeaturesForThisFeature = environmentsFeatures
.map((envFeatures) => {
const matchingFeature = envFeatures.find((f) => String(f.id) === featureId);
return matchingFeature;
})
.filter(Boolean) as Feature[];
Copy link

Choose a reason for hiding this comment

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

Would you mind extracting the formatting in a specific function (for abstraction) and just map over the result ?

Something that would like:

const featureTableData = buildFeatureTableData({
  issueFeatureIds,
  features,
  environmentsFeatures,
});

return (
  <Fragment>
    {featureTableData.map(({ featureId, baseFeature, environmentFeatures }) => (
      <Box key={id} xcss={{ marginTop: "space.300", marginBottom: "space.100" }}>

@rickary rickary requested a review from Zaimwa9 June 16, 2025 09:18
Copy link

@Zaimwa9 Zaimwa9 left a comment

Choose a reason for hiding this comment

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

LGTM thanks 👍

@matthewelwell matthewelwell merged commit 293e670 into main Jun 16, 2025
1 check passed
@matthewelwell matthewelwell deleted the 30-request-parameter-issue branch June 16, 2025 15:04
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.

4 participants