fix(cabana): search and sort to-many relation columns through subqueries

- relationSubquery reads the pivot, key and table names from GORM's parsed relationship (Schema.Relationships.Relations), never from a convention - list search matches every relation column through a correlated EXISTS, so the count query gains no JOIN and belongsToMany search stops failing - sorting a hasMany or belongsToMany column orders by a correlated string_agg subquery before the primary-key tiebreak - polymorphic relations and unknown related columns answer 422 - DryRun smoke test TestListRelationSearchSmoke needs no Docker
2026-10-09 20:56:53 +02:00
parent 3ca6a0d4b2
commit 06d75cb654
3 changed files with 353 additions and 25 deletions
--- a/docs/backend/lists-and-filters.md
+++ b/docs/backend/lists-and-filters.md
@@ -59,7 +59,7 @@ columns:
        type: datetime
 ```

-A column takes `label`, `type` (`text`, `datetime`, `date`, `time` or `switch`), `searchable` (default false) and `sortable` (default true, as in WinterCMS), and, for a column read through a relation, `relation` and `select`. Other WinterCMS column types and options are refused at boot. When `type` is omitted, a `time.Time` column is `datetime`, a `lagoon.Date` column is `date` and a `lagoon.TimeOfDay` column is `time`; every other column stays untyped (the admin treats that as text). An explicit `type` always wins. The compiled list keeps the columns in file order:
+A column takes `label`, `type` (`text`, `datetime`, `date`, `time` or `switch`), `searchable` (default false) and `sortable` (default true, as in WinterCMS), and, for a column read through a relation, `relation` and `select`. Other WinterCMS column types and options are refused at boot. When `type` is omitted, a `time.Time` column is `datetime`, a `lagoon.Date` column is `date` and a `lagoon.TimeOfDay` column is `time`; every other column stays untyped (the admin treats that as text). An explicit `type` always wins. A searchable relation column matches a record when any related record matches: a `categories` column with `relation: categories` and `select: name` on a blog post finds the posts with at least one matching category, for belongsTo, hasOne, hasMany and belongsToMany relations alike, each through a subquery, so a post is listed and counted once. A sortable hasMany or belongsToMany column sorts by its comma-joined values, the text the list shows. The compiled list keeps the columns in file order:

 ```go src=modules/cabana/example_test.go#ExampleCompileList
 // The plugin embeds its controllers/ and models/ trees; the example reads
--- a/modules/cabana/query.go
+++ b/modules/cabana/query.go
@@ -17,6 +17,7 @@ import (
 	"git.golem15.com/golem15/summercms/modules/pact"
 	"gorm.io/gorm"
 	"gorm.io/gorm/clause"
+	gormschema "gorm.io/gorm/schema"
 )

 // ListInput is the raw admin list query. Identifiers are matched later
