feat: main branch for the existing keys, tasks, imports, language stats

[bdshadow]

Summary by CodeRabbit

• New Features

  • New projects now automatically get a default "main" branch at creation.

• Bug Fixes

  • Keys, tasks and imports are now required to be associated with a branch to avoid missing references.
  • Default-branch resolution now throws an error when no default exists to prevent ambiguous behavior.

• Chores

  • Database migration added to backfill and enforce non-null branch references for existing data.

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Project-wide enforcement of non-null branch associations: default/main branch creation and migration added; branch fields made non-null in models and services; builders and import flows updated to use resolved branches; introduced DefaultBranchNotFoundException; DB migration backfills and enforces NOT NULL on branch foreign keys.

Changes

Cohort / File(s)	Summary
Exceptions backend/data/src/main/kotlin/io/tolgee/exceptions/DefaultBranchNotFoundException.kt	Adds DefaultBranchNotFoundException thrown when a project default branch is absent.
Entity models backend/data/src/main/kotlin/io/tolgee/model/key/Key.kt, backend/data/src/main/kotlin/io/tolgee/model/task/Task.kt, backend/data/src/main/kotlin/io/tolgee/model/dataImport/Import.kt	Branch associations changed from nullable to non-null (lateinit) and JPA mappings set to optional = false / nullable = false.
Project & branch helpers backend/data/src/main/kotlin/io/tolgee/model/Project.kt, backend/data/src/main/kotlin/io/tolgee/model/branching/BranchVersionedEntity.kt	getDefaultBranch() now returns non-null (throws when absent); resolveBranch simplified and nested this qualification clarified.
Branching service API backend/data/src/main/kotlin/io/tolgee/service/branching/...	getDefaultBranch() and getActiveOrDefault() signatures changed to return non-null Branch and throw when missing.
Import & importer services backend/data/src/main/kotlin/io/tolgee/service/dataImport/...	Introduce BranchService dependency; imports and importer use getActiveOrDefault and replace import.branch?.name with import.branch.name.
Security & EE Task service backend/data/src/main/kotlin/io/tolgee/service/security/SecurityService.kt, ee/backend/app/src/main/kotlin/io/tolgee/ee/service/TaskService.kt	Removed fallback to default branch in protection check; task branch resolution now non-null.
Demo / DB populator backend/data/src/main/kotlin/io/tolgee/component/demoProject/DemoProjectCreator.kt, backend/data/src/main/kotlin/io/tolgee/development/DbPopulatorReal.kt, backend/development/.../ProjectsE2eDataController.kt	Call Branch.createMainBranch(project) during demo/dev/project creation to ensure a main branch exists before further setup.
Test data builders & service backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/...	Add ProjectBuilder.defaultBranch (lazy), propagate default branch to Key/Task/Import builders, add BranchBuilder.forExistingBranch, and ensure default branch evaluated before saving branches.
Test data adjustments backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/data/..., backend/data/src/test/kotlin/...	Qualified outer this in nested builders, made branch assignments null-safe in some places, removed hardcoded "main" creation in test data, and updated test mocks to include Branch on Import.
DB migration backend/data/src/main/resources/db/changelog/schema.xml	Liquibase changeSets to insert default "main" branches per project, backfill branch_id for keys/tasks/imports/language_stats, and enforce NOT NULL constraints.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Client
  participant SingleStepImportService
  participant BranchService
  participant Database
  Client->>SingleStepImportService: request import (projectId, branchName?)
  SingleStepImportService->>BranchService: getActiveOrDefault(projectId, branchName)
  BranchService->>Database: query branch by project+name or default
  Database-->>BranchService: Branch or none
  alt Branch found
    BranchService-->>SingleStepImportService: Branch (non-null)
    SingleStepImportService->>Database: create Import with branch_id (non-null)
    Database-->>SingleStepImportService: Import persisted
    SingleStepImportService-->>Client: success
  else No branch
    BranchService-->>SingleStepImportService: throws DefaultBranchNotFoundException
    SingleStepImportService-->>Client: error (exception)
  end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• feat: branching #3246: Related branching work — default/main branch creation and non-null branch handling.

