Break grants-listing ties by the later-written grant
ListGrantsWithDelivery now orders by created_at DESC, grant_id DESC. Grants written in one transaction share created_at (a tier removal writes one default grant per vacated pool in one transaction), and the old order left those rows to the plan: their relative order was not defined, so the page cut between tied grants could list one on two pages or on none. The grant id is time-ordered, so the later-written grant lists first and each lists on exactly one page. The query comment says so; the generated code differs from the parent only in the ORDER BY and that comment. TestGrantsListingBreaksTiesByGrantID writes three grants of one organization in one rolled-back transaction (one created_at), and checks the organization call lists them by grant id descending and that search calls of one row at offsets 0, 1 and 2 return each once in that order. At the parent it fails on both checks: the grants list earliest-written first, in the same order on every page. It passes here. How it was shown to change only that: names and results of internal/server and internal/entitlements are equal at the parent and here (1587 names at the parent, 1588 here), the new test the only addition. No golden file changed. The screens run reports the same three differing screens it reports on every run (operator-domains desktop, operator-integration-stripe desktop and mobile); the Grants and organization pages are the same, as the seeded stack holds no tied grants. The break that drops the grant id from the ORDER BY is caught by the new test. Unprobed: a repeated or missing grant across pages was not reproduced, since the three-row run at the parent lists tied grants in one stable order, not a shifting one; whether a larger tie under another plan shifts between pages is left unobserved because the plan chooses it.
This commit is contained in:
@@ -766,7 +766,7 @@ SELECT listed.grant_id, listed.granted_to_org_id, listed.product_id, listed.gran
|
||||
FROM listed
|
||||
WHERE $1::text IS NULL
|
||||
OR listed.delivery_state = $1::text
|
||||
ORDER BY listed.created_at DESC
|
||||
ORDER BY listed.created_at DESC, listed.grant_id DESC
|
||||
LIMIT $3
|
||||
OFFSET $2
|
||||
`
|
||||
@@ -886,9 +886,10 @@ type ListGrantsWithDeliveryRow struct {
|
||||
// the grant holds a non-ended provision: a grant that resumed must not read
|
||||
// an earlier delivery's end beside its new activation (grant-resumption
|
||||
// D20).
|
||||
// Ordered by created_at DESC alone (tier-changes-ledger design D5): the
|
||||
// Live facet and the Active tab already select the delivering rows, so
|
||||
// ordering states creation time only and does not also pretend to filter.
|
||||
// Ordered by created_at DESC (tier-changes-ledger design D5): the Live facet
|
||||
// and the Active tab already select the delivering rows, so ordering states
|
||||
// creation time only and does not also pretend to filter. Grants that share a
|
||||
// created_at fall to grant_id DESC, so the later-written grant lists first.
|
||||
// The lineage sub-ordering (the replaced_by_grant_id subquery below) stays
|
||||
// child.created_at DESC.
|
||||
//
|
||||
@@ -911,7 +912,10 @@ type ListGrantsWithDeliveryRow struct {
|
||||
// NULL reads every row, from the first.
|
||||
//
|
||||
// total_count counts the rows every filter keeps, before the page. Rows
|
||||
// come newest first by created_at alone (tier-changes-ledger D5).
|
||||
// come newest first by created_at (tier-changes-ledger D5). Grants written in
|
||||
// one transaction share created_at, so ties fall to the grant id, which is
|
||||
// time-ordered and lists the later-written grant first; each grant lists on
|
||||
// exactly one page.
|
||||
// Each grant's lineage root, for the default-duplicate line.
|
||||
// One row per grant that passes the org and search filters, with its
|
||||
// delivery state computed once; the outer SELECT filters on it.
|
||||
|
||||
@@ -301,9 +301,10 @@ type Querier interface {
|
||||
// the grant holds a non-ended provision: a grant that resumed must not read
|
||||
// an earlier delivery's end beside its new activation (grant-resumption
|
||||
// D20).
|
||||
// Ordered by created_at DESC alone (tier-changes-ledger design D5): the
|
||||
// Live facet and the Active tab already select the delivering rows, so
|
||||
// ordering states creation time only and does not also pretend to filter.
|
||||
// Ordered by created_at DESC (tier-changes-ledger design D5): the Live facet
|
||||
// and the Active tab already select the delivering rows, so ordering states
|
||||
// creation time only and does not also pretend to filter. Grants that share a
|
||||
// created_at fall to grant_id DESC, so the later-written grant lists first.
|
||||
// The lineage sub-ordering (the replaced_by_grant_id subquery below) stays
|
||||
// child.created_at DESC.
|
||||
//
|
||||
@@ -326,7 +327,10 @@ type Querier interface {
|
||||
// NULL reads every row, from the first.
|
||||
//
|
||||
// total_count counts the rows every filter keeps, before the page. Rows
|
||||
// come newest first by created_at alone (tier-changes-ledger D5).
|
||||
// come newest first by created_at (tier-changes-ledger D5). Grants written in
|
||||
// one transaction share created_at, so ties fall to the grant id, which is
|
||||
// time-ordered and lists the later-written grant first; each grant lists on
|
||||
// exactly one page.
|
||||
// Each grant's lineage root, for the default-duplicate line.
|
||||
// One row per grant that passes the org and search filters, with its
|
||||
// delivery state computed once; the outer SELECT filters on it.
|
||||
|
||||
@@ -88,9 +88,10 @@ ORDER BY created_at DESC;
|
||||
-- the grant holds a non-ended provision: a grant that resumed must not read
|
||||
-- an earlier delivery's end beside its new activation (grant-resumption
|
||||
-- D20).
|
||||
-- Ordered by created_at DESC alone (tier-changes-ledger design D5): the
|
||||
-- Live facet and the Active tab already select the delivering rows, so
|
||||
-- ordering states creation time only and does not also pretend to filter.
|
||||
-- Ordered by created_at DESC (tier-changes-ledger design D5): the Live facet
|
||||
-- and the Active tab already select the delivering rows, so ordering states
|
||||
-- creation time only and does not also pretend to filter. Grants that share a
|
||||
-- created_at fall to grant_id DESC, so the later-written grant lists first.
|
||||
-- The lineage sub-ordering (the replaced_by_grant_id subquery below) stays
|
||||
-- child.created_at DESC.
|
||||
--
|
||||
@@ -113,7 +114,10 @@ ORDER BY created_at DESC;
|
||||
-- NULL reads every row, from the first.
|
||||
--
|
||||
-- total_count counts the rows every filter keeps, before the page. Rows
|
||||
-- come newest first by created_at alone (tier-changes-ledger D5).
|
||||
-- come newest first by created_at (tier-changes-ledger D5). Grants written in
|
||||
-- one transaction share created_at, so ties fall to the grant id, which is
|
||||
-- time-ordered and lists the later-written grant first; each grant lists on
|
||||
-- exactly one page.
|
||||
WITH RECURSIVE waits AS (
|
||||
SELECT w.grant_id::uuid AS grant_id,
|
||||
w.pool_id::uuid AS pool_id,
|
||||
@@ -320,7 +324,7 @@ SELECT listed.*, count(*) OVER() AS total_count
|
||||
FROM listed
|
||||
WHERE sqlc.narg(delivery_state)::text IS NULL
|
||||
OR listed.delivery_state = sqlc.narg(delivery_state)::text
|
||||
ORDER BY listed.created_at DESC
|
||||
ORDER BY listed.created_at DESC, listed.grant_id DESC
|
||||
LIMIT sqlc.narg(page_limit)
|
||||
OFFSET sqlc.narg(page_offset);
|
||||
|
||||
|
||||
@@ -177,9 +177,10 @@ func assertDeliveryRendering(t *testing.T, surface, body string) {
|
||||
|
||||
// TestGrantDeliveryState_OrderingByCreatedAt pins design D5 (the
|
||||
// tier-changes-ledger change): the composite's grants ledger orders by
|
||||
// created_at DESC alone, so a newer grant that stopped delivering still
|
||||
// outranks an older one still delivering -- the Live facet and the Active
|
||||
// tab already do the filtering, so ordering does not also pretend to.
|
||||
// created_at DESC with no delivery-state key, so a newer grant that stopped
|
||||
// delivering still outranks an older one still delivering -- the Live facet
|
||||
// and the Active tab already do the filtering, so ordering does not also
|
||||
// pretend to.
|
||||
func TestGrantDeliveryState_OrderingByCreatedAt(t *testing.T) {
|
||||
database := dbtest.Open(t)
|
||||
f := newOtcFixture(t, database)
|
||||
|
||||
@@ -569,9 +569,9 @@ func TestGrantsListPagingMath(t *testing.T) {
|
||||
|
||||
// TestGrantsListOrdering_CreatedAtRegardlessOfDelivery pins design D5
|
||||
// (tier-changes-ledger): the paged grants query orders by created_at DESC
|
||||
// alone, so a newer grant with no delivery still outranks an older one
|
||||
// that is live -- the Live facet and the Active tab already do the
|
||||
// filtering, so ordering does not also pretend to.
|
||||
// with no delivery-state key, so a newer grant with no delivery still
|
||||
// outranks an older one that is live -- the Live facet and the Active tab
|
||||
// already do the filtering, so ordering does not also pretend to.
|
||||
func TestGrantsListOrdering_CreatedAtRegardlessOfDelivery(t *testing.T) {
|
||||
database := dbtest.Open(t)
|
||||
f := newGLFixture(t, database)
|
||||
@@ -615,3 +615,101 @@ func TestGrantsListOrdering_CreatedAtRegardlessOfDelivery(t *testing.T) {
|
||||
t.Errorf("expected the older Live grant second, got second=%+v", rows[1])
|
||||
}
|
||||
}
|
||||
|
||||
// TestGrantsListingBreaksTiesByGrantID writes three grants of one organization
|
||||
// in one rolled-back transaction, so they share created_at, and checks that
|
||||
// the organization call lists them by grant id descending and that search
|
||||
// calls of one row at offsets 0, 1 and 2 return each once in that order.
|
||||
func TestGrantsListingBreaksTiesByGrantID(t *testing.T) {
|
||||
database := dbtest.Open(t)
|
||||
ctx := context.Background()
|
||||
tx, err := entitlements.BeginMaterializing(ctx, database)
|
||||
if err != nil {
|
||||
t.Fatalf("begin: %v", err)
|
||||
}
|
||||
defer tx.Rollback()
|
||||
|
||||
iq := identity.New(tx)
|
||||
user, err := iq.CreateUser(ctx, "gl-tie-u-"+uuid.NewString())
|
||||
if err != nil {
|
||||
t.Fatalf("user: %v", err)
|
||||
}
|
||||
person, err := iq.CreatePerson(ctx, identity.CreatePersonParams{
|
||||
UserID: user.UserID, DisplayName: "GL Tie Operator",
|
||||
PrimaryEmail: "gl-tie-" + uuid.NewString()[:8] + "@example.com", PrimaryEmailVerified: true,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("person: %v", err)
|
||||
}
|
||||
_, productID := glLadder(t, ctx, tx, "gl-tie-"+uuid.NewString()[:8])
|
||||
|
||||
marker := "GLTie" + uuid.NewString()[:8]
|
||||
org, err := organization.New(tx).CreateOrganization(ctx, organization.CreateOrganizationParams{
|
||||
Name: "GL Tie Org " + marker, OrgType: "personal", OwnerPersonID: person.PersonID,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("org: %v", err)
|
||||
}
|
||||
if _, err := entitlements.New(tx).CreateResourcePool(ctx, entitlements.CreateResourcePoolParams{
|
||||
OrgID: org.OrgID, Name: "Default", PoolType: "default", IsAutoManaged: true,
|
||||
}); err != nil {
|
||||
t.Fatalf("pool: %v", err)
|
||||
}
|
||||
|
||||
// Written first to last, the ids rise, so the listing reads them backwards.
|
||||
var want []string
|
||||
for i := 0; i < 3; i++ {
|
||||
var grantID string
|
||||
if err := tx.QueryRowContext(ctx,
|
||||
`INSERT INTO core.grants (product_id, granted_to_org_id, granted_by_person_id, grant_reason, quantity)
|
||||
VALUES ($1, $2, $3, 'manual', 1) RETURNING grant_id`,
|
||||
productID, org.OrgID, person.PersonID,
|
||||
).Scan(&grantID); err != nil {
|
||||
t.Fatalf("insert grant %d: %v", i, err)
|
||||
}
|
||||
want = append([]string{grantID}, want...)
|
||||
}
|
||||
var distinct int
|
||||
if err := tx.QueryRowContext(ctx,
|
||||
`SELECT count(DISTINCT created_at) FROM core.grants WHERE granted_to_org_id = $1`, org.OrgID,
|
||||
).Scan(&distinct); err != nil {
|
||||
t.Fatalf("count created_at: %v", err)
|
||||
}
|
||||
if distinct != 1 {
|
||||
t.Fatalf("grants carry %d distinct created_at values, want 1 (one transaction)", distinct)
|
||||
}
|
||||
|
||||
q := entitlements.New(tx)
|
||||
rows, err := q.ListGrantsWithDelivery(ctx, entitlements.ListGrantsWithDeliveryParams{
|
||||
OrgID: uuid.NullUUID{UUID: uuid.MustParse(org.OrgID), Valid: true},
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("ListGrantsWithDelivery (organization): %v", err)
|
||||
}
|
||||
var got []string
|
||||
for _, r := range rows {
|
||||
got = append(got, r.GrantID)
|
||||
}
|
||||
if strings.Join(got, ",") != strings.Join(want, ",") {
|
||||
t.Errorf("organization call lists %v, want grant id descending %v", got, want)
|
||||
}
|
||||
|
||||
var paged []string
|
||||
for offset := int32(0); offset < 3; offset++ {
|
||||
page, err := q.ListGrantsWithDelivery(ctx, entitlements.ListGrantsWithDeliveryParams{
|
||||
Q: sql.NullString{String: marker, Valid: true},
|
||||
PageLimit: sql.NullInt32{Int32: 1, Valid: true},
|
||||
PageOffset: sql.NullInt32{Int32: offset, Valid: true},
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("ListGrantsWithDelivery (search, offset %d): %v", offset, err)
|
||||
}
|
||||
if len(page) != 1 {
|
||||
t.Fatalf("search page at offset %d holds %d rows, want 1", offset, len(page))
|
||||
}
|
||||
paged = append(paged, page[0].GrantID)
|
||||
}
|
||||
if strings.Join(paged, ",") != strings.Join(want, ",") {
|
||||
t.Errorf("search pages of one row list %v, want each once in order %v", paged, want)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user