Work items show each viewer only what they may see, and close only what the closer may
Access: - get_work and claims show blockers and dependents the viewer may see, their pull requests and claims in repositories they may read; sessions only in readable repositories, redacted as every session list is; coordination events only for readable repositories; suggestions only among readable repositories, never one already linked, none for a closed work item ("1 work item"). - blocked_by refuses a number the actor may not see, as one that is not there (unless it is blocked by it already). - A work item's events are filed under its origin, or none: never a target whose readers may not see it. - closed_by_commit lists only work items the viewer may see. - WorkItem.repos and resolutions, and g1t's notes naming repositories, leave out repositories the viewer may not read. - Workspace labels are made through membership of the workspace only. - resolve_ref in a repository the viewer may not read answers as for one that does not exist. Closing and references: - Closing keywords close only what the pusher, or the merger and the pull request's owner, may triage; Part of links only what they may see. - An old number of a repository moved in from another workspace names nothing of its new one; an unknown owner/name#N names nothing. - #N in a workspace names another repository's pull request only above the workspace's floor (work_counters.floor, migration 0052): legacy per-repository numbers collide. - A landing that leaves a blocker open is told with a key of its own and keeps it among what dependents wait on. - Closing with a commit that leaves targets says which. - A claim refuses a repository or session outside the item's workspace. Migration and numbering: - New pull request numbers skip a repository's old issue numbers, after a transfer or a restore; while the job runs they come after every number. - The job leaves what repositories it did not record (deleted ones) for later, merges labels whatever their case, rewrites milestones only for repositories that had them, and keeps each workspace's floor. - The deleted_repos job (migration 0052) records repositories deleted but restorable, places what they kept and gives the workspace the labels its work items carry; repo.restored does the same for one.
- G1T-Session:
- ses_01m56vxcstesp8b57jt5edge6y
| 1 | + | -- Each workspace's floor: the highest number any of its repositories used | |
| 2 | + | -- on its own before work items moved to the workspace (a pull request | |
| 3 | + | -- opened by then, or an old issue's number). Numbers above it are the | |
| 4 | + | -- workspace's alone; at or below it, `#N` read in the workspace may be any | |
| 5 | + | -- of its repositories' old pull requests, so it names none of them from | |
| 6 | + | -- elsewhere (items/refs.rs). The job sets it from here on | |
| 7 | + | -- (items/migration.rs `place_sql`); this sets it for workspaces that moved | |
| 8 | + | -- before, as `RAISE_FLOORS` does. | |
| 9 | + | ALTER TABLE work_counters ADD COLUMN floor INTEGER NOT NULL DEFAULT 0; | |
| 10 | + | ||
| 11 | + | UPDATE work_counters SET floor = MAX(floor, | |
| 12 | + | COALESCE((SELECT MAX(p.number) FROM pulls p JOIN repo_workspaces rw ON rw.repo_id = p.repo_id | |
| 13 | + | WHERE rw.workspace = work_counters.workspace | |
| 14 | + | AND p.created_at <= COALESCE((SELECT done_at FROM work_jobs WHERE name = 'work_items'), '')), 0), | |
| 15 | + | COALESCE((SELECT MAX(l.legacy_number) FROM work_item_repos l JOIN repo_workspaces rw ON rw.repo_id = l.repo_id | |
| 16 | + | WHERE rw.workspace = work_counters.workspace), 0)) | |
| 17 | + | WHERE true; | |
| 18 | + | ||
| 19 | + | -- The job that records the repositories the first left out: deleted, but | |
| 20 | + | -- still restorable, when it ran. Their work items were placed in their | |
| 21 | + | -- workspace with no path, as private, with their labels and old numbers | |
| 22 | + | -- unmerged (items/migration.rs `migrate_deleted_repos`). It runs once the | |
| 23 | + | -- first is done, and again changes nothing. | |
| 24 | + | INSERT INTO work_jobs (name) VALUES ('deleted_repos'); |
| 3 | 3 | //! landed when it merges (`items::land`, landing.rs). | |
| 4 | 4 | ||
| 5 | 5 | use g1t_contracts::repos::Repo; | |
| 6 | + | use g1t_contracts::User; | |
| 6 | 7 | use g1t_contracts::work::*; | |
| 7 | 8 | use worker::Result; | |
| 8 | 9 | ||
| ⋯ | |||
| 15 | 16 | /// description say now: none closed where it does not merge into the | |
| 16 | 17 | /// default branch, where the repository does not close work items that | |
| 17 | 18 | /// way, or where it is kept open (`keep_open`), when its own work item | |
| 18 | − | /// is only something it is part of. | |
| 19 | − | pub(crate) async fn items_to_close(&self, pull: &Pull, repo: &Repo, settings: &RepoSettings, keep_open: bool) -> Result<NamedItems> { | |
| 19 | + | /// is only something it is part of. Only what each of `who` (who merges | |
| 20 | + | /// it, and whoever it is for) may close, or see, counts. | |
| 21 | + | pub(crate) async fn items_to_close( | |
| 22 | + | &self, | |
| 23 | + | pull: &Pull, | |
| 24 | + | repo: &Repo, | |
| 25 | + | settings: &RepoSettings, | |
| 26 | + | keep_open: bool, | |
| 27 | + | who: &[&User], | |
| 28 | + | ) -> Result<NamedItems> { | |
| 20 | 29 | let own = self.pull_item_id(&pull.id).await?; | |
| 21 | 30 | let text = format!("{}\n{}", pull.title, pull.body.as_deref().unwrap_or_default()); | |
| 22 | − | let mut named = items::named(self, repo, own.as_deref(), &text).await?; | |
| 31 | + | let mut named = items::named(self, repo, own.as_deref(), &text, who).await?; | |
| 23 | 32 | if keep_open || !settings.auto_close_items || !pull.targets_default(&repo.default_branch) { | |
| 24 | 33 | let closed = std::mem::take(&mut named.closes); | |
| 25 | 34 | let mut parts = if keep_open { closed } else { Vec::new() }; | |
| ⋯ | |||
| 29 | 38 | Ok(named) | |
| 30 | 39 | } | |
| 31 | 40 | ||
| 41 | + | /// Whoever a pull request is for, as identity knows them: its author, | |
| 42 | + | /// or the person g1t's agent opened it for. None when not known. | |
| 43 | + | pub(crate) async fn pull_owner(&self, pull: &Pull) -> Result<Option<User>> { | |
| 44 | + | let id = pull.owner().id.clone(); | |
| 45 | + | g1t_kit::call(&self.identity, "user_by_id", &g1t_contracts::identity::UserIdArgs { id }).await | |
| 46 | + | } | |
| 47 | + | ||
| 32 | 48 | /// The id of a pull request's own work item, if it has one. | |
| 33 | 49 | pub(crate) async fn pull_item_id(&self, pull_id: &str) -> Result<Option<String>> { | |
| 34 | 50 | Ok(self | |
| 68 | 68 | repos | |
| 69 | 69 | } | |
| 70 | 70 | ||
| 71 | + | /// Why a claim on a work item of `workspace` may not be made from the | |
| 72 | + | /// repository in workspace `repo` (if one is named) or from a session | |
| 73 | + | /// whose repository is in workspace `session` (if one is named; `None` | |
| 74 | + | /// inside when the repository is not known): either in another | |
| 75 | + | /// workspace. `None` when it may. | |
| 76 | + | fn outside_workspace(workspace: &str, repo: Option<&str>, session: Option<Option<&str>>) -> Option<String> { | |
| 77 | + | let other = |slug: &str| !slug.eq_ignore_ascii_case(workspace); | |
| 78 | + | if repo.is_some_and(other) { | |
| 79 | + | return Some(format!("That repository is not in {workspace}, the work item's workspace.")); | |
| 80 | + | } | |
| 81 | + | if session.is_some_and(|slug| slug.is_none_or(other)) { | |
| 82 | + | return Some(format!("That session is not in a repository of {workspace}, the work item's workspace.")); | |
| 83 | + | } | |
| 84 | + | None | |
| 85 | + | } | |
| 86 | + | ||
| 87 | + | #[cfg(test)] | |
| 88 | + | mod tests { | |
| 89 | + | use super::outside_workspace; | |
| 90 | + | ||
| 91 | + | #[test] | |
| 92 | + | fn a_claim_is_made_only_from_the_work_items_workspace() { | |
| 93 | + | assert_eq!(outside_workspace("acme", None, None), None); | |
| 94 | + | assert_eq!(outside_workspace("acme", Some("Acme"), Some(Some("acme"))), None); | |
| 95 | + | assert!(outside_workspace("acme", Some("globex"), None).is_some()); | |
| 96 | + | assert!(outside_workspace("acme", None, Some(Some("globex"))).is_some()); | |
| 97 | + | assert!(outside_workspace("acme", None, Some(None)).is_some()); | |
| 98 | + | } | |
| 99 | + | } | |
| 100 | + | ||
| 71 | 101 | impl Work { | |
| 72 | 102 | /// The caller's active claim on a work item, if they hold one. | |
| 73 | 103 | async fn own_claim(&self, item_id: &str, actor: &User, session: Option<&str>) -> Result<Option<Claim>> { | |
| ⋯ | |||
| 211 | 241 | if session.as_ref().is_some_and(|session| session.state == SessionState::Ended) { | |
| 212 | 242 | return Ok(Outcome::fail(FailureCode::Invalid, "That session has ended. Start a new one.")); | |
| 213 | 243 | } | |
| 244 | + | // Only a repository, or a session in one, of the work item's own | |
| 245 | + | // workspace is tied to it. | |
| 246 | + | let session_workspace = match &session { | |
| 247 | + | Some(session) => Some(items::repo_workspace(self, &session.repo_id).await?), | |
| 248 | + | None => None, | |
| 249 | + | }; | |
| 250 | + | if let Some(refusal) = outside_workspace( | |
| 251 | + | &held.item.workspace, | |
| 252 | + | repo.as_ref().map(|repo| repo.namespace.as_str()), | |
| 253 | + | session_workspace.as_ref().map(Option::as_deref), | |
| 254 | + | ) { | |
| 255 | + | return Ok(Outcome::fail(FailureCode::Invalid, refusal)); | |
| 256 | + | } | |
| 214 | 257 | let repo_id = repo.as_ref().map(|repo| repo.id.clone()).or_else(|| session.as_ref().map(|session| session.repo_id.clone())); | |
| 215 | 258 | let minutes = claim_minutes(a.ttl_minutes); | |
| 216 | 259 | let note = tidy(a.note.as_deref(), MAX_NOTE_CHARS); | |
| ⋯ | |||
| 222 | 265 | let (mine, others): (Vec<Claim>, Vec<Claim>) = standing | |
| 223 | 266 | .into_iter() | |
| 224 | 267 | .partition(|claim| claim_is_mine(&claim.actor.id, claim.session_id.as_deref(), &a.actor.id, session_id.as_deref())); | |
| 225 | − | let blocked_by = self.blockers(&held.item.workspace, &held.item.blocked_by).await?; | |
| 268 | + | // The others, as far as the actor may read where they are. | |
| 269 | + | let claimed_in: Vec<String> = others.iter().filter_map(|claim| claim.repo_id.clone()).collect(); | |
| 270 | + | let readable = items::readable_repo_ids(self, Some(&a.actor), &claimed_in).await?; | |
| 271 | + | let others: Vec<Claim> = | |
| 272 | + | others.into_iter().filter(|claim| claim.repo_id.as_ref().is_none_or(|id| readable.contains(id))).collect(); | |
| 273 | + | let blocked_by = self.blockers(Some(&a.actor), &held.item.workspace, &held.item.blocked_by).await?; | |
| 226 | 274 | let repos = claim_repos(&held, repo_id.as_deref()); | |
| 227 | 275 | // Claiming what you hold renews it. | |
| 228 | 276 | if let Some(held_claim) = mine.into_iter().next() { | |
| 76 | 76 | landed: &[PastLanding], | |
| 77 | 77 | touched: &[TouchedRow], | |
| 78 | 78 | ) -> Vec<SuggestedTarget> { | |
| 79 | + | // A closed work item needs nowhere else to land. | |
| 80 | + | if item.state == State::Closed { | |
| 81 | + | return Vec::new(); | |
| 82 | + | } | |
| 79 | 83 | let title_words = words(&item.title); | |
| 80 | 84 | let text = format!("{}\n{}", item.title, item.body).to_lowercase(); | |
| 81 | 85 | let mut suggested: Vec<SuggestedTarget> = Vec::new(); | |
| 86 | + | let counted = |n: u32| if n == 1 { "1 work item".to_owned() } else { format!("{n} work items") }; | |
| 82 | 87 | for (repo_id, path) in candidates { | |
| 83 | − | if item.repos.iter().any(|repo| repo.repo_id == *repo_id && repo.role == LinkRole::Target) { | |
| 88 | + | // Linked already, as its origin, a target or touched. | |
| 89 | + | if item.repos.iter().any(|repo| repo.repo_id == *repo_id) { | |
| 84 | 90 | continue; | |
| 85 | 91 | } | |
| 86 | 92 | let mut score = 0; | |
| ⋯ | |||
| 95 | 101 | .count() as u32; | |
| 96 | 102 | if labelled > 0 { | |
| 97 | 103 | score += 3 * labelled; | |
| 98 | − | why.push(format!("{labelled} work items with the same labels landed there")); | |
| 104 | + | why.push(format!("{} with the same labels landed there", counted(labelled))); | |
| 99 | 105 | } | |
| 100 | 106 | let alike = landings.iter().filter(|landing| words(&landing.title).iter().any(|word| title_words.contains(word))).count() as u32; | |
| 101 | 107 | if alike > 0 { | |
| 102 | 108 | score += 2 * alike; | |
| 103 | − | why.push(format!("{alike} work items with like titles landed there")); | |
| 109 | + | why.push(format!("{} with like titles landed there", counted(alike))); | |
| 104 | 110 | } | |
| 105 | 111 | let name = path.rsplit('/').next().unwrap_or(path).to_lowercase(); | |
| 106 | 112 | let names_it = text.split(|c: char| !(c.is_alphanumeric() || c == '-' || c == '_' || c == '.')).any(|word| word == name); | |
| ⋯ | |||
| 125 | 131 | suggested | |
| 126 | 132 | } | |
| 127 | 133 | ||
| 134 | + | /// Who is on a work item of `workspace`, as `viewer` is shown it: the | |
| 135 | + | /// claims and sessions in repositories they may read (`readable`), each | |
| 136 | + | /// session as `shown` lets them see it. | |
| 137 | + | fn shown_people(claims: Vec<Claim>, mut sessions: Vec<Session>, readable: &[String], viewer: &Viewer, workspace: &str) -> (Vec<Claim>, Vec<Session>) { | |
| 138 | + | let may_read = |repo_id: &str| readable.iter().any(|id| id == repo_id); | |
| 139 | + | let claims = claims.into_iter().filter(|claim| claim.repo_id.as_deref().is_none_or(may_read)).collect(); | |
| 140 | + | sessions.retain(|session| may_read(&session.repo_id)); | |
| 141 | + | super::shown::sessions(&mut sessions, viewer, workspace); | |
| 142 | + | (claims, sessions) | |
| 143 | + | } | |
| 144 | + | ||
| 128 | 145 | impl Work { | |
| 129 | 146 | pub(crate) async fn get_work(&self, a: WorkItemViewArgs) -> Result<Outcome<WorkContext>> { | |
| 130 | 147 | let held = unwrap_or_fail!(items::for_viewer(self, &a.viewer, &a.workspace, a.number, Capability::Read).await?); | |
| ⋯ | |||
| 154 | 171 | let pulls: Vec<Pull> = pulls.into_iter().filter(|pull| readable.iter().any(|repo| repo.id == pull.repo_id)).collect(); | |
| 155 | 172 | let mut numbers = vec![held.item.number]; | |
| 156 | 173 | numbers.extend(pulls.iter().map(|pull| pull.number)); | |
| 174 | + | let viewer = a.viewer.as_ref(); | |
| 175 | + | let may_read = |repo_id: &str| readable.iter().any(|repo| repo.id == repo_id); | |
| 157 | 176 | ||
| 158 | 177 | let people = try_join3( | |
| 159 | 178 | self.active_claims(&held.item.id), | |
| 160 | 179 | self.work_sessions(&held, &pulls), | |
| 161 | − | self.suggestions(&held), | |
| 180 | + | self.suggestions(&held, viewer), | |
| 162 | 181 | ); | |
| 163 | − | let dependencies = try_join(self.blockers(&held.item.workspace, &held.item.blocked_by), items::blocked_items(self, &held)); | |
| 164 | − | let linked = held.repo_ids(); | |
| 182 | + | let dependencies = try_join(self.blockers(viewer, &held.item.workspace, &held.item.blocked_by), items::blocked_items(self, &held, viewer)); | |
| 183 | + | // Only the repositories the viewer may read: the events service | |
| 184 | + | // takes the ids it is given as checked. | |
| 185 | + | let linked: Vec<String> = held.repo_ids().into_iter().filter(|id| may_read(id)).collect(); | |
| 165 | 186 | let history = try_join( | |
| 166 | 187 | self.coordination_events(&linked, &numbers, COORDINATION_EVENTS.to_vec(), EVENTS), | |
| 167 | 188 | self.decisions_on(&held, &a.viewer), | |
| ⋯ | |||
| 173 | 194 | })); | |
| 174 | 195 | let (((claims, sessions, suggested_targets), (blocked_by, blocks)), ((recent, decisions), repos)) = | |
| 175 | 196 | try_join(try_join(people, dependencies), try_join(history, repos)).await?; | |
| 197 | + | let readable_ids: Vec<String> = readable.iter().map(|repo| repo.id.clone()).collect(); | |
| 198 | + | let (claims, sessions) = shown_people(claims, sessions, &readable_ids, &a.viewer, &held.item.workspace); | |
| 199 | + | let item = held.shown(viewer); | |
| 176 | 200 | Ok(Outcome::Ok(WorkContext { | |
| 177 | − | definition_of_done: definition_of_done(&held.item.body), | |
| 178 | − | missing: held.item.missing(), | |
| 179 | − | item: held.item, | |
| 201 | + | definition_of_done: definition_of_done(&item.body), | |
| 202 | + | missing: item.missing(), | |
| 203 | + | item, | |
| 180 | 204 | repos, | |
| 181 | 205 | suggested_targets, | |
| 182 | 206 | claims, | |
| ⋯ | |||
| 197 | 221 | Ok(WorkRepo { link, required_checks: settings.required_checks, pulls }) | |
| 198 | 222 | } | |
| 199 | 223 | ||
| 200 | − | /// Where else a work item may belong ([`suggest_targets`]). | |
| 201 | − | async fn suggestions(&self, held: &Held) -> Result<Vec<SuggestedTarget>> { | |
| 224 | + | /// Where else a work item may belong ([`suggest_targets`]), among the | |
| 225 | + | /// repositories `viewer` may read. | |
| 226 | + | async fn suggestions(&self, held: &Held, viewer: Option<&g1t_contracts::User>) -> Result<Vec<SuggestedTarget>> { | |
| 202 | 227 | #[derive(Deserialize)] | |
| 203 | 228 | struct Candidate { | |
| 204 | 229 | repo_id: String, | |
| 205 | 230 | path: String, | |
| 231 | + | workspace: String, | |
| 232 | + | private: u8, | |
| 233 | + | } | |
| 234 | + | if held.item.state == State::Closed { | |
| 235 | + | return Ok(Vec::new()); | |
| 206 | 236 | } | |
| 207 | 237 | let results = self | |
| 208 | 238 | .db | |
| 209 | 239 | .batch(vec![ | |
| 210 | 240 | self.db | |
| 211 | − | .prepare("SELECT repo_id, path FROM repo_workspaces WHERE workspace = ? ORDER BY path LIMIT 500") | |
| 241 | + | .prepare("SELECT repo_id, path, workspace, private FROM repo_workspaces WHERE workspace = ? ORDER BY path LIMIT 500") | |
| 212 | 242 | .bind(&[held.item.workspace.as_str().into()])?, | |
| 213 | 243 | self.db.prepare(TOUCHED_FOR).bind(&[held.item.id.as_str().into()])?, | |
| 214 | 244 | ]) | |
| 215 | 245 | .await?; | |
| 216 | 246 | let mut results = results.into_iter(); | |
| 217 | 247 | let candidates: Vec<(String, String)> = match results.next() { | |
| 218 | − | Some(result) => result.results::<Candidate>()?.into_iter().map(|row| (row.repo_id, row.path)).collect(), | |
| 248 | + | Some(result) => result | |
| 249 | + | .results::<Candidate>()? | |
| 250 | + | .into_iter() | |
| 251 | + | .filter(|row| items::repo_readable(viewer, &row.repo_id, Some(&row.workspace), Some(row.private))) | |
| 252 | + | .map(|row| (row.repo_id, row.path)) | |
| 253 | + | .collect(), | |
| 219 | 254 | None => Vec::new(), | |
| 220 | 255 | }; | |
| 221 | 256 | let landed = items::landed_before(self, held, LANDINGS).await?; | |
| ⋯ | |||
| 402 | 437 | let paths: Vec<&str> = suggested.iter().map(|s| s.path.as_str()).collect(); | |
| 403 | 438 | assert_eq!(paths, ["acme/web", "acme/api"]); | |
| 404 | 439 | assert_eq!(suggested[0].why, "It names web; sessions on it touched 3 files there."); | |
| 405 | − | assert_eq!(suggested[1].why, "1 work items with the same labels landed there; 1 work items with like titles landed there."); | |
| 440 | + | assert_eq!(suggested[1].why, "1 work item with the same labels landed there; 1 work item with like titles landed there."); | |
| 406 | 441 | assert!(suggested[0].score > suggested[1].score); | |
| 407 | 442 | } | |
| 408 | 443 | ||
| 409 | 444 | #[test] | |
| 410 | − | fn a_target_already_linked_is_not_suggested() { | |
| 445 | + | fn a_repository_already_linked_is_not_suggested() { | |
| 411 | 446 | let candidates = vec![("rep_web".to_owned(), "acme/web".to_owned())]; | |
| 412 | − | let target = ItemRepo { repo_id: "rep_web".into(), path: "acme/web".into(), role: LinkRole::Target, legacy_number: None, landed: false }; | |
| 413 | − | let item = item("Fix web", "", &[], vec![target]); | |
| 414 | − | assert!(suggest_targets(&item, &candidates, &[], &[]).is_empty()); | |
| 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 }; | |
| 449 | + | let item = item("Fix web", "", &[], vec![linked]); | |
| 450 | + | assert!(suggest_targets(&item, &candidates, &[], &[]).is_empty(), "{role:?}"); | |
| 451 | + | } | |
| 452 | + | } | |
| 453 | + | ||
| 454 | + | #[test] | |
| 455 | + | fn sessions_and_claims_are_shown_as_the_viewer_may_see_them() { | |
| 456 | + | let public = super::super::shown::tests::a_session(); | |
| 457 | + | let private = Session { id: "ses_2".into(), repo_id: "rep_secret".into(), ..public.clone() }; | |
| 458 | + | let (_, sessions) = shown_people(Vec::new(), vec![public, private], &["rep_1".to_owned()], &None, "acme"); | |
| 459 | + | // Only the one in a repository they may read, and not what is inside it. | |
| 460 | + | assert_eq!(sessions.len(), 1); | |
| 461 | + | assert_eq!(sessions[0].id, "ses_1"); | |
| 462 | + | assert_eq!(sessions[0].branch, None); | |
| 463 | + | assert_eq!(sessions[0].title, None); | |
| 464 | + | } | |
| 465 | + | ||
| 466 | + | #[test] | |
| 467 | + | fn a_closed_work_item_gets_no_suggestions() { | |
| 468 | + | let candidates = vec![("rep_web".to_owned(), "acme/web".to_owned())]; | |
| 469 | + | let mut closed = item("Fix web", "", &[], Vec::new()); | |
| 470 | + | assert_eq!(suggest_targets(&closed, &candidates, &[], &[]).len(), 1); | |
| 471 | + | closed.state = State::Closed; | |
| 472 | + | assert!(suggest_targets(&closed, &candidates, &[], &[]).is_empty()); | |
| 415 | 473 | } | |
| 416 | 474 | } | |
| 5 | 5 | //! Each blocker that closes or has a pull request merged for it is a | |
| 6 | 6 | //! `dependency.changed` event on the work item it blocks. | |
| 7 | 7 | ||
| 8 | + | use std::collections::HashSet; | |
| 9 | + | ||
| 8 | 10 | use g1t_contracts::work::*; | |
| 9 | 11 | use g1t_contracts::User; | |
| 10 | 12 | use serde::Deserialize; | |
| ⋯ | |||
| 20 | 22 | title: String, | |
| 21 | 23 | status: PullStatus, | |
| 22 | 24 | item_id: String, | |
| 25 | + | repo_id: String, | |
| 23 | 26 | } | |
| 24 | 27 | ||
| 25 | 28 | impl Work { | |
| 26 | − | /// A work item closed (`closed`) or had a pull request merged for it | |
| 27 | − | /// (`merged_pull`): its claims end if it closed, and each open work | |
| 28 | − | /// item it blocks hears that it cleared. Never fails what caused it: a | |
| 29 | − | /// problem is logged. | |
| 30 | − | pub(crate) async fn item_resolved(&self, held: &Held, actor: &User, closed: bool, merged_pull: Option<u32>) { | |
| 29 | + | /// A work item closed (`closed`), or landed in one of its targets by | |
| 30 | + | /// `landing` (a pull request merged, or a commit pushed) and stays | |
| 31 | + | /// open: its claims end if it closed, and each open work item it | |
| 32 | + | /// blocks hears. A close clears it; a landing that leaves it open | |
| 33 | + | /// clears nothing, and it is still among what they wait on. Never | |
| 34 | + | /// fails what caused it: a problem is logged. | |
| 35 | + | pub(crate) async fn item_resolved(&self, held: &Held, actor: &User, closed: bool, landing: Option<&items::Landing>) { | |
| 31 | 36 | let number = held.item.number; | |
| 37 | + | let merged_pull = landing.and_then(|landing| landing.pull); | |
| 32 | 38 | let resolved = async { | |
| 33 | 39 | if closed { | |
| 34 | 40 | self.release_claims_on_close(held, Some(actor)).await?; | |
| 35 | 41 | } | |
| 36 | 42 | for dependent in items::dependents(self, held).await? { | |
| 37 | 43 | let blocked_by = dependent.item.blocked_by.clone(); | |
| 38 | − | let remaining = items::still_open(self, &dependent.item.workspace, &blocked_by, Some(number)).await?; | |
| 44 | + | let remaining = items::still_open(self, &dependent.item.workspace, &blocked_by, closed.then_some(number)).await?; | |
| 39 | 45 | let change = DependencyChanged { | |
| 40 | 46 | workspace: dependent.item.workspace.clone(), | |
| 41 | 47 | number: dependent.item.number, | |
| 42 | 48 | item: dependent.item.number, | |
| 43 | 49 | repo_ids: dependent.repo_ids(), | |
| 44 | 50 | blocked_by, | |
| 45 | − | cleared: vec![number], | |
| 51 | + | cleared: if closed { vec![number] } else { Vec::new() }, | |
| 46 | 52 | remaining, | |
| 47 | 53 | merged_pull, | |
| 48 | 54 | ..DependencyChanged::default() | |
| 49 | 55 | }; | |
| 50 | − | // Follows from a close or merge already made: named by it, so | |
| 51 | − | // a repeat of the same close or merge tells each session once. | |
| 52 | − | let how = merged_pull.map_or_else(|| "closed".to_owned(), |pull| format!("merged:{pull}")); | |
| 53 | − | let key = format!("{}:cleared:{}:{how}", dependent.item.id, held.item.id); | |
| 56 | + | // Follows from a close or landing already made: named by it, | |
| 57 | + | // so a repeat of the same one tells each session once. | |
| 58 | + | let key = resolution_key(&dependent.item.id, &held.item.id, closed, landing); | |
| 54 | 59 | let data = g1t_contracts::events::marked(change, Some(actor)); | |
| 55 | 60 | self.publish_keyed_to("dependency.changed", &dependent.repo_ids(), Some(actor.id.clone()), data, &key).await?; | |
| 56 | 61 | } | |
| ⋯ | |||
| 61 | 66 | } | |
| 62 | 67 | } | |
| 63 | 68 | ||
| 64 | − | /// The work items of `workspace` that `numbers` names, each with its | |
| 65 | − | /// active claims and its pull requests, in the order given. | |
| 66 | − | pub(crate) async fn blockers(&self, workspace: &str, numbers: &[u32]) -> Result<Vec<Blocker>> { | |
| 67 | − | let items = items::briefs(self, workspace, numbers).await?; | |
| 69 | + | /// The work items of `workspace` that `numbers` names and `viewer` may | |
| 70 | + | /// see, each with its active claims and its pull requests in | |
| 71 | + | /// repositories `viewer` may read, in the order given. | |
| 72 | + | pub(crate) async fn blockers(&self, viewer: Option<&User>, workspace: &str, numbers: &[u32]) -> Result<Vec<Blocker>> { | |
| 73 | + | let items = items::visible_numbered(self, workspace, numbers, viewer).await?; | |
| 68 | 74 | if items.is_empty() { | |
| 69 | 75 | return Ok(Vec::new()); | |
| 70 | 76 | } | |
| 71 | − | let ids = serde_json::to_string(&items.iter().map(|item| item.id.as_str()).collect::<Vec<_>>())?; | |
| 77 | + | let ids = serde_json::to_string(&items.iter().map(|held| held.item.id.as_str()).collect::<Vec<_>>())?; | |
| 72 | 78 | let bind = |sql: String| -> Result<worker::D1PreparedStatement> { self.db.prepare(sql).bind(&[ids.as_str().into()]) }; | |
| 73 | 79 | let results = self | |
| 74 | 80 | .db | |
| ⋯ | |||
| 79 | 85 | ORDER BY created_at, id" | |
| 80 | 86 | ))?, | |
| 81 | 87 | bind( | |
| 82 | − | "SELECT number, title, status, item_id FROM pulls | |
| 88 | + | "SELECT number, title, status, item_id, repo_id FROM pulls | |
| 83 | 89 | WHERE item_id IN (SELECT value FROM json_each(?1)) | |
| 84 | 90 | ORDER BY created_at" | |
| 85 | 91 | .to_owned(), | |
| ⋯ | |||
| 96 | 102 | Some(result) => result.results()?, | |
| 97 | 103 | None => Vec::new(), | |
| 98 | 104 | }; | |
| 99 | − | Ok(numbers | |
| 100 | − | .iter() | |
| 101 | − | .filter_map(|number| { | |
| 102 | − | let item = items.iter().find(|item| item.number == *number)?; | |
| 103 | − | Some(Blocker { | |
| 104 | − | number: item.number, | |
| 105 | − | title: item.title.clone(), | |
| 106 | − | state: item.state, | |
| 107 | − | claims: claims.iter().filter(|(id, _)| *id == item.id).map(|(_, claim)| claim.clone()).collect(), | |
| 108 | − | pulls: pulls | |
| 109 | − | .iter() | |
| 110 | − | .filter(|pull| pull.item_id == item.id) | |
| 111 | − | .map(|pull| PullBrief { number: pull.number, title: pull.title.clone(), status: pull.status }) | |
| 112 | − | .collect(), | |
| 113 | − | }) | |
| 105 | + | let mut repo_ids: Vec<String> = pulls.iter().map(|pull| pull.repo_id.clone()).collect(); | |
| 106 | + | repo_ids.extend(claims.iter().filter_map(|(_, claim)| claim.repo_id.clone())); | |
| 107 | + | repo_ids.sort(); | |
| 108 | + | repo_ids.dedup(); | |
| 109 | + | let readable = items::readable_repo_ids(self, viewer, &repo_ids).await?; | |
| 110 | + | Ok(assemble_blockers(numbers, &items, &claims, &pulls, &readable)) | |
| 111 | + | } | |
| 112 | + | } | |
| 113 | + | ||
| 114 | + | /// What tells `dependent` once of `blocker` resolving: closing (`:closed`, | |
| 115 | + | /// or `:merged:N` when a merge closed it), or landing somewhere while it | |
| 116 | + | /// stays open (`:landed:` and what landed), which never stands in for the | |
| 117 | + | /// close that comes later. | |
| 118 | + | fn resolution_key(dependent: &str, blocker: &str, closed: bool, landing: Option<&items::Landing>) -> String { | |
| 119 | + | let pull = landing.and_then(|landing| landing.pull); | |
| 120 | + | let commit = landing.and_then(|landing| landing.commit.as_deref()); | |
| 121 | + | let how = match (closed, pull, commit) { | |
| 122 | + | (true, Some(pull), _) => format!("merged:{pull}"), | |
| 123 | + | (true, None, _) => "closed".to_owned(), | |
| 124 | + | (false, Some(pull), _) => format!("landed:pull:{pull}"), | |
| 125 | + | (false, None, Some(commit)) => format!("landed:commit:{commit}"), | |
| 126 | + | (false, None, None) => "landed".to_owned(), | |
| 127 | + | }; | |
| 128 | + | format!("{dependent}:cleared:{blocker}:{how}") | |
| 129 | + | } | |
| 130 | + | ||
| 131 | + | /// The blockers `numbers` names among `items` (those the viewer may see), | |
| 132 | + | /// in the order given, each with the claims and pull requests that are not | |
| 133 | + | /// in a repository outside `readable`. | |
| 134 | + | fn assemble_blockers( | |
| 135 | + | numbers: &[u32], | |
| 136 | + | items: &[Held], | |
| 137 | + | claims: &[(String, Claim)], | |
| 138 | + | pulls: &[PullBriefRow], | |
| 139 | + | readable: &HashSet<String>, | |
| 140 | + | ) -> Vec<Blocker> { | |
| 141 | + | numbers | |
| 142 | + | .iter() | |
| 143 | + | .filter_map(|number| { | |
| 144 | + | let held = items.iter().find(|held| held.item.number == *number)?; | |
| 145 | + | Some(Blocker { | |
| 146 | + | number: held.item.number, | |
| 147 | + | title: held.item.title.clone(), | |
| 148 | + | state: held.item.state, | |
| 149 | + | claims: claims | |
| 150 | + | .iter() | |
| 151 | + | .filter(|(id, claim)| *id == held.item.id && claim.repo_id.as_ref().is_none_or(|repo| readable.contains(repo))) | |
| 152 | + | .map(|(_, claim)| claim.clone()) | |
| 153 | + | .collect(), | |
| 154 | + | pulls: pulls | |
| 155 | + | .iter() | |
| 156 | + | .filter(|pull| pull.item_id == held.item.id && readable.contains(&pull.repo_id)) | |
| 157 | + | .map(|pull| PullBrief { number: pull.number, title: pull.title.clone(), status: pull.status }) | |
| 158 | + | .collect(), | |
| 114 | 159 | }) | |
| 115 | − | .collect()) | |
| 160 | + | }) | |
| 161 | + | .collect() | |
| 162 | + | } | |
| 163 | + | ||
| 164 | + | #[cfg(test)] | |
| 165 | + | mod tests { | |
| 166 | + | use super::*; | |
| 167 | + | use crate::items::rows_for_test::held_for_test; | |
| 168 | + | ||
| 169 | + | #[test] | |
| 170 | + | fn a_landing_that_leaves_a_blocker_open_never_stands_in_for_its_close() { | |
| 171 | + | let commit = items::Landing { pull: None, commit: Some("abc".into()), pushed_by: None }; | |
| 172 | + | let merged = items::Landing { pull: Some(5), commit: None, pushed_by: None }; | |
| 173 | + | let landed = resolution_key("wki_2", "wki_1", false, Some(&commit)); | |
| 174 | + | let closed = resolution_key("wki_2", "wki_1", true, None); | |
| 175 | + | assert_ne!(landed, closed); | |
| 176 | + | assert_eq!(closed, "wki_2:cleared:wki_1:closed"); | |
| 177 | + | assert_eq!(landed, "wki_2:cleared:wki_1:landed:commit:abc"); | |
| 178 | + | assert_ne!(resolution_key("wki_2", "wki_1", false, Some(&merged)), resolution_key("wki_2", "wki_1", true, Some(&merged))); | |
| 179 | + | } | |
| 180 | + | ||
| 181 | + | #[test] | |
| 182 | + | fn blockers_show_only_what_the_viewer_may_read() { | |
| 183 | + | // #1 is visible (asked for by the caller, who filtered), #2 is not | |
| 184 | + | // among `items`: the viewer may not see it. | |
| 185 | + | let items = vec![held_for_test(1, &[("rep_web", "origin", "acme", false)], &[])]; | |
| 186 | + | let pull = |number: u32, item: &str, repo: &str| PullBriefRow { | |
| 187 | + | number, | |
| 188 | + | title: format!("Pull {number}"), | |
| 189 | + | status: PullStatus::Open, | |
| 190 | + | item_id: item.to_owned(), | |
| 191 | + | repo_id: repo.to_owned(), | |
| 192 | + | }; | |
| 193 | + | let pulls = vec![pull(10, "wki_1", "rep_web"), pull(11, "wki_1", "rep_secret"), pull(12, "wki_2", "rep_web")]; | |
| 194 | + | let readable: HashSet<String> = ["rep_web".to_owned()].into(); | |
| 195 | + | let blockers = assemble_blockers(&[2, 1], &items, &[], &pulls, &readable); | |
| 196 | + | assert_eq!(blockers.len(), 1); | |
| 197 | + | assert_eq!(blockers[0].number, 1); | |
| 198 | + | assert_eq!(blockers[0].pulls.iter().map(|pull| pull.number).collect::<Vec<_>>(), vec![10]); | |
| 116 | 199 | } | |
| 117 | 200 | } | |
| 171 | 171 | } | |
| 172 | 172 | ||
| 173 | 173 | #[cfg(test)] | |
| 174 | − | mod tests { | |
| 174 | + | pub(crate) mod tests { | |
| 175 | 175 | use super::*; | |
| 176 | 176 | use g1t_contracts::{Membership, User}; | |
| 177 | 177 | use g1t_contracts::credentials::Principal; | |
| ⋯ | |||
| 186 | 186 | }) | |
| 187 | 187 | } | |
| 188 | 188 | ||
| 189 | − | fn a_session() -> Session { | |
| 189 | + | pub(crate) fn a_session() -> Session { | |
| 190 | 190 | Session { | |
| 191 | 191 | id: "ses_1".into(), | |
| 192 | 192 | repo_id: "rep_1".into(), | |
| 14 | 14 | use super::rows::{Held, no_item, path_of, workspace_of}; | |
| 15 | 15 | use crate::{UNVERIFIED, Work, optional, optional_number, valid_title}; | |
| 16 | 16 | ||
| 17 | + | /// What closing work item `number` by a commit in `landed_in` says when it | |
| 18 | + | /// stays open: the targets it still has to land in (`missing`, by path; an | |
| 19 | + | /// empty one for a repository the actor may not read). | |
| 20 | + | pub(crate) fn still_open(number: u32, landed_in: &str, missing: &[String]) -> String { | |
| 21 | + | let named: Vec<&str> = missing.iter().map(String::as_str).filter(|path| !path.is_empty()).collect(); | |
| 22 | + | let hidden = missing.len() - named.len(); | |
| 23 | + | let mut targets = named.join(", "); | |
| 24 | + | if hidden > 0 { | |
| 25 | + | let more = if hidden == 1 { "1 other repository".to_owned() } else { format!("{hidden} other repositories") }; | |
| 26 | + | targets = if targets.is_empty() { more } else { format!("{targets} and {more}") }; | |
| 27 | + | } | |
| 28 | + | format!("Recorded the commit as landed in {landed_in}, but #{number} stays open until it lands in {targets} too.") | |
| 29 | + | } | |
| 30 | + | ||
| 17 | 31 | impl Work { | |
| 18 | 32 | pub(super) fn item_event(held: &Held) -> WorkItemEvent { | |
| 19 | 33 | WorkItemEvent { | |
| ⋯ | |||
| 108 | 122 | if !targets.is_empty() && origin.is_some() && !may(Capability::Triage) { | |
| 109 | 123 | return Ok(Outcome::fail(FailureCode::Forbidden, access::needs_on_item(Capability::Triage, &workspace))); | |
| 110 | 124 | } | |
| 111 | − | let colors = check!(self.ensure_labels(&a.actor, &workspace, origin_ref, &labels).await?); | |
| 125 | + | let colors = check!(self.ensure_labels(&a.actor, &workspace, &labels).await?); | |
| 112 | 126 | let milestone = match a.milestone.filter(|number| *number > 0) { | |
| 113 | 127 | Some(number) => { | |
| 114 | 128 | if !may(Capability::Triage) { | |
| ⋯ | |||
| 186 | 200 | ) | |
| 187 | 201 | .await?; | |
| 188 | 202 | } | |
| 189 | − | Ok(Outcome::Ok(held.item)) | |
| 203 | + | Ok(Outcome::Ok(held.shown(Some(&a.actor)))) | |
| 190 | 204 | } | |
| 191 | 205 | ||
| 192 | 206 | ||
| ⋯ | |||
| 209 | 223 | }; | |
| 210 | 224 | // What it is blocked by: checked before anything changes. | |
| 211 | 225 | let blocked_by = match &a.blocked_by { | |
| 212 | − | Some(asked) => Some(check!(self.checked_blocked_by(&held.item.workspace, held.item.number, asked).await?)), | |
| 226 | + | Some(asked) => Some(check!(self.checked_blocked_by(&held, &a.actor, asked).await?)), | |
| 213 | 227 | None => None, | |
| 214 | 228 | }; | |
| 215 | 229 | // Its labels and milestone first: either can be refused, and then | |
| ⋯ | |||
| 287 | 301 | ) | |
| 288 | 302 | .await?; | |
| 289 | 303 | } | |
| 290 | − | Ok(Outcome::Ok(held.item)) | |
| 304 | + | Ok(Outcome::Ok(held.shown(Some(&a.actor)))) | |
| 291 | 305 | } | |
| 292 | 306 | ||
| 293 | 307 | ||
| ⋯ | |||
| 315 | 329 | let landing = Landing { pull: None, commit: Some(commit), pushed_by: Some(a.actor.username.clone()) }; | |
| 316 | 330 | self.land(&found, &landing, &a.actor, std::slice::from_ref(&held.item.id), &[], session.as_deref(), false).await?; | |
| 317 | 331 | return Ok(match self.item_by_id(&held.item.id).await? { | |
| 318 | − | Some(held) => Outcome::Ok(held.item), | |
| 332 | + | Some(after) if after.item.state == State::Open && !after.item.missing().is_empty() => { | |
| 333 | + | let missing: Vec<String> = after | |
| 334 | + | .item | |
| 335 | + | .repos | |
| 336 | + | .iter() | |
| 337 | + | .filter(|repo| repo.role == LinkRole::Target && !repo.landed) | |
| 338 | + | .map(|repo| if after.may_read_repo(Some(&a.actor), &repo.repo_id) { repo.path.clone() } else { String::new() }) | |
| 339 | + | .collect(); | |
| 340 | + | Outcome::fail(FailureCode::Conflict, still_open(after.item.number, &path_of(&found), &missing)) | |
| 341 | + | } | |
| 342 | + | Some(after) => Outcome::Ok(after.shown(Some(&a.actor))), | |
| 319 | 343 | None => no_item(), | |
| 320 | 344 | }); | |
| 321 | 345 | } | |
| ⋯ | |||
| 341 | 365 | self.note_item(&held.item.id, &a.actor, &crate::pushed::closed_note(note, session.as_deref())).await?; | |
| 342 | 366 | // Its claims end, and the work items it blocks hear (coordination/). | |
| 343 | 367 | self.item_resolved(&held, &a.actor, true, None).await; | |
| 344 | − | let mut item = held.item; | |
| 368 | + | let mut item = held.shown(Some(&a.actor)); | |
| 345 | 369 | item.state = State::Closed; | |
| 346 | 370 | item.reason = Some(reason); | |
| 347 | 371 | item.closed_at = Some(now.clone()); | |
| ⋯ | |||
| 371 | 395 | self.publish_item("work_item.reopened", &held, Some(&a.actor), Self::item_event(&held)).await?; | |
| 372 | 396 | self.note_item(&held.item.id, &a.actor, "reopened this").await?; | |
| 373 | 397 | Ok(match self.item_by_id(&held.item.id).await? { | |
| 374 | − | Some(held) => Outcome::Ok(held.item), | |
| 398 | + | Some(held) => Outcome::Ok(held.shown(Some(&a.actor))), | |
| 375 | 399 | None => no_item(), | |
| 376 | 400 | }) | |
| 377 | 401 | } | |
| 378 | 402 | } | |
| 403 | + | ||
| 404 | + | #[cfg(test)] | |
| 405 | + | mod tests { | |
| 406 | + | use super::still_open; | |
| 407 | + | ||
| 408 | + | #[test] | |
| 409 | + | fn a_commit_that_does_not_close_says_where_it_has_still_to_land() { | |
| 410 | + | assert_eq!( | |
| 411 | + | still_open(12, "acme/web", &["acme/api".to_owned()]), | |
| 412 | + | "Recorded the commit as landed in acme/web, but #12 stays open until it lands in acme/api too." | |
| 413 | + | ); | |
| 414 | + | assert_eq!( | |
| 415 | + | still_open(12, "acme/web", &["acme/api".to_owned(), String::new()]), | |
| 416 | + | "Recorded the commit as landed in acme/web, but #12 stays open until it lands in acme/api and 1 other repository too." | |
| 417 | + | ); | |
| 418 | + | } | |
| 419 | + | } | |
| 26 | 26 | blocked_by: Option<String>, | |
| 27 | 27 | } | |
| 28 | 28 | ||
| 29 | − | /// A work item in brief: its id, number, title and state. | |
| 29 | + | /// A work item's number. | |
| 30 | 30 | #[derive(Clone, Debug, Deserialize)] | |
| 31 | − | pub(crate) struct Brief { | |
| 32 | − | pub id: String, | |
| 33 | − | pub number: u32, | |
| 34 | − | pub title: String, | |
| 35 | − | pub state: State, | |
| 31 | + | struct Numbered { | |
| 32 | + | number: u32, | |
| 33 | + | } | |
| 34 | + | ||
| 35 | + | /// The numbers of those of `found` that `viewer` may see, and of those | |
| 36 | + | /// already in `kept` (what a work item is blocked by now). | |
| 37 | + | pub(crate) fn visible_numbers(found: &[Held], viewer: Option<&User>, kept: &[u32]) -> HashSet<u32> { | |
| 38 | + | found | |
| 39 | + | .iter() | |
| 40 | + | .filter(|item| kept.contains(&item.item.number) || item.may(viewer, g1t_contracts::access::Capability::Read)) | |
| 41 | + | .map(|item| item.item.number) | |
| 42 | + | .collect() | |
| 36 | 43 | } | |
| 37 | 44 | ||
| 38 | 45 | fn numbers(json: Option<&str>) -> Vec<u32> { | |
| ⋯ | |||
| 45 | 52 | (SELECT json_group_array(b.number) FROM json_each(work_items.blocked_by) j JOIN work_items b ON b.id = j.value | |
| 46 | 53 | WHERE b.number IS NOT NULL) AS blocked_by | |
| 47 | 54 | FROM work_items WHERE workspace = ?1 AND blocked_by <> '[]' AND number IS NOT NULL"; | |
| 48 | − | ||
| 49 | − | /// The work items of workspace `?1` numbered in `?2` (a JSON array), in | |
| 50 | − | /// brief. | |
| 51 | − | const BRIEFS: &str = "SELECT id, number, title, state FROM work_items | |
| 52 | − | WHERE workspace = ?1 AND number IN (SELECT value FROM json_each(?2))"; | |
| 53 | 55 | ||
| 54 | 56 | impl Work { | |
| 55 | − | /// `asked` as `number`'s new `blocked_by` in `workspace`, or why it | |
| 56 | − | /// cannot be. | |
| 57 | − | pub(super) async fn checked_blocked_by(&self, workspace: &str, number: u32, asked: &[u32]) -> Result<Outcome<Vec<u32>>> { | |
| 57 | + | /// `asked` as `held`'s new `blocked_by`, set by `actor`, or why it | |
| 58 | + | /// cannot be. A work item `actor` may not see is refused as one that | |
| 59 | + | /// is not there, unless `held` is blocked by it already. | |
| 60 | + | pub(super) async fn checked_blocked_by(&self, held: &Held, actor: &User, asked: &[u32]) -> Result<Outcome<Vec<u32>>> { | |
| 61 | + | let workspace = held.item.workspace.as_str(); | |
| 58 | 62 | let wanted = serde_json::to_string(asked)?; | |
| 59 | 63 | let results = self | |
| 60 | 64 | .db | |
| 61 | 65 | .batch(vec![ | |
| 62 | − | self.db.prepare(BRIEFS).bind(&[workspace.into(), wanted.into()])?, | |
| 66 | + | self.db | |
| 67 | + | .prepare(select("workspace = ?1 AND number IN (SELECT value FROM json_each(?2))")) | |
| 68 | + | .bind(&[workspace.into(), wanted.into()])?, | |
| 63 | 69 | self.db.prepare(EDGES).bind(&[workspace.into()])?, | |
| 64 | 70 | ]) | |
| 65 | 71 | .await?; | |
| 66 | 72 | let mut results = results.into_iter(); | |
| 67 | − | let found: Vec<Brief> = match results.next() { | |
| 68 | − | Some(result) => result.results()?, | |
| 73 | + | let found: Vec<Held> = match results.next() { | |
| 74 | + | Some(result) => held_rows(&result)?, | |
| 69 | 75 | None => Vec::new(), | |
| 70 | 76 | }; | |
| 71 | 77 | let edges: Vec<EdgeRow> = match results.next() { | |
| 72 | 78 | Some(result) => result.results()?, | |
| 73 | 79 | None => Vec::new(), | |
| 74 | 80 | }; | |
| 75 | − | let items: HashSet<u32> = found.iter().map(|row| row.number).collect(); | |
| 81 | + | let items = visible_numbers(&found, Some(actor), &held.item.blocked_by); | |
| 82 | + | let number = held.item.number; | |
| 76 | 83 | let graph: HashMap<u32, Vec<u32>> = edges.into_iter().map(|row| (row.number, numbers(row.blocked_by.as_deref()))).collect(); | |
| 77 | 84 | Ok(match check_blocked_by(number, asked, &items, &graph) { | |
| 78 | 85 | Ok(set) => Outcome::Ok(set), | |
| ⋯ | |||
| 130 | 137 | if numbers.is_empty() { | |
| 131 | 138 | return Ok(Vec::new()); | |
| 132 | 139 | } | |
| 133 | − | let open: Vec<Brief> = self | |
| 140 | + | let open: Vec<Numbered> = self | |
| 134 | 141 | .db | |
| 135 | 142 | .prepare( | |
| 136 | − | "SELECT id, number, title, state FROM work_items | |
| 143 | + | "SELECT number FROM work_items | |
| 137 | 144 | WHERE workspace = ? AND state = 'open' AND number IN (SELECT value FROM json_each(?))", | |
| 138 | 145 | ) | |
| 139 | 146 | .bind(&[workspace.into(), serde_json::to_string(numbers)?.into()])? | |
| ⋯ | |||
| 162 | 169 | ) | |
| 163 | 170 | } | |
| 164 | 171 | ||
| 165 | − | /// The work items of `workspace` that `numbers` names, in brief, in no | |
| 166 | − | /// particular order. | |
| 167 | − | pub(super) async fn briefs(&self, workspace: &str, numbers: &[u32]) -> Result<Vec<Brief>> { | |
| 172 | + | /// The work items of `workspace` that `numbers` names and `viewer` may | |
| 173 | + | /// see, in no particular order. | |
| 174 | + | pub(super) async fn visible_numbered(&self, workspace: &str, numbers: &[u32], viewer: Option<&User>) -> Result<Vec<Held>> { | |
| 168 | 175 | if numbers.is_empty() { | |
| 169 | 176 | return Ok(Vec::new()); | |
| 170 | 177 | } | |
| 171 | − | Ok(self.db.prepare(BRIEFS).bind(&[workspace.into(), serde_json::to_string(numbers)?.into()])?.all().await?.results()?) | |
| 178 | + | let found = held_rows( | |
| 179 | + | &self | |
| 180 | + | .db | |
| 181 | + | .prepare(select("workspace = ?1 AND number IN (SELECT value FROM json_each(?2))")) | |
| 182 | + | .bind(&[workspace.into(), serde_json::to_string(numbers)?.into()])? | |
| 183 | + | .all() | |
| 184 | + | .await?, | |
| 185 | + | )?; | |
| 186 | + | Ok(found.into_iter().filter(|item| item.may(viewer, g1t_contracts::access::Capability::Read)).collect()) | |
| 172 | 187 | } | |
| 173 | 188 | ||
| 174 | − | /// The work items that name `held` in their `blocked_by`, by number. | |
| 175 | − | pub(super) async fn blocked_items(&self, held: &Held) -> Result<Vec<ItemBrief>> { | |
| 176 | − | Ok(self | |
| 177 | − | .db | |
| 178 | − | .prepare( | |
| 179 | − | "SELECT id, number, title, state FROM work_items | |
| 180 | − | WHERE workspace = ? AND number IS NOT NULL | |
| 181 | − | AND EXISTS (SELECT 1 FROM json_each(work_items.blocked_by) WHERE json_each.value = ?) | |
| 182 | − | ORDER BY number LIMIT 100", | |
| 183 | − | ) | |
| 184 | − | .bind(&[held.item.workspace.as_str().into(), held.item.id.as_str().into()])? | |
| 185 | − | .all() | |
| 186 | − | .await? | |
| 187 | − | .results::<Brief>()? | |
| 189 | + | /// The work items that name `held` in their `blocked_by`, by number: | |
| 190 | + | /// those `viewer` may see. | |
| 191 | + | pub(super) async fn blocked_items(&self, held: &Held, viewer: Option<&User>) -> Result<Vec<ItemBrief>> { | |
| 192 | + | let found = held_rows( | |
| 193 | + | &self | |
| 194 | + | .db | |
| 195 | + | .prepare(select( | |
| 196 | + | "workspace = ? AND number IS NOT NULL | |
| 197 | + | AND EXISTS (SELECT 1 FROM json_each(work_items.blocked_by) WHERE json_each.value = ?) | |
| 198 | + | ORDER BY number LIMIT 100", | |
| 199 | + | )) | |
| 200 | + | .bind(&[held.item.workspace.as_str().into(), held.item.id.as_str().into()])? | |
| 201 | + | .all() | |
| 202 | + | .await?, | |
| 203 | + | )?; | |
| 204 | + | Ok(found | |
| 188 | 205 | .into_iter() | |
| 189 | − | .map(|row| ItemBrief { number: row.number, title: row.title, state: row.state }) | |
| 206 | + | .filter(|item| item.may(viewer, g1t_contracts::access::Capability::Read)) | |
| 207 | + | .map(|item| ItemBrief { number: item.item.number, title: item.item.title, state: item.item.state }) | |
| 190 | 208 | .collect()) | |
| 191 | 209 | } | |
| 192 | 210 | } | |
| 188 | 188 | Ok(description.to_owned()) | |
| 189 | 189 | } | |
| 190 | 190 | ||
| 191 | + | /// Whether `actor` may make labels in `workspace`, which are the whole | |
| 192 | + | /// workspace's: through their membership of it alone, never their role on | |
| 193 | + | /// one repository (an outside collaborator's Write on a public repository | |
| 194 | + | /// makes no workspace labels). | |
| 195 | + | pub(crate) fn may_create_labels(actor: &User, workspace: &str) -> bool { | |
| 196 | + | access::can_item(Some(actor), workspace, None, Capability::ManageLabels) | |
| 197 | + | } | |
| 198 | + | ||
| 191 | 199 | /// Refused while issues are still being moved into work items: what is | |
| 192 | 200 | /// numbered or kept by workspace is not all there yet. | |
| 193 | 201 | pub(crate) fn still_moving<T>() -> Outcome<T> { | |
| 194 | 202 | Outcome::fail( | |
| 195 | 203 | FailureCode::Conflict, | |
| 196 | − | "Issues are moving into work items, with their labels and milestones, and new numbers wait for them; try again in a few minutes.", | |
| 204 | + | "Work items are still being moved into this workspace; try again in a few minutes.", | |
| 197 | 205 | ) | |
| 198 | 206 | } | |
| 199 | 207 | ||
| ⋯ | |||
| 410 | 418 | } | |
| 411 | 419 | ||
| 412 | 420 | /// Makes sure every label in `names` exists in the workspace: those | |
| 413 | − | /// missing are created when `actor` may manage labels (through the | |
| 414 | − | /// workspace, or their role on `repo`), and refused otherwise. Returns | |
| 415 | − | /// their colors, by name. | |
| 416 | − | pub(crate) async fn ensure_labels( | |
| 417 | − | &self, | |
| 418 | − | actor: &User, | |
| 419 | − | workspace: &str, | |
| 420 | − | repo: Option<RepoRef<'_>>, | |
| 421 | − | names: &[String], | |
| 422 | − | ) -> Result<Outcome<Vec<(String, String)>>> { | |
| 421 | + | /// missing are created when `actor` may manage the workspace's labels | |
| 422 | + | /// ([`may_create_labels`]), and refused otherwise. Returns their | |
| 423 | + | /// colors, by name. | |
| 424 | + | pub(crate) async fn ensure_labels(&self, actor: &User, workspace: &str, names: &[String]) -> Result<Outcome<Vec<(String, String)>>> { | |
| 423 | 425 | if names.is_empty() { | |
| 424 | 426 | return Ok(Outcome::Ok(Vec::new())); | |
| 425 | 427 | } | |
| 426 | 428 | let existing: Vec<Label> = self.labels_in(workspace, None).await?; | |
| 427 | 429 | let missing: Vec<&String> = names.iter().filter(|name| !existing.iter().any(|label| label.name == **name)).collect(); | |
| 428 | 430 | if !missing.is_empty() { | |
| 429 | − | if !access::can_item(Some(actor), workspace, repo, Capability::ManageLabels) { | |
| 431 | + | if !may_create_labels(actor, workspace) { | |
| 430 | 432 | let list = missing.iter().map(|name| name.as_str()).collect::<Vec<_>>().join(", "); | |
| 431 | 433 | return Ok(Outcome::fail( | |
| 432 | 434 | FailureCode::Invalid, | |
| ⋯ | |||
| 475 | 477 | format!("A work item or pull request can have up to {MAX_LABELS} labels of up to {MAX_LABEL_CHARS} characters each."), | |
| 476 | 478 | )); | |
| 477 | 479 | }; | |
| 478 | − | let colors = check!(self.ensure_labels(actor, &labeled.workspace(), labeled.repo(), &after).await?); | |
| 480 | + | let colors = check!(self.ensure_labels(actor, &labeled.workspace(), &after).await?); | |
| 479 | 481 | let before = labeled.labels().to_vec(); | |
| 480 | 482 | let (added, removed) = label_changes(&before, &after); | |
| 481 | 483 | if added.is_empty() && removed.is_empty() { | |
| ⋯ | |||
| 576 | 578 | } | |
| 577 | 579 | ||
| 578 | 580 | #[test] | |
| 581 | + | fn only_the_workspaces_members_make_its_labels() { | |
| 582 | + | use crate::items::rows_for_test::person_for_test; | |
| 583 | + | // An outside collaborator with Write on one of acme's repositories. | |
| 584 | + | let mut outsider = person_for_test("usr_9", None); | |
| 585 | + | outsider.grants.push(g1t_contracts::access::RepoGrant { | |
| 586 | + | repo_id: "rep_web".into(), | |
| 587 | + | workspace: "acme".into(), | |
| 588 | + | role: g1t_contracts::access::RepoRole::Write, | |
| 589 | + | team: None, | |
| 590 | + | }); | |
| 591 | + | assert!(!may_create_labels(&outsider, "acme")); | |
| 592 | + | assert!(may_create_labels(&person_for_test("usr_2", Some("acme")), "acme")); | |
| 593 | + | } | |
| 594 | + | ||
| 595 | + | #[test] | |
| 579 | 596 | fn labels_are_set_added_to_and_taken_off() { | |
| 580 | 597 | let current = names(&["bug", "docs"]); | |
| 581 | 598 | assert_eq!(changed_labels(¤t, &names(&["question"]), LabelChange::Set), names(&["question"])); | |
| 51 | 51 | ]; | |
| 52 | 52 | ||
| 53 | 53 | /// The work items a commit (`?2`) pushed to repository `?1` resolved. | |
| 54 | − | const CLOSED_BY_COMMIT: &str = "SELECT i.workspace, i.number, i.title, i.state = 'closed' AS closed | |
| 54 | + | const CLOSED_BY_COMMIT: &str = "SELECT i.workspace, i.number, i.title, i.state = 'closed' AS closed, i.id AS id | |
| 55 | 55 | FROM work_item_repos r JOIN work_items i ON i.id = r.item_id | |
| 56 | 56 | WHERE r.repo_id = ?1 AND i.number IS NOT NULL | |
| 57 | 57 | AND EXISTS (SELECT 1 FROM json_each(i.resolutions) j | |
| ⋯ | |||
| 67 | 67 | ||
| 68 | 68 | #[derive(Deserialize)] | |
| 69 | 69 | struct ClosedRow { | |
| 70 | + | id: String, | |
| 70 | 71 | workspace: String, | |
| 71 | 72 | number: u32, | |
| 72 | 73 | title: String, | |
| ⋯ | |||
| 82 | 83 | pub labels: String, | |
| 83 | 84 | } | |
| 84 | 85 | ||
| 86 | + | /// Those of `rows` (what a commit resolved) that `viewer` may see. | |
| 87 | + | fn seen_closed(rows: Vec<ClosedRow>, held: &[Held], viewer: Option<&User>) -> Vec<ClosedItem> { | |
| 88 | + | rows.into_iter() | |
| 89 | + | .filter(|row| held.iter().any(|held| held.item.id == row.id && held.may(viewer, g1t_contracts::access::Capability::Read))) | |
| 90 | + | .map(|row| ClosedItem { workspace: row.workspace, number: row.number, title: row.title, closed: row.closed != 0 }) | |
| 91 | + | .collect() | |
| 92 | + | } | |
| 93 | + | ||
| 85 | 94 | /// A note of a landing: where, and by what. | |
| 86 | 95 | pub(crate) fn landing_note(path: &str, pull: Option<u32>, commit: Option<&str>) -> String { | |
| 87 | 96 | match (pull, commit) { | |
| ⋯ | |||
| 173 | 182 | None => format!("closed this in {what}"), | |
| 174 | 183 | }; | |
| 175 | 184 | self.note_item(id, by, &crate::pushed::closed_note(&text, session)).await?; | |
| 176 | − | self.item_resolved(&held, actor, true, landing.pull).await; | |
| 185 | + | self.item_resolved(&held, actor, true, Some(landing)).await; | |
| 177 | 186 | closed.push(held); | |
| 178 | 187 | continue; | |
| 179 | 188 | } | |
| ⋯ | |||
| 197 | 206 | format!("landed {what}; still to land in {}", missing.join(", ")) | |
| 198 | 207 | }; | |
| 199 | 208 | self.note_item(id, by, &crate::pushed::closed_note(&text, session)).await?; | |
| 200 | − | // Merged for it: what it blocks hears, though it stays open. | |
| 201 | − | self.item_resolved(&held, actor, false, landing.pull).await; | |
| 209 | + | // Landed for it: what it blocks hears, though it stays open | |
| 210 | + | // and they still wait on it. | |
| 211 | + | self.item_resolved(&held, actor, false, Some(landing)).await; | |
| 202 | 212 | } | |
| 203 | 213 | } | |
| 204 | 214 | for id in parts { | |
| ⋯ | |||
| 311 | 321 | } | |
| 312 | 322 | ||
| 313 | 323 | /// `closed_by_commit`: the work items a commit pushed to a repository | |
| 314 | − | /// resolved, for anyone who may read the repository. | |
| 324 | + | /// resolved, for anyone who may read the repository: those of them | |
| 325 | + | /// they may see. | |
| 315 | 326 | pub(crate) async fn closed_by_commit(&self, a: ClosedByCommitArgs) -> Result<Outcome<Vec<ClosedItem>>> { | |
| 316 | 327 | let Some(commit) = crate::commit_comments::valid_commit(&a.commit).filter(|commit| commit.len() >= 40) else { | |
| 317 | 328 | return Ok(Outcome::Ok(Vec::new())); | |
| ⋯ | |||
| 320 | 331 | Outcome::Ok(repo) => repo, | |
| 321 | 332 | Outcome::Fail(failure) => return Ok(Outcome::Fail(failure)), | |
| 322 | 333 | }; | |
| 323 | − | Ok(Outcome::Ok( | |
| 324 | − | self.db | |
| 325 | − | .prepare(CLOSED_BY_COMMIT) | |
| 326 | − | .bind(&[repo.id.as_str().into(), commit.into()])? | |
| 327 | − | .all() | |
| 328 | − | .await? | |
| 329 | − | .results::<ClosedRow>()? | |
| 330 | − | .into_iter() | |
| 331 | − | .map(|row| ClosedItem { workspace: row.workspace, number: row.number, title: row.title, closed: row.closed != 0 }) | |
| 332 | − | .collect(), | |
| 333 | − | )) | |
| 334 | + | let rows = self.db.prepare(CLOSED_BY_COMMIT).bind(&[repo.id.as_str().into(), commit.into()])?.all().await?.results::<ClosedRow>()?; | |
| 335 | + | let ids: Vec<String> = rows.iter().map(|row| row.id.clone()).collect(); | |
| 336 | + | let held = self.items_by_ids(&ids).await?; | |
| 337 | + | let viewer = a.viewer.as_ref(); | |
| 338 | + | Ok(Outcome::Ok(seen_closed(rows, &held, viewer))) | |
| 334 | 339 | } | |
| 335 | 340 | ||
| 336 | 341 | /// Where the other work items of `held`'s workspace landed, newest | |
| ⋯ | |||
| 353 | 358 | use super::*; | |
| 354 | 359 | ||
| 355 | 360 | #[test] | |
| 361 | + | fn a_commit_names_only_the_work_items_the_viewer_may_see() { | |
| 362 | + | use crate::items::rows_for_test::{held_for_test, person_for_test}; | |
| 363 | + | let row = |id: &str, number: u32| ClosedRow { id: id.into(), workspace: "acme".into(), number, title: format!("Item {number}"), closed: 1 }; | |
| 364 | + | // #1 filed in a public repository, #2 a members-only one. | |
| 365 | + | let held = vec![held_for_test(1, &[("rep_web", "origin", "acme", false)], &[]), held_for_test(2, &[], &[])]; | |
| 366 | + | let numbers = |viewer: Option<&User>| -> Vec<u32> { | |
| 367 | + | seen_closed(vec![row("wki_1", 1), row("wki_2", 2)], &held, viewer).iter().map(|item| item.number).collect() | |
| 368 | + | }; | |
| 369 | + | assert_eq!(numbers(None), vec![1]); | |
| 370 | + | assert_eq!(numbers(Some(&person_for_test("usr_9", None))), vec![1]); | |
| 371 | + | assert_eq!(numbers(Some(&person_for_test("usr_2", Some("acme")))), vec![1, 2]); | |
| 372 | + | } | |
| 373 | + | ||
| 374 | + | #[test] | |
| 356 | 375 | fn a_landing_is_named_by_its_pull_request_or_commit() { | |
| 357 | 376 | assert_eq!(landing_note("acme/web", Some(12), None), "acme/web#12"); | |
| 358 | 377 | assert_eq!(landing_note("acme/web", None, Some("abcdef1234")), "acme/web@abcdef1"); | |
| 125 | 125 | return Ok(Outcome::fail(FailureCode::Invalid, "A work item's origin is where it was filed; link a target instead.")); | |
| 126 | 126 | } | |
| 127 | 127 | if current.as_ref().is_some_and(|link| link.role == a.role) { | |
| 128 | − | return Ok(Outcome::Ok(held.item)); | |
| 128 | + | return Ok(Outcome::Ok(held.shown(Some(&a.actor)))); | |
| 129 | 129 | } | |
| 130 | 130 | self.db | |
| 131 | 131 | .prepare( | |
| ⋯ | |||
| 164 | 164 | } | |
| 165 | 165 | self.publish_item(kind, &after, Some(&a.actor), WorkItemEvent { repo: Some(path), role: Some(role), ..event }).await?; | |
| 166 | 166 | self.note_item(&after.item.id, &a.actor, &text).await?; | |
| 167 | − | Ok(Outcome::Ok(after.item)) | |
| 167 | + | Ok(Outcome::Ok(after.shown(Some(&a.actor)))) | |
| 168 | 168 | } | |
| 169 | 169 | } | |
| 22 | 22 | //! ([`RENAMED`], [`DELETED`]). | |
| 23 | 23 | ||
| 24 | 24 | use g1t_contracts::events::WorkMigrated; | |
| 25 | − | use g1t_contracts::repos::{AllIdsArgs, CreatorPage, RepoVisibility, VisibilityArgs}; | |
| 25 | + | use g1t_contracts::repos::{AllIdsArgs, CreatorPage, DeletedRepoPage, RepoVisibility, VisibilityArgs}; | |
| 26 | 26 | use g1t_contracts::time::rfc3339; | |
| 27 | 27 | use g1t_contracts::work::*; | |
| 28 | 28 | use g1t_kit::now_ms; | |
| ⋯ | |||
| 55 | 55 | (SELECT s.workspace FROM rulesets s WHERE s.repo_id = o.repo_id LIMIT 1))"; | |
| 56 | 56 | ||
| 57 | 57 | /// What finishing does, in one batch, `?1` the time (see the module's | |
| 58 | − | /// notes). Each statement changes nothing run again, and those that | |
| 59 | − | /// would are guarded by the job not being done yet. | |
| 58 | + | /// notes): [`place_sql`], each workspace's floor, and the job done. | |
| 60 | 59 | pub(crate) fn finish_sql() -> Vec<String> { | |
| 61 | − | let pending = "(SELECT done_at FROM work_jobs WHERE name = 'work_items') IS NULL"; | |
| 60 | + | let mut statements = place_sql(); | |
| 61 | + | statements.push("UPDATE work_jobs SET done_at = ?1, cursor = NULL WHERE name = 'work_items' AND done_at IS NULL".to_owned()); | |
| 62 | + | statements | |
| 63 | + | } | |
| 64 | + | ||
| 65 | + | /// Placing what the repositories recorded in `repo_workspaces` kept for | |
| 66 | + | /// themselves, in one batch, `?1` the time: their moved issues' workspaces | |
| 67 | + | /// and numbers, labels, milestones and numbers become their workspaces'. | |
| 68 | + | /// Only what is still theirs is moved, and then dropped, so run again it | |
| 69 | + | /// changes nothing; a repository recorded later (one deleted while the job | |
| 70 | + | /// ran, then restored, or the job for those, [`DELETED_REPOS_JOB`]) has | |
| 71 | + | /// its own moved then, and nothing of anyone else's twice. | |
| 72 | + | pub(crate) fn place_sql() -> Vec<String> { | |
| 73 | + | let recorded = "SELECT repo_id FROM repo_workspaces"; | |
| 74 | + | // A repository's milestones are still its own: not moved yet. | |
| 75 | + | let unmoved = "SELECT repo_id FROM repo_milestones WHERE repo_id IN (SELECT repo_id FROM repo_workspaces)"; | |
| 62 | 76 | vec![ | |
| 63 | 77 | // Each moved issue's workspace: its repository's. | |
| 64 | 78 | format!( | |
| ⋯ | |||
| 97 | 111 | GROUP BY workspace | |
| 98 | 112 | ON CONFLICT (workspace) DO UPDATE SET last = MAX(work_counters.last, excluded.last)" | |
| 99 | 113 | .to_owned(), | |
| 100 | − | // Labels merge by name; the first made keeps its color and words. | |
| 114 | + | // Labels merge by name, whatever its case (names are lowercase as | |
| 115 | + | // work items carry them); the first made keeps its color and words. | |
| 101 | 116 | "INSERT INTO labels (workspace, name, color, description, created_at) | |
| 102 | − | SELECT rw.workspace, l.name, l.color, l.description, l.created_at | |
| 117 | + | SELECT rw.workspace, lower(l.name), l.color, l.description, l.created_at | |
| 103 | 118 | FROM repo_labels l JOIN repo_workspaces rw ON rw.repo_id = l.repo_id | |
| 104 | 119 | WHERE true ORDER BY l.created_at, l.repo_id | |
| 105 | 120 | ON CONFLICT (workspace, name) DO UPDATE | |
| ⋯ | |||
| 108 | 123 | .to_owned(), | |
| 109 | 124 | // Milestones are numbered on after the workspace's own, oldest | |
| 110 | 125 | // first, and what was in each moves with it. | |
| 111 | − | format!( | |
| 112 | − | "INSERT INTO milestone_moves (repo_id, old_number, workspace, number) | |
| 113 | − | SELECT m.repo_id, m.number, rw.workspace, | |
| 114 | − | COALESCE((SELECT MAX(x.number) FROM milestones x WHERE x.workspace = rw.workspace), 0) | |
| 115 | − | + ROW_NUMBER() OVER (PARTITION BY rw.workspace ORDER BY m.created_at, m.repo_id, m.number) | |
| 116 | − | FROM repo_milestones m JOIN repo_workspaces rw ON rw.repo_id = m.repo_id | |
| 117 | − | WHERE {pending} | |
| 118 | − | AND NOT EXISTS (SELECT 1 FROM milestone_moves mm WHERE mm.repo_id = m.repo_id AND mm.old_number = m.number)" | |
| 119 | − | ), | |
| 126 | + | "INSERT INTO milestone_moves (repo_id, old_number, workspace, number) | |
| 127 | + | SELECT m.repo_id, m.number, rw.workspace, | |
| 128 | + | COALESCE((SELECT MAX(x.number) FROM milestones x WHERE x.workspace = rw.workspace), 0) | |
| 129 | + | + ROW_NUMBER() OVER (PARTITION BY rw.workspace ORDER BY m.created_at, m.repo_id, m.number) | |
| 130 | + | FROM repo_milestones m JOIN repo_workspaces rw ON rw.repo_id = m.repo_id | |
| 131 | + | WHERE NOT EXISTS (SELECT 1 FROM milestone_moves mm WHERE mm.repo_id = m.repo_id AND mm.old_number = m.number)" | |
| 132 | + | .to_owned(), | |
| 120 | 133 | "INSERT INTO milestones (workspace, number, origin_repo_id, title, description, due_on, state, created_at, updated_at, closed_at) | |
| 121 | 134 | SELECT mm.workspace, mm.number, m.repo_id, m.title, m.description, m.due_on, m.state, m.created_at, m.updated_at, m.closed_at | |
| 122 | 135 | FROM milestone_moves mm JOIN repo_milestones m ON m.repo_id = mm.repo_id AND m.number = mm.old_number | |
| 123 | 136 | WHERE true | |
| 124 | 137 | ON CONFLICT DO NOTHING" | |
| 125 | 138 | .to_owned(), | |
| 139 | + | // Only what was in a milestone of a repository moving now: a | |
| 140 | + | // repository with none keeps what its work items say. | |
| 126 | 141 | format!( | |
| 127 | 142 | "UPDATE work_items SET milestone = ( | |
| 128 | 143 | SELECT mm.number FROM milestone_moves mm JOIN work_item_repos o ON o.repo_id = mm.repo_id | |
| 129 | 144 | WHERE o.item_id = work_items.id AND o.role = 'origin' AND mm.old_number = work_items.milestone) | |
| 130 | − | WHERE milestone IS NOT NULL AND {pending} | |
| 131 | − | AND id IN (SELECT item_id FROM work_item_repos WHERE legacy_number IS NOT NULL)" | |
| 145 | + | WHERE milestone IS NOT NULL | |
| 146 | + | AND id IN (SELECT item_id FROM work_item_repos WHERE legacy_number IS NOT NULL AND role = 'origin' | |
| 147 | + | AND repo_id IN ({unmoved}))" | |
| 132 | 148 | ), | |
| 133 | 149 | format!( | |
| 134 | 150 | "UPDATE pulls SET milestone = ( | |
| 135 | 151 | SELECT mm.number FROM milestone_moves mm WHERE mm.repo_id = pulls.repo_id AND mm.old_number = pulls.milestone) | |
| 136 | − | WHERE milestone IS NOT NULL AND {pending}" | |
| 152 | + | WHERE milestone IS NOT NULL AND repo_id IN ({unmoved})" | |
| 137 | 153 | ), | |
| 154 | + | // Every number they used is below the workspace's floor, and its | |
| 155 | + | // sequence goes on above it. | |
| 156 | + | "UPDATE work_counters SET floor = MAX(floor, | |
| 157 | + | COALESCE((SELECT MAX(c.last) FROM counters c JOIN repo_workspaces rw ON rw.repo_id = c.repo_id | |
| 158 | + | WHERE rw.workspace = work_counters.workspace), 0)) | |
| 159 | + | WHERE true" | |
| 160 | + | .to_owned(), | |
| 161 | + | RAISE_FLOORS.to_owned(), | |
| 162 | + | "UPDATE work_counters SET last = floor WHERE last < floor".to_owned(), | |
| 138 | 163 | // What the repositories kept for themselves is the workspace's now. | |
| 139 | − | format!("DELETE FROM counters WHERE {pending}"), | |
| 140 | − | format!("DELETE FROM repo_labels WHERE {pending}"), | |
| 141 | − | format!("DELETE FROM repo_milestones WHERE {pending}"), | |
| 142 | − | "UPDATE work_jobs SET done_at = ?1, cursor = NULL WHERE name = 'work_items' AND done_at IS NULL".to_owned(), | |
| 164 | + | format!("DELETE FROM counters WHERE repo_id IN ({recorded})"), | |
| 165 | + | format!("DELETE FROM repo_labels WHERE repo_id IN ({recorded})"), | |
| 166 | + | format!("DELETE FROM repo_milestones WHERE repo_id IN ({recorded})"), | |
| 143 | 167 | ] | |
| 144 | 168 | } | |
| 145 | 169 | ||
| 170 | + | /// The job, after the first, that records repositories deleted but still | |
| 171 | + | /// restorable ([`Work::migrate_deleted_repos`]): the first job's paging | |
| 172 | + | /// left them out, and their work items with no path, private, and their | |
| 173 | + | /// labels and old numbers unmerged. | |
| 174 | + | pub(crate) const DELETED_REPOS_JOB: &str = "deleted_repos"; | |
| 175 | + | ||
| 176 | + | /// Every label a workspace's work items or pull requests carry that the | |
| 177 | + | /// workspace does not have, as a label (gray, no description) made at | |
| 178 | + | /// `?1`: one a repository had that was never merged. Run again, it adds | |
| 179 | + | /// nothing. | |
| 180 | + | pub(crate) const CARRIED_LABELS: [&str; 2] = [ | |
| 181 | + | "INSERT INTO labels (workspace, name, color, description, created_at) | |
| 182 | + | SELECT DISTINCT i.workspace, j.value, 'ededed', '', ?1 FROM work_items i, json_each(i.labels) j | |
| 183 | + | WHERE i.workspace IS NOT NULL AND j.value <> '' | |
| 184 | + | ON CONFLICT DO NOTHING", | |
| 185 | + | "INSERT INTO labels (workspace, name, color, description, created_at) | |
| 186 | + | SELECT DISTINCT rw.workspace, j.value, 'ededed', '', ?1 FROM pulls p JOIN repo_workspaces rw ON rw.repo_id = p.repo_id, json_each(p.labels) j | |
| 187 | + | WHERE j.value <> '' | |
| 188 | + | ON CONFLICT DO NOTHING", | |
| 189 | + | ]; | |
| 190 | + | ||
| 146 | 191 | /// A workspace renamed (`g1t_kit::rename`, `?1` its slug now, `?2` before): | |
| 147 | 192 | /// its work items, labels, milestones, numbers and repositories follow. | |
| 148 | 193 | pub(crate) const RENAMED: &[&str] = &[ | |
| ⋯ | |||
| 163 | 208 | ||
| 164 | 209 | /// A repository renamed or transferred (`g1t_kit::transfer`: `?1` its path | |
| 165 | 210 | /// now, `?3` its workspace now, `?5` its id): where it is now, and in the | |
| 166 | − | /// workspace it moved to, numbers go on after its pull requests', so a new | |
| 167 | − | /// one never takes a number it has. Its work items stay in their own | |
| 168 | − | /// workspace, linked to it. | |
| 211 | + | /// workspace it moved to, numbers go on after its pull requests' and its | |
| 212 | + | /// old issues', so a new one never takes a number it has. Those numbers | |
| 213 | + | /// were never that workspace's, so its floor rises above them too: `#N` | |
| 214 | + | /// read in the workspace's other repositories names none of them | |
| 215 | + | /// (refs.rs). A rename (`?4`, its workspace before, the same) changes no | |
| 216 | + | /// numbers. Its work items stay in their own workspace, linked to it. | |
| 169 | 217 | pub(crate) const TRANSFERRED: &[&str] = &[ | |
| 170 | − | "INSERT INTO repo_workspaces (repo_id, workspace, path, private) VALUES (?5, ?3, ?1, 1) | |
| 171 | − | ON CONFLICT (repo_id) DO UPDATE SET workspace = ?3, path = ?1", | |
| 218 | + | "INSERT INTO repo_workspaces (repo_id, workspace, path, private) VALUES (?5, lower(?3), ?1, 1) | |
| 219 | + | ON CONFLICT (repo_id) DO UPDATE SET workspace = lower(?3), path = ?1", | |
| 172 | 220 | "UPDATE work_item_repos SET path = ?1 WHERE repo_id = ?5", | |
| 173 | − | "INSERT INTO work_counters (workspace, last) | |
| 174 | − | SELECT ?3, MAX(number) FROM pulls WHERE repo_id = ?5 GROUP BY repo_id | |
| 175 | − | ON CONFLICT (workspace) DO UPDATE SET last = MAX(work_counters.last, excluded.last)", | |
| 221 | + | "INSERT INTO work_counters (workspace, last, floor) | |
| 222 | + | SELECT lower(?3), MAX(n), MAX(n) FROM ( | |
| 223 | + | SELECT MAX(number) AS n FROM pulls WHERE repo_id = ?5 | |
| 224 | + | UNION ALL SELECT MAX(legacy_number) FROM work_item_repos WHERE repo_id = ?5) | |
| 225 | + | WHERE lower(?3) <> lower(?4) | |
| 226 | + | HAVING MAX(n) IS NOT NULL | |
| 227 | + | ON CONFLICT (workspace) DO UPDATE SET last = MAX(work_counters.last, excluded.last), floor = MAX(work_counters.floor, excluded.floor)", | |
| 176 | 228 | ]; | |
| 177 | 229 | ||
| 230 | + | /// Raises each workspace's floor (`work_counters.floor`) to the highest | |
| 231 | + | /// number any of its repositories used on its own: a pull request opened | |
| 232 | + | /// before work items moved (by when the job finished, or `?1`, now, while | |
| 233 | + | /// it finishes) or an old issue's number. Run again, it changes nothing. | |
| 234 | + | /// migrations/0052_work_floors.sql does the same. | |
| 235 | + | pub(crate) const RAISE_FLOORS: &str = "UPDATE work_counters SET floor = MAX(floor, | |
| 236 | + | COALESCE((SELECT MAX(p.number) FROM pulls p JOIN repo_workspaces rw ON rw.repo_id = p.repo_id | |
| 237 | + | WHERE rw.workspace = work_counters.workspace | |
| 238 | + | AND p.created_at <= COALESCE((SELECT done_at FROM work_jobs WHERE name = 'work_items'), ?1)), 0), | |
| 239 | + | COALESCE((SELECT MAX(l.legacy_number) FROM work_item_repos l JOIN repo_workspaces rw ON rw.repo_id = l.repo_id | |
| 240 | + | WHERE rw.workspace = work_counters.workspace), 0)) | |
| 241 | + | WHERE true"; | |
| 242 | + | ||
| 178 | 243 | /// A workspace deleted (`g1t_kit::deleted`, `?1` its slug): its work | |
| 179 | 244 | /// items go, with everything kept for them, and its labels, milestones | |
| 180 | 245 | /// and numbers. | |
| ⋯ | |||
| 233 | 298 | return Ok(WorkMigration { done: true, ..WorkMigration::default() }); | |
| 234 | 299 | }; | |
| 235 | 300 | if job.done_at.is_some() { | |
| 236 | − | return Ok(WorkMigration { done: true, ..WorkMigration::default() }); | |
| 301 | + | return self.migrate_deleted_repos().await; | |
| 237 | 302 | } | |
| 238 | 303 | let mut cursor = job.cursor; | |
| 239 | 304 | let mut recorded = 0; | |
| ⋯ | |||
| 256 | 321 | Ok(WorkMigration { done: false, repos: recorded, ..WorkMigration::default() }) | |
| 257 | 322 | } | |
| 258 | 323 | ||
| 324 | + | /// The job after the first ([`DELETED_REPOS_JOB`]): records each | |
| 325 | + | /// repository deleted but not purged, a few pages a run, then places | |
| 326 | + | /// what it kept ([`place_sql`]) and gives its work items' labels to | |
| 327 | + | /// the workspace ([`CARRIED_LABELS`]). Done once; run again, it | |
| 328 | + | /// changes nothing. | |
| 329 | + | async fn migrate_deleted_repos(&self) -> Result<WorkMigration> { | |
| 330 | + | let job = self | |
| 331 | + | .db | |
| 332 | + | .prepare("SELECT cursor, done_at FROM work_jobs WHERE name = ?") | |
| 333 | + | .bind(&[DELETED_REPOS_JOB.into()])? | |
| 334 | + | .first::<JobRow>(None) | |
| 335 | + | .await?; | |
| 336 | + | let Some(job) = job.filter(|job| job.done_at.is_none()) else { | |
| 337 | + | return Ok(WorkMigration { done: true, ..WorkMigration::default() }); | |
| 338 | + | }; | |
| 339 | + | let mut cursor = job.cursor; | |
| 340 | + | let mut recorded = 0; | |
| 341 | + | for _ in 0..PAGES_PER_RUN { | |
| 342 | + | let page: DeletedRepoPage = | |
| 343 | + | g1t_kit::call(&self.repos, "deleted_repos", &AllIdsArgs { after: cursor.clone(), limit: PAGE }).await?; | |
| 344 | + | let repos: Vec<(String, String, String, bool)> = page | |
| 345 | + | .repos | |
| 346 | + | .iter() | |
| 347 | + | .map(|repo| (repo.id.clone(), repo.namespace.clone(), repo.name.clone(), repo.is_private)) | |
| 348 | + | .collect(); | |
| 349 | + | recorded += self.record_repos(&repos).await?; | |
| 350 | + | match page.next { | |
| 351 | + | Some(next) => { | |
| 352 | + | self.db | |
| 353 | + | .prepare("UPDATE work_jobs SET cursor = ? WHERE name = ?") | |
| 354 | + | .bind(&[next.as_str().into(), DELETED_REPOS_JOB.into()])? | |
| 355 | + | .run() | |
| 356 | + | .await?; | |
| 357 | + | cursor = Some(next); | |
| 358 | + | } | |
| 359 | + | None => { | |
| 360 | + | self.place(true).await?; | |
| 361 | + | self.db | |
| 362 | + | .prepare("UPDATE work_jobs SET done_at = ?, cursor = NULL WHERE name = ? AND done_at IS NULL") | |
| 363 | + | .bind(&[rfc3339(now_ms()).into(), DELETED_REPOS_JOB.into()])? | |
| 364 | + | .run() | |
| 365 | + | .await?; | |
| 366 | + | return Ok(WorkMigration { done: true, repos: recorded, ..WorkMigration::default() }); | |
| 367 | + | } | |
| 368 | + | } | |
| 369 | + | } | |
| 370 | + | Ok(WorkMigration { done: false, repos: recorded, ..WorkMigration::default() }) | |
| 371 | + | } | |
| 372 | + | ||
| 373 | + | /// Records repositories (id, workspace, name, private) where they are. | |
| 374 | + | async fn record_repos(&self, repos: &[(String, String, String, bool)]) -> Result<u32> { | |
| 375 | + | if repos.is_empty() { | |
| 376 | + | return Ok(0); | |
| 377 | + | } | |
| 378 | + | let mut statements = Vec::new(); | |
| 379 | + | for (id, namespace, name, private) in repos { | |
| 380 | + | let path = format!("{namespace}/{name}"); | |
| 381 | + | let binds = [id.as_str().into(), namespace.to_lowercase().into(), path.as_str().into(), u32::from(*private).into()]; | |
| 382 | + | for sql in RECORD_REPO { | |
| 383 | + | statements.push(self.db.prepare(sql).bind(&binds[..g1t_kit::rename::parameters(sql)])?); | |
| 384 | + | } | |
| 385 | + | } | |
| 386 | + | self.db.batch(statements).await?; | |
| 387 | + | Ok(repos.len() as u32) | |
| 388 | + | } | |
| 389 | + | ||
| 390 | + | /// [`place_sql`], and with `carried`, [`CARRIED_LABELS`], in one batch. | |
| 391 | + | async fn place(&self, carried: bool) -> Result<()> { | |
| 392 | + | let now = rfc3339(now_ms()); | |
| 393 | + | let mut sql = place_sql(); | |
| 394 | + | if carried { | |
| 395 | + | sql.extend(CARRIED_LABELS.iter().map(|sql| (*sql).to_owned())); | |
| 396 | + | } | |
| 397 | + | let statements = sql | |
| 398 | + | .iter() | |
| 399 | + | .map(|sql| { | |
| 400 | + | let binds = [now.as_str().into()]; | |
| 401 | + | self.db.prepare(sql).bind(&binds[..g1t_kit::rename::parameters(sql)]) | |
| 402 | + | }) | |
| 403 | + | .collect::<Result<Vec<_>>>()?; | |
| 404 | + | self.db.batch(statements).await?; | |
| 405 | + | Ok(()) | |
| 406 | + | } | |
| 407 | + | ||
| 408 | + | /// `repo.restored`: where the repository is again, and, once work items | |
| 409 | + | /// have moved, whatever of its own is still to place (a repository | |
| 410 | + | /// deleted while the job ran). | |
| 411 | + | pub(super) async fn repo_restored(&self, data: &serde_json::Value) -> Result<()> { | |
| 412 | + | let Ok(restored) = serde_json::from_value::<Created>(data.clone()) else { | |
| 413 | + | return Ok(()); | |
| 414 | + | }; | |
| 415 | + | self.record_repos(&[(restored.repo_id, restored.namespace, restored.name, restored.is_private)]).await?; | |
| 416 | + | if self.items_moved().await? { | |
| 417 | + | self.place(false).await?; | |
| 418 | + | } | |
| 419 | + | Ok(()) | |
| 420 | + | } | |
| 421 | + | ||
| 259 | 422 | /// Records one page of repositories: where each is, and whether it is | |
| 260 | 423 | /// private. | |
| 261 | 424 | async fn record_page(&self, page: &CreatorPage) -> Result<u32> { | |
| 186 | 186 | .into_iter() | |
| 187 | 187 | .filter_map(ItemRow::held) | |
| 188 | 188 | .filter(|held| held.may(a.viewer.as_ref(), Capability::Read)) | |
| 189 | − | .map(|held| held.item) | |
| 189 | + | .map(|held| held.shown(a.viewer.as_ref())) | |
| 190 | 190 | .collect(), | |
| 191 | 191 | None => Vec::new(), | |
| 192 | 192 | }; |
| 61 | 61 | #[cfg(test)] | |
| 62 | 62 | mod tests; | |
| 63 | 63 | ||
| 64 | − | pub(crate) use dependencies::Brief; | |
| 65 | 64 | pub(crate) use labels::Labeled; | |
| 66 | 65 | pub(crate) use landing::{Landing, PastLanding}; | |
| 67 | 66 | pub(crate) use migration::{DELETED, RENAMED, TRANSFERRED}; | |
| 68 | 67 | pub(crate) use reading::{held_rows, select}; | |
| 69 | 68 | pub(crate) use refs::{MAX_CLOSES, Named, NamedItems}; | |
| 70 | − | pub(crate) use rows::{Held, path_of, workspace_of}; | |
| 69 | + | pub(crate) use rows::{Held, path_of, repo_readable, workspace_of}; | |
| 70 | + | ||
| 71 | + | /// Work items and people made up for other modules' tests. | |
| 72 | + | #[cfg(test)] | |
| 73 | + | pub(crate) mod rows_for_test { | |
| 74 | + | pub(crate) use super::rows::{held_for_test, person_for_test}; | |
| 75 | + | } | |
| 71 | 76 | ||
| 72 | 77 | // --- Reading --------------------------------------------------------------- | |
| 73 | 78 | ||
| ⋯ | |||
| 125 | 130 | work.repo_created(data).await | |
| 126 | 131 | } | |
| 127 | 132 | ||
| 133 | + | /// `repo.restored`: where the repository is again, and what it kept for | |
| 134 | + | /// itself, placed. | |
| 135 | + | pub(crate) async fn repo_restored(work: &Work, data: &serde_json::Value) -> Result<()> { | |
| 136 | + | work.repo_restored(data).await | |
| 137 | + | } | |
| 138 | + | ||
| 128 | 139 | /// `repo.visibility_changed`: whether its work items are public. | |
| 129 | 140 | pub(crate) async fn repo_visibility(work: &Work, data: &serde_json::Value) -> Result<()> { | |
| 130 | 141 | work.repo_visibility(data).await | |
| ⋯ | |||
| 144 | 155 | } | |
| 145 | 156 | ||
| 146 | 157 | /// The work items `text`, written in `repo`, closes and is part of, with | |
| 147 | − | /// `own` (a pull request's own work item) closed first. | |
| 148 | − | pub(crate) async fn named(work: &Work, repo: &Repo, own: Option<&str>, text: &str) -> Result<NamedItems> { | |
| 149 | − | work.named_items(repo, own, text).await | |
| 158 | + | /// `own` (a pull request's own work item) closed first: those each of | |
| 159 | + | /// `who` may close, or see (refs.rs `admits`). | |
| 160 | + | pub(crate) async fn named(work: &Work, repo: &Repo, own: Option<&str>, text: &str, who: &[&User]) -> Result<NamedItems> { | |
| 161 | + | work.named_items(repo, own, text, who).await | |
| 162 | + | } | |
| 163 | + | ||
| 164 | + | /// Whether each of `who` may have `held` closed by naming it (`closing`), | |
| 165 | + | /// or linked as something a change is part of. | |
| 166 | + | pub(crate) fn admits(held: &Held, who: &[&User], closing: bool) -> bool { | |
| 167 | + | refs::admits(held, who, closing) | |
| 150 | 168 | } | |
| 151 | 169 | ||
| 152 | 170 | /// Lands `closes` (work items, by id) in `repo` by `landing` and links | |
| ⋯ | |||
| 214 | 232 | work.still_open(workspace, numbers, cleared).await | |
| 215 | 233 | } | |
| 216 | 234 | ||
| 217 | − | /// The work items of `workspace` that `numbers` names, in brief. | |
| 218 | − | pub(crate) async fn briefs(work: &Work, workspace: &str, numbers: &[u32]) -> Result<Vec<Brief>> { | |
| 219 | − | work.briefs(workspace, numbers).await | |
| 235 | + | /// Those of `ids` (repositories) that `viewer` may read, as recorded in | |
| 236 | + | /// `repo_workspaces`. | |
| 237 | + | pub(crate) async fn readable_repo_ids(work: &Work, viewer: Option<&User>, ids: &[String]) -> Result<std::collections::HashSet<String>> { | |
| 238 | + | work.readable_repo_ids(viewer, ids).await | |
| 239 | + | } | |
| 240 | + | ||
| 241 | + | /// The work items of `workspace` that `numbers` names and `viewer` may | |
| 242 | + | /// see, in no particular order. | |
| 243 | + | pub(crate) async fn visible_numbered(work: &Work, workspace: &str, numbers: &[u32], viewer: Option<&User>) -> Result<Vec<Held>> { | |
| 244 | + | work.visible_numbered(workspace, numbers, viewer).await | |
| 220 | 245 | } | |
| 221 | 246 | ||
| 222 | − | /// The work items that name `held` in their `blocked_by`. | |
| 223 | − | pub(crate) async fn blocked_items(work: &Work, held: &Held) -> Result<Vec<g1t_contracts::work::ItemBrief>> { | |
| 224 | − | work.blocked_items(held).await | |
| 247 | + | /// The work items that name `held` in their `blocked_by`, those `viewer` | |
| 248 | + | /// may see. | |
| 249 | + | pub(crate) async fn blocked_items(work: &Work, held: &Held, viewer: Option<&User>) -> Result<Vec<g1t_contracts::work::ItemBrief>> { | |
| 250 | + | work.blocked_items(held, viewer).await | |
| 225 | 251 | } | |
| 9 | 9 | ||
| 10 | 10 | /// The next number of workspace `?1`'s sequence, taken in one statement so | |
| 11 | 11 | /// two openings never get the same one. A pull request of repository `?2` | |
| 12 | − | /// never takes a number one of that repository's has (a repository moved | |
| 13 | − | /// in from another workspace brings its own), and a workspace's first | |
| 14 | − | /// number comes after every number its repositories used before work | |
| 15 | − | /// items moved to the workspace. | |
| 12 | + | /// never takes a number one of that repository's has, a pull request's or | |
| 13 | + | /// an old issue's (a repository moved in from another workspace, or | |
| 14 | + | /// restored, brings its own), and a workspace's first number comes after | |
| 15 | + | /// every number its repositories used before work items moved to the | |
| 16 | + | /// workspace. While the job that moves them runs, which repositories are | |
| 17 | + | /// the workspace's is not known yet: a number then comes after every | |
| 18 | + | /// number any repository used, so it is above the floor the job finds. | |
| 16 | 19 | pub(crate) const NEXT_NUMBER: &str = "INSERT INTO work_counters (workspace, last) | |
| 17 | − | SELECT ?1, MAX(COALESCE(MAX(c.last), 0), COALESCE((SELECT MAX(number) FROM pulls WHERE repo_id = ?2), 0)) + 1 | |
| 20 | + | SELECT ?1, MAX(COALESCE(MAX(c.last), 0), COALESCE((SELECT MAX(number) FROM pulls WHERE repo_id = ?2), 0), | |
| 21 | + | COALESCE((SELECT MAX(legacy_number) FROM work_item_repos WHERE repo_id = ?2), 0)) + 1 | |
| 18 | 22 | FROM counters c | |
| 19 | 23 | WHERE c.repo_id = ?2 OR c.repo_id IN (SELECT repo_id FROM repo_workspaces WHERE workspace = ?1) | |
| 24 | + | OR EXISTS (SELECT 1 FROM work_jobs WHERE name = 'work_items' AND done_at IS NULL) | |
| 20 | 25 | ON CONFLICT (workspace) DO UPDATE SET last = MAX(work_counters.last + 1, excluded.last) | |
| 21 | 26 | RETURNING last AS n"; | |
| 22 | 27 |
| 40 | 40 | Ok(result.results::<ItemRow>()?.into_iter().filter_map(ItemRow::held).collect()) | |
| 41 | 41 | } | |
| 42 | 42 | ||
| 43 | + | /// What a repository the reader may not read is called instead. | |
| 44 | + | const UNNAMED: &str = "a private repository"; | |
| 45 | + | ||
| 46 | + | /// `text` with each of `hidden` (`owner/name`, any case) that stands as a | |
| 47 | + | /// name of its own, not inside a longer one, called [`UNNAMED`]; what | |
| 48 | + | /// follows it (`#5`, `@abc1234`) goes too. | |
| 49 | + | pub(crate) fn redact_paths(text: &str, hidden: &[String]) -> String { | |
| 50 | + | let part = |c: char| c.is_alphanumeric() || matches!(c, '-' | '_' | '.' | '/'); | |
| 51 | + | let hidden: Vec<String> = hidden.iter().map(|path| path.to_lowercase()).collect(); | |
| 52 | + | let mut out = String::with_capacity(text.len()); | |
| 53 | + | let mut at = 0; | |
| 54 | + | 'scan: while let Some(c) = text[at..].chars().next() { | |
| 55 | + | if text[..at].chars().next_back().is_none_or(|before| !part(before)) { | |
| 56 | + | for path in &hidden { | |
| 57 | + | let end = at + path.len(); | |
| 58 | + | let here = text.get(at..end).is_some_and(|found| found.eq_ignore_ascii_case(path)); | |
| 59 | + | if here && text[end..].chars().next().is_none_or(|after| !part(after)) { | |
| 60 | + | out.push_str(UNNAMED); | |
| 61 | + | // Its pull request or commit goes with it. | |
| 62 | + | let mut rest = end; | |
| 63 | + | if let Some(mark) = text[end..].chars().next().filter(|mark| matches!(mark, '#' | '@')) { | |
| 64 | + | rest += mark.len_utf8(); | |
| 65 | + | rest += text[rest..].chars().take_while(char::is_ascii_alphanumeric).map(char::len_utf8).sum::<usize>(); | |
| 66 | + | } | |
| 67 | + | at = rest; | |
| 68 | + | continue 'scan; | |
| 69 | + | } | |
| 70 | + | } | |
| 71 | + | } | |
| 72 | + | out.push(c); | |
| 73 | + | at += c.len_utf8(); | |
| 74 | + | } | |
| 75 | + | out | |
| 76 | + | } | |
| 77 | + | ||
| 43 | 78 | impl Work { | |
| 44 | 79 | /// A work item by workspace and number, whoever is asking. | |
| 45 | 80 | pub(super) async fn item(&self, workspace: &str, number: u32) -> Result<Option<Held>> { | |
| ⋯ | |||
| 78 | 113 | ) | |
| 79 | 114 | } | |
| 80 | 115 | ||
| 116 | + | /// Those of `ids` (repositories) that `viewer` may read, as | |
| 117 | + | /// `repo_workspaces` records them: one not recorded is not. | |
| 118 | + | pub(super) async fn readable_repo_ids(&self, viewer: Option<&User>, ids: &[String]) -> Result<std::collections::HashSet<String>> { | |
| 119 | + | #[derive(Deserialize)] | |
| 120 | + | struct Recorded { | |
| 121 | + | repo_id: String, | |
| 122 | + | workspace: String, | |
| 123 | + | private: u8, | |
| 124 | + | } | |
| 125 | + | if ids.is_empty() { | |
| 126 | + | return Ok(std::collections::HashSet::new()); | |
| 127 | + | } | |
| 128 | + | let recorded = self | |
| 129 | + | .db | |
| 130 | + | .prepare("SELECT repo_id, workspace, private FROM repo_workspaces WHERE repo_id IN (SELECT value FROM json_each(?))") | |
| 131 | + | .bind(&[serde_json::to_string(ids)?.into()])? | |
| 132 | + | .all() | |
| 133 | + | .await? | |
| 134 | + | .results::<Recorded>()?; | |
| 135 | + | Ok(recorded | |
| 136 | + | .into_iter() | |
| 137 | + | .filter(|row| super::rows::repo_readable(viewer, &row.repo_id, Some(&row.workspace), Some(row.private))) | |
| 138 | + | .map(|row| row.repo_id) | |
| 139 | + | .collect()) | |
| 140 | + | } | |
| 141 | + | ||
| 81 | 142 | /// A work item, if `viewer` may do `capability` on it. | |
| 82 | 143 | pub(super) async fn item_for(&self, viewer: &Viewer, workspace: &str, number: u32, capability: Capability) -> Result<Outcome<Held>> { | |
| 83 | 144 | let Some(held) = self.item(workspace, number).await? else { | |
| ⋯ | |||
| 161 | 222 | rows.into_iter() | |
| 162 | 223 | .filter_map(ItemRow::held) | |
| 163 | 224 | .filter(|held| held.may(viewer, Capability::Read)) | |
| 164 | − | .map(|held| held.item) | |
| 225 | + | .map(|held| held.shown(viewer)) | |
| 165 | 226 | .collect(), | |
| 166 | 227 | )) | |
| 167 | 228 | } | |
| ⋯ | |||
| 183 | 244 | let reacted = rows(2)?.results::<crate::reactions::ReactionRow>()?; | |
| 184 | 245 | let reactions = crate::reactions::attach(&reacted, &held.item.id, &mut comments); | |
| 185 | 246 | let pulls = self.readable_pulls(&a.viewer, pulls).await?; | |
| 186 | − | Ok(Outcome::Ok(WorkItemDetail { item: held.item, pulls, comments, reactions })) | |
| 247 | + | // What g1t noted names repositories ("added acme/api as a target", | |
| 248 | + | // "closed this by merging acme/api#5"): those the viewer may not | |
| 249 | + | // read are not named. | |
| 250 | + | let hidden = self.hidden_paths(&held, a.viewer.as_ref()).await?; | |
| 251 | + | if !hidden.is_empty() { | |
| 252 | + | for comment in comments.iter_mut().filter(|comment| comment.kind == CommentKind::Event) { | |
| 253 | + | comment.body = redact_paths(&comment.body, &hidden); | |
| 254 | + | } | |
| 255 | + | } | |
| 256 | + | Ok(Outcome::Ok(WorkItemDetail { item: held.shown(a.viewer.as_ref()), pulls, comments, reactions })) | |
| 257 | + | } | |
| 258 | + | ||
| 259 | + | /// The paths (`owner/name`) of the repositories of `held`'s workspace, | |
| 260 | + | /// and of those it is linked to, that `viewer` may not read. | |
| 261 | + | async fn hidden_paths(&self, held: &Held, viewer: Option<&User>) -> Result<Vec<String>> { | |
| 262 | + | #[derive(Deserialize)] | |
| 263 | + | struct Recorded { | |
| 264 | + | repo_id: String, | |
| 265 | + | workspace: String, | |
| 266 | + | path: String, | |
| 267 | + | private: u8, | |
| 268 | + | } | |
| 269 | + | let recorded = self | |
| 270 | + | .db | |
| 271 | + | .prepare( | |
| 272 | + | "SELECT repo_id, workspace, path, private FROM repo_workspaces | |
| 273 | + | WHERE workspace = ?1 OR repo_id IN (SELECT repo_id FROM work_item_repos WHERE item_id = ?2)", | |
| 274 | + | ) | |
| 275 | + | .bind(&[held.item.workspace.as_str().into(), held.item.id.as_str().into()])? | |
| 276 | + | .all() | |
| 277 | + | .await? | |
| 278 | + | .results::<Recorded>()?; | |
| 279 | + | Ok(recorded | |
| 280 | + | .into_iter() | |
| 281 | + | .filter(|row| !super::rows::repo_readable(viewer, &row.repo_id, Some(&row.workspace), Some(row.private))) | |
| 282 | + | .map(|row| row.path) | |
| 283 | + | .filter(|path| !path.is_empty()) | |
| 284 | + | .collect()) | |
| 187 | 285 | } | |
| 188 | 286 | ||
| 189 | 287 | /// Those of `pulls` whose repositories `viewer` may read, each naming | |
| ⋯ | |||
| 206 | 304 | .collect()) | |
| 207 | 305 | } | |
| 208 | 306 | } | |
| 307 | + | ||
| 308 | + | #[cfg(test)] | |
| 309 | + | mod tests { | |
| 310 | + | use super::redact_paths; | |
| 311 | + | ||
| 312 | + | #[test] | |
| 313 | + | fn notes_do_not_name_repositories_the_reader_may_not_read() { | |
| 314 | + | let hidden = vec!["acme/secret".to_owned()]; | |
| 315 | + | assert_eq!(redact_paths("added acme/secret as a target", &hidden), "added a private repository as a target"); | |
| 316 | + | assert_eq!(redact_paths("closed this by merging Acme/Secret#5", &hidden), "closed this by merging a private repository"); | |
| 317 | + | assert_eq!( | |
| 318 | + | redact_paths("landed acme/secret@abc1234; still to land in acme/web", &hidden), | |
| 319 | + | "landed a private repository; still to land in acme/web" | |
| 320 | + | ); | |
| 321 | + | // Only the name itself, never a longer one it starts or ends. | |
| 322 | + | assert_eq!(redact_paths("added acme/secret-tools as a target", &hidden), "added acme/secret-tools as a target"); | |
| 323 | + | assert_eq!(redact_paths("added xacme/secret", &hidden), "added xacme/secret"); | |
| 324 | + | } | |
| 325 | + | } | |
| 11 | 11 | use g1t_contracts::repos::{ReadableArgs, Repo}; | |
| 12 | 12 | use g1t_contracts::work::refs::{self, Lookup, Ref, RefScope, first_found}; | |
| 13 | 13 | use g1t_contracts::work::*; | |
| 14 | − | use g1t_contracts::Outcome; | |
| 14 | + | use g1t_contracts::{Outcome, User}; | |
| 15 | 15 | use serde::Deserialize; | |
| 16 | 16 | use worker::Result; | |
| 17 | 17 | use worker::wasm_bindgen::JsValue; | |
| 18 | 18 | ||
| 19 | − | use super::rows::path_of; | |
| 19 | + | use super::rows::{Held, path_of}; | |
| 20 | 20 | use crate::Work; | |
| 21 | 21 | use crate::rows::ValueRow; | |
| 22 | 22 | ||
| ⋯ | |||
| 24 | 24 | pub(crate) const MAX_CLOSES: usize = refs::MAX_REFS; | |
| 25 | 25 | ||
| 26 | 26 | /// What `#N` could name, read in repository `?1` (NULL for none) of | |
| 27 | − | /// workspace `?2`: a pull request of the repository, a work item filed | |
| 28 | − | /// there under that old number, the workspace's work item, and the newest | |
| 29 | − | /// pull request of that number in the workspace's other repositories. | |
| 27 | + | /// workspace `?2`: a pull request of the repository, a work item of the | |
| 28 | + | /// workspace filed there under that old number (a repository moved in from | |
| 29 | + | /// another workspace brings old numbers that name that workspace's work | |
| 30 | + | /// items, never this one's), the workspace's work item, and a pull request | |
| 31 | + | /// of that number in the workspace's other repositories. Only a number | |
| 32 | + | /// above the workspace's floor (`work_counters.floor`, the highest any of | |
| 33 | + | /// its repositories used on its own) is the workspace's alone; one at or | |
| 34 | + | /// below it may be any of its repositories' old pull requests, and names | |
| 35 | + | /// none of them from elsewhere. | |
| 30 | 36 | pub(crate) const REF_SQL: &str = "SELECT | |
| 31 | 37 | (SELECT 1 FROM pulls WHERE repo_id = ?1 AND number = ?3) AS pull_here, | |
| 32 | − | (SELECT item_id FROM work_item_repos WHERE repo_id = ?1 AND legacy_number = ?3) AS legacy, | |
| 38 | + | (SELECT item_id FROM work_item_repos WHERE repo_id = ?1 AND legacy_number = ?3 | |
| 39 | + | AND item_id IN (SELECT id FROM work_items WHERE workspace = ?2 OR workspace IS NULL)) AS legacy, | |
| 33 | 40 | (SELECT id FROM work_items WHERE workspace = ?2 AND number = ?3) AS item, | |
| 34 | 41 | (SELECT repo_id FROM pulls WHERE number = ?3 AND repo_id IS NOT ?1 | |
| 35 | 42 | AND repo_id IN (SELECT repo_id FROM repo_workspaces WHERE workspace = ?2) | |
| 43 | + | AND number > COALESCE((SELECT floor FROM work_counters WHERE workspace = ?2), 0) | |
| 36 | 44 | ORDER BY created_at DESC LIMIT 1) AS pull_there"; | |
| 37 | 45 | ||
| 38 | 46 | /// What [`REF_SQL`] found. | |
| ⋯ | |||
| 113 | 121 | /// `resolve_ref`: what `#number` names for the viewer, read in `repo` | |
| 114 | 122 | /// or in `workspace` (`refs`). | |
| 115 | 123 | pub(crate) async fn resolve_ref(&self, a: ResolveRefArgs) -> Result<RefTarget> { | |
| 116 | − | // Read in a repository the viewer may not read (a member without | |
| 117 | − | // Code following an old link), `#N` still names the work items | |
| 118 | − | // they may see; its pull requests stay out of reach. | |
| 124 | + | // Read in a repository the viewer may not read, `#N` names | |
| 125 | + | // nothing, exactly as in one that does not exist: anything else | |
| 126 | + | // would tell them a private repository is there. | |
| 119 | 127 | let (repo, workspace) = match &a.repo { | |
| 120 | − | Some(path) => match self.repo(path, &a.viewer).await? { | |
| 121 | − | Outcome::Ok(repo) => { | |
| 128 | + | Some(path) => match read_in(self.repo(path, &a.viewer).await?) { | |
| 129 | + | Some(repo) => { | |
| 122 | 130 | let workspace = repo.namespace.to_lowercase(); | |
| 123 | 131 | (Some((repo.id.clone(), Some(path_of(&repo)))), workspace) | |
| 124 | 132 | } | |
| 125 | − | Outcome::Fail(_) => match self.recorded_repo(&format!("{}/{}", path.namespace, path.name)).await? { | |
| 126 | − | Some((id, workspace)) => (Some((id, None)), workspace), | |
| 127 | − | None => return Ok(RefTarget::None), | |
| 128 | − | }, | |
| 133 | + | None => return Ok(RefTarget::None), | |
| 129 | 134 | }, | |
| 130 | 135 | None => (None, a.workspace.trim().to_lowercase()), | |
| 131 | 136 | }; | |
| ⋯ | |||
| 172 | 177 | Ok(RefTarget::None) | |
| 173 | 178 | } | |
| 174 | 179 | ||
| 175 | − | /// The id and workspace of the repository at `path` (`owner/name`), as | |
| 176 | − | /// recorded, whoever is asking. | |
| 177 | − | async fn recorded_repo(&self, path: &str) -> Result<Option<(String, String)>> { | |
| 178 | − | #[derive(Deserialize)] | |
| 179 | − | struct Recorded { | |
| 180 | − | repo_id: String, | |
| 181 | − | workspace: String, | |
| 182 | − | } | |
| 183 | − | Ok(self | |
| 184 | − | .db | |
| 185 | − | .prepare("SELECT repo_id, workspace FROM repo_workspaces WHERE lower(path) = lower(?)") | |
| 186 | − | .bind(&[path.into()])? | |
| 187 | − | .first::<Recorded>(None) | |
| 188 | − | .await? | |
| 189 | − | .map(|found| (found.repo_id, found.workspace))) | |
| 190 | − | } | |
| 191 | − | ||
| 192 | 180 | /// The work item `reference` names, read in `repo`: its id, when it is | |
| 193 | 181 | /// one of the workspace's work items. | |
| 194 | 182 | pub(super) async fn ref_item(&self, repo: &Repo, reference: &Ref) -> Result<Option<String>> { | |
| ⋯ | |||
| 215 | 203 | .bind(&[named.as_str().into()])? | |
| 216 | 204 | .first::<ValueRow>(None) | |
| 217 | 205 | .await?; | |
| 206 | + | // A repository not known names nothing, as resolve_ref | |
| 207 | + | // reads it: never the workspace's work item of that number. | |
| 218 | 208 | match known { | |
| 219 | 209 | Some(row) => (Some(row.value), true), | |
| 220 | − | None => (None, false), | |
| 210 | + | None => return Ok(None), | |
| 221 | 211 | } | |
| 222 | 212 | } | |
| 223 | 213 | } | |
| ⋯ | |||
| 230 | 220 | } | |
| 231 | 221 | ||
| 232 | 222 | /// The work items `text`, written in `repo`, closes and is part of, | |
| 233 | − | /// with `own` (a pull request's own work item) closed first. | |
| 234 | − | pub(super) async fn named_items(&self, repo: &Repo, own: Option<&str>, text: &str) -> Result<NamedItems> { | |
| 223 | + | /// with `own` (a pull request's own work item) closed first. Each of | |
| 224 | + | /// `who` (whoever wrote it, merges it or pushed it) must be able to | |
| 225 | + | /// see what it names ([`admits`]); what one of them may not is left | |
| 226 | + | /// unresolved, as a reference to nothing. | |
| 227 | + | pub(super) async fn named_items(&self, repo: &Repo, own: Option<&str>, text: &str, who: &[&User]) -> Result<NamedItems> { | |
| 235 | 228 | let named = refs::named(text); | |
| 236 | − | let mut closes: Vec<String> = own.map(str::to_owned).into_iter().collect(); | |
| 229 | + | let mut closes: Vec<String> = Vec::new(); | |
| 237 | 230 | for reference in &named.closes { | |
| 238 | 231 | if let Some(id) = self.ref_item(repo, reference).await? | |
| 239 | 232 | && !closes.contains(&id) | |
| ⋯ | |||
| 245 | 238 | for reference in &named.parts { | |
| 246 | 239 | if let Some(id) = self.ref_item(repo, reference).await? | |
| 247 | 240 | && !closes.contains(&id) | |
| 241 | + | && own != Some(id.as_str()) | |
| 248 | 242 | && !parts.contains(&id) | |
| 249 | 243 | { | |
| 250 | 244 | parts.push(id); | |
| 251 | 245 | } | |
| 252 | 246 | } | |
| 253 | − | closes.truncate(MAX_CLOSES); | |
| 254 | − | parts.truncate(MAX_CLOSES); | |
| 255 | − | let mut all = closes.clone(); | |
| 256 | − | all.extend(parts.iter().cloned()); | |
| 247 | + | let mut all: Vec<String> = own.map(str::to_owned).into_iter().collect(); | |
| 248 | + | all.extend(closes.iter().take(MAX_CLOSES * 2).cloned()); | |
| 249 | + | all.extend(parts.iter().take(MAX_CLOSES * 2).cloned()); | |
| 257 | 250 | let held = self.items_by_ids(&all).await?; | |
| 258 | − | let number = |id: &String| held.iter().find(|held| held.item.id == *id).map(|held| (id.clone(), held.item.number)); | |
| 259 | − | Ok(NamedItems { closes: closes.iter().filter_map(number).collect(), parts: parts.iter().filter_map(number).collect() }) | |
| 251 | + | let found = |id: &String| held.iter().find(|held| held.item.id == *id); | |
| 252 | + | // Its own work item first, whoever may see it: the pull request | |
| 253 | + | // was made for it. | |
| 254 | + | let mut closing: Vec<(String, u32)> = own | |
| 255 | + | .map(str::to_owned) | |
| 256 | + | .into_iter() | |
| 257 | + | .filter_map(|id| found(&id).map(|held| (id, held.item.number))) | |
| 258 | + | .collect(); | |
| 259 | + | for id in &closes { | |
| 260 | + | if let Some(held) = found(id).filter(|held| admits(held, who, true)) | |
| 261 | + | && !closing.iter().any(|(known, _)| known == id) | |
| 262 | + | { | |
| 263 | + | closing.push((id.clone(), held.item.number)); | |
| 264 | + | } | |
| 265 | + | } | |
| 266 | + | let mut parted: Vec<(String, u32)> = Vec::new(); | |
| 267 | + | for id in &parts { | |
| 268 | + | if let Some(held) = found(id).filter(|held| admits(held, who, false)) { | |
| 269 | + | parted.push((id.clone(), held.item.number)); | |
| 270 | + | } | |
| 271 | + | } | |
| 272 | + | closing.truncate(MAX_CLOSES); | |
| 273 | + | parted.truncate(MAX_CLOSES); | |
| 274 | + | Ok(NamedItems { closes: closing, parts: parted }) | |
| 275 | + | } | |
| 276 | + | } | |
| 277 | + | ||
| 278 | + | /// The repository `#N` is read in, as the viewer found it: none when they | |
| 279 | + | /// may not read it, exactly as when there is no such repository. | |
| 280 | + | fn read_in<T>(found: Outcome<T>) -> Option<T> { | |
| 281 | + | match found { | |
| 282 | + | Outcome::Ok(repo) => Some(repo), | |
| 283 | + | Outcome::Fail(_) => None, | |
| 260 | 284 | } | |
| 261 | 285 | } | |
| 262 | 286 | ||
| 287 | + | /// Whether everyone in `who` may have `held` closed (`closing`: they may | |
| 288 | + | /// triage it) or linked as something they are part of (they may see it). | |
| 289 | + | pub(crate) fn admits(held: &Held, who: &[&User], closing: bool) -> bool { | |
| 290 | + | let capability = if closing { Capability::Triage } else { Capability::Read }; | |
| 291 | + | who.iter().all(|user| held.may(Some(user), capability)) | |
| 292 | + | } | |
| 293 | + | ||
| 263 | 294 | #[cfg(test)] | |
| 264 | 295 | mod tests { | |
| 265 | 296 | use super::*; | |
| 266 | 297 | ||
| 267 | 298 | #[test] | |
| 299 | + | fn a_repository_the_viewer_may_not_read_is_read_as_none() { | |
| 300 | + | assert_eq!(read_in(Outcome::<u8>::fail(g1t_contracts::FailureCode::NotFound, "Repository not found.")), None); | |
| 301 | + | assert_eq!(read_in(Outcome::Ok(1)), Some(1)); | |
| 302 | + | } | |
| 303 | + | ||
| 304 | + | #[test] | |
| 305 | + | fn only_what_everyone_involved_may_see_is_closed_or_linked() { | |
| 306 | + | use crate::items::rows_for_test::{held_for_test, person_for_test}; | |
| 307 | + | let member = person_for_test("usr_2", Some("acme")); | |
| 308 | + | // An outside collaborator with Write on public acme/web. | |
| 309 | + | let mut outsider = person_for_test("usr_9", None); | |
| 310 | + | outsider.grants.push(g1t_contracts::access::RepoGrant { | |
| 311 | + | repo_id: "rep_web".into(), | |
| 312 | + | workspace: "acme".into(), | |
| 313 | + | role: g1t_contracts::access::RepoRole::Write, | |
| 314 | + | team: None, | |
| 315 | + | }); | |
| 316 | + | let filed_in_web = held_for_test(1, &[("rep_web", "origin", "acme", false)], &[]); | |
| 317 | + | let members_only = held_for_test(2, &[("rep_web", "target", "acme", false)], &[]); | |
| 318 | + | let private = held_for_test(3, &[("rep_secret", "origin", "acme", true)], &[]); | |
| 319 | + | assert!(admits(&filed_in_web, &[&outsider], true)); | |
| 320 | + | assert!(!admits(&members_only, &[&outsider], true)); | |
| 321 | + | assert!(!admits(&members_only, &[&outsider], false)); | |
| 322 | + | assert!(!admits(&private, &[&outsider], true)); | |
| 323 | + | assert!(admits(&private, &[&member], true)); | |
| 324 | + | // Merged by a member, written by the outsider: what both may. | |
| 325 | + | assert!(!admits(&private, &[&member, &outsider], true)); | |
| 326 | + | // Someone who may only read a public work item may not close it. | |
| 327 | + | let reader = person_for_test("usr_8", None); | |
| 328 | + | assert!(admits(&filed_in_web, &[&reader], false)); | |
| 329 | + | assert!(!admits(&filed_in_web, &[&reader], true)); | |
| 330 | + | } | |
| 331 | + | ||
| 332 | + | #[test] | |
| 268 | 333 | fn a_reference_names_the_first_thing_found_in_order() { | |
| 269 | 334 | let row = |pull_here: bool, legacy: Option<&str>, item: Option<&str>, there: Option<&str>| RefRow { | |
| 270 | 335 | pull_here: pull_here.then_some(1), | |
| 110 | 110 | /// The origin's id, workspace and privacy, while the repository is | |
| 111 | 111 | /// known: one deleted for good no longer makes it public. | |
| 112 | 112 | origin: Option<(String, String, bool)>, | |
| 113 | + | /// Each linked repository's id with its workspace and privacy, while | |
| 114 | + | /// known: what deciding who may see that it is linked needs. | |
| 115 | + | known: Vec<(String, String, bool)>, | |
| 116 | + | /// The repository of each of `item.resolutions`, by id, in order. | |
| 117 | + | resolved_in: Vec<String>, | |
| 113 | 118 | } | |
| 114 | 119 | ||
| 115 | 120 | impl Held { | |
| 121 | + | /// Whether `viewer` may read linked repository `repo_id`: one whose | |
| 122 | + | /// workspace and privacy are not known is not shown. | |
| 123 | + | pub(crate) fn may_read_repo(&self, viewer: Option<&User>, repo_id: &str) -> bool { | |
| 124 | + | self.known | |
| 125 | + | .iter() | |
| 126 | + | .find(|(id, _, _)| id == repo_id) | |
| 127 | + | .is_some_and(|(id, namespace, private)| repo_readable(viewer, id, Some(namespace), Some(u8::from(*private)))) | |
| 128 | + | } | |
| 129 | + | ||
| 130 | + | /// The work item as `viewer` is shown it: only the linked repositories | |
| 131 | + | /// and the resolutions in repositories they may read. Who may see a | |
| 132 | + | /// work item is its workspace's and its origin's business; that it is | |
| 133 | + | /// linked to a private repository is that repository's. | |
| 134 | + | pub(crate) fn shown(self, viewer: Option<&User>) -> WorkItem { | |
| 135 | + | let readable = |repo_id: &str| self.may_read_repo(viewer, repo_id); | |
| 136 | + | let repos: Vec<ItemRepo> = self.item.repos.iter().filter(|repo| readable(&repo.repo_id)).cloned().collect(); | |
| 137 | + | let resolutions: Vec<Resolution> = self | |
| 138 | + | .item | |
| 139 | + | .resolutions | |
| 140 | + | .iter() | |
| 141 | + | .zip(&self.resolved_in) | |
| 142 | + | .filter(|(_, repo_id)| readable(repo_id)) | |
| 143 | + | .map(|(resolution, _)| resolution.clone()) | |
| 144 | + | .collect(); | |
| 145 | + | WorkItem { repos, resolutions, ..self.item } | |
| 146 | + | } | |
| 147 | + | ||
| 116 | 148 | pub(crate) fn origin(&self) -> Option<RepoRef<'_>> { | |
| 117 | 149 | self.origin.as_ref().map(|(id, namespace, private)| RepoRef { id, namespace, private: *private }) | |
| 118 | 150 | } | |
| ⋯ | |||
| 122 | 154 | self.item.repos.iter().map(|repo| repo.repo_id.clone()).collect() | |
| 123 | 155 | } | |
| 124 | 156 | ||
| 125 | − | /// The repository its events are filed under: its origin, else the | |
| 126 | − | /// first it is linked to. | |
| 157 | + | /// The repository its events are filed under: its origin, or none for | |
| 158 | + | /// one filed in the workspace. Never another linked repository: its | |
| 159 | + | /// readers may not see a members-only work item, and events filed | |
| 160 | + | /// under a repository are shown to whoever reads it. The others hear | |
| 161 | + | /// of it through the event's `repoIds`. | |
| 127 | 162 | pub(crate) fn event_repo(&self) -> Option<&str> { | |
| 128 | − | self.item.repos.first().map(|repo| repo.repo_id.as_str()) | |
| 163 | + | self.item.repos.iter().find(|repo| repo.role == LinkRole::Origin).map(|repo| repo.repo_id.as_str()) | |
| 129 | 164 | } | |
| 130 | 165 | ||
| 131 | 166 | pub(crate) fn may(&self, viewer: Option<&User>, capability: Capability) -> bool { | |
| ⋯ | |||
| 143 | 178 | } | |
| 144 | 179 | } | |
| 145 | 180 | ||
| 181 | + | /// Whether `viewer` may read repository `repo_id`, as `repo_workspaces` | |
| 182 | + | /// records it (its `workspace` and `private`): one not recorded is not. | |
| 183 | + | pub(crate) fn repo_readable(viewer: Option<&User>, repo_id: &str, workspace: Option<&str>, private: Option<u8>) -> bool { | |
| 184 | + | workspace.is_some_and(|namespace| { | |
| 185 | + | access::can(viewer, RepoRef { id: repo_id, namespace, private: private != Some(0) }, Capability::Read) | |
| 186 | + | }) | |
| 187 | + | } | |
| 188 | + | ||
| 146 | 189 | pub(crate) fn no_item<T>() -> Outcome<T> { | |
| 147 | 190 | Outcome::fail(FailureCode::NotFound, "Work item not found.") | |
| 148 | 191 | } | |
| ⋯ | |||
| 172 | 215 | let namespace = link.workspace.clone()?; | |
| 173 | 216 | Some((link.repo_id.clone(), namespace, link.private != Some(0))) | |
| 174 | 217 | }); | |
| 218 | + | let known: Vec<(String, String, bool)> = links | |
| 219 | + | .iter() | |
| 220 | + | .filter_map(|link| Some((link.repo_id.clone(), link.workspace.clone()?, link.private != Some(0)))) | |
| 221 | + | .collect(); | |
| 175 | 222 | let mut repos: Vec<ItemRepo> = links | |
| 176 | 223 | .into_iter() | |
| 177 | 224 | .map(|link| ItemRepo { | |
| ⋯ | |||
| 185 | 232 | sort_repos(&mut repos); | |
| 186 | 233 | let stored: Vec<StoredResolution> = | |
| 187 | 234 | self.resolutions.as_deref().and_then(|json| serde_json::from_str(json).ok()).unwrap_or_default(); | |
| 235 | + | let resolved_in: Vec<String> = stored.iter().map(|resolution| resolution.repo_id.clone()).collect(); | |
| 188 | 236 | let resolutions = stored | |
| 189 | 237 | .into_iter() | |
| 190 | 238 | .map(|resolution| Resolution { | |
| ⋯ | |||
| 222 | 270 | repos, | |
| 223 | 271 | }, | |
| 224 | 272 | origin, | |
| 273 | + | known, | |
| 274 | + | resolved_in, | |
| 275 | + | }) | |
| 276 | + | } | |
| 277 | + | } | |
| 278 | + | ||
| 279 | + | /// A work item of `acme` linked as `links` (`[repo_id, role, workspace, | |
| 280 | + | /// private]`, a workspace of "" for a repository not known) and resolved | |
| 281 | + | /// in `resolved` (repository ids), for tests. | |
| 282 | + | #[cfg(test)] | |
| 283 | + | pub(crate) fn held_for_test(number: u32, links: &[(&str, &str, &str, bool)], resolved: &[&str]) -> Held { | |
| 284 | + | let links: Vec<serde_json::Value> = links | |
| 285 | + | .iter() | |
| 286 | + | .map(|(id, role, workspace, private)| { | |
| 287 | + | serde_json::json!({ | |
| 288 | + | "repo_id": id, "path": format!("{workspace}/{id}"), "role": role, "landed": 0, | |
| 289 | + | "workspace": if workspace.is_empty() { serde_json::Value::Null } else { (*workspace).into() }, | |
| 290 | + | "private": u8::from(*private), | |
| 291 | + | }) | |
| 225 | 292 | }) | |
| 293 | + | .collect(); | |
| 294 | + | let resolutions: Vec<serde_json::Value> = resolved.iter().map(|id| serde_json::json!({ "repo_id": id, "pull": 1 })).collect(); | |
| 295 | + | let row: ItemRow = serde_json::from_value(serde_json::json!({ | |
| 296 | + | "id": format!("wki_{number}"), "workspace": "acme", "number": number, "title": format!("Item {number}"), "body": "", | |
| 297 | + | "labels": "[]", "state": "open", "reason": null, "resolutions": serde_json::to_string(&resolutions).unwrap(), | |
| 298 | + | "author_id": "usr_1", "author_name": "ana", "created_at": "", "updated_at": "", "closed_at": null, "assignees": "[]", | |
| 299 | + | "links": serde_json::to_string(&links).unwrap(), | |
| 300 | + | })) | |
| 301 | + | .unwrap(); | |
| 302 | + | row.held().unwrap() | |
| 303 | + | } | |
| 304 | + | ||
| 305 | + | /// A verified person, a member (with the default Write) of `member_of` | |
| 306 | + | /// if given, for tests. | |
| 307 | + | #[cfg(test)] | |
| 308 | + | pub(crate) fn person_for_test(id: &str, member_of: Option<&str>) -> User { | |
| 309 | + | User { | |
| 310 | + | id: id.to_owned(), | |
| 311 | + | username: id.trim_start_matches("usr_").to_owned(), | |
| 312 | + | verified: true, | |
| 313 | + | workspaces: member_of.map(g1t_contracts::Membership::member).into_iter().collect(), | |
| 314 | + | ..User::default() | |
| 226 | 315 | } | |
| 227 | 316 | } | |
| 228 | 317 | ||
| ⋯ | |||
| 231 | 320 | use super::*; | |
| 232 | 321 | ||
| 233 | 322 | #[test] | |
| 323 | + | fn events_are_filed_under_the_origin_only() { | |
| 324 | + | let filed = held_for_test(1, &[("rep_web", "origin", "acme", false), ("rep_api", "target", "acme", false)], &[]); | |
| 325 | + | assert_eq!(filed.event_repo(), Some("rep_web")); | |
| 326 | + | // Filed in the workspace: none, not the public repository it targets. | |
| 327 | + | let members_only = held_for_test(2, &[("rep_public", "target", "acme", false)], &[]); | |
| 328 | + | assert_eq!(members_only.event_repo(), None); | |
| 329 | + | assert_eq!(members_only.repo_ids(), vec!["rep_public".to_owned()]); | |
| 330 | + | } | |
| 331 | + | ||
| 332 | + | #[test] | |
| 333 | + | fn a_viewer_is_shown_only_the_repositories_they_may_read() { | |
| 334 | + | let held = held_for_test( | |
| 335 | + | 3, | |
| 336 | + | &[("rep_web", "origin", "acme", false), ("rep_secret", "target", "acme", true), ("rep_gone", "touched", "", false)], | |
| 337 | + | &["rep_web", "rep_secret"], | |
| 338 | + | ); | |
| 339 | + | let shown = held.clone().shown(None); | |
| 340 | + | assert_eq!(shown.repos.iter().map(|repo| repo.repo_id.as_str()).collect::<Vec<_>>(), vec!["rep_web"]); | |
| 341 | + | assert_eq!(shown.resolutions.len(), 1); | |
| 342 | + | assert_eq!(shown.resolutions[0].repo, "acme/rep_web"); | |
| 343 | + | // A member who reads every repository sees them all, but for one | |
| 344 | + | // that is not known. | |
| 345 | + | let member = person_for_test("usr_2", Some("acme")); | |
| 346 | + | let shown = held.shown(Some(&member)); | |
| 347 | + | assert_eq!(shown.repos.len(), 2); | |
| 348 | + | assert_eq!(shown.resolutions.len(), 2); | |
| 349 | + | } | |
| 350 | + | ||
| 351 | + | #[test] | |
| 234 | 352 | fn a_workspace_is_the_one_given_or_its_repositorys() { | |
| 235 | 353 | assert_eq!(workspace_of(" ", Some(&RepoPath { namespace: "Acme".into(), name: "web".into() })), "acme"); | |
| 236 | 354 | assert_eq!(workspace_of("Globex", None), "globex"); | |
Comments
Sign in to comment.