Suggested labels

enhancement

Suggested reviewers

• JanCizmar

Poem

🐰 With twitching nose and nimble paws,

I stitched each branch into the laws.
Keys and tasks now find their lane,
No nulls to trip this hopping train,
Hooray for main! 🌿✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 5.88% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: ensuring existing keys, tasks, imports, and language stats have a main branch association, which aligns with the comprehensive branching refactor across multiple model entities and database migrations.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch dbocharov/branching/migrate-to-default-branch

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

backend/data/src/main/kotlin/io/tolgee/development/DbPopulatorReal.kt (1)

296-321: ⚠️ Potential issue | 🔴 Critical

Set key.branch before persisting the Key

The createTranslation method creates a Key (line 303) and persists it (line 314) without assigning key.branch. Since Key.branch is a non-nullable lateinit property with a database NOT NULL constraint, this will fail at persist time.

Assign the default branch from the project:

Proposed fix

     val key = Key()
     key.name = "sampleApp." +
       english
         .replace(" ", "_")
         .lowercase(Locale.getDefault())
         .replace("\\.+$".toRegex(), "")
     key.project = project
+    key.branch = project.getDefaultBranch()

backend/data/src/main/kotlin/io/tolgee/model/key/Key.kt (1)

106-113: ⚠️ Potential issue | 🟠 Major

Secondary constructor is missing required branch field initialization.

This constructor sets project and translations but not branch. Although all current call sites in KeyService.kt and KeysImporter.kt immediately assign branch after construction, the constructor signature is unsafe. Add branch as a parameter to this constructor or refactor to prevent accidental misuse.

🤖 Fix all issues with AI agents

In `@backend/data/src/main/kotlin/io/tolgee/model/key/Key.kt`:
- Around line 80-84: The comment above the branch property is stale and
contradicts the current non-null constraint; update or remove the misleading
comment so it reflects that in class Key the property branch (annotated
`@ManyToOne`(fetch = FetchType.LAZY, optional = false), `@JoinColumn`(name =
"branch_id"), declared as lateinit var branch: Branch and annotated
`@ActivityLoggedProp`) is non-nullable and legacy nulls are no longer
represented—replace the old note about "Nullable for backward compatibility"
with a brief accurate comment about the required non-null branch or simply
remove the comment.

In
`@backend/data/src/main/kotlin/io/tolgee/service/branching/AbstractBranchService.kt`:
- Around line 14-17: getActiveBranch currently throws
DefaultBranchNotFoundException which is semantically incorrect for arbitrary
branch lookups; change getActiveBranch to throw a generic
BranchNotFoundException (or NotFoundException) instead of
DefaultBranchNotFoundException, while keeping DefaultBranchNotFoundException
reserved for getDefaultBranch; update the throw site that uses
branchRepository.findActiveByProjectIdAndName(projectId, branchName) to
instantiate/throw the new BranchNotFoundException (or reuse an existing
NotFoundException) so the exception name matches the lookup intent.

In `@backend/data/src/main/resources/db/changelog/schema.xml`:
- Around line 5060-5128: The migration inserts a 'main' default branch
unconditionally and then backfills branch_id, which can create a conflicting
second is_default=true or leave NULLs; fix by first renaming any existing
default branch to 'main' where no 'main' exists, then run the INSERT and
backfills. Concretely, add an earlier step (before changeSet id
"1769500000000-1") that runs: UPDATE branch b SET name = 'main' WHERE
b.is_default = true AND NOT EXISTS (SELECT 1 FROM branch b2 WHERE b2.project_id
= b.project_id AND b2.name = 'main'); keep the existing INSERT in
"1769500000000-1" but guard it with WHERE NOT EXISTS (SELECT 1 FROM branch b
WHERE b.project_id = p.id AND b.name = 'main') (already present), and ensure the
subsequent updates in changeSets "1769500000000-2".."1769500000000-5" continue
to target branch_id via the SELECT FROM branch b to avoid NULLs.

