add-edit-duplicate segment modal functionality #2320
Conversation
danoswaltCL
changed the title
|
I'm able to create two segments with the same name under the same app context. I think we should not allow this. 
|
|
I don't see the "Duplicate Segment" option from the menu currently. 
Here's the design in Figma for reference: 
|
|
@zackcl yes, i disabled it on purpose because I think we need to talk about what the expected behavior is, I edited my comment above to be more clear |
Ok, I see the old modal disallows this to prevent the issue, I can do that but the backend should really not allow it to happen either. |
|
@zackcl I added in a backend change to do a duplicate name check on submit and error if yes. Also shows error in the form that makes the form invalid until changed. The error will persist and keep getting checked while the name or context changes, removing the error if changed, putting it back if the name and context again match. I put this in WIP still, I haven't added in the duplicate modal changes requested. |
|
Ok, this now adds full duplicate modal functionality also, which does include just copying the lists directly from the source segment, it works just like add since it's upsert, so as far as I can tell this is creating faithful duplications, with the understanding that currently this must keep same context as target. |
|
@danoswaltCL Shouldn't we disable the "App Context" dropdown on the"Duplicate Segment" modal? I thought that was our decision. Currently, I'm able to change the "App Context" on your branch. |
| message: `Segment name ${name} already exists in context ${context}`, | ||
| }); | ||
|
|
||
| // const error = new ErrorWithType(); |
There was a problem hiding this comment.
We can probably remove these comments
oops, yes |
|
|
||
| public upsertSegment(segment: SegmentInputValidator, logger: UpgradeLogger): Promise<Segment> { | ||
| public async upsertSegment(segment: SegmentInputValidator, logger: UpgradeLogger): Promise<Segment> { | ||
| await this.checkIsDuplicateSegmentName(segment.name, segment.context, logger); |
There was a problem hiding this comment.
This method is also used when editing - so now editing always fails unless you're changing the name.
Maybe only perform the check if there isn't an id on 'segment'.
| context: appContext, | ||
| userIds: [], | ||
| groups: [], | ||
| subSegmentIds: [], |
There was a problem hiding this comment.
We need subSegmentIds from the segment here (same as with duplicate)
|
@bcb37 updated, ready for re-re-review |
This PR resolves #2245
I have added all the necessary code for add, edit and duplicate segment modal. Currently this PR needs the segmenr table list component for a listener to close the modal after add/edit/duplicate segment. Also, tags and listType are not getting stored in the backend as of now, that also needs to be handled in a separate PR. We will need to test this PR once all these missing parts are in the dev branch. Rest all seems to work fine as expected.
EDIT DAN: