From a0d9cd881b8f2e2d52edb2008f5d5b60deef4a62 Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Sun, 31 May 2026 23:02:19 +0200 Subject: [PATCH] fix(MyShares): correct order if items in MyShares view, when grouped by Files --- .../services/subject_group_service.rs | 6 ++- .../pg/subject_group_pg_repository.rs | 12 ++---- src/infrastructure/services/pg_acl_engine.rs | 41 +++++++++---------- 3 files changed, 28 insertions(+), 31 deletions(-) diff --git a/src/application/services/subject_group_service.rs b/src/application/services/subject_group_service.rs index 18fcc63e..bf0f01f9 100644 --- a/src/application/services/subject_group_service.rs +++ b/src/application/services/subject_group_service.rs @@ -423,7 +423,11 @@ mod integration_tests { } fn rand_name(test: &str) -> String { - format!("rust-test-svc-{}-{}", test, &Uuid::new_v4().to_string()[..8]) + format!( + "rust-test-svc-{}-{}", + test, + &Uuid::new_v4().to_string()[..8] + ) } // ── 9. Virtual group cannot be deleted ───────────────────────────────── diff --git a/src/infrastructure/repositories/pg/subject_group_pg_repository.rs b/src/infrastructure/repositories/pg/subject_group_pg_repository.rs index af0551d8..4a9c6442 100644 --- a/src/infrastructure/repositories/pg/subject_group_pg_repository.rs +++ b/src/infrastructure/repositories/pg/subject_group_pg_repository.rs @@ -786,9 +786,7 @@ mod integration_tests { let mut ids = Vec::with_capacity(8); for i in 0..8 { let g = repo - .create( - &SubjectGroup::new(&rand_name(&format!("cyc8-{i}")), None).unwrap(), - ) + .create(&SubjectGroup::new(&rand_name(&format!("cyc8-{i}")), None).unwrap()) .await .unwrap(); ids.push(g.id); @@ -843,9 +841,7 @@ mod integration_tests { let mut ids = Vec::with_capacity(len); for i in 0..len { let g = repo - .create( - &SubjectGroup::new(&rand_name(&format!("depth-{i}")), None).unwrap(), - ) + .create(&SubjectGroup::new(&rand_name(&format!("depth-{i}")), None).unwrap()) .await .unwrap(); ids.push(g.id); @@ -855,9 +851,7 @@ mod integration_tests { for i in 0..(len - 1) { repo.add_member(ids[i], GroupMember::Group(ids[i + 1]), admin) .await - .unwrap_or_else(|e| { - panic!("edge {i} should fit in the depth budget: {:?}", e) - }); + .unwrap_or_else(|e| panic!("edge {i} should fit in the depth budget: {:?}", e)); } // Lift the whole chain under a new outer group → subtree depth 9. diff --git a/src/infrastructure/services/pg_acl_engine.rs b/src/infrastructure/services/pg_acl_engine.rs index f36ce33a..169c6432 100644 --- a/src/infrastructure/services/pg_acl_engine.rs +++ b/src/infrastructure/services/pg_acl_engine.rs @@ -1401,30 +1401,29 @@ impl AuthorizationEngine for PgAclEngine { .filter_map(|rid| { let (resource_type, first_shared_at, subj_map) = resource_map.remove(&rid)?; let mut grants: Vec = subj_map.into_values().collect(); - let role_rank = |perms: &[Permission]| -> u8 { - if perms.contains(&Permission::Delete) && perms.contains(&Permission::Share) { - 0 // admin → Can manage - } else if perms.contains(&Permission::Create) - || perms.contains(&Permission::Update) - { - 1 // editor → Can edit - } else { - 2 // viewer → Can view + // Per-resource subject ordering (matches the subject-sort + // branch's SQL CASE): + // 0 = group, 1 = user, 2 = token-with-password, 3 = token, + // 4 = external. + // Alphabetical tiebreak by display name. This intentionally + // ignores role/permission tier — the share dialog renders + // role as a separate pill; ordering by subject type is the + // UX contract. + let subject_rank = |e: &OutgoingGrantEntry| -> u8 { + match e.subject_type.as_str() { + "group" => 0, + "user" => 1, + "token" if e.has_password => 2, + "token" => 3, + _ => 4, } }; grants.sort_by(|a, b| { - role_rank(&a.permissions) - .cmp(&role_rank(&b.permissions)) - .then_with(|| { - // users before tokens - let type_rank = |st: &str| if st == "user" { 0u8 } else { 1 }; - type_rank(&a.subject_type).cmp(&type_rank(&b.subject_type)) - }) - .then_with(|| { - a.subject_display - .to_lowercase() - .cmp(&b.subject_display.to_lowercase()) - }) + subject_rank(a).cmp(&subject_rank(b)).then_with(|| { + a.subject_display + .to_lowercase() + .cmp(&b.subject_display.to_lowercase()) + }) }); Some(OutgoingResourceSummary { resource_type,