Skip to content

Conversation

@zackcl
Copy link
Collaborator

@zackcl zackcl commented Apr 9, 2025

Resolves #2336, resolves #2257, resolves #2258, resolves #2338

Changes Include:

  • Added addSegmentList endpoint URL (/segments/list) to environment files
  • Added addSegmentList, updateSegmentList, and deleteSegmentList functions to segments.data.service.ts for making requests to the backend
  • Updated segments actions, effects, and reducer to support adding, updating, and deleting a segment list
  • Updated flagId to id in PrivateSegmentListRequest interface for supporting both feature flags and segments
  • Updated flagId to id in FeatureFlagListValidator for consistency with the frontend
  • Added unit tests for segment list operations to SegmentController.test.ts
  • Removed unused functions from feature-flag-exclusions-section-card.component.ts

Endpoints used for adding, updating, and deleting a segment list:

  1. On ADD:
    POST /segments/list

  2. On EDIT:
    POST /segment

  3. On DELETE:
    DELETE /segments/list/:segmentId

@zackcl zackcl marked this pull request as ready for review April 9, 2025 11:25
@zackcl zackcl changed the title [WIP] Implement Add/Edit/Delete Segment List Modals 2 Implement Add/Edit/Delete Segment List Modals 2 Apr 9, 2025
@zackcl
Copy link
Collaborator Author

zackcl commented Apr 9, 2025

The PR is now ready for review.

@zackcl
Copy link
Collaborator Author

zackcl commented Apr 9, 2025

@bcb37 Just to clarify, did you try the app on branch feature/2257-upsert-segment-list2 and not the old feature/2257-upsert-segment-list?

@bcb37
Copy link
Collaborator

bcb37 commented Apr 9, 2025

@bcb37 Just to clarify, did you try the app on branch feature/2257-upsert-segment-list2 and not the old feature/2257-upsert-segment-list?

Yeah I was looking at the old one. Sorry. I'll do a real review now!

@bcb37
Copy link
Collaborator

bcb37 commented Apr 9, 2025

@bcb37 Just to clarify, did you try the app on branch feature/2257-upsert-segment-list2 and not the old feature/2257-upsert-segment-list?

Yeah I was looking at the old one. Sorry. I'll do a real review now!

@zackcl Ok, not only does this work flawlessly, but it fixes the other issue I was talking about on slack!

@bcb37 bcb37 self-requested a review April 9, 2025 17:22
bcb37
bcb37 previously approved these changes Apr 9, 2025
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.

Looks good!

danoswaltCL
danoswaltCL previously approved these changes Apr 9, 2025
@danoswaltCL
Copy link
Collaborator

approving also, only thing I see is just adding the new endpoint to the environment.local.example, but this can be merged once updated

@zackcl zackcl dismissed stale reviews from danoswaltCL and bcb37 via e5b3596 April 10, 2025 10:30
@zackcl zackcl requested a review from kawcl April 10, 2025 10:40
@zackcl zackcl merged commit 2ce3e51 into dev Apr 10, 2025
14 of 15 checks passed
@zackcl zackcl deleted the feature/2257-upsert-segment-list2 branch April 10, 2025 10:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Delete Segment List Modal Segment Add / Edit List modal Edit Segment List Modal Add Segment List Modal

5 participants