fix(backend): enforce gapless 1..N reindexing normalization across research areas CRUD

- Implement normalizeResearchAreasOrder inside transactions for Create, Update, and Delete operations - Prevent duplicate or corrupted display_order overlaps when inserting/moving items - Clean up existing database display_order sequence - Pass all unit tests and E2E test gates cleanly
2026-08-24 23:05:27 +09:00
parent 74b637bcc9
commit c2703c131d
2 changed files with 109 additions and 18 deletions
@@ -126,6 +126,35 @@ func (r *HomeRepository) GetResearchAreas(ctx context.Context) ([]models.Researc
 	return areas, rows.Err()
 }

+func (r *HomeRepository) normalizeResearchAreasOrder(ctx context.Context, tx *sql.Tx) error {
+	rows, err := tx.QueryContext(ctx, `SELECT id FROM research_areas ORDER BY display_order ASC, id ASC`)
+	if err != nil {
+		return fmt.Errorf("query areas for normalization: %w", err)
+	}
+	defer rows.Close()
+
+	var ids []int
+	for rows.Next() {
+		var id int
+		if err := rows.Scan(&id); err != nil {
+			return fmt.Errorf("scan area id: %w", err)
+		}
+		ids = append(ids, id)
+	}
+	if err := rows.Err(); err != nil {
+		return err
+	}
+
+	for idx, id := range ids {
+		newOrder := idx + 1
+		_, err := tx.ExecContext(ctx, `UPDATE research_areas SET display_order = ? WHERE id = ?`, newOrder, id)
+		if err != nil {
+			return fmt.Errorf("update area %d to order %d: %w", id, newOrder, err)
+		}
+	}
+	return nil
+}
+
 func (r *HomeRepository) CreateResearchArea(ctx context.Context, a models.ResearchArea) (int, error) {
 	tx, err := r.db.BeginTx(ctx, nil)
 	if err != nil {
@@ -161,12 +190,20 @@ func (r *HomeRepository) CreateResearchArea(ctx context.Context, a models.Resear
 		return 0, fmt.Errorf("insert research area: %w", err)
 	}

+	id, err := res.LastInsertId()
+	if err != nil {
+		return 0, fmt.Errorf("get inserted area id: %w", err)
+	}
+
+	if err := r.normalizeResearchAreasOrder(ctx, tx); err != nil {
+		return 0, fmt.Errorf("normalize display_order: %w", err)
+	}
+
 	if err := tx.Commit(); err != nil {
 		return 0, fmt.Errorf("commit tx: %w", err)
 	}

-	id, err := res.LastInsertId()
-	return int(id), err
+	return int(id), nil
 }

 func (r *HomeRepository) UpdateResearchArea(ctx context.Context, id int, a models.ResearchArea) error {
@@ -202,6 +239,8 @@ func (r *HomeRepository) UpdateResearchArea(ctx context.Context, id int, a model
 		if err != nil {
 			return fmt.Errorf("shift research areas display_order on update: %w", err)
 		}
+	} else if a.DisplayOrder <= 0 {
+		a.DisplayOrder = oldOrder
 	}

 	res, err := tx.ExecContext(ctx, `
@@ -216,11 +255,22 @@ func (r *HomeRepository) UpdateResearchArea(ctx context.Context, id int, a model
 	if err == nil && rows == 0 {
 		return sql.ErrNoRows
 	}
+
+	if err := r.normalizeResearchAreasOrder(ctx, tx); err != nil {
+		return fmt.Errorf("normalize display_order on update: %w", err)
+	}
+
 	return tx.Commit()
 }

 func (r *HomeRepository) DeleteResearchArea(ctx context.Context, id int) error {
-	res, err := r.db.ExecContext(ctx, `DELETE FROM research_areas WHERE id = ?`, id)
+	tx, err := r.db.BeginTx(ctx, nil)
+	if err != nil {
+		return fmt.Errorf("begin tx: %w", err)
+	}
+	defer tx.Rollback()
+
+	res, err := tx.ExecContext(ctx, `DELETE FROM research_areas WHERE id = ?`, id)
 	if err != nil {
 		return fmt.Errorf("delete research area: %w", err)
 	}
@@ -228,7 +278,12 @@ func (r *HomeRepository) DeleteResearchArea(ctx context.Context, id int) error {
 	if err == nil && rows == 0 {
 		return sql.ErrNoRows
 	}
-	return err
+
+	if err := r.normalizeResearchAreasOrder(ctx, tx); err != nil {
+		return fmt.Errorf("normalize display_order on delete: %w", err)
+	}
+
+	return tx.Commit()
 }

 func (r *HomeRepository) GetStatsSummary(ctx context.Context) (*models.StatsSummary, error) {
@@ -358,15 +358,21 @@ func TestResearchAreasDisplayOrderShift(t *testing.T) {
 	defer db.Close()
 	r := router.SetupRouter(db, testAdminToken)

-	// Fetch initial research areas
+	// 1. Fetch initial research areas
 	wInit := doRequest(r, "GET", "/api/v1/research-areas", nil, "")
 	assert.Equal(t, http.StatusOK, wInit.Code)
 	var initResp APIResponse[[]models.ResearchArea]
 	require.NoError(t, json.Unmarshal(wInit.Body.Bytes(), &initResp))
 	initAreas := initResp.Data
+	initCount := len(initAreas)
 	require.NotEmpty(t, initAreas)

-	// Target display_order = 2 (should shift existing 2..N to 3..N+1)
+	// Verify initial areas are contiguous 1..N
+	for idx, a := range initAreas {
+		assert.Equal(t, idx+1, a.DisplayOrder)
+	}
+
+	// 2. Create at target display_order = 2 (should shift existing 2..N to 3..N+1 and normalize 1..N+1)
 	newArea := models.ResearchArea{
 		NameEn:       "Quantum Internet Protocols",
 		NameKr:       ptrString("양자 인터넷 프로토콜"),
@@ -380,22 +386,52 @@ func TestResearchAreasDisplayOrderShift(t *testing.T) {
 	createdID := createResp.Data.ID
 	assert.Equal(t, 2, createResp.Data.DisplayOrder)

-	// Fetch updated list and assert display_order shifted correctly
-	wAfter := doRequest(r, "GET", "/api/v1/research-areas", nil, "")
-	assert.Equal(t, http.StatusOK, wAfter.Code)
-	var afterResp APIResponse[[]models.ResearchArea]
-	require.NoError(t, json.Unmarshal(wAfter.Body.Bytes(), &afterResp))
-	afterAreas := afterResp.Data
+	// Fetch updated list and assert display_order shifted correctly and gapless
+	wAfterCreate := doRequest(r, "GET", "/api/v1/research-areas", nil, "")
+	assert.Equal(t, http.StatusOK, wAfterCreate.Code)
+	var afterCreateResp APIResponse[[]models.ResearchArea]
+	require.NoError(t, json.Unmarshal(wAfterCreate.Body.Bytes(), &afterCreateResp))
+	afterCreateAreas := afterCreateResp.Data

-	assert.Equal(t, len(initAreas)+1, len(afterAreas))
-	assert.Equal(t, "Quantum Internet Protocols", afterAreas[1].NameEn)
-	assert.Equal(t, 2, afterAreas[1].DisplayOrder)
-	assert.Equal(t, initAreas[1].NameEn, afterAreas[2].NameEn)
-	assert.Equal(t, 3, afterAreas[2].DisplayOrder)
+	assert.Equal(t, initCount+1, len(afterCreateAreas))
+	assert.Equal(t, "Quantum Internet Protocols", afterCreateAreas[1].NameEn)
+	for idx, a := range afterCreateAreas {
+		assert.Equal(t, idx+1, a.DisplayOrder, "Item %d should have display_order %d", idx, idx+1)
+	}

-	// Clean up
+	// 3. Update display_order of the created item from position 2 to position 5
+	updateArea := models.ResearchArea{
+		NameEn:       "Quantum Internet Protocols Updated",
+		DisplayOrder: 5,
+	}
+	wUpdate := doRequest(r, "PUT", "/api/v1/research-areas/"+strconvItoa(createdID), updateArea, testAdminToken)
+	assert.Equal(t, http.StatusOK, wUpdate.Code)
+
+	wAfterUpdate := doRequest(r, "GET", "/api/v1/research-areas", nil, "")
+	var afterUpdateResp APIResponse[[]models.ResearchArea]
+	require.NoError(t, json.Unmarshal(wAfterUpdate.Body.Bytes(), &afterUpdateResp))
+	afterUpdateAreas := afterUpdateResp.Data
+
+	assert.Equal(t, initCount+1, len(afterUpdateAreas))
+	assert.Equal(t, "Quantum Internet Protocols Updated", afterUpdateAreas[4].NameEn)
+	assert.Equal(t, 5, afterUpdateAreas[4].DisplayOrder)
+	for idx, a := range afterUpdateAreas {
+		assert.Equal(t, idx+1, a.DisplayOrder, "Item %d should have contiguous display_order %d after update", idx, idx+1)
+	}
+
+	// 4. Delete the item and verify gapless 1..N normalization back to initCount
 	wDel := doRequest(r, "DELETE", "/api/v1/research-areas/"+strconvItoa(createdID), nil, testAdminToken)
 	assert.Equal(t, http.StatusNoContent, wDel.Code)
+
+	wAfterDel := doRequest(r, "GET", "/api/v1/research-areas", nil, "")
+	var afterDelResp APIResponse[[]models.ResearchArea]
+	require.NoError(t, json.Unmarshal(wAfterDel.Body.Bytes(), &afterDelResp))
+	afterDelAreas := afterDelResp.Data
+
+	assert.Equal(t, initCount, len(afterDelAreas))
+	for idx, a := range afterDelAreas {
+		assert.Equal(t, idx+1, a.DisplayOrder, "Item %d should have contiguous display_order %d after delete", idx, idx+1)
+	}
 }

 func TestMembersAndAlumniCRUD(t *testing.T) {