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

Import feature flag validation API and related UI changes, validation table #1798

Merged
merged 43 commits into from
Sep 7, 2024

Conversation

Yagnik56
Copy link
Collaborator

@Yagnik56 Yagnik56 commented Aug 5, 2024

This PR introduces several new features and improvements:

Validation Enhancements:

  • Added a new validation API.
  • Implemented validation-related UI changes.
  • Introduced a compatibility table with a spinner that appears during validation.

Bug Fixes:

  • Corrected the FeatureFlag Validator components. (Issue)
  • Fixed UI issues in the Import Modal File Success. (Issue)
  • Addressed the happy path scenarios for the Import Modal File UI. (Issue)
  • Added a spinner for the pending state during validation. (Issue)
  • Ensured the endpoint returns the compatibility of import validation. (Issue)

Merged PRs:

  • Merged the FeatureFlag Import API. (PR)
  • Refactored 'List' to 'Segment' in FeatureFlag. (PR)
  • Included changes to address errors thrown on updating FeatureFlag. (PR)

This PR is now ready for review
image
image

Copy link
Collaborator

@bcb37 bcb37 left a comment

Choose a reason for hiding this comment

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

I've looked at the backend. I had a couple suggestions.

@RidhamShah
Copy link
Collaborator

@bcb37 just a clarification, does the warning state only come if any public subsegments of private segments are not present?

@Yagnik56 Yagnik56 linked an issue Aug 6, 2024 that may be closed by this pull request
@Yagnik56 Yagnik56 requested a review from bcb37 August 6, 2024 08:53
@bcb37
Copy link
Collaborator

bcb37 commented Aug 6, 2024

@bcb37 just a clarification, does the warning state only come if any public subsegments of private segments are not present?

That's my understanding, yes.

bcb37
bcb37 previously approved these changes Aug 6, 2024
(await this.featureFlagsDataService.validateFeatureFlag(files).toPromise()) as ValidateFeatureFlagError[]
).filter((data) => data.compatibilityType != null) || [];
this.importFileErrorsDataSource.data = this.importFileErrors;
this.featureFlagsService.setIsLoadingImportFeatureFlag(false);
Copy link
Collaborator

@zackcl zackcl Aug 7, 2024

Choose a reason for hiding this comment

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

It looks like Observable<Object>.toPromise() is deprecated and is replaced with firstValueFrom and lastValueFrom. Could you update this?

@zackcl
Copy link
Collaborator

zackcl commented Aug 7, 2024

@Yagnik56 It looks like importing with dragging and dropping a file is not implemented yet.

@zackcl
Copy link
Collaborator

zackcl commented Aug 26, 2024

It seems like the compatibility type is always "Incompatible" even if I import the valid one (which I just exported).

@Yagnik56
Copy link
Collaborator Author

It seems like the compatibility type is always "Incompatible" even if I import the valid one (which I just exported).

@zackcl Did you change the KEY after you exported the FF? duplicate key will give you an incompatible message...

@zackcl
Copy link
Collaborator

zackcl commented Aug 26, 2024

@Yagnik56 No, I tried importing when there are no flags defined. I exported a feature flag, and then deleted the existing one, and then imported the exported one. Can you also try this yourself?

@RidhamShah
Copy link
Collaborator

@Yagnik56 No, I tried importing when there are no flags defined. I exported a feature flag, and then deleted the existing one, and then imported the exported one. Can you also try this yourself?

@zackcl The export structure is not perfect, there were some property names mismatched, this is discussed in export PR. Its merge is remaining that's why exported FeatrueFlags shows error while importing

RidhamShah and others added 2 commits September 3, 2024 17:40
* solve feature flag throwing error update

* Removed unused properties from featureFlags related interfaces

---------

Co-authored-by: RidhamShah <[email protected]>
@danoswaltCL
Copy link
Collaborator

@VivekFitkariwala will check latest

@Yagnik56 Yagnik56 merged commit bc3d70f into dev Sep 7, 2024
14 checks passed
@Yagnik56 Yagnik56 deleted the featureflag/import-validation-api branch September 7, 2024 04:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants