From 957d7d6291bd340dc027136a12536fb0258cbabb Mon Sep 17 00:00:00 2001 From: Clyde Kallahan Date: Fri, 6 Nov 2026 15:26:10 -0500 Subject: [PATCH 1/2] fix(models): allowlist order_by and direction in OrderClause (EN-229) OrderClause joined order_by and order into ORDER BY unchecked. It now takes the columns a query may sort by and accepts only asc or desc for the direction, falling back to the query's default otherwise. --- backend/src/database/order_fields_test.go | 73 +++++++++++++++++++ backend/src/database/users.go | 11 ++- .../src/handlers/query_context_order_test.go | 32 ++++++++ backend/src/models/resource.go | 17 +++-- backend/src/models/resource_test.go | 55 ++++++++++++++ 5 files changed, 180 insertions(+), 8 deletions(-) create mode 100644 backend/src/database/order_fields_test.go create mode 100644 backend/src/handlers/query_context_order_test.go create mode 100644 backend/src/models/resource_test.go diff --git a/backend/src/database/order_fields_test.go b/backend/src/database/order_fields_test.go new file mode 100644 index 000000000..5e24869f7 --- /dev/null +++ b/backend/src/database/order_fields_test.go @@ -0,0 +1,73 @@ +package database + +import ( + "context" + "slices" + "strings" + "testing" + + "github.com/UnlockedLabs/unlocked-core/backend/src/models" + "github.com/glebarez/sqlite" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "gorm.io/gorm" +) + +// Sort keys the frontend sends to the users list and the resident filters +// (StudentManagement.tsx, AddClassEnrollments.tsx, EnrollResidentsModal.tsx). +func TestOrderByFieldsKeepClientKeys(t *testing.T) { + assert.Subset(t, validUserOrderByFields, []string{"name_last", "username", "doc_id", "last_login", "created_at"}) + assert.Subset(t, residentOrderByFields, []string{"name_last", "doc_id", "created_at"}) +} + +// captureUserQueries is a DB whose queries are built but not run, and the SQL +// each one produced. +func captureUserQueries(t *testing.T) (*DB, *[]string) { + t.Helper() + gdb, err := gorm.Open(sqlite.Open("file:"+t.Name()+"?mode=memory"), &gorm.Config{DryRun: true}) + require.NoError(t, err) + var sqls []string + record := func(tx *gorm.DB) { + sqls = append(sqls, tx.Statement.SQL.String()) + // gorm skips this reset in DryRun, so a following query would reuse this SQL. + tx.Statement.SQL.Reset() + tx.Statement.Vars = nil + } + require.NoError(t, gdb.Callback().Query().After("gorm:query").Register("capture", record)) + return NewDB(gdb), &sqls +} + +func TestUserListsIgnoreHostileOrder(t *testing.T) { + const payload = "(1/(select/**/0))" + args := func(orderBy, order string) *models.QueryContext { + return &models.QueryContext{Ctx: context.Background(), PerPage: 10, Page: 1, OrderBy: orderBy, Order: order} + } + cases := []struct { + name, orderBy, order, want string + run func(db *DB, q *models.QueryContext) + }{ + {"users list direction", "name_last", "desc," + payload, "ORDER BY users.name_last desc", func(db *DB, q *models.QueryContext) { _, _ = db.GetCurrentUsers(q, "student") }}, + {"users list column", payload, "", "ORDER BY name_last asc", func(db *DB, q *models.QueryContext) { _, _ = db.GetCurrentUsers(q, "student") }}, + {"users list legit", "last_login", "desc", "login_metrics.last_login desc", func(db *DB, q *models.QueryContext) { _, _ = db.GetCurrentUsers(q, "student") }}, + {"residents direction", "name_last", "asc," + payload, "ORDER BY users.created_at desc", func(db *DB, q *models.QueryContext) { + _, _ = db.GetResidentsByIDs(q, []uint{1}, "users.created_at desc") + }}, + {"residents column", payload, "asc", "ORDER BY users.created_at desc", func(db *DB, q *models.QueryContext) { + _, _ = db.GetResidentsByIDs(q, []uint{1}, "users.created_at desc") + }}, + {"residents joinless column", "last_login", "asc", "ORDER BY users.created_at desc", func(db *DB, q *models.QueryContext) { + _, _ = db.GetResidentsByIDs(q, []uint{1}, "users.created_at desc") + }}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + db, sqls := captureUserQueries(t) + c.run(db, args(c.orderBy, c.order)) + require.NotEmpty(t, *sqls, "query was not built") + assert.True(t, slices.ContainsFunc(*sqls, func(q string) bool { return strings.Contains(q, c.want) }), "no query with %q in %v", c.want, *sqls) + for _, q := range *sqls { + assert.NotContains(t, q, "select/**/0") + } + }) + } +} diff --git a/backend/src/database/users.go b/backend/src/database/users.go index 7ca2205b0..90b999bcf 100644 --- a/backend/src/database/users.go +++ b/backend/src/database/users.go @@ -22,6 +22,12 @@ func calcOffset(page, perPage int) int { return (page - 1) * perPage } +var ( + validUserOrderByFields = []string{"name_last", "created_at", "last_login", "doc_id", "username", "role", "facility_name"} + // residentOrderByFields has no joins, so last_login and facility_name are not sortable here. + residentOrderByFields = []string{"name_last", "created_at", "doc_id", "username"} +) + func (db *DB) GetCurrentUsers(args *models.QueryContext, role string) ([]models.User, error) { tx := db.WithContext(args.Ctx).Model(models.User{}). Preload("LoginMetrics"). @@ -61,11 +67,10 @@ func (db *DB) GetCurrentUsers(args *models.QueryContext, role string) ([]models. return nil, newGetRecordsDBError(err, "users") } users := make([]models.User, 0, args.PerPage) - validUserOrderByFields := []string{"name_last", "created_at", "last_login", "doc_id", "username", "role", "facility_name"} if !slices.Contains(validUserOrderByFields, args.OrderBy) { args.OrderBy = "name_last" } - orderClause := adjustUserOrderBy(args.OrderClause("users.name_last desc")) + orderClause := adjustUserOrderBy(args.OrderClause("users.name_last desc", validUserOrderByFields...)) if args.OrderBy == "last_login" { tx = tx.Joins("LEFT JOIN login_metrics ON login_metrics.user_id = users.id") orderClause = adjustLastLoginOrderBy(orderClause) @@ -171,7 +176,7 @@ func (db *DB) GetResidentsByIDs(args *models.QueryContext, ids []uint, defaultOr return nil, newGetRecordsDBError(err, "users") } users := make([]models.User, 0, args.PerPage) - if err := tx.Order(adjustUserOrderBy(args.OrderClause(defaultOrder))). + if err := tx.Order(adjustUserOrderBy(args.OrderClause(defaultOrder, residentOrderByFields...))). Offset(args.CalcOffset()). Limit(args.PerPage). Find(&users). diff --git a/backend/src/handlers/query_context_order_test.go b/backend/src/handlers/query_context_order_test.go new file mode 100644 index 000000000..13f2c978d --- /dev/null +++ b/backend/src/handlers/query_context_order_test.go @@ -0,0 +1,32 @@ +package handlers + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" +) + +// order_by= in one param is split into the two fields the +// database layer validates, so it keeps working after the allowlist. +func TestGetQueryContextSplitsOrderBy(t *testing.T) { + cases := []struct{ query, orderBy, order string }{ + {"order_by=name_last%20desc", "name_last", "desc"}, + {"order_by=NAME_LAST%20DESC", "name_last", "desc"}, + {"order_by=name_last&order=asc", "name_last", "asc"}, + {"order_by=ps.start_dt%20asc", "ps.start_dt", "asc"}, + } + srv := &Server{} + for _, c := range cases { + t.Run(c.query, func(t *testing.T) { + r := httptest.NewRequest(http.MethodGet, "/api/users?"+c.query, nil) + r = r.WithContext(context.WithValue(r.Context(), ClaimsKey, &Claims{})) + qc := srv.getQueryContext(r) + assert.Equal(t, c.orderBy, qc.OrderBy) + assert.Equal(t, c.order, qc.Order) + assert.Equal(t, c.orderBy+" "+c.order, qc.OrderClause("fb", c.orderBy)) + }) + } +} diff --git a/backend/src/models/resource.go b/backend/src/models/resource.go index e15d1892e..763fc0a0e 100644 --- a/backend/src/models/resource.go +++ b/backend/src/models/resource.go @@ -149,13 +149,20 @@ func (q QueryContext) CalcOffset() int { return int(math.Abs(float64((q.Page - 1) * q.PerPage))) } -// fallback column is used if there is no order by in the query string -func (q QueryContext) OrderClause(fallback string) string { - val := q.OrderBy + " " + q.Order - if val == " " || q.OrderBy == "" { +// OrderClause is the request's order when order_by is one of allowed and the +// direction is asc or desc (asc when absent), otherwise fallback. allowed is +// the exact set of order_by values the query can sort by. +func (q QueryContext) OrderClause(fallback string, allowed ...string) string { + if !slices.Contains(allowed, q.OrderBy) { return fallback } - return val + switch dir := strings.ToLower(q.Order); dir { + case "": + return q.OrderBy + " asc" + case "asc", "desc": + return q.OrderBy + " " + dir + } + return fallback } func (q QueryContext) SearchQuery() string { diff --git a/backend/src/models/resource_test.go b/backend/src/models/resource_test.go new file mode 100644 index 000000000..4474fd2d3 --- /dev/null +++ b/backend/src/models/resource_test.go @@ -0,0 +1,55 @@ +package models + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestOrderClauseDirection(t *testing.T) { + cases := []struct{ name, orderBy, order, want string }{ + {"asc", "name_last", "asc", "name_last asc"}, + {"desc", "name_last", "desc", "name_last desc"}, + {"upper case direction", "name_last", "DESC", "name_last desc"}, + {"empty direction defaults to asc", "name_last", "", "name_last asc"}, + {"subselect in direction", "name_last", "desc,(select 1/0)", "fb"}, + {"statement in direction", "name_last", "desc; drop table users", "fb"}, + {"unknown direction", "name_last", "sideways", "fb"}, + {"whitespace direction", "name_last", " ", "fb"}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + q := QueryContext{OrderBy: c.orderBy, Order: c.order} + assert.Equal(t, c.want, q.OrderClause("fb", "name_last")) + }) + } +} + +func TestOrderClauseAllowlist(t *testing.T) { + allowed := []string{"name_last", "ps.start_dt"} + cases := []struct{ name, orderBy, order, want string }{ + {"allowed column", "name_last", "asc", "name_last asc"}, + {"qualified allowed column", "ps.start_dt", "desc", "ps.start_dt desc"}, + {"column not listed", "password", "asc", "fb"}, + {"empty column", "", "asc", "fb"}, + {"empty column and direction", "", "", "fb"}, + {"subselect as column", "(select 1/0)", "", "fb"}, + {"column with direction smuggled in", "name_last desc", "", "fb"}, + {"comma separated columns", "name_last,password", "asc", "fb"}, + {"comment suffix", "name_last--", "asc", "fb"}, + {"upper case column is not a member", "NAME_LAST", "asc", "fb"}, + {"over long column", strings.Repeat("a", 10000), "asc", "fb"}, + {"NUL byte", "name_last\x00", "asc", "fb"}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + q := QueryContext{OrderBy: c.orderBy, Order: c.order} + assert.Equal(t, c.want, q.OrderClause("fb", allowed...)) + }) + } +} + +func TestOrderClauseNoAllowlistFallsBack(t *testing.T) { + assert.Equal(t, "fb", QueryContext{OrderBy: "name_last", Order: "asc"}.OrderClause("fb")) +} From ea045a9e052cc3e9be907890092ffbd62022eaf2 Mon Sep 17 00:00:00 2001 From: Clyde Kallahan Date: Fri, 6 Nov 2026 15:28:32 -0500 Subject: [PATCH 2/2] fix(programs): allowlist order_by and direction on list queries (EN-229) Programs carries its own copy of OrderClause, so it gets the same allowlist and direction check. The two queries that bypassed it, the programs overview and the class outcomes, and the raw-SQL attendance roster now go through it. --- .../programs/class_enrollments_store.go | 2 +- .../programs/events_attendance_store.go | 17 ++- backend/src/modules/programs/http.go | 17 ++- .../src/modules/programs/order_clause_test.go | 138 ++++++++++++++++++ backend/src/modules/programs/order_columns.go | 15 ++ .../modules/programs/program_classes_store.go | 6 +- .../src/modules/programs/programs_store.go | 8 +- backend/src/modules/programs/users_store.go | 2 +- 8 files changed, 183 insertions(+), 22 deletions(-) create mode 100644 backend/src/modules/programs/order_clause_test.go create mode 100644 backend/src/modules/programs/order_columns.go diff --git a/backend/src/modules/programs/class_enrollments_store.go b/backend/src/modules/programs/class_enrollments_store.go index f6b8f03bb..eb49b0625 100644 --- a/backend/src/modules/programs/class_enrollments_store.go +++ b/backend/src/modules/programs/class_enrollments_store.go @@ -466,7 +466,7 @@ func (db *Store) GetProgramClassEnrollmentsForProgram(args *QueryContext, classI return nil, newNotFoundDBError(err, "program class enrollments") } if err := tx.Limit(args.PerPage). - Offset(args.CalcOffset()).Order(adjustUserOrderBy(args.OrderClause("pse.created_at desc"))). + Offset(args.CalcOffset()).Order(adjustUserOrderBy(args.OrderClause("pse.created_at desc", enrollmentOrderBy...))). Find(&content).Error; err != nil { return nil, newNotFoundDBError(err, "program class enrollments") } diff --git a/backend/src/modules/programs/events_attendance_store.go b/backend/src/modules/programs/events_attendance_store.go index 1604a0a0b..08bebefe1 100644 --- a/backend/src/modules/programs/events_attendance_store.go +++ b/backend/src/modules/programs/events_attendance_store.go @@ -30,7 +30,7 @@ func (db *Store) GetAttendees(queryParams *QueryContext, params url.Values, clas if err := tx.Count(&queryParams.Total).Error; err != nil { return nil, newGetRecordsDBError(err, "class_event_attendance") } - err := tx.Order(queryParams.OrderClause("program_class_event_attendance.created_at DESC")).Limit(queryParams.PerPage).Offset(queryParams.CalcOffset()).Find(&attendance).Error + err := tx.Order(queryParams.OrderClause("program_class_event_attendance.created_at DESC", attendanceOrderBy...)).Limit(queryParams.PerPage).Offset(queryParams.CalcOffset()).Find(&attendance).Error if err != nil { return nil, newGetRecordsDBError(err, "class_event_attendance") } @@ -201,11 +201,7 @@ func (db *Store) GetEnrollmentsWithAttendanceForEvent(qryCtx *QueryContext, clas a.minutes_attended AS minutes_attended, a.scheduled_minutes AS scheduled_minutes` finalQuery := selectClause + baseQuery - if slices.Contains([]string{"name_first", "name_last"}, qryCtx.OrderBy) { - finalQuery += " ORDER BY " + adjustUserOrderBy(qryCtx.OrderClause("e.created_at DESC")) - } else { - finalQuery += " ORDER BY e.id ASC" - } + finalQuery += " ORDER BY " + rosterOrder(qryCtx) finalQuery += " LIMIT ? OFFSET ?" args = append(args, qryCtx.PerPage, qryCtx.CalcOffset()) @@ -688,3 +684,12 @@ func (db *Store) GetCumulativeAttendanceRatesForClasses(ctx context.Context, cla } return out, nil } + +// rosterOrder is the roster's ORDER BY. The query is raw SQL, so this is what +// keeps request text out of it: name sorts only, else the stable id order. +func rosterOrder(qryCtx *QueryContext) string { + if !slices.Contains(attendanceRosterOrderBy, qryCtx.OrderBy) { + return "e.id ASC" + } + return adjustUserOrderBy(qryCtx.OrderClause("e.created_at DESC", attendanceRosterOrderBy...)) +} diff --git a/backend/src/modules/programs/http.go b/backend/src/modules/programs/http.go index 310cacfa9..428d92a4f 100644 --- a/backend/src/modules/programs/http.go +++ b/backend/src/modules/programs/http.go @@ -336,13 +336,20 @@ func (q QueryContext) CalcOffset() int { return int(math.Abs(float64((q.Page - 1) * q.PerPage))) } -// OrderClause is the request's order, or fallback when it has none. -func (q QueryContext) OrderClause(fallback string) string { - val := q.OrderBy + " " + q.Order - if val == " " || q.OrderBy == "" { +// OrderClause is the request's order when order_by is one of allowed and the +// direction is asc or desc (asc when absent), otherwise fallback. allowed is +// the exact set of order_by values the query can sort by. +func (q QueryContext) OrderClause(fallback string, allowed ...string) string { + if !slices.Contains(allowed, q.OrderBy) { return fallback } - return val + switch dir := strings.ToLower(q.Order); dir { + case "": + return q.OrderBy + " asc" + case "asc", "desc": + return q.OrderBy + " " + dir + } + return fallback } func (q QueryContext) SearchQuery() string { diff --git a/backend/src/modules/programs/order_clause_test.go b/backend/src/modules/programs/order_clause_test.go new file mode 100644 index 000000000..93bf9d38f --- /dev/null +++ b/backend/src/modules/programs/order_clause_test.go @@ -0,0 +1,138 @@ +package programs + +import ( + "context" + "slices" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "gorm.io/gorm" +) + +func TestOrderClauseDirection(t *testing.T) { + cases := []struct{ name, orderBy, order, want string }{ + {"asc", "name_last", "asc", "name_last asc"}, + {"desc", "name_last", "desc", "name_last desc"}, + {"upper case direction", "name_last", "DESC", "name_last desc"}, + {"empty direction defaults to asc", "name_last", "", "name_last asc"}, + {"subselect in direction", "name_last", "desc,(select 1/0)", "fb"}, + {"statement in direction", "name_last", "desc; drop table users", "fb"}, + {"unknown direction", "name_last", "sideways", "fb"}, + {"whitespace direction", "name_last", " ", "fb"}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + q := QueryContext{OrderBy: c.orderBy, Order: c.order} + assert.Equal(t, c.want, q.OrderClause("fb", "name_last")) + }) + } +} + +func TestOrderClauseAllowlist(t *testing.T) { + allowed := []string{"name_last", "ps.start_dt"} + cases := []struct{ name, orderBy, order, want string }{ + {"allowed column", "name_last", "asc", "name_last asc"}, + {"qualified allowed column", "ps.start_dt", "desc", "ps.start_dt desc"}, + {"column not listed", "password", "asc", "fb"}, + {"empty column", "", "asc", "fb"}, + {"empty column and direction", "", "", "fb"}, + {"subselect as column", "(1/(select/**/0))", "", "fb"}, + {"column with direction smuggled in", "name_last desc", "", "fb"}, + {"comma separated columns", "name_last,password", "asc", "fb"}, + {"comment suffix", "name_last--", "asc", "fb"}, + {"upper case column is not a member", "NAME_LAST", "asc", "fb"}, + {"over long column", strings.Repeat("a", 10000), "asc", "fb"}, + {"NUL byte", "name_last\x00", "asc", "fb"}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + q := QueryContext{OrderBy: c.orderBy, Order: c.order} + assert.Equal(t, c.want, q.OrderClause("fb", allowed...)) + }) + } + assert.Equal(t, "fb", QueryContext{OrderBy: "name_last", Order: "asc"}.OrderClause("fb")) +} + +// Sort keys the frontend and the trusted internal caller (api.go) send. +func TestOrderByListsKeepClientKeys(t *testing.T) { + assert.Subset(t, programOverviewOrderBy, []string{"programs.name", "mr.total_active_enrollments", "completion_rate"}) + assert.Subset(t, programOrderBy, []string{"name"}) + assert.Subset(t, classDetailsOrderBy, []string{"ps.start_dt"}) + assert.Subset(t, enrollmentOrderBy, []string{"name_last", "doc_id", "start_dt"}) + assert.Subset(t, userProgramOrderBy, []string{"start_dt"}) +} + +// captureSQL is a store whose queries are built but not run, and the SQL each +// one produced. It lets the Postgres-only queries be checked on SQLite. +func captureSQL(t *testing.T) (*Store, *[]string) { + t.Helper() + db := newTestDB(t) + var sqls []string + record := func(tx *gorm.DB) { + sqls = append(sqls, tx.Statement.SQL.String()) + // gorm skips this reset in DryRun, so a following Find would reuse the Count's SQL. + tx.Statement.SQL.Reset() + tx.Statement.Vars = nil + } + require.NoError(t, db.Callback().Query().After("gorm:query").Register("capture", record)) + require.NoError(t, db.Callback().Row().After("gorm:row").Register("capture_row", record)) + return &Store{db.Session(&gorm.Session{DryRun: true})}, &sqls +} + +func hostile(orderBy, order string) *QueryContext { + return &QueryContext{Ctx: context.Background(), PerPage: 10, Page: 1, OrderBy: orderBy, Order: order} +} + +func TestStoreQueriesIgnoreHostileOrder(t *testing.T) { + const payload = "(1/(select/**/0))" + cases := []struct { + name string + run func(s *Store, q *QueryContext) + want string + }{ + {"programs", func(s *Store, q *QueryContext) { _, _ = s.GetPrograms(q) }, "programs.created_at desc"}, + {"programs overview", func(s *Store, q *QueryContext) { + _, _ = s.GetProgramsOverviewTable(q, 30, false, "", nil) + }, "programs.created_at desc"}, + {"class details", func(s *Store, q *QueryContext) { _, _ = s.GetProgramClassDetailsByID(1, q) }, "ps.created_at desc"}, + {"class outcomes", func(s *Store, q *QueryContext) { _, _ = s.GetProgramClassOutcomes(1, q) }, "months.month asc"}, + {"class history", func(s *Store, q *QueryContext) { _, _ = s.GetProgramClassesHistory(1, "t", q) }, "created_at desc"}, + {"user programs", func(s *Store, q *QueryContext) { _, _ = s.GetUserProgramInfo(q, 1) }, "pce.created_at desc"}, + {"enrollments", func(s *Store, q *QueryContext) { _, _ = s.GetProgramClassEnrollmentsForProgram(q, 1, "all") }, "pse.created_at desc"}, + {"attendees", func(s *Store, q *QueryContext) { _, _ = s.GetAttendees(q, nil, 1) }, "program_class_event_attendance.created_at DESC"}, + } + for _, c := range cases { + for _, in := range [][2]string{{payload, ""}, {"name_last", "desc," + payload}} { + t.Run(c.name+"/"+in[0]+" "+in[1], func(t *testing.T) { + s, sqls := captureSQL(t) + c.run(s, hostile(in[0], in[1])) + require.NotEmpty(t, *sqls, "query was not built") + for _, q := range *sqls { + assert.NotContains(t, q, "select/**/0") + } + assert.True(t, slices.ContainsFunc(*sqls, func(q string) bool { return strings.Contains(q, "ORDER BY "+c.want) }), + "no query ordered by %q", c.want) + }) + } + } +} + +// The roster query builds its ORDER BY into a raw SQL string. +func TestRosterOrderIgnoresHostileOrder(t *testing.T) { + const payload = "(1/(select/**/0))" + cases := []struct{ orderBy, order, want string }{ + {"name_last", "asc", "name_last asc, name_first"}, + {"name_first", "desc", "name_first desc"}, + {"name_last", "desc," + payload, "e.created_at DESC"}, + {"name_last", "desc;" + payload, "e.created_at DESC"}, + {payload, "", "e.id ASC"}, + {"name_last, " + payload, "asc", "e.id ASC"}, + } + for _, c := range cases { + t.Run(c.orderBy+" "+c.order, func(t *testing.T) { + assert.Equal(t, c.want, rosterOrder(hostile(c.orderBy, c.order))) + }) + } +} diff --git a/backend/src/modules/programs/order_columns.go b/backend/src/modules/programs/order_columns.go new file mode 100644 index 000000000..29376a134 --- /dev/null +++ b/backend/src/modules/programs/order_columns.go @@ -0,0 +1,15 @@ +package programs + +// The order_by values each list query accepts. A value outside its list falls +// back to the query's default order, so request text never reaches ORDER BY. +var ( + programOrderBy = []string{"name", "programs.created_at"} + programOverviewOrderBy = []string{"programs.name", "mr.total_active_enrollments", "completion_rate", "programs.created_at"} + classDetailsOrderBy = []string{"ps.start_dt", "ps.created_at"} + classOutcomesOrderBy = []string{"months.month", "month", "completions", "drops"} + classHistoryOrderBy = []string{"created_at"} + attendanceOrderBy = []string{"program_class_event_attendance.created_at"} + attendanceRosterOrderBy = []string{"name_first", "name_last"} + userProgramOrderBy = []string{"pce.created_at", "start_dt"} + enrollmentOrderBy = []string{"name_last", "doc_id", "start_dt", "pse.created_at"} +) diff --git a/backend/src/modules/programs/program_classes_store.go b/backend/src/modules/programs/program_classes_store.go index 49a6becd1..7a850cfbe 100644 --- a/backend/src/modules/programs/program_classes_store.go +++ b/backend/src/modules/programs/program_classes_store.go @@ -411,7 +411,7 @@ func (db *Store) GetProgramClassDetailsByID(id int, args *QueryContext) ([]Progr if err := query.Count(&args.Total).Error; err != nil { return nil, newGetRecordsDBError(err, "programs") } - if err := query.Limit(args.PerPage).Offset(args.CalcOffset()).Order(args.OrderClause("ps.created_at desc")).Find(&classDetails).Error; err != nil { + if err := query.Limit(args.PerPage).Offset(args.CalcOffset()).Order(args.OrderClause("ps.created_at desc", classDetailsOrderBy...)).Find(&classDetails).Error; err != nil { return nil, newGetRecordsDBError(err, "programs") } @@ -483,7 +483,7 @@ func (db *Store) GetProgramClassOutcomes(id int, args *QueryContext) ([]ProgramC ON TO_CHAR(DATE_TRUNC('month', pce.updated_at), 'YYYY-MM') = months.month `, enrollmentsSubquery), id, facilityID). Group("months.month"). - Order(args.OrderBy) + Order(args.OrderClause("months.month asc", classOutcomesOrderBy...)) if err := query.Find(&outcomes).Error; err != nil { return nil, newGetRecordsDBError(err, "program_class_enrollments") } @@ -492,7 +492,7 @@ func (db *Store) GetProgramClassOutcomes(id int, args *QueryContext) ([]ProgramC func (db *Store) GetProgramClassesHistory(id int, tableName string, args *QueryContext) ([]ProgramClassesHistory, error) { history := []ProgramClassesHistory{} - if err := db.WithContext(args.Ctx).Order(args.OrderClause("created_at desc")). + if err := db.WithContext(args.Ctx).Order(args.OrderClause("created_at desc", classHistoryOrderBy...)). Find(&history, "parent_ref_id = ? and table_name = ?", id, tableName). Error; err != nil { return nil, newGetRecordsDBError(err, "program_classes_history") diff --git a/backend/src/modules/programs/programs_store.go b/backend/src/modules/programs/programs_store.go index 2fedef2b5..42d5ae587 100644 --- a/backend/src/modules/programs/programs_store.go +++ b/backend/src/modules/programs/programs_store.go @@ -127,7 +127,7 @@ func (db *Store) GetPrograms(args *QueryContext) ([]Program, error) { if len(args.Tags) > 0 { tx = tx.Joins("JOIN program_types t ON t.program_id = programs.id").Where("t.id IN (?)", args.Tags) } - tx = tx.Order(args.OrderClause("programs.created_at desc")) + tx = tx.Order(args.OrderClause("programs.created_at desc", programOrderBy...)) if args.Search != "" { tx = tx.Where("LOWER(name) LIKE ? OR LOWER(description) LIKE ? ", args.SearchQuery(), args.SearchQuery()) @@ -934,11 +934,7 @@ func (db *Store) GetProgramsOverviewTable(args *QueryContext, timeFilter int, in return nil, newGetRecordsDBError(err, "programs table count") } - if args.OrderBy != "" && args.Order != "" { - tx = tx.Order(args.OrderBy + " " + args.Order) - } else { - tx = tx.Order(args.OrderClause("programs.created_at desc")) - } + tx = tx.Order(args.OrderClause("programs.created_at desc", programOverviewOrderBy...)) if !args.All { tx = tx.Limit(args.PerPage).Offset(args.CalcOffset()) diff --git a/backend/src/modules/programs/users_store.go b/backend/src/modules/programs/users_store.go index b804601e1..85ce29339 100644 --- a/backend/src/modules/programs/users_store.go +++ b/backend/src/modules/programs/users_store.go @@ -168,7 +168,7 @@ func (db *Store) GetUserProgramInfo(args *QueryContext, userId int) ([]ResidentP pce.id, pce.enrollment_status, p.name, p.id, cl.name, pc.status, pc.start_dt, pc.end_dt, pc.id, pce.updated_at, - pce.created_at, pce.change_reason`).Order(args.OrderClause("pce.created_at desc")) + pce.created_at, pce.change_reason`).Order(args.OrderClause("pce.created_at desc", userProgramOrderBy...)) if !args.All { if err := base.Count(&args.Total).Error; err != nil {