feat: add branching support to single-step import - #3471
Conversation
📝 WalkthroughWalkthroughThis pull request implements branch-aware single-step file import functionality. Changes include feature guard enforcement in the controller, branch resolution and storage in the service layer, and comprehensive test coverage for various branching scenarios. Changes
Sequence DiagramsequenceDiagram
participant Client as Client
participant Controller as SingleStepImportController
participant Guard as ProjectFeatureGuard
participant Service as SingleStepImportService
participant BranchSvc as BranchService
participant Importer as StoredDataImporter
participant DB as Database
Client->>Controller: POST single-step import with branch param
Controller->>Guard: checkIfUsed(BRANCHING, branch)
Guard->>DB: verify feature enabled
alt Feature not enabled & branch specified
Guard-->>Controller: throw exception
Controller-->>Client: 400 error
else Feature enabled or no branch
Controller->>Service: singleStepImport(files, params)
Service->>BranchSvc: getActiveOrDefault(projectId, branch)
BranchSvc-->>Service: Branch entity
Service->>Importer: process with branch-aware Import
Importer->>DB: store keys with branch reference
DB-->>Importer: success
Importer-->>Service: import complete
Service-->>Controller: result
Controller-->>Client: success response
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/data/src/main/kotlin/io/tolgee/service/dataImport/SingleStepImportService.kt (1)
106-172:⚠️ Potential issue | 🟡 MinorAdd feature guard check to
singleStepResolvableImportendpoint or clarify branching limitation.The
singleStepResolvableImportcontroller endpoint lacks aprojectFeatureGuard.checkIfUsed(Feature.BRANCHING, ...)check, unlike the regularsingleStepFromFilesendpoint. Additionally,SingleStepImportResolvableRequesthas no branch field to forward to the import logic, meaning the resolvable import path cannot support branching. Either add the feature guard (and branch field to the request DTO) if branching should be supported, or add a comment documenting that branching is intentionally unsupported for resolvable imports.
🤖 Fix all issues with AI agents
In
`@ee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/branching/SingleStepImportBranchingTest.kt`:
- Around line 35-41: Add an `@AfterEach` teardown to mirror the setup: implement a
method (annotated with `@AfterEach`) that calls
testDataService.cleanTestData(testData.root) and resets
enabledFeaturesProvider.forceEnabled = null to avoid test state leakage; place
it alongside the existing setup() in SingleStepImportBranchingTest so it runs
after each test and uses the same testData instance created in setup().
🧹 Nitpick comments (3)
backend/data/src/main/kotlin/io/tolgee/service/dataImport/SingleStepImportService.kt (1)
155-161: Unnecessary block wrapping for single-expression branches.These
whenbranches now wrap single expressions in{}blocks. While harmless, it's inconsistent with typical Kotlin style where single-expression branches don't need braces. Minor nit — not blocking.backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/data/dataImport/SingleStepImportBranchTestData.kt (1)
7-7:germanLanguageappears unused.This member is declared but none of the tests in
SingleStepImportBranchingTestreference it. If it's not needed, consider removing it to keep the test data minimal.#!/bin/bash # Verify if germanLanguage is referenced anywhere rg -n 'germanLanguage' --type=ktee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/branching/SingleStepImportBranchingTest.kt (1)
27-28:newJsonresource is declared but never used in any test.Consider removing it to avoid confusion.
| @BeforeEach | ||
| fun setup() { | ||
| testData = SingleStepImportBranchTestData() | ||
| testDataService.saveTestData(testData.root) | ||
| userAccount = testData.user | ||
| projectSupplier = { testData.project } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the ProjectAuthControllerTest base class and check for cleanup methods
fd 'ProjectAuthControllerTest' --type f -e ktRepository: tolgee/tolgee-platform
Length of output: 136
🏁 Script executed:
# Search for `@AfterEach` and cleanup methods in the base class area
rg -n '@AfterEach|fun tearDown|fun cleanup' --type=kt -C5 | head -100Repository: tolgee/tolgee-platform
Length of output: 92
🏁 Script executed:
# Check how enabledFeaturesProvider.forceEnabled is used in tests
rg -n 'forceEnabled|enabledFeaturesProvider' --type=kt -C3 | head -150Repository: tolgee/tolgee-platform
Length of output: 92
🏁 Script executed:
# Look at the full SingleStepImportBranchingTest file to understand its structure
fd 'SingleStepImportBranchingTest' --type f -e kt -exec cat -n {} \;Repository: tolgee/tolgee-platform
Length of output: 296
🏁 Script executed:
# Read the ProjectAuthControllerTest base class
cat -n backend/testing/src/main/kotlin/io/tolgee/ProjectAuthControllerTest.ktRepository: tolgee/tolgee-platform
Length of output: 4396
🏁 Script executed:
# Search for `@AfterEach` and forceEnabled with correct rg syntax
rg '@AfterEach|tearDown|cleanup' -A 5 --type-list | grep -i kotlinRepository: tolgee/tolgee-platform
Length of output: 455
🏁 Script executed:
# Read the SingleStepImportBranchingTest file
cat -n ee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/branching/SingleStepImportBranchingTest.kt | head -100Repository: tolgee/tolgee-platform
Length of output: 4058
🏁 Script executed:
# Search for forceEnabled usage
rg 'forceEnabled' -C 3 | head -100Repository: tolgee/tolgee-platform
Length of output: 11342
🏁 Script executed:
# Get complete SingleStepImportBranchingTest file
cat ee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/branching/SingleStepImportBranchingTest.kt | wc -lRepository: tolgee/tolgee-platform
Length of output: 69
🏁 Script executed:
# Get the full file content
tail -n +100 ee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/branching/SingleStepImportBranchingTest.ktRepository: tolgee/tolgee-platform
Length of output: 1152
🏁 Script executed:
# Check the AuthorizedControllerTest parent class
find . -name "AuthorizedControllerTest.kt" -type fRepository: tolgee/tolgee-platform
Length of output: 145
🏁 Script executed:
# Check AuthorizedControllerTest for cleanup methods
cat -n ./backend/testing/src/main/kotlin/io/tolgee/testing/AuthorizedControllerTest.kt | head -150Repository: tolgee/tolgee-platform
Length of output: 5093
🏁 Script executed:
# Check the entire file length first
wc -l ./backend/testing/src/main/kotlin/io/tolgee/testing/AuthorizedControllerTest.ktRepository: tolgee/tolgee-platform
Length of output: 149
🏁 Script executed:
# Check AbstractControllerTest to see if it has testDataService cleanup
fd 'AbstractControllerTest.kt' --type fRepository: tolgee/tolgee-platform
Length of output: 141
🏁 Script executed:
# Check if there's a testDataService cleanup pattern used elsewhere
rg 'testDataService.cleanTestData' -A 2 -B 2 | head -50Repository: tolgee/tolgee-platform
Length of output: 5395
🏁 Script executed:
# Verify the pattern in AbstractControllerTest
cat -n backend/testing/src/main/kotlin/io/tolgee/testing/AbstractControllerTest.kt | grep -A 5 -B 5 'testDataService\|@AfterEach'Repository: tolgee/tolgee-platform
Length of output: 48
Add @AfterEach cleanup method to reset test state.
The test sets enabledFeaturesProvider.forceEnabled and creates test data via testDataService.saveTestData() but never cleans up. This causes state leakage across test runs. Following the established pattern in similar tests (e.g., GlossaryTermControllerTest, SsoProviderControllerTest), add an @AfterEach method that calls testDataService.cleanTestData(testData.root) and resets enabledFeaturesProvider.forceEnabled = null.
🤖 Prompt for AI Agents
In
`@ee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/branching/SingleStepImportBranchingTest.kt`
around lines 35 - 41, Add an `@AfterEach` teardown to mirror the setup: implement
a method (annotated with `@AfterEach`) that calls
testDataService.cleanTestData(testData.root) and resets
enabledFeaturesProvider.forceEnabled = null to avoid test state leakage; place
it alongside the existing setup() in SingleStepImportBranchingTest so it runs
after each test and uses the same testData instance created in setup().
# [3.158.0](v3.157.0...v3.158.0) (2026-02-17) ### Features * add branching support to single-step import ([#3471](#3471)) ([a4e87e7](a4e87e7))
2 participants
Summary
BranchService.getActiveOrDefaultand setting it on theImportentity, instead of relying onStoredDataImporterto look up the branch fromproject.branchesProjectFeatureGuard.checkIfUsedvalidation in the controller to reject branch params when the branching feature is not enabledTest plan
Summary by CodeRabbit
New Features
Tests