Name the way past the last-tier refusal
The refusal of a default ladder's only tier said what was refused and why, but not what to do instead (#187). It now reads, for one org type: "New Personal organizations start on this tier, the only one in their default plan. Add another tier first, or choose a new default for Personal." With two or more types it names them all and ends "choose a new default for each". LastTierRefusal writes it, so the plain delete, the removal, the refused removal preview and the disabled Remove on the tier card all read the same sentence. A table test covers one, two and three types, and the three goldens that carry the sentence change only in it. Ride-along: three tests failed a page that leaked nothing whenever the hex digits of an id on it contained "23505", because they looked for the unique-violation code as a substring. requireNoDriverText matches it as a word of its own. The full suite passes.
This commit is contained in:
@@ -239,16 +239,29 @@ func nullString(s string) sql.NullString {
|
||||
// type defaults to. It returns the sentence that refuses the deletion of
|
||||
// productID's tier when that tier is the ladder's only one and
|
||||
// defaultingTypes, the display names of the organization types whose default
|
||||
// ladder it is, is not empty; otherwise it returns "". It reads no row, so the
|
||||
// plain delete and the removal apply the one rule: new signups land on the
|
||||
// ladder's rank-0 tier, and a ladder with no tier leaves them without a
|
||||
// starting plan. Emptying a ladder no type defaults to stays legal.
|
||||
// ladder it is, is not empty; otherwise it returns "". The sentence says that
|
||||
// new organizations of those types start on this tier, the only one in their
|
||||
// default plan, and names the way past the refusal: add another tier first,
|
||||
// or choose a new default (for the type when one is named, for each when
|
||||
// several are). It reads no row, so the plain delete and the removal apply the
|
||||
// one rule: new signups land on the ladder's rank-0 tier, and a ladder with no
|
||||
// tier leaves them without a starting plan. Emptying a ladder no type defaults
|
||||
// to stays legal.
|
||||
func LastTierRefusal(tiers []billing.PlanLadderTier, productID string, defaultingTypes []string) string {
|
||||
if len(tiers) != 1 || tiers[0].ProductID != productID || len(defaultingTypes) == 0 {
|
||||
return ""
|
||||
}
|
||||
return "This ladder's only remaining tier; " + strings.Join(defaultingTypes, ", ") +
|
||||
" defaults new signups to it. Deleting it would leave those signups without a starting plan, so the removal is refused."
|
||||
n := len(defaultingTypes)
|
||||
names := defaultingTypes[0]
|
||||
if n > 1 {
|
||||
names = strings.Join(defaultingTypes[:n-1], ", ") + " and " + defaultingTypes[n-1]
|
||||
}
|
||||
newDefault := "for each"
|
||||
if n == 1 {
|
||||
newDefault = "for " + defaultingTypes[0]
|
||||
}
|
||||
return "New " + names + " organizations start on this tier, the only one in their default plan. " +
|
||||
"Add another tier first, or choose a new default " + newDefault + "."
|
||||
}
|
||||
|
||||
// orgTypesDefaultingTo returns the display names of the organization types
|
||||
|
||||
@@ -143,8 +143,11 @@ func TestRunPlanActNamesAFailedCommit(t *testing.T) {
|
||||
func TestLastTierRefusal(t *testing.T) {
|
||||
only := []billing.PlanLadderTier{{ProductID: "basic"}}
|
||||
two := []billing.PlanLadderTier{{ProductID: "basic"}, {ProductID: "plus"}}
|
||||
const sentence = "This ladder's only remaining tier; Personal, Team defaults new signups to it. " +
|
||||
"Deleting it would leave those signups without a starting plan, so the removal is refused."
|
||||
const (
|
||||
one = "New Personal organizations start on this tier, the only one in their default plan. Add another tier first, or choose a new default for Personal."
|
||||
two2 = "New Personal and Team organizations start on this tier, the only one in their default plan. Add another tier first, or choose a new default for each."
|
||||
three = "New Personal, Team and Club organizations start on this tier, the only one in their default plan. Add another tier first, or choose a new default for each."
|
||||
)
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
tiers []billing.PlanLadderTier
|
||||
@@ -152,7 +155,10 @@ func TestLastTierRefusal(t *testing.T) {
|
||||
defaults []string
|
||||
want string
|
||||
}{
|
||||
{"the only tier of a default ladder", only, "basic", []string{"Personal", "Team"}, sentence},
|
||||
{"the only tier of a ladder one type defaults to", only, "basic", []string{"Personal"}, one},
|
||||
{"the only tier of a ladder two types default to", only, "basic", []string{"Personal", "Team"}, two2},
|
||||
{"the only tier of a ladder three types default to", only, "basic", []string{"Personal", "Team", "Club"}, three},
|
||||
{"the only tier of a ladder with an empty list of types", only, "basic", []string{}, ""},
|
||||
{"the only tier of a ladder no type defaults to", only, "basic", nil, ""},
|
||||
{"a tier of a ladder with another tier", two, "basic", []string{"Personal"}, ""},
|
||||
{"a product that is not the only tier", only, "plus", []string{"Personal"}, ""},
|
||||
@@ -175,7 +181,7 @@ func TestTierRemovalRefusal(t *testing.T) {
|
||||
viaType = " 2 of them hold it through their org type's default; "
|
||||
change = "changing that default with Migrate moves them in the background."
|
||||
reorder = "changing that default, or reordering this ladder, with Migrate moves them in the background."
|
||||
lastTier = "This ladder's only remaining tier; Personal defaults new signups to it. Deleting it would leave those signups without a starting plan, so the removal is refused."
|
||||
lastTier = "New Personal organizations start on this tier, the only one in their default plan. Add another tier first, or choose a new default for Personal."
|
||||
)
|
||||
personal := []string{"Personal"}
|
||||
for _, tc := range []struct {
|
||||
|
||||
@@ -4,7 +4,7 @@
|
||||
package server_test
|
||||
|
||||
// Tests for DeletePlanLadderTier's last-tier guard (schema-hardening tasks
|
||||
// 2.3 + 2.5, design D4a): deleting a ladder's only remaining tier is refused
|
||||
// 2.3 + 2.5, design D4a): deleting a ladder's last tier is refused
|
||||
// when some org type's default_plan_ladder_id references that ladder (new
|
||||
// signups would land on a plan-less ladder), but stays allowed for a ladder
|
||||
// no org type defaults to (the 2026-07-03 remediation's narrow guard).
|
||||
@@ -54,7 +54,7 @@ func TestDeletePlanLadderTier_RefusesLastTierOfDefaultLadder(t *testing.T) {
|
||||
if code != http.StatusOK {
|
||||
t.Fatalf("status=%d body=%s", code, body)
|
||||
}
|
||||
if !strings.Contains(body, "only remaining tier") {
|
||||
if !strings.Contains(body, "the only one in their default plan") {
|
||||
t.Errorf("refusal copy missing; body:\n%s", body)
|
||||
}
|
||||
// The fixture's org type display name is "OTC "+suffix (newOtcFixture).
|
||||
|
||||
@@ -17,6 +17,7 @@ import (
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"net/url"
|
||||
"regexp"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
@@ -113,11 +114,7 @@ func TestCreatePlanLadder_DuplicateNameRenders422OnTheNameField(t *testing.T) {
|
||||
if !strings.Contains(body, "is-invalid") || !strings.Contains(body, "invalid-feedback") {
|
||||
t.Errorf("expected the message to render on the name field, got: %s", body)
|
||||
}
|
||||
for _, leak := range []string{"uq_plan_ladders_name_ci", "SQLSTATE", "23505"} {
|
||||
if strings.Contains(body, leak) {
|
||||
t.Errorf("response leaked driver text %q: %s", leak, body)
|
||||
}
|
||||
}
|
||||
requireNoDriverText(t, body, "uq_plan_ladders_name_ci")
|
||||
}
|
||||
|
||||
// TestUpdatePlanLadder_DuplicateNameRenders422OnTheNameField covers the same
|
||||
@@ -172,11 +169,7 @@ func TestUpdatePlanLadder_DuplicateNameRenders422OnTheNameField(t *testing.T) {
|
||||
if !strings.Contains(body, "is-invalid") || !strings.Contains(body, "invalid-feedback") {
|
||||
t.Errorf("expected the message to render on the name field, got: %s", body)
|
||||
}
|
||||
for _, leak := range []string{"uq_plan_ladders_name_ci", "SQLSTATE", "23505"} {
|
||||
if strings.Contains(body, leak) {
|
||||
t.Errorf("response leaked driver text %q: %s", leak, body)
|
||||
}
|
||||
}
|
||||
requireNoDriverText(t, body, "uq_plan_ladders_name_ci")
|
||||
|
||||
reloaded, err := bq.GetPlanLadderByID(ctx, ladder.PlanLadderID)
|
||||
if err != nil {
|
||||
@@ -190,3 +183,22 @@ func TestUpdatePlanLadder_DuplicateNameRenders422OnTheNameField(t *testing.T) {
|
||||
func uniqueLadderSuffix() string {
|
||||
return uuid.New().String()[:8]
|
||||
}
|
||||
|
||||
// uniqueViolationCode matches the unique-violation SQLSTATE as a word of its
|
||||
// own. A substring match would also find it among the hex digits of an id the
|
||||
// page renders, and fail a page that leaks nothing.
|
||||
var uniqueViolationCode = regexp.MustCompile(`\b23505\b`)
|
||||
|
||||
// requireNoDriverText fails when body carries the constraint's name, the word
|
||||
// SQLSTATE, or the unique-violation code: the driver text of a duplicate name.
|
||||
func requireNoDriverText(t *testing.T, body, constraint string) {
|
||||
t.Helper()
|
||||
for _, leak := range []string{constraint, "SQLSTATE"} {
|
||||
if strings.Contains(body, leak) {
|
||||
t.Errorf("response leaked driver text %q: %s", leak, body)
|
||||
}
|
||||
}
|
||||
if uniqueViolationCode.MatchString(body) {
|
||||
t.Errorf("response leaked driver text %q: %s", "23505", body)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -308,7 +308,7 @@ func TestTierRemoval_CommitRefusesWhatThePreviewRefuses(t *testing.T) {
|
||||
|
||||
rec := r.commit(h, r.basic, disposition)
|
||||
|
||||
if got := htmltest.Parse(t, rec.Body.String()).Text(); !strings.Contains(got, "This ladder's only remaining tier;") || !strings.Contains(got, "so the removal is refused.") {
|
||||
if got := htmltest.Parse(t, rec.Body.String()).Text(); !strings.Contains(got, "start on this tier, the only one in their default plan.") || !strings.Contains(got, "Add another tier first") {
|
||||
t.Errorf("the answer lacks the last-tier sentence: %.500s", got)
|
||||
}
|
||||
if !r.tierExists(t, r.basic) || r.acts(t) != 0 {
|
||||
|
||||
@@ -1,8 +1,8 @@
|
||||
{
|
||||
"answer": [
|
||||
"status 200",
|
||||
"Hx-Trigger showErrorToast: This ladder's only remaining tier; T defaults new signups to it. Deleting it would leave those signups without a starting plan, so the removal is refused.",
|
||||
"[role=\"alert\"]: This ladder's only remaining tier; T defaults new signups to it. Deleting it would leave those signups without a starting plan, so the removal is refused."
|
||||
"Hx-Trigger showErrorToast: New T organizations start on this tier, the only one in their default plan. Add another tier first, or choose a new default for T.",
|
||||
"[role=\"alert\"]: New T organizations start on this tier, the only one in their default plan. Add another tier first, or choose a new default for T."
|
||||
],
|
||||
"checked": [],
|
||||
"logs": [],
|
||||
|
||||
@@ -1,8 +1,8 @@
|
||||
{
|
||||
"answer": [
|
||||
"status 200",
|
||||
"Hx-Trigger showErrorToast: This ladder's only remaining tier; T defaults new signups to it. Deleting it would leave those signups without a starting plan, so the removal is refused.",
|
||||
"[role=\"alert\"]: This ladder's only remaining tier; T defaults new signups to it. Deleting it would leave those signups without a starting plan, so the removal is refused."
|
||||
"Hx-Trigger showErrorToast: New T organizations start on this tier, the only one in their default plan. Add another tier first, or choose a new default for T.",
|
||||
"[role=\"alert\"]: New T organizations start on this tier, the only one in their default plan. Add another tier first, or choose a new default for T."
|
||||
],
|
||||
"checked": [],
|
||||
"logs": [],
|
||||
|
||||
@@ -1,8 +1,8 @@
|
||||
{
|
||||
"answer": [
|
||||
"status 200",
|
||||
"Hx-Trigger showErrorToast: This ladder's only remaining tier; T defaults new signups to it. Deleting it would leave those signups without a starting plan, so the removal is refused.",
|
||||
"[role=\"alert\"]: This ladder's only remaining tier; T defaults new signups to it. Deleting it would leave those signups without a starting plan, so the removal is refused."
|
||||
"Hx-Trigger showErrorToast: New T organizations start on this tier, the only one in their default plan. Add another tier first, or choose a new default for T.",
|
||||
"[role=\"alert\"]: New T organizations start on this tier, the only one in their default plan. Add another tier first, or choose a new default for T."
|
||||
],
|
||||
"checked": [],
|
||||
"logs": [],
|
||||
|
||||
@@ -219,11 +219,7 @@ func TestCreateWorkspace_DuplicateNameRenders422OnTheNameField(t *testing.T) {
|
||||
if !strings.Contains(body, "is-invalid") || !strings.Contains(body, "invalid-feedback") {
|
||||
t.Errorf("expected the message to render on the name field, got: %s", body)
|
||||
}
|
||||
for _, leak := range []string{"uq_workspaces_org_id_name_ci", "SQLSTATE", "23505"} {
|
||||
if strings.Contains(body, leak) {
|
||||
t.Errorf("response leaked driver text %q: %s", leak, body)
|
||||
}
|
||||
}
|
||||
requireNoDriverText(t, body, "uq_workspaces_org_id_name_ci")
|
||||
|
||||
workspaces, err := organization.New(database).GetWorkspacesByOrgID(context.Background(), orgID)
|
||||
if err != nil {
|
||||
|
||||
@@ -819,7 +819,7 @@ Every sentence a person reads that this change adds, changes, reuses or removes.
|
||||
|
||||
| # | Sentence | New place | Since the previous draft |
|
||||
|---|---|---|---|
|
||||
| 21 | "This ladder's only remaining tier; {types} defaults new signups to it. Deleting it would leave those signups without a starting plan, so the removal is refused." | the plain delete's commit refusal, the removal's under either disposition, the refused removal preview, in place of its outcome lines and disposition choice, with Discard alone, and the reason of the tier card's disabled Remove or Preview removal on that tier | "Where" changed: Discard alone, the disabled Remove |
|
||||
| 21 | "New {type} organizations start on this tier, the only one in their default plan. Add another tier first, or choose a new default for {type}." With more than one type: "New {A} and {B} organizations start on this tier, the only one in their default plan. Add another tier first, or choose a new default for each." (three or more: "{A}, {B} and {C}") | the plain delete's commit refusal, the removal's under either disposition, the refused removal preview, in place of its outcome lines and disposition choice, with Discard alone, and the reason of the tier card's disabled Remove or Preview removal on that tier | "Where" changed: Discard alone, the disabled Remove |
|
||||
| 22 | "Already the default." | the default change's card error when a concurrent commit saved the same default first | |
|
||||
| 23 | "Another change collided with this one; try again." | tier add, tier delete, tier removal, the reorder, the default change's card and Retry, and the rule change's Apply changes, on a lock wait past the timeout or a deadlock (#216); today the tier add's field error for a rank collision | |
|
||||
| 24 | withdrawn | was the tier add's "Failed to add tier. Reference: {request id}." on its new failure paths | withdrawn: the tier add answers copy 50 on every failure path |
|
||||
|
||||
@@ -389,7 +389,7 @@ attaches to a product whose set is empty is unfinished rather than stranded.
|
||||
|
||||
### Requirement: The last tier of a live default ladder cannot be deleted
|
||||
|
||||
Deleting a plan ladder tier, whether by the plain delete of a tier no organization holds or by the removal of a tier with holders under either disposition, SHALL be refused when it is the ladder's only remaining tier AND any org type's default plan ladder references that ladder, with copy explaining that new signups land on this ladder's bottom tier. The guard, and the plain delete's count of live holders, SHALL be read inside the transaction that would delete the tier, and a failed read of the org types SHALL refuse the deletion rather than allow it. The tier card SHALL render that tier's Remove or Preview removal disabled with the explanation as its reason, because the deletion is certain to be refused. Deleting the last tier of a ladder no org type defaults to SHALL remain allowed, because emptying a non-default ladder to rebuild it is a legitimate workflow.
|
||||
Deleting a plan ladder tier, whether by the plain delete of a tier no organization holds or by the removal of a tier with holders under either disposition, SHALL be refused when it is the ladder's only remaining tier AND any org type's default plan ladder references that ladder, with copy explaining that new organizations of those types start on this tier and naming the way past the refusal: add another tier first, or choose a new default. The guard, and the plain delete's count of live holders, SHALL be read inside the transaction that would delete the tier, and a failed read of the org types SHALL refuse the deletion rather than allow it. The tier card SHALL render that tier's Remove or Preview removal disabled with the explanation as its reason, because the deletion is certain to be refused. Deleting the last tier of a ladder no org type defaults to SHALL remain allowed, because emptying a non-default ladder to rebuild it is a legitimate workflow.
|
||||
|
||||
#### Scenario: Last tier of the signup default is protected
|
||||
|
||||
|
||||
Reference in New Issue
Block a user