Sort notes within stack view (#1858)

inovex · Jan 21, 2023 · abc45c8 · abc45c8
1 parent e591c1a
commit abc45c8
Show file tree

Hide file tree

Showing 6 changed files with 310 additions and 36 deletions.
diff --git a/server/src/database/notes.go b/server/src/database/notes.go
@@ -161,7 +161,7 @@ func (d *Database) updateNoteWithoutStack(update NoteUpdate) (Note, error) {
 		With("update_when_new_is_higher", updateWhenNewIsHigher).
 		With("update_child_notes", updateChildNotes).
 		Set("\"column\" = ?", update.Position.Column).
-		Set("stack = ?", update.Position.Stack).
+		Set("\"stack\" = ?", update.Position.Stack).
 		Set("rank = (SELECT new_rank FROM rank_selection)").
 		Where("id = ?", update.ID).
 		Where("board = ?", update.Board).
@@ -176,38 +176,111 @@ func (d *Database) updateNoteWithoutStack(update NoteUpdate) (Note, error) {
 }
 
 func (d *Database) updateNoteWithStack(update NoteUpdate) (Note, error) {
+	newRank := update.Position.Rank
+	if update.Position.Rank < 0 {
+		newRank = 0
+	}
+
 	// select previous configuration of note to update
 	previous := d.db.NewSelect().Model((*Note)(nil)).Where("id = ?", update.ID).Where("board = ?", update.Board)
+
+	// select previous configuration of stack target
+	stackTarget := d.db.NewSelect().Model((*Note)(nil)).Where("id = ?", update.Position.Stack).Where("board = ?", update.Board)
+
 	// check whether this note should be updated
-	updateCheck := d.db.NewSelect().ColumnExpr("CASE WHEN (SELECT \"stack\" FROM previous) IS NOT NULL AND (SELECT \"stack\" FROM previous) <> ? THEN true WHEN (SELECT \"stack\" FROM previous) IS NULL THEN true ELSE false END as should_update", update.Position.Stack).ColumnExpr("CASE WHEN (SELECT \"column\" FROM notes WHERE id = ?) = ? THEN true ELSE false END as valid_update", update.Position.Stack, update.Position.Column)
+	updateCheck := d.db.
+		NewSelect().
+		ColumnExpr("CASE WHEN (SELECT \"stack\" FROM previous) IS NOT NULL AND (SELECT \"stack\" FROM previous) <> ? THEN true WHEN (SELECT \"stack\" FROM previous) IS NULL THEN true ELSE false END as is_new_in_stack", update.Position.Stack).
+		ColumnExpr("CASE WHEN (SELECT \"stack\" FROM previous) = ? AND (SELECT \"rank\" FROM previous) <> ? THEN true ELSE false END as is_same_stack", update.Position.Stack, update.Position.Rank).
+		ColumnExpr("CASE WHEN (SELECT \"stack\" FROM stack_target) = ? THEN true ELSE false END as is_stack_swap", update.ID).
+		ColumnExpr("CASE WHEN (SELECT \"column\" FROM notes WHERE id = ?) = ? THEN true ELSE false END as valid_update", update.Position.Stack, update.Position.Column)
+
 	// select the children of the note to update
 	children := d.db.NewSelect().Model((*Note)(nil)).Column("*").ColumnExpr("row_number() over (ORDER BY rank DESC) as index").Where("stack = ?", update.ID)
+
 	// select the new rank for the note based on the limits of the ranks pre-existing
-	rankSelection := d.db.NewSelect().Model((*Note)(nil)).ColumnExpr("CASE WHEN (SELECT should_update FROM update_check) THEN COUNT(*) + (SELECT COUNT(*) FROM children) ELSE (SELECT rank FROM previous) END as new_rank").Where("\"column\" = ?", update.Position.Column).Where("board = ?", update.Board).Where("stack = ?", update.Position.Stack)
+	rankSelection := d.db.NewSelect().Model((*Note)(nil)).
+		ColumnExpr("CASE "+
+			"WHEN (SELECT is_stack_swap FROM update_check) THEN (SELECT rank FROM stack_target) "+
+			"WHEN (SELECT is_same_stack FROM update_check) THEN LEAST((SELECT COUNT(*) FROM notes WHERE \"stack\" = ?)-1, ?) "+
+			"WHEN (SELECT is_new_in_stack FROM update_check) THEN COUNT(*) + (SELECT COUNT(*) FROM children) "+
+			"ELSE (SELECT rank FROM previous) END as new_rank", update.Position.Stack, newRank).
+		Where("\"column\" = ?", update.Position.Column).
+		Where("board = ?", update.Board).
+		Where("stack = ?", update.Position.Stack)
+
+	// shift notes within stack if the new rank is lower than before
+	updateWhenNewIsLower := d.db.NewUpdate().Model((*Note)(nil)).Set("rank=rank+1").Where("(SELECT is_same_stack FROM update_check)").Where("(SELECT new_rank FROM rank_selection) < (SELECT rank FROM previous)").Where("\"stack\" = ?", update.Position.Stack).Where("board = ?", update.Board).Where("rank >= (SELECT new_rank FROM rank_selection)").Where("rank < (SELECT rank FROM previous)")
+
+	// shift notes within stack if the new rank is higher than before
+	updateWhenNewIsHigher := d.db.NewUpdate().Model((*Note)(nil)).Set("rank=rank-1").Where("(SELECT is_same_stack FROM update_check)").Where("(SELECT new_rank FROM rank_selection) > (SELECT rank FROM previous)").Where("\"stack\" = ?", update.Position.Stack).Where("board = ?", update.Board).Where("rank <= (SELECT new_rank FROM rank_selection)").Where("rank > (SELECT rank FROM previous)")
+
 	// update the ranks of other notes if this note is moved freshly into a new stack
-	updateWhenPreviouslyNotInStack := d.db.NewUpdate().Model((*Note)(nil)).Set("rank=rank-1").Where("(SELECT should_update FROM update_check)").Where("(SELECT valid_update FROM update_check)").Where("board = ?", update.Board).Where("\"column\" = (SELECT \"column\" FROM previous)").Where("rank >= (SELECT rank FROM previous)").WhereGroup(" AND ", func(q *bun.UpdateQuery) *bun.UpdateQuery {
-		return q.
-			WhereGroup(" OR ", func(q *bun.UpdateQuery) *bun.UpdateQuery {
-				return q.
-					Where("(SELECT stack FROM previous) IS NULL").
-					Where("stack IS NULL")
-			}).
-			WhereGroup(" OR ", func(q *bun.UpdateQuery) *bun.UpdateQuery {
-				return q.
-					Where("(SELECT stack FROM previous) IS NOT NULL").
-					Where("stack = (SELECT stack FROM previous)")
-			})
-	})
+	updateWhenPreviouslyNotInStack := d.db.NewUpdate().Model((*Note)(nil)).
+		Set("rank=rank-1").
+		Where("(SELECT is_new_in_stack FROM update_check)").
+		Where("NOT (SELECT is_stack_swap FROM update_check)").
+		Where("(SELECT valid_update FROM update_check)").
+		Where("board = ?", update.Board).
+		Where("\"column\" = (SELECT \"column\" FROM previous)").
+		Where("rank >= (SELECT rank FROM previous)").
+		WhereGroup(" AND ", func(q *bun.UpdateQuery) *bun.UpdateQuery {
+			return q.
+				WhereGroup(" OR ", func(q *bun.UpdateQuery) *bun.UpdateQuery {
+					return q.
+						Where("(SELECT stack FROM previous) IS NULL").
+						Where("stack IS NULL")
+				}).
+				WhereGroup(" OR ", func(q *bun.UpdateQuery) *bun.UpdateQuery {
+					return q.
+						Where("(SELECT stack FROM previous) IS NOT NULL").
+						Where("stack = (SELECT stack FROM previous)")
+				})
+		})
+
 	// update the stack and rank of the children of the note to update, so that it matches the new configuration
-	updateChildren := d.db.NewUpdate().TableExpr("notes as n").TableExpr("children as c").Set("stack = ?", update.Position.Stack).Set("rank = (SELECT new_rank FROM rank_selection) - c.index").Set("\"column\" = ?", update.Position.Column).Where("(SELECT valid_update FROM update_check)").Where("(SELECT should_update FROM update_check)").Where("n.id = c.id")
+	updateChildren := d.db.NewUpdate().
+		TableExpr("notes as n").
+		TableExpr("children as c").
+		Set("stack = ?", update.Position.Stack).
+		Set("rank = (SELECT new_rank FROM rank_selection) - c.index").
+		Set("\"column\" = ?", update.Position.Column).
+		Where("(SELECT valid_update FROM update_check)").
+		Where("(SELECT is_new_in_stack FROM update_check)").
+		Where("NOT (SELECT is_stack_swap FROM update_check)").
+		Where("n.id = c.id")
+
+	// update the stack and rank of the children of the note to update, so that it matches the new configuration
+	updateChildrenInSwap := d.db.NewUpdate().
+		TableExpr("notes as n").
+		TableExpr("children as c").
+		Set("stack = ?", update.Position.Stack).
+		Set("\"column\" = ?", update.Position.Column).
+		Where("(SELECT valid_update FROM update_check)").
+		Where("(SELECT is_stack_swap FROM update_check)").
+		Where("n.id = c.id")
+
+	// update new stack root
+	updateSwapNote := d.db.NewUpdate().Model((*Note)(nil)).
+		Set("rank = (SELECT rank FROM previous)").
+		Set("stack = ?", nil).
+		Where("(SELECT valid_update FROM update_check)").
+		Where("(SELECT is_stack_swap FROM update_check)").
+		Where("id = (SELECT id FROM stack_target)").
+		Where("board = ?", update.Board)
 
 	query := d.db.NewUpdate().Model(&update).
 		With("previous", previous).
+		With("stack_target", stackTarget).
 		With("update_check", updateCheck).
 		With("children", children).
 		With("rank_selection", rankSelection).
+		With("update_lower", updateWhenNewIsLower).
+		With("update_higher", updateWhenNewIsHigher).
 		With("update_when_previously_not_in_stack", updateWhenPreviouslyNotInStack).
 		With("update_children", updateChildren).
+		With("update_children_in_swap", updateChildrenInSwap).
+		With("update_stack_target", updateSwapNote).
 		Set("\"column\" = ?", update.Position.Column).
 		Set("stack = ?", update.Position.Stack).
 		Set("rank = (SELECT new_rank FROM rank_selection)").

diff --git a/server/src/database/notes_test.go b/server/src/database/notes_test.go
@@ -29,6 +29,12 @@ func TestRunnerForNotes(t *testing.T) {
 	t.Run("Update=10", testOrderOnShiftToStackWithinOtherColumn)
 	t.Run("Update=11", testOrderWhenMergingStacks)
 
+	t.Run("Update=12", testChangeOrderWhenMoveWithinStackToLower)
+	t.Run("Update=13", testChangeOrderWhenMoveWithinStackToHigher)
+	t.Run("Update=14", testChangeOrderWhenMoveWithinStackToNegative)
+	t.Run("Update=15", testChangeOrderWhenMoveWithinStackToLargeRank)
+	t.Run("Update=16", testOrderWhenChangeStackParent)
+
 	t.Run("Delete=0", testDeleteNote)
 	t.Run("Delete=1", testDeleteStackParent)
 	t.Run("Delete=2", testDeleteSharedNote)
@@ -50,6 +56,14 @@ var noteB2 *Note
 var noteB3 *Note
 var noteC1 *Note
 
+var stackTestBoard *Board
+var stackTestColumn *Column
+var stackA *Note
+var stackB *Note
+var stackC *Note
+var stackD *Note
+var stackUser *User
+
 var author *User
 
 func testGetNote(t *testing.T) {
@@ -332,6 +346,114 @@ func testOrderWhenMergingStacks(t *testing.T) {
 	verifyNoteOrder(t, notes, noteA1, noteA4, noteA2, noteA6, noteA3, noteA5)
 }
 
+func testChangeOrderWhenMoveWithinStackToLower(t *testing.T) {
+	stackTestBoard = fixture.MustRow("Board.stackTestBoard").(*Board)
+	stackTestColumn = fixture.MustRow("Column.stackTestColumn").(*Column)
+	stackA = fixture.MustRow("Note.stackTestNote1").(*Note)
+	stackB = fixture.MustRow("Note.stackTestNote2").(*Note)
+	stackC = fixture.MustRow("Note.stackTestNote3").(*Note)
+	stackD = fixture.MustRow("Note.stackTestNote4").(*Note)
+	stackUser = fixture.MustRow("User.justin").(*User)
+
+	/*
+	   A: Rank 1337, Stack null
+	   B: Rank 0, Stack A
+	   C: Rank 1, Stack A
+	   D: Rank 2, Stack A
+	*/
+
+	notes, _ := testDb.GetNotes(stackTestBoard.ID, stackTestColumn.ID)
+	verifyNoteOrder(t, notes, stackA, stackD, stackC, stackB)
+
+	note, err := testDb.UpdateNote(stackUser.ID, NoteUpdate{
+		ID:    stackD.ID,
+		Board: stackTestBoard.ID,
+		Position: &NoteUpdatePosition{
+			Column: stackD.Column,
+			Stack:  uuid.NullUUID{UUID: stackA.ID, Valid: true},
+			Rank:   0,
+		},
+	})
+
+	assert.Nil(t, err)
+	assert.Equal(t, 0, note.Rank)
+
+	notes, _ = testDb.GetNotes(stackTestBoard.ID, stackTestColumn.ID)
+	verifyNoteOrder(t, notes, stackA, stackC, stackB, stackD)
+}
+
+func testChangeOrderWhenMoveWithinStackToHigher(t *testing.T) {
+	note, err := testDb.UpdateNote(stackUser.ID, NoteUpdate{
+		ID:    stackD.ID,
+		Board: stackTestBoard.ID,
+		Position: &NoteUpdatePosition{
+			Column: stackA.Column,
+			Stack:  uuid.NullUUID{UUID: stackA.ID, Valid: true},
+			Rank:   2,
+		},
+	})
+
+	assert.Nil(t, err)
+	assert.Equal(t, 2, note.Rank)
+
+	notes, _ := testDb.GetNotes(stackTestBoard.ID, stackTestColumn.ID)
+	verifyNoteOrder(t, notes, stackA, stackD, stackC, stackB)
+}
+
+func testChangeOrderWhenMoveWithinStackToNegative(t *testing.T) {
+	note, err := testDb.UpdateNote(stackUser.ID, NoteUpdate{
+		ID:    stackD.ID,
+		Board: stackTestBoard.ID,
+		Position: &NoteUpdatePosition{
+			Column: stackA.Column,
+			Stack:  uuid.NullUUID{UUID: stackA.ID, Valid: true},
+			Rank:   -100,
+		},
+	})
+
+	assert.Nil(t, err)
+	assert.Equal(t, 0, note.Rank)
+
+	notes, _ := testDb.GetNotes(stackTestBoard.ID, stackTestColumn.ID)
+	verifyNoteOrder(t, notes, stackA, stackC, stackB, stackD)
+}
+
+func testChangeOrderWhenMoveWithinStackToLargeRank(t *testing.T) {
+	note, err := testDb.UpdateNote(stackUser.ID, NoteUpdate{
+		ID:    stackD.ID,
+		Board: stackTestBoard.ID,
+		Position: &NoteUpdatePosition{
+			Column: stackA.Column,
+			Stack:  uuid.NullUUID{UUID: stackA.ID, Valid: true},
+			Rank:   9999,
+		},
+	})
+
+	assert.Nil(t, err)
+	assert.Equal(t, 2, note.Rank)
+
+	notes, _ := testDb.GetNotes(stackTestBoard.ID, stackTestColumn.ID)
+	verifyNoteOrder(t, notes, stackA, stackD, stackC, stackB)
+}
+
+func testOrderWhenChangeStackParent(t *testing.T) {
+	note, err := testDb.UpdateNote(stackUser.ID, NoteUpdate{
+		ID:    stackA.ID,
+		Board: stackTestBoard.ID,
+		Position: &NoteUpdatePosition{
+			Column: stackD.Column,
+			Stack:  uuid.NullUUID{UUID: stackD.ID, Valid: true},
+			Rank:   9999,
+		},
+	})
+
+	assert.Nil(t, err)
+	assert.Equal(t, 2, note.Rank)
+
+	notes, _ := testDb.GetNotes(stackTestBoard.ID, stackTestColumn.ID)
+	verifyNoteOrder(t, notes, stackD, stackA, stackC, stackB)
+}
+
 func testDeleteNote(t *testing.T) {
 	err := testDb.DeleteNote(author.ID, notesTestBoard.ID, noteB1.ID)
 	assert.Nil(t, err)

diff --git a/server/src/database/testdata/fixture.yml b/server/src/database/testdata/fixture.yml
@@ -152,6 +152,13 @@
       show_authors: true
       show_notes_of_other_users: true
       created_at: '{{ now }}'
+    - _id: stackTestBoard
+      id: "1acd6899-ad71-4479-8ffe-6c6401b20811"
+      name: Stack test board
+      access_policy: PUBLIC
+      show_authors: true
+      show_notes_of_other_users: true
+      created_at: '{{ now }}'
 
 - model: BoardSessionInsert
   rows:
@@ -207,6 +214,10 @@
       board: '{{ $.Board.votingSortingTestBoard.ID }}'
       user: '{{ $.User.justin.ID }}'
       role: OWNER
+    - _id: justinSessionOnStackTestBoard
+      board: '{{ $.Board.stackTestBoard.ID }}'
+      user: '{{ $.User.justin.ID }}'
+      role: OWNER
 
 - model: Column
   rows:
@@ -301,6 +312,13 @@
       color: backlog-blue
       visible: true
       index: 0
+    - _id: stackTestColumn
+      id: "00c8c51b-d391-4472-8136-06eadff7bbd6"
+      board: '{{ $.Board.stackTestBoard.ID }}'
+      name: Stack test column
+      color: backlog-blue
+      visible: true
+      index: 0
 
 - model: Note
   rows:
@@ -472,6 +490,38 @@
       text: "Note 6"
       stack: "8d77f76d-803c-4c37-9c03-20d38772ef7f"
       rank: 0
+    - _id: stackTestNote1
+      id: "c98dd000-66ae-4c59-a7de-b7fee44e452a"
+      board: '{{ $.Board.stackTestBoard.ID }}'
+      author: '{{ $.User.justin.ID }}'
+      column: '{{ $.Column.stackTestColumn.ID }}'
+      text: "A"
+      stack: null
+      rank: 1337
+    - _id: stackTestNote2
+      id: "c98dd001-66ae-4c59-a7de-b7fee44e452a"
+      board: '{{ $.Board.stackTestBoard.ID }}'
+      author: '{{ $.User.justin.ID }}'
+      column: '{{ $.Column.stackTestColumn.ID }}'
+      text: "B"
+      stack: "c98dd000-66ae-4c59-a7de-b7fee44e452a"
+      rank: 0
+    - _id: stackTestNote3
+      id: "c98dd002-66ae-4c59-a7de-b7fee44e452a"
+      board: '{{ $.Board.stackTestBoard.ID }}'
+      author: '{{ $.User.justin.ID }}'
+      column: '{{ $.Column.stackTestColumn.ID }}'
+      text: "C"
+      stack: "c98dd000-66ae-4c59-a7de-b7fee44e452a"
+      rank: 1
+    - _id: stackTestNote4
+      id: "c98dd003-66ae-4c59-a7de-b7fee44e452a"
+      board: '{{ $.Board.stackTestBoard.ID }}'
+      author: '{{ $.User.justin.ID }}'
+      column: '{{ $.Column.stackTestColumn.ID }}'
+      text: "D"
+      stack: "c98dd000-66ae-4c59-a7de-b7fee44e452a"
+      rank: 2
 
 - model: Voting
   rows:

diff --git a/server/src/go.mod b/server/src/go.mod
@@ -29,7 +29,7 @@ require (
 )
 
 require (
-	cloud.google.com/go/compute v1.6.1 // indirect
+	cloud.google.com/go/compute v1.7.0 // indirect
 	github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1 // indirect
 	github.com/Microsoft/go-winio v0.5.2 // indirect
 	github.com/Nvveen/Gotty v0.0.0-20120604004816-cd527374f1e5 // indirect