Skip to content

Commit a4e87e7

Browse files
dkrizandkrizan
authored
feat: add branching support to single-step import (#3471)
## Summary - Fixes single-step import to properly support branching by resolving the branch via `BranchService.getActiveOrDefault` and setting it on the `Import` entity, instead of relying on `StoredDataImporter` to look up the branch from `project.branches` - Adds `ProjectFeatureGuard.checkIfUsed` validation in the controller to reject branch params when the branching feature is not enabled - Adds comprehensive EE tests covering import to specific branch, default branch fallback, branch isolation, feature gate enforcement, and non-existent branch handling ## Test plan - [x] Import to specified branch assigns keys to that branch - [x] Import without branch param assigns keys to default branch - [x] Keys imported to a branch are not visible on default branch - [x] Specifying branch when feature is disabled returns 400 - [x] Import without branch works when feature is disabled - [x] Non-existent branch returns 404 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Branch-aware import: import directly into a specified branch when branching is enabled; defaults to main branch when unspecified. Import attempts to a branch now enforce branching feature availability. * **Tests** * Added end-to-end tests covering imports to feature and default branches, visibility rules, error cases for disabled or non-existent branches, and improved test teardown for isolation. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent 6f745b8 commit a4e87e7

6 files changed

Lines changed: 199 additions & 13 deletions

File tree

‎backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/dataImport/SingleStepImportController.kt‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import io.swagger.v3.oas.annotations.media.Content
99
import io.swagger.v3.oas.annotations.media.Encoding
1010
import io.tolgee.activity.RequestActivity
1111
import io.tolgee.activity.data.ActivityType
12+
import io.tolgee.constants.Feature
1213
import io.tolgee.dtos.ImportResult
1314
import io.tolgee.dtos.dataImport.ImportFileDto
1415
import io.tolgee.dtos.request.SingleStepImportRequest
@@ -20,6 +21,7 @@ import io.tolgee.security.authentication.AllowApiAccess
2021
import io.tolgee.security.authentication.AuthenticationFacade
2122
import io.tolgee.security.authorization.RequiresProjectPermissions
2223
import io.tolgee.service.dataImport.SingleStepImportService
24+
import io.tolgee.service.project.ProjectFeatureGuard
2325
import io.tolgee.service.security.SecurityService
2426
import io.tolgee.util.Logging
2527
import io.tolgee.util.filterFiles
@@ -43,6 +45,7 @@ class SingleStepImportController(
4345
private val projectHolder: ProjectHolder,
4446
private val securityService: SecurityService,
4547
private val singleStepImportService: SingleStepImportService,
48+
private val projectFeatureGuard: ProjectFeatureGuard,
4649
) : Logging {
4750
@PostMapping("single-step-import", consumes = [MediaType.MULTIPART_FORM_DATA_VALUE])
4851
@Operation(
@@ -72,6 +75,7 @@ class SingleStepImportController(
7275
@RequestPart
7376
@Valid params: SingleStepImportRequest,
7477
): ImportResult {
78+
projectFeatureGuard.checkIfUsed(Feature.BRANCHING, params.branch)
7579
val filteredFiles = filterFiles(files.map { (it.originalFilename ?: "") to it })
7680
val fileDtos =
7781
filteredFiles.map {

‎backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/data/dataImport/SingleStepImportBranchTestData.kt‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
package io.tolgee.development.testDataBuilder.data.dataImport
2+
3+
import io.tolgee.development.testDataBuilder.data.BaseTestData
4+
import io.tolgee.model.branching.Branch
5+
6+
class SingleStepImportBranchTestData : BaseTestData() {
7+
val germanLanguage = projectBuilder.addGerman()
8+
lateinit var defaultBranch: Branch
9+
lateinit var featureBranch: Branch
10+
11+
init {
12+
this.root.apply {
13+
projectBuilder.apply {
14+
self.useBranching = true
15+
defaultBranch =
16+
addBranch {
17+
name = Branch.DEFAULT_BRANCH_NAME
18+
project = projectBuilder.self
19+
isDefault = true
20+
isProtected = true
21+
}.self
22+
featureBranch =
23+
addBranch {
24+
name = "feature"
25+
project = projectBuilder.self
26+
isDefault = false
27+
isProtected = false
28+
originBranch = defaultBranch
29+
}.self
30+
}
31+
}
32+
}
33+
}

‎backend/data/src/main/kotlin/io/tolgee/service/dataImport/SingleStepImportService.kt‎

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,10 @@ import io.tolgee.dtos.request.importKeysResolvable.SingleStepImportResolvableReq
1313
import io.tolgee.exceptions.BadRequestException
1414
import io.tolgee.model.Project
1515
import io.tolgee.model.UserAccount
16+
import io.tolgee.model.branching.Branch
1617
import io.tolgee.model.dataImport.Import
1718
import io.tolgee.model.dataImport.ImportTranslation
19+
import io.tolgee.service.branching.BranchService
1820
import io.tolgee.service.dataImport.ScreenshotImporter.Companion.ScreenshotToImport
1921
import io.tolgee.service.dataImport.status.ImportApplicationStatus
2022
import jakarta.persistence.EntityManager
@@ -33,6 +35,7 @@ class SingleStepImportService(
3335
private val currentDateProvider: CurrentDateProvider,
3436
private val applicationContext: ApplicationContext,
3537
private val entityManager: EntityManager,
38+
private val branchingService: BranchService,
3639
) {
3740
@Transactional
3841
fun singleStepImport(
@@ -45,7 +48,11 @@ class SingleStepImportService(
4548
resolveConflict: ((translation: ImportTranslation) -> ForceMode?)? = null,
4649
): ImportResult {
4750
reportStatus?.invoke(ImportApplicationStatus.ANALYZING_FILES)
48-
val import = Import(project).also { it.author = userAccount }
51+
val import =
52+
Import(project).also {
53+
it.author = userAccount
54+
it.branch = getBranch(project, params.branch)
55+
}
4956

5057
importService.publishImportBusinessEvent(project.id, userAccount.id)
5158

@@ -145,8 +152,14 @@ class SingleStepImportService(
145152
?.get(translation.key.name)
146153

147154
when (resolution) {
148-
null -> ForceMode.OVERRIDE
149-
ResolvableTranslationResolution.OVERRIDE -> ForceMode.OVERRIDE
155+
null -> {
156+
ForceMode.OVERRIDE
157+
}
158+
159+
ResolvableTranslationResolution.OVERRIDE -> {
160+
ForceMode.OVERRIDE
161+
}
162+
150163
ResolvableTranslationResolution.EXPECT_NO_CONFLICT -> {
151164
if (translation.text != translation.conflict?.text) {
152165
throw BadRequestException(Message.EXPECT_NO_CONFLICT_FAILED, getConflictingKeys(translation))
@@ -164,4 +177,11 @@ class SingleStepImportService(
164177
listOf(SimpleKeyResult(it.id, it.key.name, it.key.namespace?.name))
165178
} ?: emptyList()
166179
}
180+
181+
private fun getBranch(
182+
project: Project,
183+
name: String?,
184+
): Branch? {
185+
return branchingService.getActiveOrDefault(project.id, name)
186+
}
167187
}

‎backend/data/src/main/kotlin/io/tolgee/service/dataImport/StoredDataImporter.kt‎

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import io.tolgee.dtos.dataImport.SimpleImportConflictResult
77
import io.tolgee.dtos.request.SingleStepImportRequest
88
import io.tolgee.exceptions.BadRequestException
99
import io.tolgee.exceptions.ImportConflictNotResolvedException
10-
import io.tolgee.model.branching.Branch
1110
import io.tolgee.model.dataImport.Import
1211
import io.tolgee.model.dataImport.ImportLanguage
1312
import io.tolgee.model.dataImport.ImportTranslation
@@ -383,7 +382,7 @@ class StoredDataImporter(
383382
// or get it from conflict or create new one
384383
val newKey =
385384
importDataManager.existingKeys[this.key.file.namespace to this.key.name]
386-
?: createNewKey(this.key.name, this.key.file.namespace, import.branch?.name)
385+
?: createNewKey(this.key.name, this.key.file.namespace)
387386
newKey
388387
}
389388
}
@@ -393,19 +392,18 @@ class StoredDataImporter(
393392
keyName: String,
394393
): Key {
395394
return keysToSave.computeIfAbsent(namespace to keyName) {
396-
importDataManager.existingKeys[namespace to keyName] ?: createNewKey(keyName, namespace, import.branch?.name)
395+
importDataManager.existingKeys[namespace to keyName] ?: createNewKey(keyName, namespace)
397396
}
398397
}
399398

400399
private fun createNewKey(
401400
name: String,
402401
namespace: String?,
403-
branch: String?,
404402
): Key {
405403
return Key(name = name).apply {
406404
project = import.project
407405
this.namespace = getNamespace(namespace)
408-
this.branch = getBranch(branch)
406+
this.branch = import.branch
409407
newKeys.add(this)
410408
}
411409
}
@@ -446,11 +444,6 @@ class StoredDataImporter(
446444
}
447445
}
448446

449-
private fun getBranch(name: String?): Branch? {
450-
name ?: return null
451-
return import.project.branches.find { it.name == name }
452-
}
453-
454447
private val tagService by lazy {
455448
applicationContext.getBean(TagService::class.java)
456449
}

‎ee/backend/tests/src/test/kotlin/io/tolgee/ee/api/v2/controllers/branching/SingleStepImportBranchingTest.kt‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
package io.tolgee.ee.api.v2.controllers.branching
2+
3+
import io.tolgee.ProjectAuthControllerTest
4+
import io.tolgee.constants.Feature
5+
import io.tolgee.constants.Message
6+
import io.tolgee.development.testDataBuilder.data.dataImport.SingleStepImportBranchTestData
7+
import io.tolgee.ee.component.PublicEnabledFeaturesProvider
8+
import io.tolgee.fixtures.andHasErrorMessage
9+
import io.tolgee.fixtures.andIsBadRequest
10+
import io.tolgee.fixtures.andIsNotFound
11+
import io.tolgee.fixtures.andIsOk
12+
import io.tolgee.testing.annotations.ProjectJWTAuthTestMethod
13+
import io.tolgee.testing.assert
14+
import io.tolgee.util.performSingleStepImport
15+
import org.junit.jupiter.api.BeforeEach
16+
import org.junit.jupiter.api.Test
17+
import org.springframework.beans.factory.annotation.Autowired
18+
import org.springframework.beans.factory.annotation.Value
19+
import org.springframework.core.io.Resource
20+
import org.springframework.test.web.servlet.ResultActions
21+
22+
@Suppress("SpringJavaInjectionPointsAutowiringInspection")
23+
class SingleStepImportBranchingTest : ProjectAuthControllerTest("/v2/projects/") {
24+
@Value("classpath:import/simple.json")
25+
lateinit var simpleJson: Resource
26+
27+
@Value("classpath:import/new.json")
28+
lateinit var newJson: Resource
29+
30+
lateinit var testData: SingleStepImportBranchTestData
31+
32+
@Autowired
33+
lateinit var enabledFeaturesProvider: PublicEnabledFeaturesProvider
34+
35+
@BeforeEach
36+
fun setup() {
37+
testData = SingleStepImportBranchTestData()
38+
testDataService.saveTestData(testData.root)
39+
userAccount = testData.user
40+
projectSupplier = { testData.project }
41+
}
42+
43+
@Test
44+
@ProjectJWTAuthTestMethod
45+
fun `imports new keys to specified branch`() {
46+
enabledFeaturesProvider.forceEnabled = setOf(Feature.BRANCHING)
47+
performImport(
48+
listOf(Pair("en.json", simpleJson)),
49+
params = mapOf("branch" to testData.featureBranch.name),
50+
).andIsOk
51+
52+
executeInNewTransaction {
53+
val key = keyService.getAllByBranch(testData.project.id, "feature").find { it.name == "test" }
54+
key.assert.isNotNull
55+
key!!
56+
.translations
57+
.find { it.language.tag == "en" }!!
58+
.text.assert
59+
.isEqualTo("test")
60+
}
61+
}
62+
63+
@Test
64+
@ProjectJWTAuthTestMethod
65+
fun `imports to default branch when no branch specified`() {
66+
performImport(
67+
listOf(Pair("en.json", simpleJson)),
68+
).andIsOk
69+
70+
executeInNewTransaction {
71+
val key = keyService.getAllByBranch(testData.project.id, "main").find { it.name == "test" }
72+
key.assert.isNotNull
73+
key!!
74+
.translations
75+
.find { it.language.tag == "en" }!!
76+
.text.assert
77+
.isEqualTo("test")
78+
}
79+
}
80+
81+
@Test
82+
@ProjectJWTAuthTestMethod
83+
fun `keys imported to branch are not visible on default branch`() {
84+
enabledFeaturesProvider.forceEnabled = setOf(Feature.BRANCHING)
85+
performImport(
86+
listOf(Pair("en.json", simpleJson)),
87+
params = mapOf("branch" to testData.featureBranch.name),
88+
).andIsOk
89+
90+
executeInNewTransaction {
91+
val keysOnDefault = keyService.getAllByBranch(testData.project.id, "main")
92+
keysOnDefault.find { it.name == "test" }.assert.isNull()
93+
94+
val keysOnFeature = keyService.getAllByBranch(testData.project.id, "feature")
95+
keysOnFeature.find { it.name == "test" }.assert.isNotNull
96+
}
97+
}
98+
99+
@Test
100+
@ProjectJWTAuthTestMethod
101+
fun `fails when branch specified but feature not enabled`() {
102+
enabledFeaturesProvider.forceEnabled = emptySet()
103+
performImport(
104+
listOf(Pair("en.json", simpleJson)),
105+
params = mapOf("branch" to testData.featureBranch.name),
106+
).andIsBadRequest.andHasErrorMessage(Message.FEATURE_NOT_ENABLED)
107+
}
108+
109+
@Test
110+
@ProjectJWTAuthTestMethod
111+
fun `succeeds without branch when feature not enabled`() {
112+
enabledFeaturesProvider.forceEnabled = emptySet()
113+
performImport(
114+
listOf(Pair("en.json", simpleJson)),
115+
).andIsOk
116+
}
117+
118+
@Test
119+
@ProjectJWTAuthTestMethod
120+
fun `fails when non-existent branch specified`() {
121+
enabledFeaturesProvider.forceEnabled = setOf(Feature.BRANCHING)
122+
performImport(
123+
listOf(Pair("en.json", simpleJson)),
124+
params = mapOf("branch" to "non-existent"),
125+
).andIsNotFound
126+
}
127+
128+
private fun performImport(
129+
files: List<Pair<String, Resource>>?,
130+
params: Map<String, Any?> = mapOf(),
131+
): ResultActions {
132+
return performSingleStepImport(mvc, testData.project.id, files, params)
133+
}
134+
}

‎ee/backend/tests/src/test/kotlin/io/tolgee/ee/service/EeSubscriptionProviderImplTest.kt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,8 @@ class EeSubscriptionProviderImplTest : AbstractSpringTest() {
5555

5656
@AfterEach
5757
fun cleanup() {
58+
schedulingManager.cancelAll()
59+
eeSubscriptionServiceImpl.delete()
5860
eeProperties.checkPeriodInMs = oldCheckPeriodProperty
5961
}
6062

0 commit comments

Comments
 (0)