Implement consistent email sorting for individual_permissions arrays

- Backend: Add itertools imports and case-insensitive email sorting to collections, dashboards, and metrics handlers
- Frontend: Add email sorting to React Query mutation onMutate callbacks for share/unshare operations
- Ensures consistent alphabetical ordering by email across API responses and UI state
- Addresses BUS-1477 requirements for predictable individual_permissions ordering

Co-Authored-By: nate@buster.so <nate@buster.so>
This commit is contained in:
Devin AI 2025-07-22 12:41:40 +00:00
parent 98926c5109
commit 1820dd6842
6 changed files with 30 additions and 18 deletions

View File

@ -11,6 +11,7 @@ use database::{
}; };
use diesel::{ExpressionMethods, JoinOnDsl, NullableExpressionMethods, QueryDsl, Queryable}; use diesel::{ExpressionMethods, JoinOnDsl, NullableExpressionMethods, QueryDsl, Queryable};
use diesel_async::RunQueryDsl; use diesel_async::RunQueryDsl;
use itertools::Itertools;
use middleware::AuthenticatedUser; use middleware::AuthenticatedUser;
use sharing::{check_permission_access, compute_effective_permission}; use sharing::{check_permission_access, compute_effective_permission};
use tracing; use tracing;
@ -133,7 +134,6 @@ pub async fn get_collection_handler(
users::name, users::name,
users::avatar_url, users::avatar_url,
)) ))
.order_by(users::email)
.load::<AssetPermissionInfo>(&mut conn) .load::<AssetPermissionInfo>(&mut conn)
.await; .await;
@ -157,7 +157,10 @@ pub async fn get_collection_handler(
name: p.name, name: p.name,
avatar_url: p.avatar_url, avatar_url: p.avatar_url,
}) })
.collect::<Vec<BusterShareIndividual>>(), .collect::<Vec<BusterShareIndividual>>()
.into_iter()
.sorted_by(|a, b| a.email.to_lowercase().cmp(&b.email.to_lowercase()))
.collect(),
) )
} }
} }

View File

@ -5,6 +5,7 @@ use chrono::{DateTime, Utc};
use diesel::{BoolExpressionMethods, ExpressionMethods, JoinOnDsl, QueryDsl, Queryable, Selectable}; use diesel::{BoolExpressionMethods, ExpressionMethods, JoinOnDsl, QueryDsl, Queryable, Selectable};
use diesel_async::RunQueryDsl; use diesel_async::RunQueryDsl;
use futures::future::join_all; use futures::future::join_all;
use itertools::Itertools;
use middleware::AuthenticatedUser; use middleware::AuthenticatedUser;
use serde_json::Value; use serde_json::Value;
use serde_yaml; use serde_yaml;
@ -346,7 +347,6 @@ pub async fn get_dashboard_handler(
.filter(asset_permissions::identity_type.eq(IdentityType::User)) .filter(asset_permissions::identity_type.eq(IdentityType::User))
.filter(asset_permissions::deleted_at.is_null()) .filter(asset_permissions::deleted_at.is_null())
.select((asset_permissions::role, users::email, users::name, users::avatar_url)) .select((asset_permissions::role, users::email, users::name, users::avatar_url))
.order_by(users::email)
.load::<AssetPermissionInfo>(&mut conn) .load::<AssetPermissionInfo>(&mut conn)
.await; .await;
@ -391,7 +391,10 @@ pub async fn get_dashboard_handler(
name: p.name, name: p.name,
avatar_url: p.avatar_url, avatar_url: p.avatar_url,
}) })
.collect::<Vec<BusterShareIndividual>>(), .collect::<Vec<BusterShareIndividual>>()
.into_iter()
.sorted_by(|a, b| a.email.to_lowercase().cmp(&b.email.to_lowercase()))
.collect(),
) )
} }
} }

View File

@ -2,6 +2,7 @@ use anyhow::{anyhow, Result};
use diesel::{BoolExpressionMethods, ExpressionMethods, JoinOnDsl, QueryDsl, Queryable}; use diesel::{BoolExpressionMethods, ExpressionMethods, JoinOnDsl, QueryDsl, Queryable};
use diesel_async::RunQueryDsl; use diesel_async::RunQueryDsl;
use futures::future::join; use futures::future::join;
use itertools::Itertools;
use middleware::AuthenticatedUser; use middleware::AuthenticatedUser;
use serde_yaml; use serde_yaml;
use sharing::asset_access_checks::check_metric_collection_access; use sharing::asset_access_checks::check_metric_collection_access;
@ -399,7 +400,6 @@ pub async fn get_metric_handler(
.filter(asset_permissions::identity_type.eq(IdentityType::User)) .filter(asset_permissions::identity_type.eq(IdentityType::User))
.filter(asset_permissions::deleted_at.is_null()) .filter(asset_permissions::deleted_at.is_null())
.select((asset_permissions::role, users::email, users::name, users::avatar_url)) .select((asset_permissions::role, users::email, users::name, users::avatar_url))
.order_by(users::email)
.load::<AssetPermissionInfo>(&mut conn) .load::<AssetPermissionInfo>(&mut conn)
.await; .await;
@ -465,7 +465,10 @@ pub async fn get_metric_handler(
name: p.name, name: p.name,
avatar_url: p.avatar_url, avatar_url: p.avatar_url,
}) })
.collect::<Vec<crate::metrics::types::BusterShareIndividual>>(), .collect::<Vec<crate::metrics::types::BusterShareIndividual>>()
.into_iter()
.sorted_by(|a, b| a.email.to_lowercase().cmp(&b.email.to_lowercase()))
.collect(),
) )
} }
} }

