Skip to content

Feature/issue 261 export tags -Prune unrelated channel IDs from exported channel tags#288

Open
eddcomeau wants to merge 3 commits intoOpenIntegrationEngine:mainfrom
eddcomeau:feature/issue-261-export-tags
Open

Feature/issue 261 export tags -Prune unrelated channel IDs from exported channel tags#288
eddcomeau wants to merge 3 commits intoOpenIntegrationEngine:mainfrom
eddcomeau:feature/issue-261-export-tags

Conversation

@eddcomeau
Copy link
Copy Markdown

Summary

Prunes exported channel tag membership so channel exports only include the exported channel's ID in each tag.

Changes

  • prune tag channel IDs in server-side export assembly
  • prune tag channel IDs in client-side channel export
  • update ChannelServletTest expectations for exported tag membership

Verification

  • ChannelServletTest passes when run directly (It was edited as part of this commit)
  • Rebuilt with Ant -DdisableTests=true
  • Tested the UI (windows 10) by exporting a single channel, importing, and exporting a group, importing.

mgaffigan
mgaffigan previously approved these changes Apr 6, 2026
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 6, 2026

Test Results

  111 files  ±0    214 suites  ±0   6m 53s ⏱️ + 1m 8s
  654 tests ±0    654 ✅ ±0  0 💤 ±0  0 ❌ ±0 
1 308 runs  ±0  1 308 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit ba793e4. ± Comparison against base commit be058f6.

♻️ This comment has been updated with latest results.

NicoPiel
NicoPiel previously approved these changes Apr 10, 2026
@eddcomeau eddcomeau force-pushed the feature/issue-261-export-tags branch from 11557b3 to ba793e4 Compare April 10, 2026 11:25
Copy link
Copy Markdown
Member

@tonygermano tonygermano left a comment

Choose a reason for hiding this comment

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

I have a few questions about these changes and the associated issue.

  1. Where is the import code that looks at this information? Presumably it would need to create the tag in some situations and not create it in others. Can we be certain that this will not cause unwanted behavior in some scenarios?
  2. The issue suggests emptying the channel list from the exported tag entirely. Is there a reason to keep the current channel Id rather than using an empty Set?
  3. The issue also identifies a problem with the tags reporting incorrect channel values/counts. Can we confirm why that was the case, and whether this PR resolves that issue or not?

Comment on lines +1927 to +1939
private void addTagsToChannel(Channel channel) {
List<ChannelTag> channelTags = new ArrayList<ChannelTag>();
for (ChannelTag channelTag : getCachedChannelTags()) {
if (channelTag.getChannelIds().contains(channel.getId())) {
ChannelTag exportTag = new ChannelTag(channelTag);
exportTag.setChannelIds(Collections.singleton(channel.getId()));
channelTags.add(exportTag);
}
}

if (CollectionUtils.isNotEmpty(channelTags)) {
channel.getExportData().setChannelTags(channelTags);
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

chore: there appears to be a whitespace issue here

Most of this method did not change.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I fixed this up in the newer one

@eddcomeau eddcomeau dismissed stale reviews from mgaffigan and NicoPiel via 248e7e9 April 10, 2026 16:31
Signed-off-by: DESKTOP-9AKG4SL\ecomeau <ecomeau@caredx.com>
Signed-off-by: DESKTOP-9AKG4SL\ecomeau <ecomeau@caredx.com>
@eddcomeau eddcomeau force-pushed the feature/issue-261-export-tags branch from 248e7e9 to 61cbf7a Compare April 10, 2026 17:59
@eddcomeau
Copy link
Copy Markdown
Author

I have a few questions about these changes and the associated issue.

  1. Where is the import code that looks at this information? Presumably it would need to create the tag in some situations and not create it in others. Can we be certain that this will not cause unwanted behavior in some scenarios?
  2. The issue suggests emptying the channel list from the exported tag entirely. Is there a reason to keep the current channel Id rather than using an empty Set?
  3. The issue also identifies a problem with the tags reporting incorrect channel values/counts. Can we confirm why that was the case, and whether this PR resolves that issue or not?

Thanks

The serialized list of channels is really only used for NEW tags.
If it's an existing tag it could only ever add the imported channel.
This PR would change that behavior a bit, so brand new tags used to get added to every channel in the serialized list, now a brand new tag will only get added to the single channel you are importing. I think the only case you get this is if you export a channel with a tag, delete the tag everywhere, then re-import the channel it will now only add the tag to the imported channel than adding across all channels. So, I think this change would avoid unwanted side effects.

  1. Using an empty set would work for existing tags but would likely cause an error for new tags. I could change the import, but I think if I export a channel with a tag, I expect that tag to be there when I import it, even if it means making a new one.

  2. To be honest I couldn't replicate that. Although I believe because NEW tags were being added to the entire serialized list of channels, non-existent channels could still be added to the list. Throwing off the number. My hope was that the commit is a good move and if it resolved this that would be a bonus. But let me know if you disagree and I'm willing to dig more.

@eddcomeau eddcomeau requested a review from tonygermano April 10, 2026 18:06
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.

5 participants