Commit 540541e119
Verified · cmc ci/build: success ci/test: success
Layout: unified · split
internal/store/dashboard.go +5 −5
| @@ -60,7 +60,7 @@ const dashboardMRsQuery = ` | |||
| 60 | LEFT JOIN orgs o ON r.owner_kind = 'org' AND o.id = r.owner_id | 60 | LEFT JOIN orgs o ON r.owner_kind = 'org' AND o.id = r.owner_id |
| 61 | JOIN users au ON au.id = x.author_id | 61 | JOIN users au ON au.id = x.author_id |
| 62 | WHERE x.state IN ('open', 'source_gone') AND ` + involvedCond + ` | 62 | WHERE x.state IN ('open', 'source_gone') AND ` + involvedCond + ` |
| 63 | ORDER BY x.updated_at DESC LIMIT 50` | 63 | ORDER BY x.updated_at DESC, x.id DESC LIMIT 50` |
| 64 | 64 | ||
| 65 | // DashboardMRs returns open merge requests involving the user: on their | 65 | // DashboardMRs returns open merge requests involving the user: on their |
| 66 | // repositories (owned, granted, org) or authored by them anywhere. | 66 | // repositories (owned, granted, org) or authored by them anywhere. |
| @@ -80,7 +80,7 @@ const dashboardIssuesQuery = ` | |||
| 80 | WHERE x.state = 'open' AND ` + involvedCond + ` | 80 | WHERE x.state = 'open' AND ` + involvedCond + ` |
| 81 | AND NOT EXISTS (SELECT 1 FROM issue_assignees ia | 81 | AND NOT EXISTS (SELECT 1 FROM issue_assignees ia |
| 82 | WHERE ia.issue_id = x.id AND ia.user_id = ?1) | 82 | WHERE ia.issue_id = x.id AND ia.user_id = ?1) |
| 83 | ORDER BY x.updated_at DESC LIMIT 50` | 83 | ORDER BY x.updated_at DESC, x.id DESC LIMIT 50` |
| 84 | 84 | ||
| 85 | func (s *Store) DashboardIssues(userID int64) ([]DashboardItem, error) { | 85 | func (s *Store) DashboardIssues(userID int64) ([]DashboardItem, error) { |
| 86 | return s.dashboardQuery(dashboardIssuesQuery, userID) | 86 | return s.dashboardQuery(dashboardIssuesQuery, userID) |
| @@ -152,7 +152,7 @@ const reviewQueueQuery = ` | |||
| 152 | WHERE rv.mr_id = x.id AND rv.reviewer_id = ?1 | 152 | WHERE rv.mr_id = x.id AND rv.reviewer_id = ?1 |
| 153 | AND rv.head_sha = x.head_sha) | 153 | AND rv.head_sha = x.head_sha) |
| 154 | AND ` + involvedCond + ` | 154 | AND ` + involvedCond + ` |
| 155 | ORDER BY x.updated_at DESC LIMIT 8` | 155 | ORDER BY x.updated_at DESC, x.id DESC LIMIT 8` |
| 156 | 156 | ||
| 157 | // requestedReviewsQuery is ReviewQueue's other half: MRs where the user was | 157 | // requestedReviewsQuery is ReviewQueue's other half: MRs where the user was |
| 158 | // asked directly, regardless of involvement — the same exemption | 158 | // asked directly, regardless of involvement — the same exemption |
| @@ -177,7 +177,7 @@ const requestedReviewsQuery = ` | |||
| 177 | AND NOT EXISTS (SELECT 1 FROM mr_reviews rv | 177 | AND NOT EXISTS (SELECT 1 FROM mr_reviews rv |
| 178 | WHERE rv.mr_id = x.id AND rv.reviewer_id = ?1 | 178 | WHERE rv.mr_id = x.id AND rv.reviewer_id = ?1 |
| 179 | AND rv.head_sha = x.head_sha) | 179 | AND rv.head_sha = x.head_sha) |
| 180 | ORDER BY x.updated_at DESC LIMIT 8` | 180 | ORDER BY x.updated_at DESC, x.id DESC LIMIT 8` |
| 181 | 181 | ||
| 182 | // ReviewQueue returns open merge requests the user is involved in, has not | 182 | // ReviewQueue returns open merge requests the user is involved in, has not |
| 183 | // authored, and has not reviewed at the current head — what the rail shows | 183 | // authored, and has not reviewed at the current head — what the rail shows |
| @@ -245,7 +245,7 @@ const assignedIssuesQuery = ` | |||
| 245 | LEFT JOIN orgs o ON r.owner_kind = 'org' AND o.id = r.owner_id | 245 | LEFT JOIN orgs o ON r.owner_kind = 'org' AND o.id = r.owner_id |
| 246 | JOIN users au ON au.id = x.author_id | 246 | JOIN users au ON au.id = x.author_id |
| 247 | WHERE ia.user_id = ?1 AND x.state = 'open' | 247 | WHERE ia.user_id = ?1 AND x.state = 'open' |
| 248 | ORDER BY x.updated_at DESC LIMIT 20` | 248 | ORDER BY x.updated_at DESC, x.id DESC LIMIT 20` |
| 249 | 249 | ||
| 250 | func (s *Store) AssignedIssues(userID int64) ([]DashboardItem, error) { | 250 | func (s *Store) AssignedIssues(userID int64) ([]DashboardItem, error) { |
| 251 | return s.dashboardQuery(assignedIssuesQuery, userID) | 251 | return s.dashboardQuery(assignedIssuesQuery, userID) |
internal/store/dashboard_test.go +74 −1
| @@ -1,6 +1,79 @@ | |||
| 1 | package store | 1 | package store |
| 2 | 2 | ||
| 3 | import "testing" | 3 | import ( |
| 4 | "reflect" | ||
| 5 | "testing" | ||
| 6 | ) | ||
| 7 | |||
| 8 | // Rows sharing an updated_at (timestamps are milliseconds) come back | ||
| 9 | // newest first, the same order distinct times give, so the dashboard does | ||
| 10 | // not reshuffle depending on whether two writes landed in one millisecond. | ||
| 11 | func TestDashboardEqualUpdatedAtStableOrder(t *testing.T) { | ||
| 12 | s := open(t) | ||
| 13 | if err := s.MigrateUp(); err != nil { | ||
| 14 | t.Fatal(err) | ||
| 15 | } | ||
| 16 | uid, err := s.CreateUser("cmc", false) | ||
| 17 | if err != nil { | ||
| 18 | t.Fatal(err) | ||
| 19 | } | ||
| 20 | other, err := s.CreateUser("alice", false) | ||
| 21 | if err != nil { | ||
| 22 | t.Fatal(err) | ||
| 23 | } | ||
| 24 | repoID, err := s.CreateRepo("user", uid, "gitbay", "public") | ||
| 25 | if err != nil { | ||
| 26 | t.Fatal(err) | ||
| 27 | } | ||
| 28 | for _, title := range []string{"first", "second", "third"} { | ||
| 29 | n, err := s.CreateIssue(repoID, uid, title, "", "markdown") | ||
| 30 | if err != nil { | ||
| 31 | t.Fatal(err) | ||
| 32 | } | ||
| 33 | issue, err := s.IssueByNumber(repoID, n) | ||
| 34 | if err != nil { | ||
| 35 | t.Fatal(err) | ||
| 36 | } | ||
| 37 | if err := s.SetIssueAssignee(issue.ID, other, true); err != nil { | ||
| 38 | t.Fatal(err) | ||
| 39 | } | ||
| 40 | if _, err := s.CreateMR(repoID, other, repoID, title, "main", title, "", "abc", "md", false); err != nil { | ||
| 41 | t.Fatal(err) | ||
| 42 | } | ||
| 43 | } | ||
| 44 | for _, table := range []string{"issues", "merge_requests"} { | ||
| 45 | if _, err := s.DB.Exec("UPDATE " + table + " SET updated_at = '2026-10-01T12:00:00.000Z'"); err != nil { | ||
| 46 | t.Fatal(err) | ||
| 47 | } | ||
| 48 | } | ||
| 49 | |||
| 50 | titles := func(items []DashboardItem, err error) []string { | ||
| 51 | t.Helper() | ||
| 52 | if err != nil { | ||
| 53 | t.Fatal(err) | ||
| 54 | } | ||
| 55 | var out []string | ||
| 56 | for _, d := range items { | ||
| 57 | out = append(out, d.Title) | ||
| 58 | } | ||
| 59 | return out | ||
| 60 | } | ||
| 61 | want := []string{"third", "second", "first"} | ||
| 62 | cases := []struct { | ||
| 63 | name string | ||
| 64 | got []string | ||
| 65 | }{ | ||
| 66 | {"DashboardIssues", titles(s.DashboardIssues(uid))}, | ||
| 67 | {"DashboardMRs", titles(s.DashboardMRs(uid))}, | ||
| 68 | {"ReviewQueue", titles(s.ReviewQueue(uid))}, | ||
| 69 | {"AssignedIssues", titles(s.AssignedIssues(other))}, | ||
| 70 | } | ||
| 71 | for _, tc := range cases { | ||
| 72 | if !reflect.DeepEqual(tc.got, want) { | ||
| 73 | t.Errorf("%s = %v, want %v", tc.name, tc.got, want) | ||
| 74 | } | ||
| 75 | } | ||
| 76 | } | ||
| 4 | 77 | ||
| 5 | // An issue assigned to the user is not repeated under DashboardIssues: | 78 | // An issue assigned to the user is not repeated under DashboardIssues: |
| 6 | // AssignedIssues already covers it, and a repository the user can | 79 | // AssignedIssues already covers it, and a repository the user can |
internal/store/dashboardplan_test.go +1 −1
| @@ -60,7 +60,7 @@ func TestDashboardQueriesUseIndexes(t *testing.T) { | |||
| 60 | if !strings.Contains(tc.plan, tc.want) { | 60 | if !strings.Contains(tc.plan, tc.want) { |
| 61 | t.Errorf("%s does not use %s:\n%s", tc.name, tc.want, tc.plan) | 61 | t.Errorf("%s does not use %s:\n%s", tc.name, tc.want, tc.plan) |
| 62 | } | 62 | } |
| 63 | if tc.ordered && strings.Contains(tc.plan, "USE TEMP B-TREE FOR ORDER BY") { | 63 | if tc.ordered && strings.Contains(tc.plan, "USE TEMP B-TREE") { |
| 64 | t.Errorf("%s sorts instead of walking an index:\n%s", tc.name, tc.plan) | 64 | t.Errorf("%s sorts instead of walking an index:\n%s", tc.name, tc.plan) |
| 65 | } | 65 | } |
| 66 | } | 66 | } |
internal/store/migrations/0078_recent_id_order.down.sql added +4
| @@ -0,0 +1,4 @@ | |||
| 1 | DROP INDEX issues_recent; | ||
| 2 | DROP INDEX merge_requests_recent; | ||
| 3 | CREATE INDEX issues_recent ON issues(updated_at DESC); | ||
| 4 | CREATE INDEX merge_requests_recent ON merge_requests(updated_at DESC); | ||
internal/store/migrations/0078_recent_id_order.up.sql added +7
| @@ -0,0 +1,7 @@ | |||
| 1 | -- The dashboard breaks updated_at ties by id, newest first. The 0035 | ||
| 2 | -- indexes carry rowid ascending after updated_at, so that order would | ||
| 3 | -- sort the table instead of walking the index. | ||
| 4 | DROP INDEX issues_recent; | ||
| 5 | DROP INDEX merge_requests_recent; | ||
| 6 | CREATE INDEX issues_recent ON issues(updated_at DESC, id DESC); | ||
| 7 | CREATE INDEX merge_requests_recent ON merge_requests(updated_at DESC, id DESC); | ||