Work item links say when their repository is deleted, and work lists page on
A repository deleted but still restorable kept its path in repo_workspaces with nothing to say it was gone, so anything reading an item's links (the Board, the item page) asked it for pull requests and got not found every time: flagon-io/hello behind #59. repo.deleted now marks it (deleted_at), repo.restored clears it, repo.purged drops the row as before, and each link carries `deleted`. A one-off job, deleted_marks, marks those deleted before the column was there. list_work_items takes closed_since (closed at or after, most recently closed first) and before (numbered below, the next page); list_pulls takes before and merged_since. Each still gives at most 100, so a caller can read every page instead of silently stopping at the first. Part of #161
- G1t-Session:
- ses_01m5780qevefmb5pmg2kcjsgba
| 1679 | 1679 | pub repo_id: Option<String>, | |
| 1680 | 1680 | } | |
| 1681 | 1681 | ||
| 1682 | − | /// `list_pulls`, newest first. Returns `Outcome<Vec<Pull>>`. | |
| 1682 | + | /// `list_pulls`, newest first, at most 100 (`before` pages on). Returns | |
| 1683 | + | /// `Outcome<Vec<Pull>>`. | |
| 1683 | 1684 | #[derive(Debug, Serialize, Deserialize)] | |
| 1684 | 1685 | pub struct ListPullsArgs { | |
| 1685 | 1686 | pub repo: RepoPath, | |
| ⋯ | |||
| 1695 | 1696 | /// Only pull requests into this branch. | |
| 1696 | 1697 | #[serde(default)] | |
| 1697 | 1698 | pub base: Option<String>, | |
| 1699 | + | /// Only pull requests numbered below this: the next page, after one | |
| 1700 | + | /// whose lowest number this is. | |
| 1701 | + | #[serde(default)] | |
| 1702 | + | pub before: Option<u32>, | |
| 1703 | + | /// Only pull requests merged at or after this time (RFC 3339). | |
| 1704 | + | #[serde(default)] | |
| 1705 | + | pub merged_since: Option<String>, | |
| 1698 | 1706 | } | |
| 1699 | 1707 | ||
| 1700 | 1708 | /// `pulls_for_repos`: the newest open and the newest closed pull requests | |
| 61 | 61 | /// A pull request or commit that closes it landed in this repository. | |
| 62 | 62 | #[serde(default)] | |
| 63 | 63 | pub landed: bool, | |
| 64 | + | /// The repository is deleted, and can still be restored until it is | |
| 65 | + | /// purged: `path` is where it was. It has nothing to open or read. | |
| 66 | + | #[serde(default)] | |
| 67 | + | pub deleted: bool, | |
| 64 | 68 | } | |
| 65 | 69 | ||
| 66 | 70 | /// What resolved a work item in one repository: a pull request merged, or | |
| ⋯ | |||
| 247 | 251 | pub milestone: Option<u32>, | |
| 248 | 252 | } | |
| 249 | 253 | ||
| 250 | − | /// `list_work_items`, newest first: a workspace's work items the viewer | |
| 251 | − | /// may see. With `repo` (then `workspace` may be empty, meaning the | |
| 254 | + | /// `list_work_items`, newest first, at most 100 (`before` pages on): a | |
| 255 | + | /// workspace's work items the viewer may see. With `repo` (then `workspace` may be empty, meaning the | |
| 252 | 256 | /// repository's), only those linked to it, in any role or in `role`. | |
| 253 | 257 | /// Returns `Outcome<Vec<WorkItem>>`. | |
| 254 | 258 | #[derive(Debug, Serialize, Deserialize)] | |
| ⋯ | |||
| 272 | 276 | /// Only those assigned to this username. | |
| 273 | 277 | #[serde(default)] | |
| 274 | 278 | pub assignee: Option<String>, | |
| 279 | + | /// Only those closed at or after this time (RFC 3339), the most | |
| 280 | + | /// recently closed first instead of the newest. | |
| 281 | + | #[serde(default)] | |
| 282 | + | pub closed_since: Option<String>, | |
| 283 | + | /// Only those numbered below this: the next page, after one whose | |
| 284 | + | /// lowest number this is. | |
| 285 | + | #[serde(default)] | |
| 286 | + | pub before: Option<u32>, | |
| 275 | 287 | } | |
| 276 | 288 | ||
| 277 | 289 | /// `get_work_item` (`Outcome<WorkItemDetail>`), `get_work` | |
| ⋯ | |||
| 450 | 462 | use super::*; | |
| 451 | 463 | ||
| 452 | 464 | fn link(path: &str, role: LinkRole, landed: bool) -> ItemRepo { | |
| 453 | − | ItemRepo { repo_id: format!("rep_{path}"), path: path.into(), role, legacy_number: None, landed } | |
| 465 | + | ItemRepo { repo_id: format!("rep_{path}"), path: path.into(), role, legacy_number: None, landed, deleted: false } | |
| 454 | 466 | } | |
| 455 | 467 | ||
| 456 | 468 | #[test] | |
| 493 | 493 | label: filter.label ?? null, | |
| 494 | 494 | milestone: filter.milestone ?? null, | |
| 495 | 495 | assignee: filter.assignee ?? null, | |
| 496 | + | closed_since: filter.closedSince ?? null, | |
| 497 | + | before: filter.before ?? null, | |
| 496 | 498 | }), | |
| 497 | 499 | getWorkItem: (workspace, number, viewer) => call("get_work_item", { workspace, number, viewer }), | |
| 498 | 500 | updateWorkItem: (actor, workspace, number, input) => | |
| ⋯ | |||
| 562 | 564 | updateSettings: (actor, repo, settings) => | |
| 563 | 565 | call("update_settings", { actor, repo, settings }), | |
| 564 | 566 | openPull: (actor, repo, input) => call("open_pull", { actor, repo, ...input }), | |
| 565 | − | listPulls: (repo, viewer, state, filter = {}) => call("list_pulls", { repo, viewer, state, ...filter }), | |
| 567 | + | listPulls: (repo, viewer, state, { mergedSince, ...filter } = {}) => | |
| 568 | + | call("list_pulls", { repo, viewer, state, ...filter, merged_since: mergedSince ?? null }), | |
| 566 | 569 | deliveryInsights: (repoIds, viewer, from, until) => call("delivery_insights", { repoIds, viewer, from, until }), | |
| 567 | 570 | workspaceSignals: (args, viewer) => call("workspace_signals", { ...args, viewer }), | |
| 568 | 571 | pullsForRepos: (repoIds, viewer, limit) => call("pulls_for_repos", { repoIds, viewer, limit }), | |
| 34 | 34 | legacyNumber: number | null; | |
| 35 | 35 | /** A pull request or commit that closes it landed in this repository. */ | |
| 36 | 36 | landed: boolean; | |
| 37 | + | /** | |
| 38 | + | * The repository is deleted, and can still be restored until it is | |
| 39 | + | * purged: `path` is where it was. It has nothing to open or read. | |
| 40 | + | */ | |
| 41 | + | deleted?: boolean; | |
| 37 | 42 | }; | |
| 38 | 43 | ||
| 39 | 44 | /** What resolved a work item in one repository: a pull request merged, or a commit pushed. */ | |
| ⋯ | |||
| 871 | 876 | blocked_by?: number[]; | |
| 872 | 877 | }; | |
| 873 | 878 | ||
| 879 | + | /** How many `listWorkItems` and `listPulls` give at most in one call. */ | |
| 880 | + | export const LIST_PAGE = 100; | |
| 881 | + | ||
| 874 | 882 | /** The filters on `listWorkItems`. */ | |
| 875 | 883 | export type WorkItemFilter = { | |
| 876 | 884 | /** Only those linked to this repository (in any role, or in `role`). */ | |
| ⋯ | |||
| 881 | 889 | milestone?: number; | |
| 882 | 890 | /** Only those assigned to this username. */ | |
| 883 | 891 | assignee?: string; | |
| 892 | + | /** Only those closed at or after this time (RFC 3339), the most recently closed first instead of the newest. */ | |
| 893 | + | closedSince?: string; | |
| 894 | + | /** Only those numbered below this: the next page, after one whose lowest number this is. */ | |
| 895 | + | before?: number; | |
| 884 | 896 | }; | |
| 885 | 897 | ||
| 886 | 898 | export type OpenPullInput = { | |
| ⋯ | |||
| 931 | 943 | * meant to land. | |
| 932 | 944 | */ | |
| 933 | 945 | openWorkItem(actor: User, input: OpenWorkItemInput): Promise<Result<WorkItem>>; | |
| 934 | − | /** A workspace's work items the viewer may see, newest first. */ | |
| 946 | + | /** A workspace's work items the viewer may see, newest first, at most [`LIST_PAGE`] (`before` pages on). */ | |
| 935 | 947 | listWorkItems(workspace: string, viewer: Viewer, filter?: WorkItemFilter): Promise<Result<WorkItem[]>>; | |
| 936 | 948 | getWorkItem(workspace: string, number: number, viewer: Viewer): Promise<Result<WorkItemDetail>>; | |
| 937 | 949 | /** Its owner, or someone with the Triage role on it, may. */ | |
| ⋯ | |||
| 1071 | 1083 | * branch, one ready for review. | |
| 1072 | 1084 | */ | |
| 1073 | 1085 | openPull(actor: User, repo: RepoPath, input: OpenPullInput): Promise<Result<Pull>>; | |
| 1074 | − | /** Newest first. */ | |
| 1086 | + | /** Newest first, at most [`LIST_PAGE`] (`before` pages on). */ | |
| 1075 | 1087 | listPulls( | |
| 1076 | 1088 | repo: RepoPath, | |
| 1077 | 1089 | viewer: Viewer, | |
| 1078 | 1090 | state?: State, | |
| 1079 | − | filter?: { label?: string; milestone?: number; base?: string }, | |
| 1091 | + | filter?: { | |
| 1092 | + | label?: string; | |
| 1093 | + | milestone?: number; | |
| 1094 | + | base?: string; | |
| 1095 | + | /** Only those numbered below this: the next page, after one whose lowest number this is. */ | |
| 1096 | + | before?: number; | |
| 1097 | + | /** Only those merged at or after this time (RFC 3339). */ | |
| 1098 | + | mergedSince?: string; | |
| 1099 | + | }, | |
| 1080 | 1100 | ): Promise<Result<Pull[]>>; | |
| 1081 | 1101 | /** | |
| 1082 | 1102 | * Delivery for up to 100 repositories over UTC days `from` through | |
| 1149 | 1149 | label: None, | |
| 1150 | 1150 | milestone: None, | |
| 1151 | 1151 | base: None, | |
| 1152 | + | before: None, | |
| 1153 | + | merged_since: None, | |
| 1152 | 1154 | }, | |
| 1153 | 1155 | ) | |
| 1154 | 1156 | .await |
| 767 | 767 | label: None, | |
| 768 | 768 | milestone: None, | |
| 769 | 769 | assignee: None, | |
| 770 | + | closed_since: None, | |
| 771 | + | before: None, | |
| 770 | 772 | }, | |
| 771 | 773 | ) | |
| 772 | 774 | .await?; | |
| ⋯ | |||
| 788 | 790 | let pulls: Outcome<Vec<Pull>> = g1t_kit::call( | |
| 789 | 791 | &self.work, | |
| 790 | 792 | "list_pulls", | |
| 791 | − | &ListPullsArgs { repo: path.clone(), viewer: Some(actor), state: None, label: None, milestone: None, base: None }, | |
| 793 | + | &ListPullsArgs { repo: path.clone(), viewer: Some(actor), state: None, label: None, milestone: None, base: None, before: None, merged_since: None }, | |
| 792 | 794 | ) | |
| 793 | 795 | .await?; | |
| 794 | 796 | let Outcome::Ok(pulls) = pulls else { return Ok(()) }; | |
| 1 | + | -- When a repository was deleted, while it can still be restored: its work | |
| 2 | + | -- items' links say so, and the board reads nothing from it. Set on | |
| 3 | + | -- repo.deleted, cleared on repo.restored, and the row goes on repo.purged. | |
| 4 | + | ALTER TABLE repo_workspaces ADD COLUMN deleted_at TEXT; | |
| 5 | + | ||
| 6 | + | -- The job that marks the repositories deleted before this column was | |
| 7 | + | -- there (items/migration.rs `mark_deleted_repos`). It runs once the jobs | |
| 8 | + | -- before it are done, and again changes nothing. | |
| 9 | + | INSERT INTO work_jobs (name) VALUES ('deleted_marks'); |
| 445 | 445 | fn a_repository_already_linked_is_not_suggested() { | |
| 446 | 446 | let candidates = vec![("rep_web".to_owned(), "acme/web".to_owned())]; | |
| 447 | 447 | for role in LinkRole::ALL { | |
| 448 | − | let linked = ItemRepo { repo_id: "rep_web".into(), path: "acme/web".into(), role, legacy_number: None, landed: false }; | |
| 448 | + | let linked = ItemRepo { repo_id: "rep_web".into(), path: "acme/web".into(), role, legacy_number: None, landed: false, deleted: false }; | |
| 449 | 449 | let item = item("Fix web", "", &[], vec![linked]); | |
| 450 | 450 | assert!(suggest_targets(&item, &candidates, &[], &[]).is_empty(), "{role:?}"); | |
| 451 | 451 | } |
| 15 | 15 | use crate::{UNVERIFIED, Work}; | |
| 16 | 16 | ||
| 17 | 17 | /// Records where repository `?1` is: workspace `?2`, path `?3`, and | |
| 18 | − | /// whether it is private (`?4`). | |
| 18 | + | /// whether it is private (`?4`). It is there, so not deleted. | |
| 19 | 19 | pub(crate) const REMEMBER_REPO: &str = "INSERT INTO repo_workspaces (repo_id, workspace, path, private) | |
| 20 | 20 | VALUES (?1, ?2, ?3, ?4) | |
| 21 | − | ON CONFLICT (repo_id) DO UPDATE SET workspace = excluded.workspace, path = excluded.path, private = excluded.private"; | |
| 21 | + | ON CONFLICT (repo_id) DO UPDATE SET workspace = excluded.workspace, path = excluded.path, private = excluded.private, | |
| 22 | + | deleted_at = NULL"; | |
| 23 | + | ||
| 24 | + | /// `repo.deleted` (`?1` its id, at `?2`): its links say it is deleted | |
| 25 | + | /// until `repo.restored` records it again ([`REMEMBER_REPO`]) or | |
| 26 | + | /// `repo.purged` drops its row. | |
| 27 | + | pub(crate) const MARK_DELETED: &str = "UPDATE repo_workspaces SET deleted_at = COALESCE(deleted_at, ?2) WHERE repo_id = ?1"; | |
| 22 | 28 | ||
| 23 | 29 | /// Linking a work item (`?1`) that a change in repository `?2` is part of, | |
| 24 | 30 | /// at `?3`, by `?4`, unless it is linked already. |
| 28 | 28 | use g1t_kit::now_ms; | |
| 29 | 29 | use serde::Deserialize; | |
| 30 | 30 | use worker::Result; | |
| 31 | + | use worker::wasm_bindgen::JsValue; | |
| 31 | 32 | ||
| 32 | 33 | use crate::Work; | |
| 33 | 34 | use crate::optional; | |
| ⋯ | |||
| 173 | 174 | /// labels and old numbers unmerged. | |
| 174 | 175 | pub(crate) const DELETED_REPOS_JOB: &str = "deleted_repos"; | |
| 175 | 176 | ||
| 177 | + | /// The job after that ([`Work::mark_deleted_repos`]): marks each | |
| 178 | + | /// repository deleted but not purged as deleted in `repo_workspaces` | |
| 179 | + | /// ([`super::links::MARK_DELETED`]), for those deleted before that was | |
| 180 | + | /// kept. Run again, it changes nothing. | |
| 181 | + | pub(crate) const DELETED_MARKS_JOB: &str = "deleted_marks"; | |
| 182 | + | ||
| 176 | 183 | /// Every label a workspace's work items or pull requests carry that the | |
| 177 | 184 | /// workspace does not have, as a label (gray, no description) made at | |
| 178 | 185 | /// `?1`: one a repository had that was never merged. Run again, it adds | |
| ⋯ | |||
| 298 | 305 | return Ok(WorkMigration { done: true, ..WorkMigration::default() }); | |
| 299 | 306 | }; | |
| 300 | 307 | if job.done_at.is_some() { | |
| 301 | − | return self.migrate_deleted_repos().await; | |
| 308 | + | let deleted = self.migrate_deleted_repos().await?; | |
| 309 | + | if !deleted.done { | |
| 310 | + | return Ok(deleted); | |
| 311 | + | } | |
| 312 | + | return self.mark_deleted_repos().await; | |
| 302 | 313 | } | |
| 303 | 314 | let mut cursor = job.cursor; | |
| 304 | 315 | let mut recorded = 0; | |
| ⋯ | |||
| 347 | 358 | .map(|repo| (repo.id.clone(), repo.namespace.clone(), repo.name.clone(), repo.is_private)) | |
| 348 | 359 | .collect(); | |
| 349 | 360 | recorded += self.record_repos(&repos).await?; | |
| 361 | + | // Recording one says it is there; these are deleted. | |
| 362 | + | self.mark_deleted(&page).await?; | |
| 350 | 363 | match page.next { | |
| 351 | 364 | Some(next) => { | |
| 352 | 365 | self.db | |
| ⋯ | |||
| 370 | 383 | Ok(WorkMigration { done: false, repos: recorded, ..WorkMigration::default() }) | |
| 371 | 384 | } | |
| 372 | 385 | ||
| 386 | + | /// The job after [`Self::migrate_deleted_repos`] ([`DELETED_MARKS_JOB`]): | |
| 387 | + | /// marks each repository deleted but not purged as deleted, a few | |
| 388 | + | /// pages a run. Done once; run again, it changes nothing. | |
| 389 | + | async fn mark_deleted_repos(&self) -> Result<WorkMigration> { | |
| 390 | + | let job = self | |
| 391 | + | .db | |
| 392 | + | .prepare("SELECT cursor, done_at FROM work_jobs WHERE name = ?") | |
| 393 | + | .bind(&[DELETED_MARKS_JOB.into()])? | |
| 394 | + | .first::<JobRow>(None) | |
| 395 | + | .await?; | |
| 396 | + | let Some(job) = job.filter(|job| job.done_at.is_none()) else { | |
| 397 | + | return Ok(WorkMigration { done: true, ..WorkMigration::default() }); | |
| 398 | + | }; | |
| 399 | + | let mut cursor = job.cursor; | |
| 400 | + | let mut marked = 0; | |
| 401 | + | for _ in 0..PAGES_PER_RUN { | |
| 402 | + | let page: DeletedRepoPage = | |
| 403 | + | g1t_kit::call(&self.repos, "deleted_repos", &AllIdsArgs { after: cursor.clone(), limit: PAGE }).await?; | |
| 404 | + | marked += self.mark_deleted(&page).await?; | |
| 405 | + | let done = page.next.is_none(); | |
| 406 | + | self.db | |
| 407 | + | .prepare("UPDATE work_jobs SET cursor = ?, done_at = ? WHERE name = ? AND done_at IS NULL") | |
| 408 | + | .bind(&[ | |
| 409 | + | optional(&page.next), | |
| 410 | + | if done { rfc3339(now_ms()).into() } else { JsValue::NULL }, | |
| 411 | + | DELETED_MARKS_JOB.into(), | |
| 412 | + | ])? | |
| 413 | + | .run() | |
| 414 | + | .await?; | |
| 415 | + | if done { | |
| 416 | + | return Ok(WorkMigration { done: true, repos: marked, ..WorkMigration::default() }); | |
| 417 | + | } | |
| 418 | + | cursor = page.next; | |
| 419 | + | } | |
| 420 | + | Ok(WorkMigration { done: false, repos: marked, ..WorkMigration::default() }) | |
| 421 | + | } | |
| 422 | + | ||
| 423 | + | /// Marks a page of deleted repositories as deleted where they are recorded. | |
| 424 | + | async fn mark_deleted(&self, page: &DeletedRepoPage) -> Result<u32> { | |
| 425 | + | if page.repos.is_empty() { | |
| 426 | + | return Ok(0); | |
| 427 | + | } | |
| 428 | + | let now = rfc3339(now_ms()); | |
| 429 | + | let statements = page | |
| 430 | + | .repos | |
| 431 | + | .iter() | |
| 432 | + | .map(|repo| self.db.prepare(super::links::MARK_DELETED).bind(&[repo.id.as_str().into(), now.as_str().into()])) | |
| 433 | + | .collect::<Result<Vec<_>>>()?; | |
| 434 | + | self.db.batch(statements).await?; | |
| 435 | + | Ok(page.repos.len() as u32) | |
| 436 | + | } | |
| 437 | + | ||
| 373 | 438 | /// Records repositories (id, workspace, name, private) where they are. | |
| 374 | 439 | async fn record_repos(&self, repos: &[(String, String, String, bool)]) -> Result<u32> { | |
| 375 | 440 | if repos.is_empty() { | |
| 62 | 62 | mod tests; | |
| 63 | 63 | ||
| 64 | 64 | pub(crate) use labels::Labeled; | |
| 65 | + | pub(crate) use links::MARK_DELETED; | |
| 65 | 66 | pub(crate) use landing::{Landing, PastLanding}; | |
| 66 | 67 | pub(crate) use migration::{DELETED, RENAMED, TRANSFERRED}; | |
| 67 | 68 | pub(crate) use reading::{held_rows, select}; |
| 16 | 16 | /// What `list_work_items` reads: `?1` the workspace, then the filters, and | |
| 17 | 17 | /// `?8` whether the viewer is a member (else only work items filed in a | |
| 18 | 18 | /// public repository or one in `?9`, the repositories they were given a | |
| 19 | − | /// role on). | |
| 19 | + | /// role on), at most `?10`. With `?11`, only those closed since then, the | |
| 20 | + | /// most recently closed first; with `?12`, only those numbered below it. | |
| 20 | 21 | const LIST_SQL: &str = "SELECT {ITEM_COLUMNS} FROM work_items | |
| 21 | 22 | WHERE workspace = ?1 AND (?2 IS NULL OR state = ?2) | |
| 22 | 23 | AND (?3 IS NULL OR EXISTS (SELECT 1 FROM json_each(work_items.labels) WHERE json_each.value = ?3)) | |
| ⋯ | |||
| 27 | 28 | AND (?8 = 1 OR EXISTS (SELECT 1 FROM work_item_repos o JOIN repo_workspaces rw ON rw.repo_id = o.repo_id | |
| 28 | 29 | WHERE o.item_id = work_items.id AND o.role = 'origin' | |
| 29 | 30 | AND (rw.private = 0 OR o.repo_id IN (SELECT value FROM json_each(?9))))) | |
| 30 | − | ORDER BY number DESC LIMIT ?10"; | |
| 31 | + | AND (?11 IS NULL OR closed_at >= ?11) | |
| 32 | + | AND (?12 IS NULL OR number < ?12) | |
| 33 | + | ORDER BY CASE WHEN ?11 IS NULL THEN NULL ELSE closed_at END DESC, number DESC LIMIT ?10"; | |
| 31 | 34 | ||
| 32 | 35 | /// The work items matching `condition` (on `work_items`), with everything | |
| 33 | 36 | /// they are read with. | |
| ⋯ | |||
| 216 | 219 | u32::from(member).into(), | |
| 217 | 220 | serde_json::to_string(&granted)?.into(), | |
| 218 | 221 | LIST_PAGE.into(), | |
| 222 | + | optional(&a.closed_since.map(|at| at.trim().to_owned()).filter(|at| !at.is_empty())), | |
| 223 | + | optional_number(a.before), | |
| 219 | 224 | ])?; | |
| 220 | 225 | let rows = self.timing.db(1, query.all()).await?.results::<ItemRow>()?; | |
| 221 | 226 | Ok(Outcome::Ok( | |
| ⋯ | |||
| 307 | 312 | ||
| 308 | 313 | #[cfg(test)] | |
| 309 | 314 | mod tests { | |
| 310 | − | use super::redact_paths; | |
| 315 | + | use rusqlite::{Connection, ToSql}; | |
| 316 | + | ||
| 317 | + | use super::super::links::{MARK_DELETED, REMEMBER_REPO}; | |
| 318 | + | use super::super::rows::ITEM_COLUMNS; | |
| 319 | + | use super::{LIST_SQL, redact_paths}; | |
| 320 | + | use crate::migrated::database; | |
| 321 | + | ||
| 322 | + | /// acme's work items 1 to 5, linked to acme/web; 3, 4 and 5 closed, | |
| 323 | + | /// in the order 4, 5, 3; 2 also to acme/hello. | |
| 324 | + | fn acme() -> Connection { | |
| 325 | + | let db = database(); | |
| 326 | + | db.execute_batch( | |
| 327 | + | "INSERT INTO repo_workspaces (repo_id, workspace, path, private) | |
| 328 | + | VALUES ('rep_web', 'acme', 'acme/web', 0), ('rep_hello', 'acme', 'acme/hello', 0); | |
| 329 | + | INSERT INTO work_items (id, workspace, number, title, body, labels, author_id, author_name, created_at, updated_at, | |
| 330 | + | state, closed_at) | |
| 331 | + | VALUES ('wki_1', 'acme', 1, 'One', '', '[]', 'usr_1', 'ana', '', '', 'open', NULL), | |
| 332 | + | ('wki_2', 'acme', 2, 'Two', '', '[]', 'usr_1', 'ana', '', '', 'open', NULL), | |
| 333 | + | ('wki_3', 'acme', 3, 'Three', '', '[]', 'usr_1', 'ana', '', '', 'closed', '2026-10-17T00:00:00Z'), | |
| 334 | + | ('wki_4', 'acme', 4, 'Four', '', '[]', 'usr_1', 'ana', '', '', 'closed', '2026-10-01T00:00:00Z'), | |
| 335 | + | ('wki_5', 'acme', 5, 'Five', '', '[]', 'usr_1', 'ana', '', '', 'closed', '2026-10-10T00:00:00Z'); | |
| 336 | + | INSERT INTO work_item_repos (item_id, repo_id, role, path, added_at) | |
| 337 | + | VALUES ('wki_1', 'rep_web', 'origin', 'acme/web', ''), ('wki_2', 'rep_hello', 'origin', 'acme/hello', ''), | |
| 338 | + | ('wki_2', 'rep_web', 'target', 'acme/web', ''), ('wki_3', 'rep_web', 'origin', 'acme/web', ''), | |
| 339 | + | ('wki_4', 'rep_web', 'origin', 'acme/web', ''), ('wki_5', 'rep_web', 'origin', 'acme/web', '');", | |
| 340 | + | ) | |
| 341 | + | .unwrap(); | |
| 342 | + | db | |
| 343 | + | } | |
| 344 | + | ||
| 345 | + | /// The numbers `list_work_items` reads for acme as a member, in its order. | |
| 346 | + | fn listed(db: &Connection, state: Option<&str>, limit: u32, closed_since: Option<&str>, before: Option<u32>) -> Vec<u32> { | |
| 347 | + | let sql = LIST_SQL.replace("{ITEM_COLUMNS}", ITEM_COLUMNS); | |
| 348 | + | let values: [&dyn ToSql; 12] = [&"acme", &state, &None::<String>, &None::<u32>, &None::<String>, &None::<String>, &None::<String>, &1, &"[]", &limit, &closed_since, &before]; | |
| 349 | + | let mut statement = db.prepare(&sql).unwrap(); | |
| 350 | + | let numbers = statement.query_map(&values[..], |row| row.get::<_, u32>("number")).unwrap(); | |
| 351 | + | numbers.map(Result::unwrap).collect() | |
| 352 | + | } | |
| 353 | + | ||
| 354 | + | #[test] | |
| 355 | + | fn work_items_page_on_by_number_and_closed_ones_come_by_when_they_closed() { | |
| 356 | + | let db = acme(); | |
| 357 | + | assert_eq!(listed(&db, None, 100, None, None), [5, 4, 3, 2, 1]); | |
| 358 | + | // A page, then the next from below its lowest number. | |
| 359 | + | assert_eq!(listed(&db, None, 2, None, None), [5, 4]); | |
| 360 | + | assert_eq!(listed(&db, None, 2, None, Some(4)), [3, 2]); | |
| 361 | + | assert_eq!(listed(&db, None, 2, None, Some(2)), [1]); | |
| 362 | + | // Closed since a time: the most recently closed first, older ones left out. | |
| 363 | + | assert_eq!(listed(&db, Some("closed"), 100, Some("2026-10-04T00:00:00Z"), None), [3, 5]); | |
| 364 | + | } | |
| 365 | + | ||
| 366 | + | /// Whether each of work item 2's links says its repository is deleted, by path. | |
| 367 | + | fn deleted_links(db: &Connection) -> Vec<(String, bool)> { | |
| 368 | + | let links: String = db | |
| 369 | + | .query_row(&format!("SELECT links FROM (SELECT {ITEM_COLUMNS} FROM work_items WHERE id = 'wki_2')"), [], |row| row.get(0)) | |
| 370 | + | .unwrap(); | |
| 371 | + | let mut links: Vec<(String, bool)> = serde_json::from_str::<Vec<serde_json::Value>>(&links) | |
| 372 | + | .unwrap() | |
| 373 | + | .into_iter() | |
| 374 | + | .map(|link| (link["path"].as_str().unwrap().to_owned(), link["deleted"].as_i64() == Some(1))) | |
| 375 | + | .collect(); | |
| 376 | + | links.sort(); | |
| 377 | + | links | |
| 378 | + | } | |
| 379 | + | ||
| 380 | + | #[test] | |
| 381 | + | fn a_deleted_repository_is_marked_on_its_links_until_it_is_restored() { | |
| 382 | + | let db = acme(); | |
| 383 | + | assert_eq!(deleted_links(&db), [("acme/hello".to_owned(), false), ("acme/web".to_owned(), false)]); | |
| 384 | + | // repo.deleted: its path is kept, and its links say it is deleted. | |
| 385 | + | db.execute(MARK_DELETED, rusqlite::params!["rep_hello", "2026-10-15T00:00:00Z"]).unwrap(); | |
| 386 | + | assert_eq!(deleted_links(&db), [("acme/hello".to_owned(), true), ("acme/web".to_owned(), false)]); | |
| 387 | + | // Marked again (the job for those deleted before), it keeps when. | |
| 388 | + | db.execute(MARK_DELETED, rusqlite::params!["rep_hello", "2026-10-18T00:00:00Z"]).unwrap(); | |
| 389 | + | let at: String = db.query_row("SELECT deleted_at FROM repo_workspaces WHERE repo_id = 'rep_hello'", [], |row| row.get(0)).unwrap(); | |
| 390 | + | assert_eq!(at, "2026-10-15T00:00:00Z"); | |
| 391 | + | // repo.restored records it where it is: no longer deleted. | |
| 392 | + | db.execute(REMEMBER_REPO, rusqlite::params!["rep_hello", "acme", "acme/hello", 0]).unwrap(); | |
| 393 | + | assert_eq!(deleted_links(&db), [("acme/hello".to_owned(), false), ("acme/web".to_owned(), false)]); | |
| 394 | + | // repo.purged drops its row: the link keeps its path, and is not marked. | |
| 395 | + | db.execute(MARK_DELETED, rusqlite::params!["rep_hello", "2026-10-18T00:00:00Z"]).unwrap(); | |
| 396 | + | db.execute("DELETE FROM repo_workspaces WHERE repo_id = ?1", ["rep_hello"]).unwrap(); | |
| 397 | + | assert_eq!(deleted_links(&db), [("acme/hello".to_owned(), false), ("acme/web".to_owned(), false)]); | |
| 398 | + | } | |
| 311 | 399 | ||
| 312 | 400 | #[test] | |
| 313 | 401 | fn notes_do_not_name_repositories_the_reader_may_not_read() { | |
| 26 | 26 | WHERE b.number IS NOT NULL ORDER BY j.key)) AS blocked_numbers, | |
| 27 | 27 | (SELECT json_group_array(json_object('repo_id', l.repo_id, 'path', COALESCE(rw.path, l.path, ''), 'role', l.role, | |
| 28 | 28 | 'legacy_number', l.legacy_number, 'landed', l.landed_at IS NOT NULL, 'workspace', rw.workspace, | |
| 29 | − | 'private', rw.private)) | |
| 29 | + | 'private', rw.private, 'deleted', rw.deleted_at IS NOT NULL)) | |
| 30 | 30 | FROM work_item_repos l LEFT JOIN repo_workspaces rw ON rw.repo_id = l.repo_id | |
| 31 | 31 | WHERE l.item_id = work_items.id) AS links"; | |
| 32 | 32 | ||
| ⋯ | |||
| 88 | 88 | workspace: Option<String>, | |
| 89 | 89 | #[serde(default)] | |
| 90 | 90 | private: Option<u8>, | |
| 91 | + | /// The repository is deleted, not yet purged (`repo_workspaces.deleted_at`). | |
| 92 | + | #[serde(default)] | |
| 93 | + | deleted: u8, | |
| 91 | 94 | } | |
| 92 | 95 | ||
| 93 | 96 | /// A resolution as stored. | |
| ⋯ | |||
| 227 | 230 | role: link.role, | |
| 228 | 231 | legacy_number: link.legacy_number, | |
| 229 | 232 | landed: link.landed != 0, | |
| 233 | + | deleted: link.deleted != 0, | |
| 230 | 234 | }) | |
| 231 | 235 | .collect(); | |
| 232 | 236 | sort_repos(&mut repos); | |
| 309 | 309 | .unwrap(); | |
| 310 | 310 | migrate_from(&db, "0052_work_floors.sql"); | |
| 311 | 311 | assert_eq!(numbers(&db, "SELECT workspace, floor FROM work_counters"), [("acme".to_owned(), Some(9))]); | |
| 312 | − | assert_eq!(strings(&db, "SELECT name FROM work_jobs WHERE done_at IS NULL"), ["deleted_repos"]); | |
| 312 | + | // The jobs after it wait their turn: the one 0052 adds, and 0055's. | |
| 313 | + | assert_eq!(strings(&db, "SELECT name FROM work_jobs WHERE done_at IS NULL ORDER BY name"), ["deleted_marks", "deleted_repos"]); | |
| 313 | 314 | run(&db, super::migration::RAISE_FLOORS, &[&"2026-10-18T12:00:00Z"]); | |
| 314 | 315 | assert_eq!(numbers(&db, "SELECT workspace, floor FROM work_counters"), [("acme".to_owned(), Some(9))]); | |
| 315 | 316 | } |
| 1137 | 1137 | .map(|label| label.split_whitespace().collect::<Vec<_>>().join(" ").to_lowercase()) | |
| 1138 | 1138 | .filter(|label| !label.is_empty()); | |
| 1139 | 1139 | let milestone = a.milestone; | |
| 1140 | + | let before = a.before; | |
| 1141 | + | let merged_since = a.merged_since.map(|at| at.trim().to_owned()).filter(|at| !at.is_empty()); | |
| 1140 | 1142 | let read = |repo_id: String| { | |
| 1141 | 1143 | let label = label.clone(); | |
| 1144 | + | let merged_since = merged_since.clone(); | |
| 1142 | 1145 | async move { | |
| 1143 | 1146 | let query = self | |
| 1144 | 1147 | .db | |
| 1145 | 1148 | .prepare(format!( | |
| 1146 | − | "SELECT {PULL_COLUMNS} FROM pulls WHERE repo_id = ? {filter} | |
| 1147 | − | AND (? IS NULL OR EXISTS | |
| 1148 | − | (SELECT 1 FROM json_each(pulls.labels) WHERE json_each.value = ?)) | |
| 1149 | − | AND (? IS NULL OR milestone = ?) | |
| 1150 | − | ORDER BY number DESC LIMIT ?" | |
| 1149 | + | "SELECT {PULL_COLUMNS} FROM pulls WHERE repo_id = ?1 {filter} | |
| 1150 | + | AND (?2 IS NULL OR EXISTS | |
| 1151 | + | (SELECT 1 FROM json_each(pulls.labels) WHERE json_each.value = ?2)) | |
| 1152 | + | AND (?3 IS NULL OR milestone = ?3) | |
| 1153 | + | AND (?5 IS NULL OR number < ?5) | |
| 1154 | + | AND (?6 IS NULL OR (status = 'merged' AND merged_at >= ?6)) | |
| 1155 | + | ORDER BY number DESC LIMIT ?4" | |
| 1151 | 1156 | )) | |
| 1152 | 1157 | .bind(&[ | |
| 1153 | 1158 | repo_id.into(), | |
| 1154 | 1159 | optional(&label), | |
| 1155 | − | optional(&label), | |
| 1156 | − | optional_number(milestone), | |
| 1157 | 1160 | optional_number(milestone), | |
| 1158 | 1161 | LIST_PAGE.into(), | |
| 1162 | + | optional_number(before), | |
| 1163 | + | optional(&merged_since), | |
| 1159 | 1164 | ])?; | |
| 1160 | 1165 | self.timing.db(1, query.all()).await?.results::<PullRow>() | |
| 1161 | 1166 | } |
| 179 | 179 | } | |
| 180 | 180 | ||
| 181 | 181 | /// `repo.deleted` and `repo.archived`: its runs stop (the runner stops | |
| 182 | − | /// their sandboxes, see [`RunsInRepoArgs`]). Rows are kept, for a restore. | |
| 182 | + | /// their sandboxes, see [`RunsInRepoArgs`]). Rows are kept, for a | |
| 183 | + | /// restore; a deleted one is marked so in `repo_workspaces`. | |
| 183 | 184 | async fn stop_runs_in(&self, repo_id: &str) -> Result<()> { | |
| 184 | 185 | let now = rfc3339(now_ms()); | |
| 185 | 186 | self.db | |
| ⋯ | |||
| 198 | 199 | } | |
| 199 | 200 | if let Some((repo_id, _, _)) = stopping(event) { | |
| 200 | 201 | self.stop_runs_in(&repo_id).await?; | |
| 202 | + | // Its work items' links say it is deleted, until it is restored or purged. | |
| 203 | + | if event.kind == "repo.deleted" { | |
| 204 | + | let at = rfc3339(now_ms()); | |
| 205 | + | self.db | |
| 206 | + | .prepare(crate::items::MARK_DELETED) | |
| 207 | + | .bind(&[repo_id.as_str().into(), at.as_str().into()])? | |
| 208 | + | .run() | |
| 209 | + | .await?; | |
| 210 | + | } | |
| 201 | 211 | return Ok(true); | |
| 202 | 212 | } | |
| 203 | 213 | if event.kind != "branch.renamed" { | |
Comments
Sign in to comment.