🧹 Nitpick comments (5)

backend/data/src/main/kotlin/io/tolgee/service/security/SecurityService.kt (1)

547-554: Consider tightening Branch? to Branch now that both callers supply non-null values.

With key.branch now non-null and branchService.getActiveOrDefault also returning a non-null Branch (per this PR), the branch parameter here is never actually null. Keeping it nullable means branch?.isProtected silently skips the permission check if a future caller passes null—a potential security gap. Changing the parameter to non-null Branch would make the compiler enforce the invariant.

Suggested diff

-  private fun checkProtectedBranchModify(
-    branch: Branch?,
-    projectId: Long,
-  ) {
-    if (branch?.isProtected == true) {
+  private fun checkProtectedBranchModify(
+    branch: Branch,
+    projectId: Long,
+  ) {
+    if (branch.isProtected) {
       checkProjectPermission(projectId, Scope.BRANCH_PROTECTED_MODIFY)
     }
   }

backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/builders/BranchBuilder.kt (1)

16-31: Factory method creates and immediately discards a branch — consider a cleaner approach.

forExistingBranch instantiates a BranchBuilder (which adds an auto-created branch to project.branches in the constructor), then immediately removes it and replaces self. This works but is somewhat wasteful. A private constructor or an init flag could avoid the throwaway branch creation. That said, for a test data builder this is perfectly acceptable.

ee/backend/app/src/main/kotlin/io/tolgee/ee/service/TaskService.kt (2)

495-511: getOnlyProjectKeys parameter branch is still nullable, but callers now always pass non-null.

Since getBranchForTask now returns Branch (non-null), the branch: Branch? parameter on line 501 and the safe-call branch?.name on line 510 are no longer necessary. Consider tightening the signature to branch: Branch for consistency with the new non-null contract.

Proposed fix

   private fun getOnlyProjectKeys(
     projectId: Long,
     languageId: Long,
     type: TaskType,
     keys: Collection<Long>,
     filters: TranslationScopeFilters,
-    branch: Branch?,
+    branch: Branch,
   ): MutableSet<Long> {
     return taskRepository
       .getKeysWithoutConflicts(
         projectId,
         languageId,
         type.toString(),
         keys,
         filters,
-        branch?.name,
+        branch.name,
       ).toMutableSet()
   }

---

423-440: Unnecessary safe-call operators on non-null branch in calculateScope.

getBranchForTask now returns Branch (non-null), so branch?.name on lines 430 and 439 can be simplified to branch.name.

Proposed fix

     val keysIncludingConflicts =
       taskRepository.getKeysIncludingConflicts(
         projectEntity.id,
         language.id,
         dto.keys,
         filters,
-        branch?.name,
+        branch.name,
       )
     val relevantKeys =
       taskRepository.getKeysWithoutConflicts(
         projectEntity.id,
         language.id,
         dto.type.toString(),
         dto.keys,
         filters,
-        branch?.name,
+        branch.name,
       )

backend/data/src/main/kotlin/io/tolgee/model/dataImport/Import.kt (1)

38-40: Missing @field:NotNull annotation — inconsistent with sibling fields.

Both project (line 26) and author (line 31) use @field:NotNull alongside @ManyToOne(optional = false). The new branch field omits it. While optional = false and nullable = false enforce the constraint at the JPA/DDL level, adding @field:NotNull keeps bean-validation behaviour consistent across the entity.

Suggested fix

+  `@field`:NotNull
   `@ManyToOne`(targetEntity = Branch::class, optional = false)
   `@JoinColumn`(name = "branch_id", nullable = false)
   lateinit var branch: Branch