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

FF: Endpoint to Validate Imported JSON files #1716

Closed
danoswaltCL opened this issue Jul 2, 2024 · 11 comments · Fixed by #1798 or #2022
Closed

FF: Endpoint to Validate Imported JSON files #1716

danoswaltCL opened this issue Jul 2, 2024 · 11 comments · Fixed by #1798 or #2022

Comments

@danoswaltCL
Copy link
Collaborator

On drag end or select of valid feature-flag JSON file(s) in the import modal, we will need to support a view that gives the user a list of files, their compatibility status (COMPATIBLE, WARNING and INCOMPATIBLE, and a text warning message for the warning and incompatible states that will give more complete information about what the user can expect or need to correct in order to proceed.

@ppratikcr7 @VivekFitkariwala please help me to fill out the details for acceptance criteria of the response and request so that it will look familiar to the way we do it for experiments.

Image

@bcb37
Copy link
Collaborator

bcb37 commented Jul 8, 2024

The response and request objects should look like they do for /api/flags/:id , with "featureFlagSegmentInclusion" and "featureFlagSegmentExclusion" arrays that define the inclusion and exclusion lists, which should be created via calls to the /segment endpoints as part of the import action. I'm not sure we do it for experiments, but maybe we should verify that all included/excluded (public) segments already exist?

@amurphy-cl
Copy link
Collaborator

Reminder that for v1, imports will be one at a time.

Imported feature flag design IDs should not be processed (a new ID will be assigned).

Required fields: Name, Key, Description (can be null if empty), App Context, Tags (can be null), Include List, Exclude List

If there are public segments that don't exist during the import, the compatibility status will be WARNING
UI will say that "At least one segment listed in this feature flag could not be found".

@ppratikcr7 ppratikcr7 self-assigned this Jul 19, 2024
@ppratikcr7
Copy link
Collaborator

For backend:

the /validation api will expect the below format:

[{
fileName: string;
fileContent: string;
}]
This api will return the below format:

[{
fileName: string;
compatibilityType: FF_COMPATIBILITY_TYPE;
}]
enum FF_COMPATIBILITY_TYPE {
COMPATIBLE = 'compatible',
WARNING = 'warning',
INCOMPATIBLE = 'incompatible',
}
Based on the returned filenames and their compatibilityType, we can show the required message in the modal

@danoswaltCL danoswaltCL moved this to Refined ToDo in UpGrade Project Jul 19, 2024
@Yagnik56 Yagnik56 moved this from Refined ToDo to In Progress in UpGrade Project Jul 22, 2024
@ppratikcr7
Copy link
Collaborator

ppratikcr7 commented Jul 24, 2024

@danoswaltCL @bcb37 @zackcl @amurphy-cl Just to confirm once, are we going to have the includeList and excludeList in import FF file? In the Add feature button next to the hamburger's import ff we only take the below details and allow to add or import the includeList and excludeList separately later.

{
"id": "be3ae74f-370a-4015-93f3-7761d16f8b11",
"name": "test",
"key": "FF_1",
"description": null,
"context": ["sub"],
"tags": [
"ff",
"test"
]
}

@RidhamShah RidhamShah self-assigned this Jul 30, 2024
@amurphy-cl
Copy link
Collaborator

Yeah, only the constructed lists, not segments that are public.

@ppratikcr7 ppratikcr7 moved this from In Progress to Code Review in UpGrade Project Aug 21, 2024
@RidhamShah RidhamShah moved this from Code Review to In Progress in UpGrade Project Sep 2, 2024
@Yagnik56 Yagnik56 moved this from Code Review to QA in UpGrade Project Sep 10, 2024
@amurphy-cl amurphy-cl added this to the Program Increment PI13 milestone Sep 10, 2024
@amurphy-cl amurphy-cl assigned bcb37 and unassigned ppratikcr7 and RidhamShah Sep 10, 2024
@Yagnik56
Copy link
Collaborator

Yagnik56 commented Sep 10, 2024

This bug is related to this issue.

@bcb37
Copy link
Collaborator

bcb37 commented Sep 24, 2024

This works as expected. Moving to done

@ppratikcr7
Copy link
Collaborator

QA: Found a bug in this. If we try to import a duplicate feature flag which already exists, we show compatible label and we get a 500 error while clicking on import due to duplicate key constraint. I feel this was addressed and resolved for add feature flag but is remaining for import feature flag. @RidhamShah Can you please resolve this?
Screenshot 2024-10-03 at 3 04 39 PM
Screenshot 2024-10-03 at 2 45 22 PM

@ppratikcr7 ppratikcr7 moved this from Done to Refined ToDo in UpGrade Project Oct 3, 2024
@ppratikcr7 ppratikcr7 reopened this Oct 3, 2024
@Yagnik56 Yagnik56 self-assigned this Oct 3, 2024
@Yagnik56
Copy link
Collaborator

Yagnik56 commented Oct 3, 2024

I am working on this issue.

@ppratikcr7 ppratikcr7 added the priority: high High priority issue label Oct 3, 2024
@Yagnik56 Yagnik56 linked a pull request Oct 4, 2024 that will close this issue
@ppratikcr7 ppratikcr7 assigned ppratikcr7 and unassigned Yagnik56 Oct 4, 2024
@ppratikcr7
Copy link
Collaborator

I will QA this

@ppratikcr7 ppratikcr7 moved this from In Progress to QA in UpGrade Project Oct 4, 2024
@ppratikcr7
Copy link
Collaborator

QA passed

@ppratikcr7 ppratikcr7 moved this from QA to Done in UpGrade Project Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment