Skip to content

fix(teams): trim team names on xls import and allow deleting teams linked to injects (#7558) - #7609

Open
Johanah LEKEU (johanah29) wants to merge 5 commits into
mainfrom
issue/7558
Open

fix(teams): trim team names on xls import and allow deleting teams linked to injects (#7558)#7609
Johanah LEKEU (johanah29) wants to merge 5 commits into
mainfrom
issue/7558

Conversation

@johanah29

Copy link
Copy Markdown
Member

Proposed changes

*Add a trim for the creation of teams from xls files
*Allow hte deletion of teams linked to injects

Testing Instructions

  1. Import injects from an xls file. Make sure to have several teams names separated by a comma in the teams cell
  2. The teams must be created and attached to their corresponding injects, no duplicate
  3. Try deleting a team when it is still linked to injects, deletion should succeed without any error

@github-actions

github-actions Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

📖 Documentation check — ✅ Passed

2 functional file(s), 0 doc file(s) changed.

No documentation gaps detected.

@johanah29 Johanah LEKEU (johanah29) changed the title Issue/7558 fix(teams): XLS inject import creates duplicate contextual teams and contextual team delete fails with HTTP Aug 26, 2026
@github-actions github-actions Bot added the filigran team Item from the Filigran team. label Aug 26, 2026
@Filigran-Automation Filigran Automation (Filigran-Automation) changed the title fix(teams): XLS inject import creates duplicate contextual teams and contextual team delete fails with HTTP fix(teams): trim team names on xls import and allow deleting teams linked to injects (#7558) Aug 26, 2026
@johanah29
Johanah LEKEU (johanah29) marked this pull request as ready for review August 26, 2026 09:49
Copilot AI lite review requested due to automatic review settings August 26, 2026 09:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR targets team handling consistency by (1) normalizing team names during XLS inject import and (2) allowing teams to be deleted even when they are linked to injects.

Changes:

  • Trim imported team name tokens when parsing comma-separated teams during XLS inject import.
  • Update DELETE /teams/{teamId} to load the tenant-scoped team, detach it from linked injects, and then delete it.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
openaev-api/src/main/java/io/openaev/service/InjectImportService.java Trims parsed team-name tokens from XLS import before resolving/creating teams.
openaev-api/src/main/java/io/openaev/rest/team/TeamApi.java Changes delete behavior to fetch the team in-tenant and detach from injects prior to deletion.
Suppressed comments (1)

openaev-api/src/main/java/io/openaev/rest/team/TeamApi.java:253

  • issue (blocking): Removing the team from inject.getTeams() mutates the join table but does not update inject_updated_at. Inject indexing uses inject_updated_at as part of its cursor/sort key, so team membership changes may not be propagated to Elasticsearch (stale inject_teams until the inject is otherwise updated/reindexed).
      team.getInjects().forEach(inject -> inject.getTeams().remove(team));
      teamRepository.delete(team);

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +891 to 892
.map(String::trim)
.toList();
Comment on lines +891 to 892
.map(String::trim)
.toList();
Comment on lines +247 to +253
Team team =
teamRepository
.findByIdAndTenantId(teamId, TenantContext.getCurrentTenant())
.orElseThrow(ElementNotFoundException::new);
try {
teamRepository.deleteById(teamId);
team.getInjects().forEach(inject -> inject.getTeams().remove(team));
teamRepository.delete(team);
Comment thread openaev-api/src/test/java/io/openaev/service/InjectImportServiceTest.java Outdated
Comment thread openaev-api/src/test/java/io/openaev/service/InjectImportServiceTest.java Outdated
…nd restrictions (#7199)

Signed-off-by: johanah29 <johanah.lekeu@filigran.io>
…contextual team delete fails with HTTP

Signed-off-by: johanah29 <johanah.lekeu@filigran.io>
Signed-off-by: johanah29 <johanah.lekeu@filigran.io>
Signed-off-by: johanah29 <johanah.lekeu@filigran.io>
Signed-off-by: johanah29 <johanah.lekeu@filigran.io>
@github-actions

Copy link
Copy Markdown
Contributor

Container vulnerability scan — Report incomplete

The latest run did not produce both readable reports. Check the workflow run for details.

Image Critical High Total Status
Standard ❔ Unavailable
UBI9 ❔ Unavailable

View workflow run

Updated from CI run attempt 1.

.split(","))
.map(String::trim)
.filter(value -> !value.isBlank())
.distinct()

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.

todo: that's a huge issue. Having a disctinct here instead of having it on the prod code. Please put the distinct method in openaev-api/src/main/java/io/openaev/service/InjectImportService.java


@DisplayName("Test parse teams cell with spaces and trailing comma")
@Test
void testParseTeamsCellWithSpacesAndTrailingComma() {

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.

TODO: this test don't test your changes. Please do a real import.

if (!teamRepository.existsByIdAndTenantId(teamId, TenantContext.getCurrentTenant())) {
throw new ElementNotFoundException();
}
Team team =

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.

Todo: you should add a test for this case.

if (!teamRepository.existsByIdAndTenantId(teamId, TenantContext.getCurrentTenant())) {
throw new ElementNotFoundException();
}
Team team =

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.

TODO: can you test on bulk delete team if we have the issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

filigran team Item from the Filigran team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(teams): XLS inject import creates duplicate contextual teams (no trimming), and contextual team delete fails with HTTP 500

3 participants