tenseleyflow/shithub / d8a3df3

Browse files

S23: settings UI for required reviews + pull_view review templates + CSS

Authored by mfwolffe <wolffemf@dukes.jmu.edu>
SHA
d8a3df3c112d95a0c0d6b93941a380353609b2b2
Parents
736c0b3
Tree
5047267

4 changed files

StatusFile+-
M internal/web/handlers/repo/settings_branches.go 22 0
M internal/web/static/css/shithub.css 41 0
M internal/web/templates/repo/pull_view.html 121 0
M internal/web/templates/repo/settings_branches.html 6 1
internal/web/handlers/repo/settings_branches.gomodified
@@ -75,6 +75,12 @@ func (h *Handlers) settingsBranchesUpsert(w http.ResponseWriter, r *http.Request
7575
 	preventDeletion := r.PostFormValue("prevent_deletion") == "on"
7676
 	requirePR := r.PostFormValue("require_pr_for_push") == "on"
7777
78
+	requiredReviews, _ := strconv.Atoi(strings.TrimSpace(r.PostFormValue("required_review_count")))
79
+	if requiredReviews < 0 {
80
+		requiredReviews = 0
81
+	}
82
+	dismissStale := r.PostFormValue("dismiss_stale_reviews_on_push") == "on"
83
+
7884
 	allowed, err := resolveUsernameList(r, h, r.PostFormValue("allowed_pushers"))
7985
 	if err != nil {
8086
 		http.Error(w, err.Error(), http.StatusBadRequest)
@@ -100,6 +106,14 @@ func (h *Handlers) settingsBranchesUpsert(w http.ResponseWriter, r *http.Request
100106
 			http.Error(w, "failed to create rule", http.StatusInternalServerError)
101107
 			return
102108
 		}
109
+		if err := h.rq.UpdateBranchProtectionReviewSettings(r.Context(), h.d.Pool, reposdb.UpdateBranchProtectionReviewSettingsParams{
110
+			ID:                          newID,
111
+			RequiredReviewCount:         int32(requiredReviews),
112
+			DismissStaleReviewsOnPush:   dismissStale,
113
+			RequireCodeOwnerReview:      false,
114
+		}); err != nil {
115
+			h.d.Logger.WarnContext(r.Context(), "branch-protection: review settings", "error", err)
116
+		}
103117
 		_ = h.d.Audit.Record(r.Context(), h.d.Pool, viewer.ID,
104118
 			audit.ActionRepoCreated, audit.TargetRepo, row.ID,
105119
 			map[string]any{"branch_protection_rule_id": newID, "pattern": pattern, "action": "create"})
@@ -127,6 +141,14 @@ func (h *Handlers) settingsBranchesUpsert(w http.ResponseWriter, r *http.Request
127141
 			http.Error(w, "failed to update rule", http.StatusInternalServerError)
128142
 			return
129143
 		}
144
+		if err := h.rq.UpdateBranchProtectionReviewSettings(r.Context(), h.d.Pool, reposdb.UpdateBranchProtectionReviewSettingsParams{
145
+			ID:                          id,
146
+			RequiredReviewCount:         int32(requiredReviews),
147
+			DismissStaleReviewsOnPush:   dismissStale,
148
+			RequireCodeOwnerReview:      false,
149
+		}); err != nil {
150
+			h.d.Logger.WarnContext(r.Context(), "branch-protection: review settings", "error", err)
151
+		}
130152
 		_ = h.d.Audit.Record(r.Context(), h.d.Pool, viewer.ID,
131153
 			audit.ActionRepoCreated, audit.TargetRepo, row.ID,
132154
 			map[string]any{"branch_protection_rule_id": id, "pattern": pattern, "action": "update"})
internal/web/static/css/shithub.cssmodified
@@ -1306,3 +1306,44 @@ code {
13061306
   padding: 0.4rem 0; border-bottom: 1px solid var(--border-default);
13071307
   display: flex; gap: 0.6rem; align-items: baseline;
13081308
 }
1309
+
1310
+/* ========== PR Reviews (S23) ========== */
1311
+.shithub-pull-reviews, .shithub-pull-reviewers {
1312
+  border-top: 1px solid var(--border-default);
1313
+  padding: 0.6rem 0;
1314
+}
1315
+.shithub-pull-reviews h3, .shithub-pull-reviewers h3 {
1316
+  font-size: 0.85rem; text-transform: uppercase; color: var(--fg-muted); margin: 0 0 0.4rem;
1317
+}
1318
+.shithub-pull-reviews-list, .shithub-pull-reviewers-list {
1319
+  list-style: none; padding: 0; margin: 0; font-size: 0.9rem;
1320
+}
1321
+.shithub-pull-reviews-list li { padding: 0.2rem 0; display: flex; gap: 0.35rem; flex-wrap: wrap; align-items: baseline; }
1322
+.shithub-pull-review-state { font-weight: 600; }
1323
+.shithub-pull-review-approve .shithub-pull-review-state { color: #1a7f37; }
1324
+.shithub-pull-review-request_changes .shithub-pull-review-state { color: #cf222e; }
1325
+.shithub-pull-review-comment .shithub-pull-review-state { color: var(--fg-muted); }
1326
+.shithub-pull-review-dismissed { opacity: 0.5; text-decoration: line-through; }
1327
+.shithub-pull-request-reviewer summary, .shithub-pull-submit-review summary {
1328
+  margin: 0.5rem 0 0.25rem;
1329
+}
1330
+.shithub-pull-review-form { display: flex; flex-direction: column; gap: 0.4rem; padding: 0.4rem 0; }
1331
+.shithub-pull-review-form textarea, .shithub-pull-review-form select {
1332
+  padding: 0.4rem; border: 1px solid var(--border-default); border-radius: 6px; font: inherit;
1333
+}
1334
+.shithub-pull-threads { margin-top: 1.5rem; padding: 0.75rem; border: 1px solid var(--border-default); border-radius: 6px; }
1335
+.shithub-pull-thread-file { padding: 0.4rem 0; border-bottom: 1px solid var(--border-default); }
1336
+.shithub-pull-thread-file:last-child { border-bottom: none; }
1337
+.shithub-pull-thread-file summary { cursor: pointer; font-weight: 600; }
1338
+.shithub-pull-thread { padding: 0.5rem; border-left: 3px solid var(--border-default); margin: 0.5rem 0; }
1339
+.shithub-pull-thread-outdated { opacity: 0.6; border-left-color: #9a6700; }
1340
+.shithub-pull-thread-resolved { opacity: 0.7; border-left-color: #1a7f37; }
1341
+.shithub-pull-thread-actions { display: flex; gap: 0.3rem; margin-top: 0.4rem; flex-wrap: wrap; }
1342
+.shithub-pull-thread-actions textarea, .shithub-pull-thread-actions input[type=text] {
1343
+  padding: 0.3rem; border: 1px solid var(--border-default); border-radius: 6px; font: inherit;
1344
+}
1345
+.shithub-pull-add-comment summary { margin-top: 0.5rem; }
1346
+.shithub-pull-add-comment form { display: flex; flex-direction: column; gap: 0.3rem; padding: 0.4rem 0; }
1347
+.shithub-pull-add-comment input, .shithub-pull-add-comment textarea {
1348
+  padding: 0.3rem 0.5rem; border: 1px solid var(--border-default); border-radius: 6px; font: inherit;
1349
+}
internal/web/templates/repo/pull_view.htmlmodified
@@ -62,6 +62,71 @@
6262
       </article>
6363
6464
       <aside class="shithub-pull-merge">
65
+        {{ if .Reviews }}
66
+        <section class="shithub-pull-reviews">
67
+          <h3>Reviews</h3>
68
+          <ul class="shithub-pull-reviews-list">
69
+            {{ range .Reviews }}
70
+            <li class="shithub-pull-review-{{ printf "%s" .R.State }}{{ if .R.DismissedAt.Valid }} shithub-pull-review-dismissed{{ end }}">
71
+              <span class="shithub-pull-review-state">
72
+                {{ if eq (printf "%s" .R.State) "approve" }}✓ approved{{ else if eq (printf "%s" .R.State) "request_changes" }}✗ changes requested{{ else }}● commented{{ end }}
73
+              </span>
74
+              {{ if .AuthorName }}<a href="/{{ .AuthorName }}">{{ .AuthorName }}</a>{{ end }}
75
+              <time datetime="{{ .R.SubmittedAt.Time.Format "2006-01-02T15:04:05Z" }}">{{ relativeTime .R.SubmittedAt.Time }}</time>
76
+              {{ if .R.DismissedAt.Valid }}<small>dismissed</small>{{ end }}
77
+            </li>
78
+            {{ end }}
79
+          </ul>
80
+        </section>
81
+        {{ end }}
82
+
83
+        {{ if .ReviewRequests }}
84
+        <section class="shithub-pull-reviewers">
85
+          <h3>Reviewers</h3>
86
+          <ul class="shithub-pull-reviewers-list">
87
+            {{ range .ReviewRequests }}
88
+              {{ if not .R.DismissedAt.Valid }}
89
+              <li>
90
+                <a href="/{{ .Username }}">@{{ .Username }}</a>
91
+                {{ if .R.SatisfiedByReviewID.Valid }}<small>reviewed</small>{{ else }}<small>pending</small>{{ end }}
92
+              </li>
93
+              {{ end }}
94
+            {{ end }}
95
+          </ul>
96
+        </section>
97
+        {{ end }}
98
+
99
+        {{ if .Viewer.ID }}
100
+        <details class="shithub-pull-request-reviewer">
101
+          <summary class="shithub-button">Request review</summary>
102
+          <form method="post" action="/{{ .Owner }}/{{ .Repo.Name }}/pulls/{{ .PR.INumber }}/reviewers">
103
+            <input type="hidden" name="csrf_token" value="{{ .CSRFToken }}">
104
+            <input type="text" name="username" placeholder="username" required>
105
+            <button type="submit" class="shithub-button">Request</button>
106
+          </form>
107
+        </details>
108
+
109
+        {{ if not .PR.MergedAt.Valid }}
110
+        {{ if eq (printf "%s" .PR.IState) "open" }}
111
+        <details class="shithub-pull-submit-review">
112
+          <summary class="shithub-button">Submit review</summary>
113
+          <form method="post" action="/{{ .Owner }}/{{ .Repo.Name }}/pulls/{{ .PR.INumber }}/reviews" class="shithub-pull-review-form">
114
+            <input type="hidden" name="csrf_token" value="{{ .CSRFToken }}">
115
+            <textarea name="body" rows="4" placeholder="Leave a review comment (optional)"></textarea>
116
+            <select name="state">
117
+              <option value="comment">Comment</option>
118
+              {{ if not (and .PR.IAuthorUserID.Valid (eq .PR.IAuthorUserID.Int64 .Viewer.ID)) }}
119
+              <option value="approve">Approve</option>
120
+              <option value="request_changes">Request changes</option>
121
+              {{ end }}
122
+            </select>
123
+            <button type="submit" class="shithub-button shithub-button-primary">Submit</button>
124
+          </form>
125
+        </details>
126
+        {{ end }}
127
+        {{ end }}
128
+        {{ end }}
129
+
65130
         {{ if .PR.MergedAt.Valid }}
66131
           <p>Merged via <code>{{ printf "%s" .PR.MergeMethod.PrMergeMethod }}</code>.</p>
67132
         {{ else if eq (printf "%s" .PR.IState) "closed" }}
@@ -121,6 +186,62 @@
121186
     {{ else }}
122187
       <p class="shithub-muted">No diff available.</p>
123188
     {{ end }}
189
+
190
+    {{ if .ThreadsByFile }}
191
+    <section class="shithub-pull-threads">
192
+      <h3>Inline comments</h3>
193
+      {{ range $path, $threads := .ThreadsByFile }}
194
+        {{ if $threads }}
195
+        <details class="shithub-pull-thread-file" open>
196
+          <summary><code>{{ $path }}</code> · {{ len $threads }}</summary>
197
+          {{ range $threads }}
198
+            <div class="shithub-pull-thread{{ if not .C.CurrentPosition.Valid }} shithub-pull-thread-outdated{{ end }}{{ if .C.ResolvedAt.Valid }} shithub-pull-thread-resolved{{ end }}">
199
+              <div class="shithub-comment-head">
200
+                {{ if .AuthorName }}<a href="/{{ .AuthorName }}">{{ .AuthorName }}</a>{{ end }}
201
+                line {{ .C.OriginalLine }}
202
+                <time datetime="{{ .C.CreatedAt.Time.Format "2006-01-02T15:04:05Z" }}">{{ relativeTime .C.CreatedAt.Time }}</time>
203
+                {{ if not .C.CurrentPosition.Valid }}<span class="shithub-pill">outdated</span>{{ end }}
204
+                {{ if .C.ResolvedAt.Valid }}<span class="shithub-pill">resolved</span>{{ end }}
205
+              </div>
206
+              <div class="shithub-comment-body markdown-body">
207
+                {{ if .C.BodyHtmlCached.Valid }}{{ safeHTML .C.BodyHtmlCached.String }}{{ else }}<p>{{ .C.Body }}</p>{{ end }}
208
+              </div>
209
+              {{ if $.Viewer.ID }}
210
+              <div class="shithub-pull-thread-actions">
211
+                <form method="post" action="/{{ $.Owner }}/{{ $.Repo.Name }}/pulls/{{ $.PR.INumber }}/review-comments/{{ .C.ID }}/reply">
212
+                  <input type="hidden" name="csrf_token" value="{{ $.CSRFToken }}">
213
+                  <textarea name="body" rows="2" placeholder="Reply…"></textarea>
214
+                  <button type="submit" class="shithub-button">Reply</button>
215
+                </form>
216
+                <form method="post" action="/{{ $.Owner }}/{{ $.Repo.Name }}/pulls/{{ $.PR.INumber }}/review-comments/{{ .C.ID }}/{{ if .C.ResolvedAt.Valid }}reopen{{ else }}resolve{{ end }}">
217
+                  <input type="hidden" name="csrf_token" value="{{ $.CSRFToken }}">
218
+                  <button type="submit" class="shithub-button">{{ if .C.ResolvedAt.Valid }}Reopen{{ else }}Resolve{{ end }}</button>
219
+                </form>
220
+              </div>
221
+              {{ end }}
222
+            </div>
223
+          {{ end }}
224
+        </details>
225
+        {{ end }}
226
+      {{ end }}
227
+
228
+      {{ if .Viewer.ID }}
229
+      <details class="shithub-pull-add-comment">
230
+        <summary class="shithub-button">Add inline comment</summary>
231
+        <form method="post" action="/{{ .Owner }}/{{ .Repo.Name }}/pulls/{{ .PR.INumber }}/review-comments">
232
+          <input type="hidden" name="csrf_token" value="{{ .CSRFToken }}">
233
+          <input type="text" name="file_path" placeholder="path" required>
234
+          <input type="number" name="line" placeholder="line" required>
235
+          <input type="number" name="position" placeholder="position" required>
236
+          <input type="hidden" name="commit_sha" value="{{ .PR.HeadOid }}">
237
+          <input type="hidden" name="side" value="right">
238
+          <textarea name="body" rows="3" required></textarea>
239
+          <button type="submit" class="shithub-button">Add comment</button>
240
+        </form>
241
+      </details>
242
+      {{ end }}
243
+    </section>
244
+    {{ end }}
124245
   {{ else if eq .Tab "checks" }}
125246
     <p class="shithub-muted">Check engine ships in S24. Today this tab is a placeholder.</p>
126247
   {{ end }}
internal/web/templates/repo/settings_branches.htmlmodified
@@ -27,7 +27,7 @@
2727
     {{ if .Rules }}
2828
     <table class="shithub-branches-table">
2929
       <thead>
30
-        <tr><th>Pattern</th><th>Force-push</th><th>Deletion</th><th>Allowed pushers</th><th></th></tr>
30
+        <tr><th>Pattern</th><th>Force-push</th><th>Deletion</th><th>Allowed pushers</th><th>Required reviews</th><th></th></tr>
3131
       </thead>
3232
       <tbody>
3333
         {{ range .Rules }}
@@ -36,6 +36,7 @@
3636
           <td>{{ if .PreventForcePush }}🚫{{ else }}allowed{{ end }}</td>
3737
           <td>{{ if .PreventDeletion }}🚫{{ else }}allowed{{ end }}</td>
3838
           <td>{{ len .AllowedPusherUserIds }} {{ if eq (len .AllowedPusherUserIds) 1 }}user{{ else }}users{{ end }}</td>
39
+          <td>{{ .RequiredReviewCount }}{{ if .DismissStaleReviewsOnPush }} · dismiss-stale{{ end }}</td>
3940
           <td>
4041
             <form method="POST" action="/{{ $.Owner }}/{{ $.Repo.Name }}/settings/branches/{{ .ID }}/delete" style="display:inline">
4142
               <input type="hidden" name="csrf_token" value="{{ $.CSRFToken }}">
@@ -64,6 +65,10 @@
6465
       <label>Allowed pushers (comma-separated usernames; leave blank for "any collaborator")
6566
         <input type="text" name="allowed_pushers" placeholder="alice, bob">
6667
       </label>
68
+      <label>Required reviews (S23) — number of approving reviews required before merge
69
+        <input type="number" name="required_review_count" min="0" value="0">
70
+      </label>
71
+      <label><input type="checkbox" name="dismiss_stale_reviews_on_push"> Dismiss stale reviews on new push (S23)</label>
6772
       <button type="submit" class="shithub-button shithub-button-primary">Create rule</button>
6873
     </form>
6974
   </section>