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

Feature flag admin endpoints #1474

Merged
merged 10 commits into from
May 21, 2024
Merged

Feature flag admin endpoints #1474

merged 10 commits into from
May 21, 2024

Conversation

bcb37
Copy link
Collaborator

@bcb37 bcb37 commented Apr 24, 2024

Adds (or updates) the following endpoints:

GET /flags

  • get all feature flags

POST /flags

  • create feature flag

GET /flags/{id}

  • get flag by id

DELETE /flags/{id}

  • delete flag by id

PUT /flags/{id}

  • update flag by id

POST /flags/paginated

  • get all flags paginated

POST /flags/status

  • update flag status

The request (for POST and PUT) and response bodies for these endpoints can be viewed by looking at the swagger page.

@RidhamShah
Copy link
Collaborator

We also need FILTER_MODE (includeAll, excludeAll) in featureFlags for Segments logic too, as it is also present in Experiments.

if we don't want to use FILTER_MODE then we need to discuss how the Segments would work for it.

@bcb37
Copy link
Collaborator Author

bcb37 commented May 2, 2024

We also need FILTER_MODE (includeAll, excludeAll) in featureFlags for Segments logic too, as it is also present in Experiments.

if we don't want to use FILTER_MODE then we need to discuss how the Segments would work for it.

@RidhamShah I've created a pr that adds FILTER_MODE to the db. When that's merged we can continue with this one.

@bcb37 bcb37 requested review from zackcl, danoswaltCL and RidhamShah May 6, 2024 14:31

// saving variations
let variationDocs: FlagVariation[];
const setSegmentInEx = (inEx: FeatureFlagSegmentExclusion | FeatureFlagSegmentInclusion) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

what does inEx mean? inclusion / exclusion?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, I'd meant to eventually give that a better name. Good catch

* sortAs:
* type: string
* enum: [ASC, DESC]
* tags:
* - Feature flags
* - Feature Flags
Copy link
Collaborator

Choose a reason for hiding this comment

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

Line 240 says Experiments instead of Feature Flags

: inEx;
};

let includeSegmentExists = true;
Copy link
Collaborator

@danoswaltCL danoswaltCL May 10, 2024

Choose a reason for hiding this comment

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

I asked copilot to pull this out 159-221 into own function without the repetition for the segment include and segment exclude. It needs some help with types but I thought something like this looked pretty reasonable, what do you think?

async function handleSegment(segmentInclusionExclusion: any, flagDoc: any, flag: any, type: string, logger: any) {

  let segmentExists = true;
  let segmentDoc: Segment;
  let segmentDocToSave: Partial<FeatureFlagSegmentInclusion | FeatureFlagSegmentExclusion> = {};
  if (segmentInclusionExclusion) {
    const segment: any = setSegmentInEx(segmentInclusionExclusion);

    const segmentData: SegmentInputValidator = {
      ...segment,
      id: segment.id || uuid(),
      name: flagDoc.id + ' ' + type + ' Segment',
      description: flagDoc.id + ' ' + type + ' Segment',
      context: flagDoc.context[0],
      type: SEGMENT_TYPE.PRIVATE,
    };
    try {
      segmentDoc = await this.segmentService.upsertSegment(segmentData, logger);
    } catch (err) {
      const error = err as ErrorWithType;
      error.details = 'Error in adding segment in DB';
      error.type = SERVER_ERROR.QUERY_FAILED;
      logger.error(error);
      throw error;
    }
    // creating segment doc
    const tempDoc = type === 'Inclusion' ? new FeatureFlagSegmentInclusion() : new FeatureFlagSegmentExclusion();
    tempDoc.segment = segmentDoc;
    tempDoc.featureFlag = flag;
    segmentDocToSave = this.getSegmentDoc(tempDoc);
  } else {
    segmentExists = false;
  }
  return { segmentExists, segmentDoc, segmentDocToSave };
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it makes sense to reduce duplication. I'll see what I can do about wrangling the types.

@danoswaltCL danoswaltCL self-requested a review May 16, 2024 21:46
danoswaltCL
danoswaltCL previously approved these changes May 16, 2024
@danoswaltCL
Copy link
Collaborator

looks good to me, other than whatever's in the merge conflicts

@danoswaltCL
Copy link
Collaborator

hey it works. @VivekFitkariwala let us know if this can be merged, this will be helpful for getting the UI moving. I see some outstanding comments but I think Ben has responded.

@danoswaltCL danoswaltCL merged commit e1b0fab into dev May 21, 2024
8 checks passed
@danoswaltCL danoswaltCL deleted the feature/feature-flag-admin-api branch May 21, 2024 14:35
KD1712 pushed a commit that referenced this pull request Jun 6, 2024
* WIP: Feature flag admin endpoints

* update feature flag service tests

* review changes

* merge dev into feature/feature-flag-admin-api

* review changes

---------

Co-authored-by: danoswaltCL <[email protected]>
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