Skip to content
This repository was archived by the owner on Oct 19, 2026. It is now read-only.

Commit 6b8daf5

Browse files
authored
fix: order_index unique problem (#1710)
* feat: add sync_sidebar router * feat: change ID default to NoSet * chore: address code review feedback * fix: order_index unique problem * fix: clippy closure error * feat: add test
1 parent cb3e0d0 commit 6b8daf5

4 files changed

Lines changed: 135 additions & 2 deletions

File tree

‎jupiter/callisto/src/dynamic_sidebar.rs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ pub struct Model {
1313
pub label: String,
1414
pub href: String,
1515
pub visible: bool,
16-
#[sea_orm(unique)]
1716
pub order_index: i32,
1817
}
1918

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
use sea_orm_migration::prelude::*;
2+
3+
#[derive(DeriveMigrationName)]
4+
pub struct Migration;
5+
6+
#[async_trait::async_trait]
7+
impl MigrationTrait for Migration {
8+
async fn up(&self, manager: &SchemaManager) -> Result<(), DbErr> {
9+
manager
10+
.drop_index(
11+
Index::drop()
12+
.name("idx_dynamic_sidebar_order_index")
13+
.table(DynamicSidebar::Table)
14+
.to_owned(),
15+
)
16+
.await
17+
}
18+
19+
async fn down(&self, manager: &SchemaManager) -> Result<(), DbErr> {
20+
manager
21+
.create_index(
22+
Index::create()
23+
.unique()
24+
.name("idx_dynamic_sidebar_order_index")
25+
.table(DynamicSidebar::Table)
26+
.col(DynamicSidebar::OrderIndex)
27+
.to_owned(),
28+
)
29+
.await
30+
}
31+
}
32+
33+
#[derive(DeriveIden)]
34+
enum DynamicSidebar {
35+
Table,
36+
OrderIndex,
37+
}

‎jupiter/src/migration/mod.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ mod m20251119_145041_add_draft_status;
7373
mod m20251125_135032_add_draft_conv_type;
7474
mod m20251128_000001_create_buck_session;
7575
mod m20251203_013745_add_dynamic_sidebar;
76+
mod m20251210_113942_remove_unique_constraint_from_order_index;
7677

7778
/// Creates a primary key column definition with big integer type.
7879
///
@@ -134,6 +135,7 @@ impl MigratorTrait for Migrator {
134135
Box::new(m20251125_135032_add_draft_conv_type::Migration),
135136
Box::new(m20251128_000001_create_buck_session::Migration),
136137
Box::new(m20251203_013745_add_dynamic_sidebar::Migration),
138+
Box::new(m20251210_113942_remove_unique_constraint_from_order_index::Migration),
137139
]
138140
}
139141
}

‎jupiter/src/storage/dynamic_sidebar_storage.rs‎

Lines changed: 96 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use std::ops::Deref;
1+
use std::{collections::HashSet, ops::Deref};
22

33
use crate::{
44
model::sidebar_dto::SidebarSyncDto,
@@ -122,6 +122,21 @@ impl DynamicSidebarStorage {
122122
Ok(model)
123123
}
124124

125+
/// Synchronize sidebar items by applying updates and inserts.
126+
///
127+
/// This function does **not** perform full replacement.
128+
/// It assumes:
129+
/// - The frontend sends the **complete list of sidebar items**.
130+
/// - Existing items include a valid `id` and will be updated.
131+
/// - New items have `id = None` and will be inserted.
132+
/// - Deletions are handled separately by another API.
133+
///
134+
/// Validation rules:
135+
/// - `order_index` values in the incoming list must be unique.
136+
///
137+
/// Transaction behavior:
138+
/// - All updates and inserts happen inside a single transaction.
139+
/// - Any failure will roll back the entire operation.
125140
pub async fn sync_sidebar(
126141
&self,
127142
items: Vec<SidebarSyncDto>,
@@ -130,6 +145,9 @@ impl DynamicSidebarStorage {
130145
return Ok(Vec::new());
131146
}
132147

148+
// Validate that order_index values in items are unique
149+
validate_order_index_unique(&items).map_err(MegaError::Other)?;
150+
133151
// Begin a transaction
134152
let txn = self.get_connection().begin().await?;
135153

@@ -175,3 +193,80 @@ impl DynamicSidebarStorage {
175193
Ok(res_models)
176194
}
177195
}
196+
197+
fn validate_order_index_unique(items: &[SidebarSyncDto]) -> Result<(), String> {
198+
let mut seen = HashSet::new();
199+
200+
for item in items {
201+
if !seen.insert(item.order_index) {
202+
// If insert returns false, it means a duplicate order_index exists
203+
return Err(format!("Duplicate order_index found: {}", item.order_index));
204+
}
205+
}
206+
207+
Ok(())
208+
}
209+
210+
#[cfg(test)]
211+
mod tests {
212+
use super::*;
213+
214+
fn dto(id: Option<i32>, order: i32) -> SidebarSyncDto {
215+
SidebarSyncDto {
216+
id,
217+
public_id: format!("id_{}", order),
218+
label: "label".into(),
219+
href: "/".into(),
220+
visible: true,
221+
order_index: order,
222+
}
223+
}
224+
225+
#[test]
226+
fn test_validate_unique_order_index_ok() {
227+
// All unique order indices
228+
let items = vec![dto(Some(1), 0), dto(Some(2), 1), dto(Some(3), 2)];
229+
230+
let result = validate_order_index_unique(&items);
231+
232+
assert!(result.is_ok(), "Expected unique order indices to pass.");
233+
}
234+
235+
#[test]
236+
fn test_validate_unique_order_index_duplicate() {
237+
// Contains duplicate order indices
238+
let items = vec![dto(Some(1), 0), dto(Some(2), 1), dto(Some(3), 1)];
239+
240+
let result = validate_order_index_unique(&items);
241+
242+
assert!(
243+
result.is_err(),
244+
"Expected duplicate order indices to produce error."
245+
);
246+
247+
assert!(
248+
result.unwrap_err().contains("1"),
249+
"Error message should contain the duplicate order index."
250+
);
251+
}
252+
253+
#[test]
254+
fn test_validate_unique_order_index_empty() {
255+
// Empty list should be valid
256+
let items: Vec<SidebarSyncDto> = vec![];
257+
258+
let result = validate_order_index_unique(&items);
259+
260+
assert!(result.is_ok(), "Empty list should be considered valid.");
261+
}
262+
263+
#[test]
264+
fn test_validate_unique_order_index_single_item() {
265+
// A single item is always valid
266+
let items = vec![dto(Some(1), 10)];
267+
268+
let result = validate_order_index_unique(&items);
269+
270+
assert!(result.is_ok(), "Single item should always be valid.");
271+
}
272+
}

0 commit comments

Comments
 (0)