@@ -129,7 +130,10 @@ func ExecuteList(ctx context.Context, db *gorm.DB, cc *CompiledController, in Li
 	if err := q.Session(&gorm.Session{}).Count(&total).Error; err != nil {
 		return nil, err
 	}
-	q = applyListOrder(q, cc.Controller, cc.List, model, sortCol, sortDesc)
+	q, err = applyListOrder(q, cc.Controller, cc.List, model, sortCol, sortDesc)
+	if err != nil {
+		return nil, err
+	}
 	for _, col := range cc.List.Columns {
 		if col.Relation == "" {
 			continue
@@ -290,12 +294,11 @@ func normalizeSort(schema *ListSchema, in ListInput) (string, bool, error) {
 	return match.Key, dir == "desc", nil
 }

-func applyListOrder(db *gorm.DB, controller pact.AdminController, schema *ListSchema, model any, column string, desc bool) *gorm.DB {
+func applyListOrder(db *gorm.DB, controller pact.AdminController, schema *ListSchema, model any, column string, desc bool) (*gorm.DB, error) {
 	table := tableName(model)
 	pk := primaryColumn(model)
 	if column != "" {
-		orderTable := table
-		orderColumn := column
+		order := clause.OrderByColumn{Column: clause.Column{Table: table, Name: column}, Desc: desc}
 		for _, col := range schema.Columns {
 			if col.Key != column || col.Relation == "" {
 				continue
@@ -304,22 +307,149 @@ func applyListOrder(db *gorm.DB, controller pact.AdminController, schema *ListSc
 			if !ok {
 				continue
 			}
-			orderTable = field
-			orderColumn = listRelationColumn(controller, col.Relation, col.Select)
+			sub, err := relationSubquery(db, model, field, listRelationColumn(controller, col.Relation, col.Select), 0, "sort")
+			if err != nil {
+				return nil, err
+			}
+			if sub.toMany {
+				// Winter sorts a to-many column on its group_concat sub-select;
+				// string_agg in the same order is the comma-joined value the
+				// list displays (relatedSelectMany). A raw column merges with
+				// the primary-key tiebreak below into one ORDER BY.
+				value := "CAST(" + sub.column + " AS TEXT)"
+				order.Column = clause.Column{
+					Name: fmt.Sprintf("(SELECT string_agg(%s, ', ' ORDER BY %s) FROM %s WHERE %s)", value, value, sub.from, sub.where),
+					Raw:  true,
+				}
+				break
+			}
+			// A to-one relation is joined; GORM aliases the joined
+			// association with the Go field name.
 			db = db.Joins(field)
+			order.Column = clause.Column{Table: field, Name: sub.name}
 			break
 		}
-		db = db.Order(clause.OrderByColumn{
-			Column: clause.Column{Table: orderTable, Name: orderColumn},
-			Desc:   desc,
-		})
+		db = db.Order(order)
 	}
 	if column != pk {
 		db = db.Order(clause.OrderByColumn{Column: clause.Column{Table: table, Name: pk}})
 	}
-	return db
+	return db, nil
 }

+// relationSQL is the FROM and WHERE of a subquery over the related records
+// of one relation, correlated to the outer list query's main table. It holds
+// no bound values: every name in it comes from GORM's parsed relationship and
+// is quoted by the dialector.
+type relationSQL struct {
+	// from is the related table aliased cabana_r<n>; a belongsToMany
+	// relation adds its pivot aliased cabana_p<n>, joined to the related
+	// table.
+	from string
+	// where correlates the related records to the main table's row and
+	// skips soft-deleted related records.
+	where string
+	// column is the selected related column, qualified with the alias.
+	column string
+	// name is the selected column's physical name.
+	name string
+	// toMany is true for hasMany and belongsToMany relations.
+	toMany bool
+}
+
+// relationSubquery builds the subquery parts for the relation behind the Go
+// field of model, selecting column from the related table. Table, pivot and
+// key names come from GORM's parsed relationship, never from a naming
+// convention, so custom foreignKey, joinForeignKey and many2many tags and the
+// naming strategy are honored. The aliases carry n, which keeps a
+// self-referencing relation apart from the main table and two subqueries of
+// one statement apart from each other. A missing relation, a column the
+// related model lacks and polymorphic relations are refused as validation
+// failures on param.
+func relationSubquery(db *gorm.DB, model any, field, column string, n int, param string) (relationSQL, error) {
+	invalid := listInvalid(param, "is not a searchable relation column")
+	if param == "sort" {
+		invalid = listInvalid(param, "is not a sortable column")
+	}
+	stmt := &gorm.Statement{DB: db}
+	if err := stmt.Parse(model); err != nil {
+		return relationSQL{}, err
+	}
+	rel, ok := stmt.Schema.Relationships.Relations[field]
+	if !ok || rel == nil || rel.FieldSchema == nil || rel.Polymorphic != nil || !identifier(column) {
+		return relationSQL{}, invalid
+	}
+	selected := rel.FieldSchema.LookUpField(column)
+	if selected == nil || selected.DBName == "" {
+		return relationSQL{}, invalid
+	}
+	for _, ref := range rel.References {
+		if ref == nil || ref.PrimaryValue != "" || ref.PrimaryKey == nil || ref.ForeignKey == nil {
+			return relationSQL{}, invalid
+		}
+	}
+	main := stmt.Schema.Table
+	related := fmt.Sprintf("cabana_r%d", n)
+	out := relationSQL{
+		column: qualifiedColumn(db, related, selected.DBName),
+		name:   selected.DBName,
+	}
+	var on, where []string
+	switch rel.Type {
+	case gormschema.Many2Many:
+		if rel.JoinTable == nil {
+			return relationSQL{}, invalid
+		}
+		pivot := fmt.Sprintf("cabana_p%d", n)
+		for _, ref := range rel.References {
+			if ref.OwnPrimaryKey {
+				// pivot owner key = main primary key
+				where = append(where, qualifiedColumn(db, pivot, ref.ForeignKey.DBName)+" = "+qualifiedColumn(db, main, ref.PrimaryKey.DBName))
+			} else {
+				// pivot related key = related primary key
+				on = append(on, qualifiedColumn(db, pivot, ref.ForeignKey.DBName)+" = "+qualifiedColumn(db, related, ref.PrimaryKey.DBName))
+			}
+		}
+		if len(on) == 0 || len(where) == 0 {
+			return relationSQL{}, invalid
+		}
+		out.from = fmt.Sprintf("%s AS %s JOIN %s AS %s ON %s",
+			quotedIdent(db, rel.JoinTable.Table), quotedIdent(db, pivot),
+			quotedIdent(db, rel.FieldSchema.Table), quotedIdent(db, related),
+			strings.Join(on, " AND "))
+		out.toMany = true
+	case gormschema.HasMany, gormschema.HasOne:
+		for _, ref := range rel.References {
+			// related foreign key = main primary key
+			where = append(where, qualifiedColumn(db, related, ref.ForeignKey.DBName)+" = "+qualifiedColumn(db, main, ref.PrimaryKey.DBName))
+		}
+		out.from = quotedIdent(db, rel.FieldSchema.Table) + " AS " + quotedIdent(db, related)
+		out.toMany = rel.Type == gormschema.HasMany
+	case gormschema.BelongsTo:
+		for _, ref := range rel.References {
+			// related primary key = main foreign key
+			where = append(where, qualifiedColumn(db, related, ref.PrimaryKey.DBName)+" = "+qualifiedColumn(db, main, ref.ForeignKey.DBName))
+		}
+		out.from = quotedIdent(db, rel.FieldSchema.Table) + " AS " + quotedIdent(db, related)
+	default:
+		return relationSQL{}, invalid
+	}
+	if len(where) == 0 {
+		return relationSQL{}, invalid
+	}
+	// Winter's whereHas applies the related model's soft-delete scope.
+	for _, f := range rel.FieldSchema.Fields {
+		if f.DBName != "" && f.FieldType == deletedAtType {
+			where = append(where, qualifiedColumn(db, related, f.DBName)+" IS NULL")
+			break
+		}
+	}
+	out.where = strings.Join(where, " AND ")
+	return out, nil
+}
+
+var deletedAtType = reflect.TypeOf(gorm.DeletedAt{})
+
 func applyListSearch(db *gorm.DB, controller pact.AdminController, schema *ListSchema, model any, term string) (*gorm.DB, error) {
 	term = strings.TrimSpace(term)
 	if term == "" {
@@ -335,32 +465,36 @@ func applyListSearch(db *gorm.DB, controller pact.AdminController, schema *ListS
 		return nil, listInvalid("search", "is not searchable")
 	}
 	pattern := "%" + escapeLike(strings.ToLower(term)) + "%"
-	joined := map[string]struct{}{}
 	parts := make([]string, 0, len(cols))
 	args := make([]any, 0, len(cols))
 	main := tableName(model)
+	n := 0
 	for _, col := range cols {
-		table := main
-		column := col.Key
 		if col.Relation != "" {
 			field, ok := relationFieldName(model, col.Relation)
-			physical := listRelationColumn(controller, col.Relation, col.Select)
-			if !ok || !identifier(physical) {
+			if !ok {
 				return nil, listInvalid("search", "is not searchable")
 			}
-			// GORM aliases the joined association with the Go field name.
-			table = field
-			column = physical
-			if _, done := joined[field]; !done {
-				db = db.Joins(field)
-				joined[field] = struct{}{}
+			// Every relation type is searched through a correlated EXISTS
+			// (Winter's orWhereHas), so the outer query gains no JOIN: a
+			// record matching through several related rows is counted and
+			// listed once, and unqualified ListExtendQuery or scope
+			// predicates stay unambiguous.
+			sub, err := relationSubquery(db, model, field, listRelationColumn(controller, col.Relation, col.Select), n, "search")
+			if err != nil {
+				return nil, err
 			}
-		} else if !identifier(column) {
+			n++
+			parts = append(parts, fmt.Sprintf("EXISTS (SELECT 1 FROM %s WHERE %s AND LOWER(CAST(%s AS TEXT)) LIKE ? ESCAPE '\\')", sub.from, sub.where, sub.column))
+			args = append(args, pattern)
+			continue
+		}
+		if !identifier(col.Key) {
 			return nil, listInvalid("search", "is not searchable")
 		}
 		// CAST makes a numeric, date or boolean column searchable as its text
 		// form (Winter casts to TEXT too); PostgreSQL has no lower(integer).
-		parts = append(parts, fmt.Sprintf("LOWER(CAST(%s AS TEXT)) LIKE ? ESCAPE '\\'", qualifiedColumn(db, table, column)))
+		parts = append(parts, fmt.Sprintf("LOWER(CAST(%s AS TEXT)) LIKE ? ESCAPE '\\'", qualifiedColumn(db, main, col.Key)))
 		args = append(args, pattern)
 	}
 	return db.Where(strings.Join(parts, " OR "), args...), nil
--- a/modules/cabana/query_relation_test.go
+++ b/modules/cabana/query_relation_test.go
@@ -0,0 +1,194 @@
+package cabana
+
+import (
+	"context"
+	"net/http"
+	"strings"
+	"sync"
+	"testing"
+	"time"
+
+	"git.golem15.com/golem15/summercms/modules/backpack"
+	"git.golem15.com/golem15/summercms/modules/pact"
+	pgdriver "gorm.io/driver/postgres"
+	"gorm.io/gorm"
+	"gorm.io/gorm/logger"
+)
+
+// The relation fixtures cover every relation type a list column can name:
+// belongsTo (Author), belongsToMany through a pivot (Tags) and hasMany (Notes).
+
+type relAuthor struct {
+	ID   uint   `gorm:"column:id;primaryKey"`
+	Name string `gorm:"column:name"`
+}
+
+func (relAuthor) TableName() string { return "cabana_rel_authors" }
+
+type relTag struct {
+	ID   uint   `gorm:"column:id;primaryKey"`
+	Name string `gorm:"column:name"`
+}
+
+func (relTag) TableName() string { return "cabana_rel_tags" }
+
+type relNote struct {
+	ID     uint   `gorm:"column:id;primaryKey"`
+	PostID uint   `gorm:"column:post_id"`
+	Body   string `gorm:"column:body"`
+}
+
+func (relNote) TableName() string { return "cabana_rel_notes" }
+
+type relPost struct {
+	ID       uint      `gorm:"column:id;primaryKey"`
+	Title    string    `gorm:"column:title"`
+	AuthorID *uint     `gorm:"column:author_id"`
+	Author   relAuthor `gorm:"foreignKey:AuthorID"`
+	Tags     []relTag  `gorm:"many2many:cabana_rel_post_tags"`
+	Notes    []relNote `gorm:"foreignKey:PostID"`
+}
+
+func (relPost) TableName() string { return "cabana_rel_posts" }
+
+type relController struct{}
+
+func (relController) ID() string        { return "acme.demo.widgets" }
+func (relController) ModelName() string { return "Widget" }
+func (relController) ConfigDir() string { return "controllers/widgets" }
+func (relController) NewRecord() any    { return &relPost{} }
+
+var _ pact.AdminRecordSource = relController{}
+
+const relListConfig = `modelClass: Widget
+list: ~/plugins/acme/demo/models/widget/columns.yaml
+recordsPerPage: 20
+showSorting: true
+`
+
+const relColumns = `columns:
+    title:
+        label: Title
+        searchable: true
+    tags:
+        label: Tags
+        relation: tags
+        select: name
+        searchable: true
+        sortable: true
+    notes:
+        label: Notes
+        relation: notes
+        select: body
+        searchable: true
+    author:
+        label: Author
+        relation: author
+        select: name
+        searchable: true
+        sortable: true
+`
+
+// sqlRecorder is a GORM logger that keeps every statement GORM traces, with
+// its bound values inlined, so DryRun tests can assert on the SQL shape.
+type sqlRecorder struct {
+	mu   sync.Mutex
+	sqls []string
+}
+
+func (r *sqlRecorder) LogMode(logger.LogLevel) logger.Interface { return r }
+func (r *sqlRecorder) Info(context.Context, string, ...any)     {}
+func (r *sqlRecorder) Warn(context.Context, string, ...any)     {}
+func (r *sqlRecorder) Error(context.Context, string, ...any)    {}
+func (r *sqlRecorder) Trace(_ context.Context, _ time.Time, fc func() (string, int64), _ error) {
+	sql, _ := fc()
+	r.mu.Lock()
+	defer r.mu.Unlock()
+	r.sqls = append(r.sqls, sql)
+}
+
+// statements returns the recorded statements and forgets them.
+func (r *sqlRecorder) statements() []string {
+	r.mu.Lock()
+	defer r.mu.Unlock()
+	out := r.sqls
+	r.sqls = nil
+	return out
+}
+
+// statement returns the first recorded statement starting with prefix.
+func (r *sqlRecorder) statement(t *testing.T, prefix string, all []string) string {
+	t.Helper()
+	for _, sql := range all {
+		if strings.HasPrefix(sql, prefix) {
+			return sql
+		}
+	}
+	t.Fatalf("no statement starting with %q in %q", prefix, all)
+	return ""
+}
+
+// openDryRunDB opens a Postgres-dialect GORM handle that never connects:
+// DryRun builds every statement without executing it, and the DSN points at
+// a port nothing listens on, so a test that needs no Docker can still assert
+// on the SQL that ExecuteList would send.
+func openDryRunDB(t *testing.T) (*gorm.DB, *sqlRecorder) {
+	t.Helper()
+	rec := &sqlRecorder{}
+	db, err := gorm.Open(pgdriver.New(pgdriver.Config{DSN: "host=127.0.0.1 port=1 user=cabana dbname=cabana sslmode=disable"}),
+		&gorm.Config{DryRun: true, DisableAutomaticPing: true, Logger: rec})
+	if err != nil {
+		t.Fatalf("dry run db: %v", err)
+	}
+	return db, rec
+}
+
+// newDryRunListService builds a service whose app publishes db and whose
+// registry holds the compiled list of ctl as acme.demo.widgets, the
+// controller controllerRequest targets.
+func newDryRunListService(t *testing.T, db *gorm.DB, ctl pact.AdminController, config, columns string) *service {
+	t.Helper()
+	schema, err := CompileList("acme.demo", ctl, filterFS(config, columns, ""))
+	if err != nil {
+		t.Fatalf("schema: %v", err)
+	}
+	app := backpack.New(nil)
+	if err := app.Publish(db); err != nil {
+		t.Fatal(err)
+	}
+	return &service{app: app, reg: &Registry{byID: map[string]*CompiledController{
+		"acme.demo.widgets": {Controller: ctl, List: schema},
+	}}}
+}
+
+func TestListRelationSearchSmoke(t *testing.T) {
+	db, rec := openDryRunDB(t)
+	svc := newDryRunListService(t, db, relController{}, relListConfig, relColumns)
+
+	decodeList(t, getList(t, svc, "search=fungi"), http.StatusOK)
+	count := rec.statement(t, "SELECT count(*)", rec.statements())
+	for _, want := range []string{
+		`EXISTS (SELECT 1 FROM "cabana_rel_post_tags" AS "cabana_p0" JOIN "cabana_rel_tags" AS "cabana_r0" ON "cabana_p0"."rel_tag_id" = "cabana_r0"."id"`,
+		`WHERE "cabana_p0"."rel_post_id" = "cabana_rel_posts"."id" AND LOWER(CAST("cabana_r0"."name" AS TEXT)) LIKE '%fungi%'`,
+		`EXISTS (SELECT 1 FROM "cabana_rel_notes" AS "cabana_r1" WHERE "cabana_r1"."post_id" = "cabana_rel_posts"."id"`,
+		`EXISTS (SELECT 1 FROM "cabana_rel_authors" AS "cabana_r2" WHERE "cabana_r2"."id" = "cabana_rel_posts"."author_id"`,
+	} {
+		if !strings.Contains(count, want) {
+			t.Fatalf("count statement lacks %s:\n%s", want, count)
+		}
+	}
+	if strings.Contains(count, "JOIN \"cabana_rel_tags\" \"Tags\"") || strings.Contains(count, "LEFT JOIN") {
+		t.Fatalf("count statement joins a relation:\n%s", count)
+	}
+
+	decodeList(t, getList(t, svc, "sort=tags&dir=desc"), http.StatusOK)
+	page := rec.statement(t, "SELECT * FROM", rec.statements())
+	order := page[strings.Index(page, "ORDER BY"):]
+	if !strings.Contains(order, `ORDER BY (SELECT string_agg(CAST("cabana_r0"."name" AS TEXT), ', ' ORDER BY CAST("cabana_r0"."name" AS TEXT)) FROM "cabana_rel_post_tags" AS "cabana_p0"`) ||
+		!strings.Contains(order, `) DESC,"cabana_rel_posts"."id"`) {
+		t.Fatalf("to-many sort is not a string_agg subquery with the id tiebreak:\n%s", page)
+	}
+	if strings.Contains(page, "LEFT JOIN") {
+		t.Fatalf("to-many sort joins the relation:\n%s", page)
+	}
+}