View File

@ -182,7 +182,7 @@ export const useShareCollection = () => {
draft.individual_permissions = [ draft.individual_permissions = [
...params.map((p) => ({ ...p })), ...params.map((p) => ({ ...p })),
...(draft.individual_permissions || []) ...(draft.individual_permissions || [])
]; ].sort((a, b) => a.email.localeCompare(b.email));
}); });
}); });
}, },
@ -208,7 +208,8 @@ export const useUnshareCollection = () => {
if (!previousData) return previousData; if (!previousData) return previousData;
return create(previousData, (draft: BusterCollection) => { return create(previousData, (draft: BusterCollection) => {
draft.individual_permissions = draft.individual_permissions =
draft.individual_permissions?.filter((t) => !variables.data.includes(t.email)) || []; (draft.individual_permissions?.filter((t) => !variables.data.includes(t.email)) || [])
.sort((a, b) => a.email.localeCompare(b.email));
}); });
}); });
}, },
@ -231,11 +232,11 @@ export const useUpdateCollectionShare = () => {
if (!previousData) return previousData; if (!previousData) return previousData;
return create(previousData, (draft) => { return create(previousData, (draft) => {
draft.individual_permissions = draft.individual_permissions =
draft.individual_permissions?.map((t) => { (draft.individual_permissions?.map((t) => {
const found = params.users?.find((v) => v.email === t.email); const found = params.users?.find((v) => v.email === t.email);
if (found) return { ...t, ...found }; if (found) return { ...t, ...found };
return t; return t;
}) || []; }) || []).sort((a, b) => a.email.localeCompare(b.email));
if (params.publicly_accessible !== undefined) { if (params.publicly_accessible !== undefined) {
draft.publicly_accessible = params.publicly_accessible; draft.publicly_accessible = params.publicly_accessible;

View File

@ -368,7 +368,7 @@ export const useShareDashboard = () => {
avatar_url: p.avatar_url || null avatar_url: p.avatar_url || null
})), })),
...(draft.individual_permissions || []) ...(draft.individual_permissions || [])
]; ].sort((a, b) => a.email.localeCompare(b.email));
}); });
}); });
}, },
@ -398,7 +398,8 @@ export const useUnshareDashboard = () => {
if (!previousData) return previousData; if (!previousData) return previousData;
return create(previousData, (draft) => { return create(previousData, (draft) => {
draft.individual_permissions = draft.individual_permissions =
draft.individual_permissions?.filter((t) => !variables.data.includes(t.email)) || []; (draft.individual_permissions?.filter((t) => !variables.data.includes(t.email)) || [])
.sort((a, b) => a.email.localeCompare(b.email));
}); });
}); });
}, },
@ -419,11 +420,11 @@ export const useUpdateDashboardShare = () => {
if (!previousData) return previousData; if (!previousData) return previousData;
return create(previousData, (draft) => { return create(previousData, (draft) => {
draft.individual_permissions = draft.individual_permissions =
draft.individual_permissions?.map((t) => { (draft.individual_permissions?.map((t) => {
const found = params.users?.find((v) => v.email === t.email); const found = params.users?.find((v) => v.email === t.email);
if (found) return { ...t, ...found }; if (found) return { ...t, ...found };
return t; return t;
}) || []; }) || []).sort((a, b) => a.email.localeCompare(b.email));
if (params.publicly_accessible !== undefined) { if (params.publicly_accessible !== undefined) {
draft.publicly_accessible = params.publicly_accessible; draft.publicly_accessible = params.publicly_accessible;

View File

@ -233,7 +233,7 @@ export const useShareMetric = () => {
avatar_url: p.avatar_url || null avatar_url: p.avatar_url || null
})), })),
...(draft.individual_permissions || []) ...(draft.individual_permissions || [])
]; ].sort((a, b) => a.email.localeCompare(b.email));
}); });
}); });
}, },
@ -264,7 +264,8 @@ export const useUnshareMetric = () => {
if (!previousData) return previousData; if (!previousData) return previousData;
return create(previousData, (draft: BusterMetric) => { return create(previousData, (draft: BusterMetric) => {
draft.individual_permissions = draft.individual_permissions =
draft.individual_permissions?.filter((t) => !variables.data.includes(t.email)) || []; (draft.individual_permissions?.filter((t) => !variables.data.includes(t.email)) || [])
.sort((a, b) => a.email.localeCompare(b.email));
}); });
}); });
}, },
@ -297,11 +298,11 @@ export const useUpdateMetricShare = () => {
if (!previousData) return previousData; if (!previousData) return previousData;
return create(previousData, (draft: BusterMetric) => { return create(previousData, (draft: BusterMetric) => {
draft.individual_permissions = draft.individual_permissions =
draft.individual_permissions?.map((t) => { (draft.individual_permissions?.map((t) => {
const found = variables.params.users?.find((v) => v.email === t.email); const found = variables.params.users?.find((v) => v.email === t.email);
if (found) return { ...t, ...found }; if (found) return { ...t, ...found };
return t; return t;
}) || []; }) || []).sort((a, b) => a.email.localeCompare(b.email));
if (variables.params.publicly_accessible !== undefined) { if (variables.params.publicly_accessible !== undefined) {
draft.publicly_accessible = variables.params.publicly_accessible; draft.publicly_accessible = variables.params.publicly_accessible;