Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,13 @@ class OrganizationStatsServiceTest : AbstractSpringTest() {
@Test
fun `getKeyCount counts unique keys across all projects in organization`() {
// Organization total:
// First project: 6 unique keys
// Second project: 2 unique keys
// Total: 8 unique keys
// First project (branching enabled): 6 unique keys
// Second project (branching enabled): 2 unique keys
// Third project (branching disabled): nb-key1 + nb-key3 (default branch) + nb-key4 = 3 keys
// (nb-key2 on orphan branch excluded)
// Total: 11 unique keys
val orgKeyCount = organizationStatsService.getKeyCount(testData.organization.id)
assertThat(orgKeyCount).isEqualTo(8)
assertThat(orgKeyCount).isEqualTo(11)
}

@Test
Expand All @@ -70,8 +72,39 @@ class OrganizationStatsServiceTest : AbstractSpringTest() {
// - key5 (main): EN, DE = 2
// Second project total: 3
//
// Organization total: 8
// Third project translations (branching disabled):
// - nb-key1: EN = 1
// - nb-key2 on orphan branch: excluded (branching disabled)
// - nb-key3 (default branch): EN = 1
// Third project total: 2
//
// Organization total: 10
val translationCount = organizationStatsService.getTranslationCount(testData.organization.id)
assertThat(translationCount).isEqualTo(10)
}

@Test
fun `getKeyCount excludes branch keys when project has branching disabled`() {
// The no-branching project has useBranching=false with:
// - nb-key1 (no branch) = counted
// - nb-key2 (orphan feature branch) = NOT counted (branching disabled)
// - nb-key3 (default branch, branch_id set) = counted
// - nb-key4 (no branch) = counted
// This is verified via the org-wide count which includes all three projects:
// First project: 6 + Second project: 2 + Third project: 3 = 11
val orgKeyCount = organizationStatsService.getKeyCount(testData.organization.id)
assertThat(orgKeyCount).isEqualTo(11)
}

@Test
fun `getTranslationCount excludes branch translations when project has branching disabled`() {
// The no-branching project has useBranching=false with:
// - nb-key1 EN translation = counted
// - nb-key2 EN translation on orphan branch = NOT counted (branching disabled)
// - nb-key3 EN translation on default branch = counted
// This is verified via the org-wide count:
// First project: 5 + Second project: 3 + Third project: 2 = 10
val translationCount = organizationStatsService.getTranslationCount(testData.organization.id)
assertThat(translationCount).isEqualTo(8)
assertThat(translationCount).isEqualTo(10)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ class OrganizationStatsTestData : BaseTestData("org-stats", "Stats Project") {
lateinit var secondProjectFeatureBranch: Branch
lateinit var germanLanguage: Language
lateinit var secondProjectGermanLanguage: Language
lateinit var noBranchingProject: Project

init {
root.apply {
Expand All @@ -28,6 +29,7 @@ class OrganizationStatsTestData : BaseTestData("org-stats", "Stats Project") {
}

addSecondProject()
addNoBranchingProject()
}
}

Expand Down Expand Up @@ -196,4 +198,66 @@ class OrganizationStatsTestData : BaseTestData("org-stats", "Stats Project") {
}
}.self
}

/**
* Third project with useBranching=false but containing orphan branch keys.
* These branch keys should NOT be counted in org stats.
*/
private fun TestDataBuilder.addNoBranchingProject() {
noBranchingProject =
addProject {
name = "No Branching Project"
organizationOwner = this@OrganizationStatsTestData.organization
useBranching = false
}.build {
addLanguage {
name = "English"
tag = "en"
originalName = "English"
this@build.self.baseLanguage = this
}

val noBranchingMainBranch =
addBranch {
name = "main"
project = self
isDefault = true
}.build { self }.self

val noBranchingFeatureBranch =
addBranch {
name = "orphan-feature"
project = self
originBranch = noBranchingMainBranch
}.build { self }.self

// Key on main branch (branch_id is null for default) — should be counted
addKey {
name = "nb-key1"
}.build {
addTranslation("en", "NB Key 1 English")
}

// Key on orphan feature branch — should NOT be counted (branching disabled)
addKey {
name = "nb-key2"
branch = noBranchingFeatureBranch
}.build {
addTranslation("en", "NB Key 2 English on orphan branch")
}

// Key explicitly on default branch (branch_id is set) — should still be counted
addKey {
name = "nb-key3"
branch = noBranchingMainBranch
}.build {
addTranslation("en", "NB Key 3 English on default branch")
}

// Key on main branch without translation — should be counted as key but not translation
addKey {
name = "nb-key4"
}
}.self
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,23 +44,31 @@ class OrganizationStatsService(
return entityManager
.createNativeQuery(
"""
with org_keys as materialized (
select k.id, k.project_id, k.name, k.namespace_id
from key k
join project p on p.id = k.project_id and p.deleted_at is null
left join branch b on b.id = k.branch_id
where p.organization_owner_id = :organizationId
and k.deleted_at is null
and (k.branch_id is null or b.deleted_at is null)
and (p.use_branching = true or k.branch_id is null or b.is_default = true)
),
org_translations as materialized (
select ok.project_id, ok.name, ok.namespace_id, t.language_id
from org_keys ok
join translation t on t.key_id = ok.id
and t.text is not null
and t.text <> ''
)
select count(*) from (
select distinct k.project_id, k.name, k.namespace_id, t.language_id
from translation t
join key k on k.id = t.key_id
where k.project_id in (
select p.id from project p
where p.organization_owner_id = :organizationId
and p.deleted_at is null
)
and k.deleted_at is null
and exists (
select distinct ot.project_id, ot.name, ot.namespace_id, ot.language_id
from org_translations ot
where exists (
select 1 from language l
where l.id = t.language_id
where l.id = ot.language_id
and l.deleted_at is null
)
and t.text is not null
and t.text <> ''
) sub
""".trimIndent(),
).setParameter("organizationId", organizationId)
Expand All @@ -77,8 +85,11 @@ class OrganizationStatsService(
select distinct k.project_id, k.name, k.namespace_id
from key k
join project p on p.id = k.project_id and p.deleted_at is null
left join branch b on b.id = k.branch_id
where p.organization_owner_id = :organizationId
and k.deleted_at is null
and (k.branch_id is null or b.deleted_at is null)
and (p.use_branching = true or k.branch_id is null or b.is_default = true)
) sub
""".trimIndent(),
).setParameter("organizationId", organizationId)
Expand Down
12 changes: 12 additions & 0 deletions backend/data/src/main/resources/db/changelog/schema.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5174,4 +5174,16 @@
referencedTableName="user_account" referencedColumnNames="id"
constraintName="fk_key_deleted_by_user_account"/>
</changeSet>

<!-- Partial index for org-wide key/translation count queries (fixes #3527) -->
<changeSet author="dkrizan" id="1741855200000-1" runInTransaction="false">
<sql>
create index concurrently if not exists key_project_name_ns_not_deleted
on public.key (project_id, name, namespace_id)
Copy link
Contributor

Choose a reason for hiding this comment

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

it looks like the same index, but not partial, already exists. It was added in the changeset 1771846650000-1. So it's not clear why the new one is required here.

Maybe, for the usage query it must be for 2 fields: project_id and deleted_at? Not sure, just asking

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It improved the performance by 25%, I think due to where deleted_at is null .. but I will dive deeper and find a better way.

Copy link
Contributor

Choose a reason for hiding this comment

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

hm, got it. It's interesting why the existing index isn't taken then... Maybe because of more fields in it 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the reason Postgres does not choose it is the condition WHERE deleted_at IS NOT NULL, in this case, the opposite is needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like this is the best solution + the refactor of SQLs.

where deleted_at is null;
</sql>
<rollback>
drop index concurrently if exists key_project_name_ns_not_deleted;
</rollback>
</changeSet>
</databaseChangeLog>
Loading