tenseleyflow/shithub / 23e382a

Browse files

web/handlers/repo: rewire advanced BP gate to fire on prevent_* + sign + checks; user-tier upgrade copy + multi-reviewer variant

Authored by mfwolffe <wolffemf@dukes.jmu.edu>
SHA
23e382a9a0d5a22577b0a034bbcb5e2f23ff3eb5
Parents
eadda53
Tree
0bad269

1 changed file

StatusFile+-
M internal/web/handlers/repo/settings_branches.go 95 21
internal/web/handlers/repo/settings_branches.gomodified
@@ -79,6 +79,11 @@ func (h *Handlers) settingsBranchesUpsert(w http.ResponseWriter, r *http.Request
7979
 	preventForcePush := r.PostFormValue("prevent_force_push") == "on"
8080
 	preventDeletion := r.PostFormValue("prevent_deletion") == "on"
8181
 	requirePR := r.PostFormValue("require_pr_for_push") == "on"
82
+	// PRO08 C1: require_signed_commits is a Pro paygate input. The
83
+	// column existed since SP05 but had no form parser; PRO01 ratified
84
+	// it as part of Pro v1's "advanced branch protection" set even
85
+	// though underlying enforcement is a no-op until signing lands.
86
+	requireSignedCommits := r.PostFormValue("require_signed_commits") == "on"
8287
8388
 	requiredReviews, _ := strconv.Atoi(strings.TrimSpace(r.PostFormValue("required_review_count")))
8489
 	if requiredReviews < 0 {
@@ -92,7 +97,15 @@ func (h *Handlers) settingsBranchesUpsert(w http.ResponseWriter, r *http.Request
9297
 	requiredChecks := splitCommaList(r.PostFormValue("required_status_check_names"))
9398
 	dismissStaleChecks := r.PostFormValue("dismiss_stale_status_checks_on_push") == "on"
9499
95
-	noticeCode, err := h.branchProtectionEntitlementNotice(r.Context(), row, requiredReviews, dismissStale, requiredChecks, dismissStaleChecks)
100
+	noticeCode, err := h.branchProtectionEntitlementNotice(r.Context(), row, branchProtectionInputs{
101
+		RequiredReviews:      requiredReviews,
102
+		DismissStale:         dismissStale,
103
+		PreventForcePush:     preventForcePush,
104
+		PreventDeletion:      preventDeletion,
105
+		RequireSignedCommits: requireSignedCommits,
106
+		RequiredChecks:       requiredChecks,
107
+		DismissStaleChecks:   dismissStaleChecks,
108
+	})
96109
 	if err != nil {
97110
 		h.d.Render.HTTPError(w, r, http.StatusInternalServerError, "")
98111
 		return
@@ -119,6 +132,7 @@ func (h *Handlers) settingsBranchesUpsert(w http.ResponseWriter, r *http.Request
119132
 			PreventForcePush:     preventForcePush,
120133
 			PreventDeletion:      preventDeletion,
121134
 			RequirePrForPush:     requirePR,
135
+			RequireSignedCommits: requireSignedCommits,
122136
 			AllowedPusherUserIds: allowed,
123137
 			CreatedByUserID:      pgtype.Int8{Int64: viewer.ID, Valid: viewer.ID != 0},
124138
 		})
@@ -165,6 +179,7 @@ func (h *Handlers) settingsBranchesUpsert(w http.ResponseWriter, r *http.Request
165179
 			PreventForcePush:     preventForcePush,
166180
 			PreventDeletion:      preventDeletion,
167181
 			RequirePrForPush:     requirePR,
182
+			RequireSignedCommits: requireSignedCommits,
168183
 			AllowedPusherUserIds: allowed,
169184
 		}); err != nil {
170185
 			http.Error(w, "failed to update rule", http.StatusInternalServerError)
@@ -193,17 +208,36 @@ func (h *Handlers) settingsBranchesUpsert(w http.ResponseWriter, r *http.Request
193208
 	http.Redirect(w, r, "/"+owner.Username+"/"+row.Name+"/settings/branches?notice=saved", http.StatusSeeOther)
194209
 }
195210
196
-// branchProtectionEntitlementNotice gates the required-reviewers
197
-// and advanced-branch-protection knobs on private repos. PRO05
198
-// extended it to personal private repos in report-only mode — a
199
-// user-kind would-deny logs an "entitlements.report_only_deny"
200
-// event but does not block the save. PRO07 flips per-feature
201
-// enforce on for user kind once production telemetry confirms no
202
-// existing Free user is over-quota.
211
+// branchProtectionInputs bundles the form values consumed by the
212
+// entitlement gate. Centralizing them in a struct keeps the gate
213
+// signature stable as PRO01's v1 paygate set evolves.
214
+type branchProtectionInputs struct {
215
+	RequiredReviews      int
216
+	DismissStale         bool
217
+	PreventForcePush     bool
218
+	PreventDeletion      bool
219
+	RequireSignedCommits bool
220
+	RequiredChecks       []string
221
+	DismissStaleChecks   bool
222
+}
223
+
224
+// branchProtectionEntitlementNotice gates the required-reviewers and
225
+// advanced-branch-protection knobs on private repos.
203226
 //
204
-// Public repos and non-private personal/org repos return empty
205
-// (no gating).
206
-func (h *Handlers) branchProtectionEntitlementNotice(ctx context.Context, row reposdb.Repo, requiredReviews int, dismissStale bool, requiredChecks []string, dismissStaleChecks bool) (string, error) {
227
+// PRO08 C2: PRO07 mis-wired this — FeatureAdvancedBranchProtection
228
+// fired on required status checks only, NOT on the PRO01-ratified
229
+// flags (prevent_force_push, prevent_deletion, require_signed_commits).
230
+// The corrected predicate fires on ANY advanced input:
231
+// {prevent_force_push, prevent_deletion, require_signed_commits,
232
+//  required_status_check_names, dismiss_stale_status_checks_on_push}.
233
+// FeatureRequiredReviewers stays on required_review_count + dismissStale.
234
+//
235
+// PRO05 extended gating to personal private repos in report-only
236
+// mode; PRO07's enforce flags flip per-feature once the operator
237
+// finishes the 7-day telemetry soak.
238
+//
239
+// Public repos and non-private personal/org repos return empty.
240
+func (h *Handlers) branchProtectionEntitlementNotice(ctx context.Context, row reposdb.Repo, inputs branchProtectionInputs) (string, error) {
207241
 	if row.Visibility != reposdb.RepoVisibilityPrivate {
208242
 		return "", nil
209243
 	}
@@ -211,14 +245,15 @@ func (h *Handlers) branchProtectionEntitlementNotice(ctx context.Context, row re
211245
 	if !ok {
212246
 		return "", nil
213247
 	}
214
-	if requiredReviews > 0 || dismissStale {
215
-		code, err := h.evaluateBranchProtectionFeature(ctx, principal, entitlements.FeatureRequiredReviewers, true)
248
+	if inputs.RequiredReviews > 0 || inputs.DismissStale {
249
+		code, err := h.evaluateBranchProtectionFeature(ctx, principal, entitlements.FeatureRequiredReviewers, inputs.RequiredReviews)
216250
 		if err != nil || code != "" {
217251
 			return code, err
218252
 		}
219253
 	}
220
-	if len(requiredChecks) > 0 || dismissStaleChecks {
221
-		code, err := h.evaluateBranchProtectionFeature(ctx, principal, entitlements.FeatureAdvancedBranchProtection, false)
254
+	if inputs.PreventForcePush || inputs.PreventDeletion || inputs.RequireSignedCommits ||
255
+		len(inputs.RequiredChecks) > 0 || inputs.DismissStaleChecks {
256
+		code, err := h.evaluateBranchProtectionFeature(ctx, principal, entitlements.FeatureAdvancedBranchProtection, 0)
222257
 		if err != nil || code != "" {
223258
 			return code, err
224259
 		}
@@ -245,6 +280,11 @@ func principalFromRepo(row reposdb.Repo) (billing.Principal, bool) {
245280
 // report-only-allowed); a non-empty code means the save is blocked
246281
 // and the caller redirects to the settings page with a banner.
247282
 //
283
+// `attemptedReviewers` is meaningful only when `feature` is
284
+// FeatureRequiredReviewers — it lets the notice code distinguish
285
+// single-reviewer-upgrade from multi-reviewer-upgrade copy (PRO08 C3).
286
+// Pass 0 for any other feature.
287
+//
248288
 // Enforcement semantics:
249289
 //   - Org kind: SP05 enforcement — would-denies return the notice
250290
 //     code unconditionally.
@@ -254,7 +294,7 @@ func principalFromRepo(row reposdb.Repo) (billing.Principal, bool) {
254294
 //     (blocks the save). The flag is per-feature; see
255295
 //     EnforceConfig.UserAdvancedBranchProtection and
256296
 //     EnforceConfig.UserRequiredReviewers in internal/infra/config.
257
-func (h *Handlers) evaluateBranchProtectionFeature(ctx context.Context, p billing.Principal, feature entitlements.Feature, requiredReviewers bool) (string, error) {
297
+func (h *Handlers) evaluateBranchProtectionFeature(ctx context.Context, p billing.Principal, feature entitlements.Feature, attemptedReviewers int) (string, error) {
258298
 	if !entitlements.FeatureAppliesToKind(feature, p.Kind) {
259299
 		return "", nil
260300
 	}
@@ -275,7 +315,7 @@ func (h *Handlers) evaluateBranchProtectionFeature(ctx context.Context, p billin
275315
 			"required_plan", string(decision.RequiredPlan))
276316
 		return "", nil
277317
 	}
278
-	return branchProtectionNoticeCode(decision, requiredReviewers), nil
318
+	return branchProtectionNoticeCode(decision, feature, p, attemptedReviewers), nil
279319
 }
280320
281321
 // userBranchProtectionEnforceOn maps a feature to the operator's
@@ -293,14 +333,37 @@ func (h *Handlers) userBranchProtectionEnforceOn(feature entitlements.Feature) b
293333
 	}
294334
 }
295335
296
-func branchProtectionNoticeCode(decision entitlements.Decision, requiredReviewers bool) string {
297
-	if requiredReviewers {
336
+// branchProtectionNoticeCode encodes the upgrade-banner copy variant
337
+// the settings page should render. PRO08 split the previously
338
+// kind-agnostic codes into user / org variants so the user-tier path
339
+// can say "upgrade to Pro" (with the /settings/billing href) rather
340
+// than the org-flavored "upgrade your org to Team" (PRO08 C4).
341
+//
342
+// For FeatureRequiredReviewers, the attempted count distinguishes
343
+// "Pro lets you require reviews" (count == 1) from "Pro lets you
344
+// require MORE THAN ONE review" (count > 1) per PRO01's sub-check
345
+// ratification (PRO08 C3).
346
+//
347
+// Enterprise + billing-action-needed reasons retain their pre-PRO08
348
+// codes — they're independent of plan kind.
349
+func branchProtectionNoticeCode(decision entitlements.Decision, feature entitlements.Feature, p billing.Principal, attemptedReviewers int) string {
350
+	if feature == entitlements.FeatureRequiredReviewers {
298351
 		switch decision.Reason {
299352
 		case entitlements.ReasonBillingActionNeeded:
300353
 			return "required-reviewers-billing"
301354
 		case entitlements.ReasonEnterpriseContactSales:
302355
 			return "required-reviewers-enterprise"
303356
 		}
357
+		multi := attemptedReviewers > 1
358
+		if p.IsUser() {
359
+			if multi {
360
+				return "required-reviewers-multi-upgrade-pro"
361
+			}
362
+			return "required-reviewers-upgrade-pro"
363
+		}
364
+		if multi {
365
+			return "required-reviewers-multi-upgrade"
366
+		}
304367
 		return "required-reviewers-upgrade"
305368
 	}
306369
 	switch decision.Reason {
@@ -309,6 +372,9 @@ func branchProtectionNoticeCode(decision entitlements.Decision, requiredReviewer
309372
 	case entitlements.ReasonEnterpriseContactSales:
310373
 		return "branch-protection-enterprise"
311374
 	}
375
+	if p.IsUser() {
376
+		return "branch-protection-upgrade-pro"
377
+	}
312378
 	return "branch-protection-upgrade"
313379
 }
314380
@@ -412,14 +478,22 @@ func settingsBranchesNoticeMessage(code string) string {
412478
 		return "Default branch updated."
413479
 	case "branch-protection-upgrade":
414480
 		return "Advanced branch protection on private organization repositories requires Team billing."
481
+	case "branch-protection-upgrade-pro":
482
+		return "Advanced branch protection on private personal repositories requires Pro. Upgrade at /settings/billing."
415483
 	case "branch-protection-billing":
416
-		return "Advanced branch protection is read-only until Team billing is brought back into good standing."
484
+		return "Advanced branch protection is read-only until billing is brought back into good standing."
417485
 	case "branch-protection-enterprise":
418486
 		return "Advanced branch protection is unavailable for Enterprise preview organizations. Contact sales to enable it."
419487
 	case "required-reviewers-upgrade":
420488
 		return "Required reviewers on private organization repositories require Team billing."
489
+	case "required-reviewers-upgrade-pro":
490
+		return "Required reviewers on private personal repositories require Pro. Upgrade at /settings/billing."
491
+	case "required-reviewers-multi-upgrade":
492
+		return "Requiring more than one reviewer on private organization repositories requires Team billing."
493
+	case "required-reviewers-multi-upgrade-pro":
494
+		return "Requiring more than one reviewer on private personal repositories requires Pro. Upgrade at /settings/billing."
421495
 	case "required-reviewers-billing":
422
-		return "Required reviewers are read-only until Team billing is brought back into good standing."
496
+		return "Required reviewers are read-only until billing is brought back into good standing."
423497
 	case "required-reviewers-enterprise":
424498
 		return "Required reviewers are unavailable for Enterprise preview organizations. Contact sales to enable them."
425499
 	default: