Signed by hannaSSH key fingerprint: SHA256:4g9cWhkAAw8gwqhJUcVbSnGEvdGwklW+1Aa/fMUu59k
assets/base.css +36 −0
| @@ -706,6 +706,42 @@ body.drawer-open .drawer-backdrop { | |||
| 706 | 706 | } | |
| 707 | 707 | } | |
| 708 | 708 | ||
| 709 | + | /* Issue/PR dependency lists in the sidebar. */ | |
| 710 | + | .dep-list { | |
| 711 | + | list-style: none; | |
| 712 | + | display: flex; | |
| 713 | + | flex-direction: column; | |
| 714 | + | gap: 0.35rem; | |
| 715 | + | margin-bottom: 0.5rem; | |
| 716 | + | } | |
| 717 | + | .dep-row { | |
| 718 | + | display: flex; | |
| 719 | + | align-items: center; | |
| 720 | + | gap: 0.4rem; | |
| 721 | + | font-size: 0.9em; | |
| 722 | + | } | |
| 723 | + | .dep-row > a { | |
| 724 | + | flex: 1; | |
| 725 | + | min-width: 0; | |
| 726 | + | color: var(--fb-fg); | |
| 727 | + | overflow: hidden; | |
| 728 | + | text-overflow: ellipsis; | |
| 729 | + | white-space: nowrap; | |
| 730 | + | } | |
| 731 | + | .dep-row .icon-btn { | |
| 732 | + | width: 1.5rem; | |
| 733 | + | height: 1.5rem; | |
| 734 | + | flex: none; | |
| 735 | + | } | |
| 736 | + | .dep-add { | |
| 737 | + | display: flex; | |
| 738 | + | gap: 0.4rem; | |
| 739 | + | } | |
| 740 | + | .dep-add input { | |
| 741 | + | flex: 1; | |
| 742 | + | min-width: 0; | |
| 743 | + | } | |
| 744 | + | ||
| 709 | 745 | /* Pagination controls for long lists. */ | |
| 710 | 746 | .pagination { | |
| 711 | 747 | display: flex; | |
crates/store/src/issues.rs +122 −0
| @@ -111,6 +111,26 @@ impl Store { | |||
| 111 | 111 | Ok(row.as_ref().map(|r| self.map_issue(r)).transpose()?) | |
| 112 | 112 | } | |
| 113 | 113 | ||
| 114 | + | /// Fetch an issue or PR by repo and number, of either kind (for dependency | |
| 115 | + | /// references, which cross issues and PRs). | |
| 116 | + | /// | |
| 117 | + | /// # Errors | |
| 118 | + | /// | |
| 119 | + | /// Returns [`StoreError::Query`] on a database failure. | |
| 120 | + | pub async fn issue_by_number_any( | |
| 121 | + | &self, | |
| 122 | + | repo_id: &str, | |
| 123 | + | number: i64, | |
| 124 | + | ) -> Result<Option<Issue>, StoreError> { | |
| 125 | + | let sql = format!("SELECT {ISSUE_COLUMNS} FROM issues WHERE repo_id = $1 AND number = $2"); | |
| 126 | + | let row = sqlx::query(AssertSqlSafe(sql)) | |
| 127 | + | .bind(repo_id) | |
| 128 | + | .bind(number) | |
| 129 | + | .fetch_optional(&self.pool) | |
| 130 | + | .await?; | |
| 131 | + | Ok(row.as_ref().map(|r| self.map_issue(r)).transpose()?) | |
| 132 | + | } | |
| 133 | + | ||
| 114 | 134 | /// Fetch an issue/PR by id. | |
| 115 | 135 | /// | |
| 116 | 136 | /// # Errors | |
| @@ -530,6 +550,108 @@ impl Store { | |||
| 530 | 550 | Ok(()) | |
| 531 | 551 | } | |
| 532 | 552 | ||
| 553 | + | // ---- Dependencies ---- | |
| 554 | + | ||
| 555 | + | /// Record that `issue_id` is blocked by `depends_on_id` (idempotent). | |
| 556 | + | /// | |
| 557 | + | /// # Errors | |
| 558 | + | /// | |
| 559 | + | /// Returns [`StoreError::Query`] on a database failure. | |
| 560 | + | pub async fn add_dependency( | |
| 561 | + | &self, | |
| 562 | + | issue_id: &str, | |
| 563 | + | depends_on_id: &str, | |
| 564 | + | ) -> Result<(), StoreError> { | |
| 565 | + | sqlx::query( | |
| 566 | + | "INSERT INTO issue_dependencies (issue_id, depends_on_id, created_at) \ | |
| 567 | + | VALUES ($1, $2, $3) ON CONFLICT (issue_id, depends_on_id) DO NOTHING", | |
| 568 | + | ) | |
| 569 | + | .bind(issue_id) | |
| 570 | + | .bind(depends_on_id) | |
| 571 | + | .bind(now_ms()) | |
| 572 | + | .execute(&self.pool) | |
| 573 | + | .await?; | |
| 574 | + | Ok(()) | |
| 575 | + | } | |
| 576 | + | ||
| 577 | + | /// Remove a dependency edge. | |
| 578 | + | /// | |
| 579 | + | /// # Errors | |
| 580 | + | /// | |
| 581 | + | /// Returns [`StoreError::Query`] on a database failure. | |
| 582 | + | pub async fn remove_dependency( | |
| 583 | + | &self, | |
| 584 | + | issue_id: &str, | |
| 585 | + | depends_on_id: &str, | |
| 586 | + | ) -> Result<(), StoreError> { | |
| 587 | + | sqlx::query("DELETE FROM issue_dependencies WHERE issue_id = $1 AND depends_on_id = $2") | |
| 588 | + | .bind(issue_id) | |
| 589 | + | .bind(depends_on_id) | |
| 590 | + | .execute(&self.pool) | |
| 591 | + | .await?; | |
| 592 | + | Ok(()) | |
| 593 | + | } | |
| 594 | + | ||
| 595 | + | /// The issues/PRs that `issue_id` is blocked by (its dependencies). | |
| 596 | + | /// | |
| 597 | + | /// # Errors | |
| 598 | + | /// | |
| 599 | + | /// Returns [`StoreError::Query`] on a database failure. | |
| 600 | + | pub async fn dependencies_of(&self, issue_id: &str) -> Result<Vec<Issue>, StoreError> { | |
| 601 | + | self.related_issues( | |
| 602 | + | "SELECT i.id, i.repo_id, i.number, i.author_id, i.title, i.body, i.state, \ | |
| 603 | + | i.is_pull, i.assignee_id, i.created_at, i.updated_at, i.closed_at \ | |
| 604 | + | FROM issue_dependencies d JOIN issues i ON i.id = d.depends_on_id \ | |
| 605 | + | WHERE d.issue_id = $1 ORDER BY i.number ASC", | |
| 606 | + | issue_id, | |
| 607 | + | ) | |
| 608 | + | .await | |
| 609 | + | } | |
| 610 | + | ||
| 611 | + | /// The issues/PRs blocked by `issue_id` (its dependents). | |
| 612 | + | /// | |
| 613 | + | /// # Errors | |
| 614 | + | /// | |
| 615 | + | /// Returns [`StoreError::Query`] on a database failure. | |
| 616 | + | pub async fn dependents_of(&self, issue_id: &str) -> Result<Vec<Issue>, StoreError> { | |
| 617 | + | self.related_issues( | |
| 618 | + | "SELECT i.id, i.repo_id, i.number, i.author_id, i.title, i.body, i.state, \ | |
| 619 | + | i.is_pull, i.assignee_id, i.created_at, i.updated_at, i.closed_at \ | |
| 620 | + | FROM issue_dependencies d JOIN issues i ON i.id = d.issue_id \ | |
| 621 | + | WHERE d.depends_on_id = $1 ORDER BY i.number ASC", | |
| 622 | + | issue_id, | |
| 623 | + | ) | |
| 624 | + | .await | |
| 625 | + | } | |
| 626 | + | ||
| 627 | + | /// Count `issue_id`'s dependencies that are still open (i.e. blocking it). | |
| 628 | + | /// | |
| 629 | + | /// # Errors | |
| 630 | + | /// | |
| 631 | + | /// Returns [`StoreError::Query`] on a database failure. | |
| 632 | + | pub async fn open_dependency_count(&self, issue_id: &str) -> Result<i64, StoreError> { | |
| 633 | + | let row = sqlx::query( | |
| 634 | + | "SELECT COUNT(*) AS n FROM issue_dependencies d JOIN issues i ON i.id = d.depends_on_id \ | |
| 635 | + | WHERE d.issue_id = $1 AND i.state = 'open'", | |
| 636 | + | ) | |
| 637 | + | .bind(issue_id) | |
| 638 | + | .fetch_one(&self.pool) | |
| 639 | + | .await?; | |
| 640 | + | Ok(row.try_get("n")?) | |
| 641 | + | } | |
| 642 | + | ||
| 643 | + | /// Run a dependency join query bound to one issue id and map the rows. | |
| 644 | + | async fn related_issues(&self, sql: &str, issue_id: &str) -> Result<Vec<Issue>, StoreError> { | |
| 645 | + | let rows = sqlx::query(AssertSqlSafe(sql.to_string())) | |
| 646 | + | .bind(issue_id) | |
| 647 | + | .fetch_all(&self.pool) | |
| 648 | + | .await?; | |
| 649 | + | rows.iter() | |
| 650 | + | .map(|r| self.map_issue(r)) | |
| 651 | + | .collect::<Result<_, _>>() | |
| 652 | + | .map_err(Into::into) | |
| 653 | + | } | |
| 654 | + | ||
| 533 | 655 | /// Map an `issues` row (selected via [`ISSUE_COLUMNS`]) to an [`Issue`]. | |
| 534 | 656 | fn map_issue(&self, row: &AnyRow) -> Result<Issue, sqlx::Error> { | |
| 535 | 657 | Ok(Issue { | |
crates/web/src/issues.rs +151 −0
| @@ -220,6 +220,8 @@ pub(crate) async fn view( | |||
| 220 | 220 | Some(id) => Some(username(state, id).await), | |
| 221 | 221 | None => None, | |
| 222 | 222 | }; | |
| 223 | + | let deps = state.store.dependencies_of(&issue.id).await?; | |
| 224 | + | let dependents = state.store.dependents_of(&issue.id).await?; | |
| 223 | 225 | ||
| 224 | 226 | let can_write = viewer.is_some() && ctx.access >= auth::Access::Write; | |
| 225 | 227 | let is_author = viewer.as_ref().is_some_and(|v| v.id == issue.author_id); | |
| @@ -279,6 +281,7 @@ pub(crate) async fn view( | |||
| 279 | 281 | (label_form(&issue, &repo_labels, &issue_labels, &csrf)) | |
| 280 | 282 | } | |
| 281 | 283 | } | |
| 284 | + | (deps_section(&issue, &ctx, &deps, &dependents, &csrf, can_write)) | |
| 282 | 285 | } | |
| 283 | 286 | } | |
| 284 | 287 | } | |
| @@ -392,6 +395,58 @@ pub(crate) fn label_form( | |||
| 392 | 395 | } | |
| 393 | 396 | } | |
| 394 | 397 | ||
| 398 | + | /// The dependencies sidebar section: "Blocked by" (with add/remove for writers) | |
| 399 | + | /// and "Blocks" (read-only). Shared by the issue and PR views. | |
| 400 | + | pub(crate) fn deps_section( | |
| 401 | + | issue: &Issue, | |
| 402 | + | ctx: &RepoCtx, | |
| 403 | + | deps: &[Issue], | |
| 404 | + | dependents: &[Issue], | |
| 405 | + | csrf: &str, | |
| 406 | + | can_write: bool, | |
| 407 | + | ) -> Markup { | |
| 408 | + | let base = format!("/{}/{}", ctx.owner.username, ctx.repo.path); | |
| 409 | + | let row = |dep: &Issue, removable: bool| { | |
| 410 | + | let seg = if dep.is_pull { "pulls" } else { "issues" }; | |
| 411 | + | html! { | |
| 412 | + | li class="dep-row" { | |
| 413 | + | (state_icon(dep.state, dep.is_pull)) | |
| 414 | + | a href=(format!("{base}/-/{seg}/{}", dep.number)) { | |
| 415 | + | span class="muted mono" { "#" (dep.number) } " " (dep.title) | |
| 416 | + | } | |
| 417 | + | @if removable && can_write { | |
| 418 | + | form method="post" action=(format!("/issue/{}/deps/remove", issue.id)) class="dep-remove" { | |
| 419 | + | input type="hidden" name="_csrf" value=(csrf); | |
| 420 | + | input type="hidden" name="depends_on" value=(dep.id); | |
| 421 | + | button class="icon-btn" type="submit" aria-label="Remove dependency" { (icon(Icon::X)) } | |
| 422 | + | } | |
| 423 | + | } | |
| 424 | + | } | |
| 425 | + | } | |
| 426 | + | }; | |
| 427 | + | html! { | |
| 428 | + | section { | |
| 429 | + | h3 { "Blocked by" } | |
| 430 | + | @if deps.is_empty() { | |
| 431 | + | p class="muted" { "Nothing" } | |
| 432 | + | } @else { | |
| 433 | + | ul class="dep-list" { @for d in deps { (row(d, true)) } } | |
| 434 | + | } | |
| 435 | + | @if can_write { | |
| 436 | + | form method="post" action=(format!("/issue/{}/deps/add", issue.id)) class="side-form dep-add" { | |
| 437 | + | input type="hidden" name="_csrf" value=(csrf); | |
| 438 | + | input type="number" name="number" min="1" placeholder="#123" aria-label="Depends on number" required; | |
| 439 | + | button class="btn" type="submit" { "Add" } | |
| 440 | + | } | |
| 441 | + | } | |
| 442 | + | @if !dependents.is_empty() { | |
| 443 | + | h3 { "Blocks" } | |
| 444 | + | ul class="dep-list" { @for d in dependents { (row(d, false)) } } | |
| 445 | + | } | |
| 446 | + | } | |
| 447 | + | } | |
| 448 | + | } | |
| 449 | + | ||
| 395 | 450 | /// Resolve a username for display, falling back to the id. | |
| 396 | 451 | pub(crate) async fn username(state: &AppState, user_id: &str) -> String { | |
| 397 | 452 | state | |
| @@ -707,6 +762,102 @@ pub async fn set_labels( | |||
| 707 | 762 | respond_redirect(result) | |
| 708 | 763 | } | |
| 709 | 764 | ||
| 765 | + | /// Add-dependency form: the number of the blocking issue/PR. | |
| 766 | + | #[derive(Debug, Deserialize)] | |
| 767 | + | pub struct DepAddForm { | |
| 768 | + | /// The number of the issue/PR this one depends on. | |
| 769 | + | #[serde(default)] | |
| 770 | + | number: i64, | |
| 771 | + | /// CSRF token. | |
| 772 | + | #[serde(rename = "_csrf")] | |
| 773 | + | csrf: String, | |
| 774 | + | } | |
| 775 | + | ||
| 776 | + | /// `POST /issue/{id}/deps/add` — mark this issue/PR blocked by another (Write). | |
| 777 | + | pub async fn deps_add( | |
| 778 | + | State(state): State<AppState>, | |
| 779 | + | MaybeUser(viewer): MaybeUser, | |
| 780 | + | jar: CookieJar, | |
| 781 | + | headers: HeaderMap, | |
| 782 | + | Path(id): Path<String>, | |
| 783 | + | Form(form): Form<DepAddForm>, | |
| 784 | + | ) -> Response { | |
| 785 | + | if verify_csrf(&jar, &headers, Some(&form.csrf)).is_err() { | |
| 786 | + | return AppError::Forbidden.into_response(); | |
| 787 | + | } | |
| 788 | + | let result = async { | |
| 789 | + | let user = viewer.ok_or(AppError::Forbidden)?; | |
| 790 | + | let issue = state | |
| 791 | + | .store | |
| 792 | + | .issue_by_id(&id) | |
| 793 | + | .await? | |
| 794 | + | .ok_or(AppError::NotFound)?; | |
| 795 | + | let (repo, access) = repo_of_issue(&state, Some(&user), &issue).await?; | |
| 796 | + | if access < auth::Access::Write { | |
| 797 | + | return Err(AppError::Forbidden); | |
| 798 | + | } | |
| 799 | + | // Resolve the referenced number within the same repo (issue or PR). | |
| 800 | + | let target = state | |
| 801 | + | .store | |
| 802 | + | .issue_by_number_any(&repo.id, form.number) | |
| 803 | + | .await? | |
| 804 | + | .ok_or_else(|| AppError::BadRequest(format!("No issue or PR #{}.", form.number)))?; | |
| 805 | + | if target.id == issue.id { | |
| 806 | + | return Err(AppError::BadRequest( | |
| 807 | + | "An item cannot depend on itself.".to_string(), | |
| 808 | + | )); | |
| 809 | + | } | |
| 810 | + | state.store.add_dependency(&issue.id, &target.id).await?; | |
| 811 | + | Ok::<String, AppError>(issue_url(&state, &repo, &issue).await) | |
| 812 | + | } | |
| 813 | + | .await; | |
| 814 | + | respond_redirect(result) | |
| 815 | + | } | |
| 816 | + | ||
| 817 | + | /// Remove-dependency form: the depended-on issue id. | |
| 818 | + | #[derive(Debug, Deserialize)] | |
| 819 | + | pub struct DepRemoveForm { | |
| 820 | + | /// The `issues` id this one no longer depends on. | |
| 821 | + | #[serde(default)] | |
| 822 | + | depends_on: String, | |
| 823 | + | /// CSRF token. | |
| 824 | + | #[serde(rename = "_csrf")] | |
| 825 | + | csrf: String, | |
| 826 | + | } | |
| 827 | + | ||
| 828 | + | /// `POST /issue/{id}/deps/remove` — drop a dependency edge (Write). | |
| 829 | + | pub async fn deps_remove( | |
| 830 | + | State(state): State<AppState>, | |
| 831 | + | MaybeUser(viewer): MaybeUser, | |
| 832 | + | jar: CookieJar, | |
| 833 | + | headers: HeaderMap, | |
| 834 | + | Path(id): Path<String>, | |
| 835 | + | Form(form): Form<DepRemoveForm>, | |
| 836 | + | ) -> Response { | |
| 837 | + | if verify_csrf(&jar, &headers, Some(&form.csrf)).is_err() { | |
| 838 | + | return AppError::Forbidden.into_response(); | |
| 839 | + | } | |
| 840 | + | let result = async { | |
| 841 | + | let user = viewer.ok_or(AppError::Forbidden)?; | |
| 842 | + | let issue = state | |
| 843 | + | .store | |
| 844 | + | .issue_by_id(&id) | |
| 845 | + | .await? | |
| 846 | + | .ok_or(AppError::NotFound)?; | |
| 847 | + | let (repo, access) = repo_of_issue(&state, Some(&user), &issue).await?; | |
| 848 | + | if access < auth::Access::Write { | |
| 849 | + | return Err(AppError::Forbidden); | |
| 850 | + | } | |
| 851 | + | state | |
| 852 | + | .store | |
| 853 | + | .remove_dependency(&issue.id, &form.depends_on) | |
| 854 | + | .await?; | |
| 855 | + | Ok::<String, AppError>(issue_url(&state, &repo, &issue).await) | |
| 856 | + | } | |
| 857 | + | .await; | |
| 858 | + | respond_redirect(result) | |
| 859 | + | } | |
| 860 | + | ||
| 710 | 861 | /// Redirect on success, render the error otherwise. | |
| 711 | 862 | pub(crate) fn respond_redirect(result: AppResult<String>) -> Response { | |
| 712 | 863 | match result { | |
crates/web/src/lib.rs +2 −0
| @@ -198,6 +198,8 @@ pub fn build_router(state: AppState) -> Router { | |||
| 198 | 198 | .route("/issue/{id}/state", post(issues::set_state)) | |
| 199 | 199 | .route("/issue/{id}/assignee", post(issues::set_assignee)) | |
| 200 | 200 | .route("/issue/{id}/labels", post(issues::set_labels)) | |
| 201 | + | .route("/issue/{id}/deps/add", post(issues::deps_add)) | |
| 202 | + | .route("/issue/{id}/deps/remove", post(issues::deps_remove)) | |
| 201 | 203 | .route("/pull-new/{repo_id}", post(pulls::create)) | |
| 202 | 204 | .route("/pull/{id}/merge", post(pulls::merge)) | |
| 203 | 205 | .route("/healthz", get(serve_assets::healthz)) | |
crates/web/src/pulls.rs +36 −4
| @@ -25,8 +25,8 @@ use crate::AppState; | |||
| 25 | 25 | use crate::diff; | |
| 26 | 26 | use crate::error::{AppError, AppResult}; | |
| 27 | 27 | use crate::issues::{ | |
| 28 | - | Kind, access_for, assignee_form, comment_card, issue_url, label_form, repo_of_issue, | |
| 29 | - | respond_redirect, state_badge, state_button, username, | |
| 28 | + | Kind, access_for, assignee_form, comment_card, deps_section, issue_url, label_form, | |
| 29 | + | repo_of_issue, respond_redirect, state_badge, state_button, username, | |
| 30 | 30 | }; | |
| 31 | 31 | use crate::layout::page; | |
| 32 | 32 | use crate::pages::build_chrome; | |
| @@ -127,6 +127,9 @@ pub(crate) async fn view( | |||
| 127 | 127 | commits, | |
| 128 | 128 | files, | |
| 129 | 129 | mergeable, | |
| 130 | + | deps, | |
| 131 | + | dependents, | |
| 132 | + | blocked, | |
| 130 | 133 | } = gather(state, &ctx, &issue, &pr, can_write).await?; | |
| 131 | 134 | ||
| 132 | 135 | let (jar, chrome) = build_chrome(state, jar, viewer, uri.path().to_string()); | |
| @@ -150,7 +153,7 @@ pub(crate) async fn view( | |||
| 150 | 153 | @for (c, cauthor) in &comment_rows { | |
| 151 | 154 | (comment_card(cauthor, &c.body, c.created_at)) | |
| 152 | 155 | } | |
| 153 | - | (merge_panel(&issue, &pr, mergeable, &csrf)) | |
| 156 | + | (merge_panel(&issue, &pr, mergeable, blocked, &csrf)) | |
| 154 | 157 | @if can_write { | |
| 155 | 158 | div class="card comment-form" { | |
| 156 | 159 | form method="post" action=(format!("/issue/{}/comment", issue.id)) { | |
| @@ -180,6 +183,7 @@ pub(crate) async fn view( | |||
| 180 | 183 | (label_form(&issue, &repo_labels, &issue_labels, &csrf)) | |
| 181 | 184 | } | |
| 182 | 185 | } | |
| 186 | + | (deps_section(&issue, &ctx, &deps, &dependents, &csrf, can_write)) | |
| 183 | 187 | } | |
| 184 | 188 | } | |
| 185 | 189 | @if issue.state == model::IssueState::Open { | |
| @@ -209,6 +213,12 @@ struct Loaded { | |||
| 209 | 213 | files: Vec<crate::diff::RenderedFile>, | |
| 210 | 214 | /// Mergeability, computed only for an open PR the viewer can merge. | |
| 211 | 215 | mergeable: Option<git::merge::Mergeability>, | |
| 216 | + | /// The issues/PRs this PR is blocked by. | |
| 217 | + | deps: Vec<model::Issue>, | |
| 218 | + | /// The issues/PRs blocked by this PR. | |
| 219 | + | dependents: Vec<model::Issue>, | |
| 220 | + | /// Whether any dependency is still open (blocking the merge). | |
| 221 | + | blocked: bool, | |
| 212 | 222 | } | |
| 213 | 223 | ||
| 214 | 224 | /// Load the comments, labels, assignee, diff, and mergeability for the PR view. | |
| @@ -247,6 +257,9 @@ async fn gather( | |||
| 247 | 257 | } else { | |
| 248 | 258 | None | |
| 249 | 259 | }; | |
| 260 | + | let deps = state.store.dependencies_of(&issue.id).await?; | |
| 261 | + | let dependents = state.store.dependents_of(&issue.id).await?; | |
| 262 | + | let blocked = deps.iter().any(|d| d.state == model::IssueState::Open); | |
| 250 | 263 | Ok(Loaded { | |
| 251 | 264 | author, | |
| 252 | 265 | comment_rows, | |
| @@ -256,6 +269,9 @@ async fn gather( | |||
| 256 | 269 | commits, | |
| 257 | 270 | files, | |
| 258 | 271 | mergeable, | |
| 272 | + | deps, | |
| 273 | + | dependents, | |
| 274 | + | blocked, | |
| 259 | 275 | }) | |
| 260 | 276 | } | |
| 261 | 277 | ||
| @@ -316,11 +332,13 @@ fn pr_state_badge(issue: &model::Issue, pr: &model::PullRequest) -> Markup { | |||
| 316 | 332 | state_badge(issue.state, true) | |
| 317 | 333 | } | |
| 318 | 334 | ||
| 319 | - | /// The merge panel: shows mergeability and, when clean, the strategy buttons. | |
| 335 | + | /// The merge panel: shows mergeability and, when clean and unblocked, the | |
| 336 | + | /// strategy buttons. Open dependencies block the merge. | |
| 320 | 337 | fn merge_panel( | |
| 321 | 338 | issue: &model::Issue, | |
| 322 | 339 | pr: &model::PullRequest, | |
| 323 | 340 | mergeable: Option<git::merge::Mergeability>, | |
| 341 | + | blocked: bool, | |
| 324 | 342 | csrf: &str, | |
| 325 | 343 | ) -> Markup { | |
| 326 | 344 | use git::merge::Mergeability; | |
| @@ -336,6 +354,14 @@ fn merge_panel( | |||
| 336 | 354 | if issue.state != model::IssueState::Open { | |
| 337 | 355 | return html! {}; | |
| 338 | 356 | } | |
| 357 | + | // A merge is blocked until every dependency is closed. | |
| 358 | + | if blocked && mergeable.is_some() { | |
| 359 | + | return html! { | |
| 360 | + | div class="card merge-panel conflicts" { | |
| 361 | + | p { "This pull request is blocked by open dependencies and cannot be merged yet." } | |
| 362 | + | } | |
| 363 | + | }; | |
| 364 | + | } | |
| 339 | 365 | match mergeable { | |
| 340 | 366 | None => html! {}, | |
| 341 | 367 | Some(Mergeability::AlreadyMerged) => html! { | |
| @@ -523,6 +549,12 @@ pub async fn merge( | |||
| 523 | 549 | "This pull request is already closed.".to_string(), | |
| 524 | 550 | )); | |
| 525 | 551 | } | |
| 552 | + | // Refuse to merge while any dependency is still open. | |
| 553 | + | if state.store.open_dependency_count(&issue.id).await? > 0 { | |
| 554 | + | return Err(AppError::BadRequest( | |
| 555 | + | "This pull request is blocked by open dependencies.".to_string(), | |
| 556 | + | )); | |
| 557 | + | } | |
| 526 | 558 | ||
| 527 | 559 | let strategy = git::merge::MergeStrategy::from_token(&form.strategy); | |
| 528 | 560 | let sha = run_merge(&state, &repo, &pr, &user, strategy, &issue).await?; | |
crates/web/src/tests.rs +76 −0
| @@ -725,6 +725,82 @@ async fn issue_list_paginates() { | |||
| 725 | 725 | } | |
| 726 | 726 | ||
| 727 | 727 | #[tokio::test] | |
| 728 | + | async fn issue_dependencies_block_and_unblock() { | |
| 729 | + | let (state, app) = app().await; | |
| 730 | + | let ada = state.store.user_by_username("ada").await.unwrap().unwrap(); | |
| 731 | + | let repo = state | |
| 732 | + | .store | |
| 733 | + | .create_repo(NewRepo { | |
| 734 | + | owner_id: ada.id.clone(), | |
| 735 | + | group_id: None, | |
| 736 | + | name: "proj".to_string(), | |
| 737 | + | path: "proj".to_string(), | |
| 738 | + | description: None, | |
| 739 | + | visibility: model::Visibility::Public, | |
| 740 | + | default_branch: "main".to_string(), | |
| 741 | + | }) | |
| 742 | + | .await | |
| 743 | + | .unwrap(); | |
| 744 | + | // #1 will depend on #2. | |
| 745 | + | let one = state | |
| 746 | + | .store | |
| 747 | + | .create_issue(&repo.id, &ada.id, "feature", "", false) | |
| 748 | + | .await | |
| 749 | + | .unwrap(); | |
| 750 | + | let two = state | |
| 751 | + | .store | |
| 752 | + | .create_issue(&repo.id, &ada.id, "prerequisite", "", false) | |
| 753 | + | .await | |
| 754 | + | .unwrap(); | |
| 755 | + | ||
| 756 | + | let cookie = login(&app).await; | |
| 757 | + | let token = cookie | |
| 758 | + | .split("fabrica_csrf=") | |
| 759 | + | .nth(1) | |
| 760 | + | .and_then(|s| s.split(';').next()) | |
| 761 | + | .unwrap() | |
| 762 | + | .to_string(); | |
| 763 | + | ||
| 764 | + | // Add the dependency: #1 depends on #2. | |
| 765 | + | let req = Request::builder() | |
| 766 | + | .method("POST") | |
| 767 | + | .uri(format!("/issue/{}/deps/add", one.id)) | |
| 768 | + | .header(header::CONTENT_TYPE, "application/x-www-form-urlencoded") | |
| 769 | + | .header(header::COOKIE, cookie.clone()) | |
| 770 | + | .body(Body::from(format!("number=2&_csrf={token}"))) | |
| 771 | + | .unwrap(); | |
| 772 | + | let res = app.clone().oneshot(req).await.unwrap(); | |
| 773 | + | assert_eq!(res.status(), StatusCode::SEE_OTHER); | |
| 774 | + | assert_eq!( | |
| 775 | + | state.store.open_dependency_count(&one.id).await.unwrap(), | |
| 776 | + | 1, | |
| 777 | + | "one open blocker" | |
| 778 | + | ); | |
| 779 | + | ||
| 780 | + | // #1's view lists the blocker; #2's view lists what it blocks. | |
| 781 | + | let res = app | |
| 782 | + | .clone() | |
| 783 | + | .oneshot(get("/ada/proj/-/issues/1")) | |
| 784 | + | .await | |
| 785 | + | .unwrap(); | |
| 786 | + | let html = body_string(res).await; | |
| 787 | + | assert!(html.contains("Blocked by"), "blocked-by section"); | |
| 788 | + | assert!(html.contains("prerequisite"), "blocker listed"); | |
| 789 | + | ||
| 790 | + | // Closing #2 clears the block. | |
| 791 | + | state | |
| 792 | + | .store | |
| 793 | + | .set_issue_state(&two.id, model::IssueState::Closed) | |
| 794 | + | .await | |
| 795 | + | .unwrap(); | |
| 796 | + | assert_eq!( | |
| 797 | + | state.store.open_dependency_count(&one.id).await.unwrap(), | |
| 798 | + | 0, | |
| 799 | + | "no open blockers once closed" | |
| 800 | + | ); | |
| 801 | + | } | |
| 802 | + | ||
| 803 | + | #[tokio::test] | |
| 728 | 804 | async fn issues_404_when_disabled() { | |
| 729 | 805 | let (state, app) = app().await; | |
| 730 | 806 | let ada = state.store.user_by_username("ada").await.unwrap().unwrap(); | |
migrations/postgres/0008_issue_dependencies.sql +10 −0
| @@ -0,0 +1,10 @@ | |||
| 1 | + | -- Dependencies between issues/PRs: `issue_id` is blocked by `depends_on_id`. | |
| 2 | + | -- The reverse direction ("blocks") is read by querying depends_on_id. Works | |
| 3 | + | -- across issues and PRs uniformly (both live in `issues`), so PRs can be stacked. | |
| 4 | + | CREATE TABLE issue_dependencies ( | |
| 5 | + | issue_id TEXT NOT NULL REFERENCES issues(id) ON DELETE CASCADE, | |
| 6 | + | depends_on_id TEXT NOT NULL REFERENCES issues(id) ON DELETE CASCADE, | |
| 7 | + | created_at BIGINT NOT NULL, | |
| 8 | + | PRIMARY KEY (issue_id, depends_on_id) | |
| 9 | + | ); | |
| 10 | + | CREATE INDEX idx_issue_deps_depends_on ON issue_dependencies (depends_on_id); | |
migrations/sqlite/0008_issue_dependencies.sql +10 −0
| @@ -0,0 +1,10 @@ | |||
| 1 | + | -- Dependencies between issues/PRs: `issue_id` is blocked by `depends_on_id`. | |
| 2 | + | -- The reverse direction ("blocks") is read by querying depends_on_id. Works | |
| 3 | + | -- across issues and PRs uniformly (both live in `issues`), so PRs can be stacked. | |
| 4 | + | CREATE TABLE issue_dependencies ( | |
| 5 | + | issue_id TEXT NOT NULL REFERENCES issues(id) ON DELETE CASCADE, | |
| 6 | + | depends_on_id TEXT NOT NULL REFERENCES issues(id) ON DELETE CASCADE, | |
| 7 | + | created_at BIGINT NOT NULL, | |
| 8 | + | PRIMARY KEY (issue_id, depends_on_id) | |
| 9 | + | ); | |
| 10 | + | CREATE INDEX idx_issue_deps_depends_on ON issue_dependencies (depends_on_id); | |