From f56db1b41b0a13cc9440bf97e3e018d967ccf130 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 22 Jun 2023 15:24:48 -0500 Subject: [PATCH] feat: add user search query param on last_seen (#8139) * feat: add sql filter for before/after on last_seen column --- coderd/database/dbfake/dbfake.go | 20 +++++++++++ coderd/database/dbgen/dbgen.go | 9 +++++ coderd/database/querier_test.go | 58 ++++++++++++++++++++++++++++++ coderd/database/queries.sql.go | 31 +++++++++++----- coderd/database/queries/users.sql | 11 ++++++ coderd/httpapi/queryparams.go | 27 ++++++++++++-- coderd/httpapi/queryparams_test.go | 14 ++------ coderd/searchquery/search.go | 8 +++-- coderd/users.go | 14 ++++---- coderd/users_test.go | 41 +++++++++++++++++++-- 10 files changed, 200 insertions(+), 33 deletions(-) diff --git a/coderd/database/dbfake/dbfake.go b/coderd/database/dbfake/dbfake.go index 1183db530c..3b3e3dbfda 100644 --- a/coderd/database/dbfake/dbfake.go +++ b/coderd/database/dbfake/dbfake.go @@ -2688,6 +2688,26 @@ func (q *fakeQuerier) GetUsers(_ context.Context, params database.GetUsersParams users = usersFilteredByRole } + if !params.LastSeenBefore.IsZero() { + usersFilteredByLastSeen := make([]database.User, 0, len(users)) + for i, user := range users { + if user.LastSeenAt.Before(params.LastSeenBefore) { + usersFilteredByLastSeen = append(usersFilteredByLastSeen, users[i]) + } + } + users = usersFilteredByLastSeen + } + + if !params.LastSeenAfter.IsZero() { + usersFilteredByLastSeen := make([]database.User, 0, len(users)) + for i, user := range users { + if user.LastSeenAt.After(params.LastSeenAfter) { + usersFilteredByLastSeen = append(usersFilteredByLastSeen, users[i]) + } + } + users = usersFilteredByLastSeen + } + beforePageCount := len(users) if params.OffsetOpt > 0 { diff --git a/coderd/database/dbgen/dbgen.go b/coderd/database/dbgen/dbgen.go index 1a428ba2ba..f4acd6e933 100644 --- a/coderd/database/dbgen/dbgen.go +++ b/coderd/database/dbgen/dbgen.go @@ -206,6 +206,15 @@ func User(t testing.TB, db database.Store, orig database.User) database.User { LoginType: takeFirst(orig.LoginType, database.LoginTypePassword), }) require.NoError(t, err, "insert user") + + if !orig.LastSeenAt.IsZero() { + user, err = db.UpdateUserLastSeenAt(genCtx, database.UpdateUserLastSeenAtParams{ + ID: user.ID, + LastSeenAt: orig.LastSeenAt, + UpdatedAt: user.UpdatedAt, + }) + require.NoError(t, err, "user last seen") + } return user } diff --git a/coderd/database/querier_test.go b/coderd/database/querier_test.go index 5fe89991f7..b79b071d38 100644 --- a/coderd/database/querier_test.go +++ b/coderd/database/querier_test.go @@ -387,3 +387,61 @@ func TestQueuePosition(t *testing.T) { require.Equal(t, job.ProvisionerJob.ID, jobs[index].ID) } } + +func TestUserLastSeenFilter(t *testing.T) { + t.Parallel() + if testing.Short() { + t.SkipNow() + } + t.Run("Before", func(t *testing.T) { + t.Parallel() + sqlDB := testSQLDB(t) + err := migrations.Up(sqlDB) + require.NoError(t, err) + db := database.New(sqlDB) + ctx := context.Background() + now := time.Now() + + yesterday := dbgen.User(t, db, database.User{ + LastSeenAt: now.Add(time.Hour * -25), + }) + today := dbgen.User(t, db, database.User{ + LastSeenAt: now, + }) + lastWeek := dbgen.User(t, db, database.User{ + LastSeenAt: now.Add((time.Hour * -24 * 7) + (-1 * time.Hour)), + }) + + beforeToday, err := db.GetUsers(ctx, database.GetUsersParams{ + LastSeenBefore: now.Add(time.Hour * -24), + }) + require.NoError(t, err) + database.ConvertUserRows(beforeToday) + + requireUsersMatch(t, []database.User{yesterday, lastWeek}, beforeToday, "before today") + + justYesterday, err := db.GetUsers(ctx, database.GetUsersParams{ + LastSeenBefore: now.Add(time.Hour * -24), + LastSeenAfter: now.Add(time.Hour * -24 * 2), + }) + require.NoError(t, err) + requireUsersMatch(t, []database.User{yesterday}, justYesterday, "just yesterday") + + all, err := db.GetUsers(ctx, database.GetUsersParams{ + LastSeenBefore: now.Add(time.Hour), + }) + require.NoError(t, err) + requireUsersMatch(t, []database.User{today, yesterday, lastWeek}, all, "all") + + allAfterLastWeek, err := db.GetUsers(ctx, database.GetUsersParams{ + LastSeenAfter: now.Add(time.Hour * -24 * 7), + }) + require.NoError(t, err) + requireUsersMatch(t, []database.User{today, yesterday}, allAfterLastWeek, "after last week") + }) +} + +func requireUsersMatch(t testing.TB, expected []database.User, found []database.GetUsersRow, msg string) { + t.Helper() + require.ElementsMatch(t, expected, database.ConvertUserRows(found), msg) +} diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 1040af85df..6a466cc2ee 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -5190,22 +5190,35 @@ WHERE rbac_roles && $4 :: text[] ELSE true END + -- Filter by last_seen + AND CASE + WHEN $5 :: timestamp with time zone != '0001-01-01 00:00:00Z' THEN + last_seen_at <= $5 + ELSE true + END + AND CASE + WHEN $6 :: timestamp with time zone != '0001-01-01 00:00:00Z' THEN + last_seen_at >= $6 + ELSE true + END -- End of filters ORDER BY -- Deterministic and consistent ordering of all users. This is to ensure consistent pagination. - LOWER(username) ASC OFFSET $5 + LOWER(username) ASC OFFSET $7 LIMIT -- A null limit means "no limit", so 0 means return all - NULLIF($6 :: int, 0) + NULLIF($8 :: int, 0) ` type GetUsersParams struct { - AfterID uuid.UUID `db:"after_id" json:"after_id"` - Search string `db:"search" json:"search"` - Status []UserStatus `db:"status" json:"status"` - RbacRole []string `db:"rbac_role" json:"rbac_role"` - OffsetOpt int32 `db:"offset_opt" json:"offset_opt"` - LimitOpt int32 `db:"limit_opt" json:"limit_opt"` + AfterID uuid.UUID `db:"after_id" json:"after_id"` + Search string `db:"search" json:"search"` + Status []UserStatus `db:"status" json:"status"` + RbacRole []string `db:"rbac_role" json:"rbac_role"` + LastSeenBefore time.Time `db:"last_seen_before" json:"last_seen_before"` + LastSeenAfter time.Time `db:"last_seen_after" json:"last_seen_after"` + OffsetOpt int32 `db:"offset_opt" json:"offset_opt"` + LimitOpt int32 `db:"limit_opt" json:"limit_opt"` } type GetUsersRow struct { @@ -5231,6 +5244,8 @@ func (q *sqlQuerier) GetUsers(ctx context.Context, arg GetUsersParams) ([]GetUse arg.Search, pq.Array(arg.Status), pq.Array(arg.RbacRole), + arg.LastSeenBefore, + arg.LastSeenAfter, arg.OffsetOpt, arg.LimitOpt, ) diff --git a/coderd/database/queries/users.sql b/coderd/database/queries/users.sql index 22b3c9c1d6..bb17946162 100644 --- a/coderd/database/queries/users.sql +++ b/coderd/database/queries/users.sql @@ -181,6 +181,17 @@ WHERE rbac_roles && @rbac_role :: text[] ELSE true END + -- Filter by last_seen + AND CASE + WHEN @last_seen_before :: timestamp with time zone != '0001-01-01 00:00:00Z' THEN + last_seen_at <= @last_seen_before + ELSE true + END + AND CASE + WHEN @last_seen_after :: timestamp with time zone != '0001-01-01 00:00:00Z' THEN + last_seen_at >= @last_seen_after + ELSE true + END -- End of filters ORDER BY -- Deterministic and consistent ordering of all users. This is to ensure consistent pagination. diff --git a/coderd/httpapi/queryparams.go b/coderd/httpapi/queryparams.go index c2def95774..3f16565e1d 100644 --- a/coderd/httpapi/queryparams.go +++ b/coderd/httpapi/queryparams.go @@ -111,14 +111,35 @@ func (p *QueryParamParser) UUIDs(vals url.Values, def []uuid.UUID, queryParam st }) } -func (p *QueryParamParser) Time(vals url.Values, def time.Time, queryParam string, format string) time.Time { +func (p *QueryParamParser) Time(vals url.Values, def time.Time, queryParam, layout string) time.Time { + return p.timeWithMutate(vals, def, queryParam, layout, nil) +} + +// Time uses the default time format of RFC3339Nano and always returns a UTC time. +func (p *QueryParamParser) Time3339Nano(vals url.Values, def time.Time, queryParam string) time.Time { + layout := time.RFC3339Nano + return p.timeWithMutate(vals, def, queryParam, layout, func(term string) string { + // All search queries are forced to lowercase. But the RFC format requires + // upper case letters. So just uppercase the term. + return strings.ToUpper(term) + }) +} + +func (p *QueryParamParser) timeWithMutate(vals url.Values, def time.Time, queryParam, layout string, mutate func(term string) string) time.Time { v, err := parseQueryParam(p, vals, func(term string) (time.Time, error) { - return time.Parse(format, term) + if mutate != nil { + term = mutate(term) + } + t, err := time.Parse(layout, term) + if err != nil { + return time.Time{}, err + } + return t.UTC(), nil }, def, queryParam) if err != nil { p.Errors = append(p.Errors, codersdk.ValidationError{ Field: queryParam, - Detail: fmt.Sprintf("Query param %q must be a valid date format (%s): %s", queryParam, format, err.Error()), + Detail: fmt.Sprintf("Query param %q must be a valid date format (%s): %s", queryParam, layout, err.Error()), }) } return v diff --git a/coderd/httpapi/queryparams_test.go b/coderd/httpapi/queryparams_test.go index 4a7649e704..ecf7b3a99c 100644 --- a/coderd/httpapi/queryparams_test.go +++ b/coderd/httpapi/queryparams_test.go @@ -69,13 +69,12 @@ func TestParseQueryParams(t *testing.T) { t.Run("Time", func(t *testing.T) { t.Parallel() - const layout = "2006-01-02" expParams := []queryParamTestCase[time.Time]{ { QueryParam: "date", - Value: "2010-01-01", - Expected: must(time.Parse(layout, "2010-01-01")), + Value: "2023-01-16T00:00:00+12:00", + Expected: time.Date(2023, 1, 15, 12, 0, 0, 0, time.UTC), }, { QueryParam: "bad_date", @@ -86,7 +85,7 @@ func TestParseQueryParams(t *testing.T) { parser := httpapi.NewQueryParamParser() testQueryParams(t, expParams, parser, func(vals url.Values, def time.Time, queryParam string) time.Time { - return parser.Time(vals, time.Time{}, queryParam, layout) + return parser.Time3339Nano(vals, time.Time{}, queryParam) }) }) @@ -309,10 +308,3 @@ func testQueryParams[T any](t *testing.T, testCases []queryParamTestCase[T], par }) } } - -func must[T any](value T, err error) T { - if err != nil { - panic(err) - } - return value -} diff --git a/coderd/searchquery/search.go b/coderd/searchquery/search.go index a96d2cbaec..07bb3a6db1 100644 --- a/coderd/searchquery/search.go +++ b/coderd/searchquery/search.go @@ -59,9 +59,11 @@ func Users(query string) (database.GetUsersParams, []codersdk.ValidationError) { parser := httpapi.NewQueryParamParser() filter := database.GetUsersParams{ - Search: parser.String(values, "", "search"), - Status: httpapi.ParseCustomList(parser, values, []database.UserStatus{}, "status", httpapi.ParseEnum[database.UserStatus]), - RbacRole: parser.Strings(values, []string{}, "role"), + Search: parser.String(values, "", "search"), + Status: httpapi.ParseCustomList(parser, values, []database.UserStatus{}, "status", httpapi.ParseEnum[database.UserStatus]), + RbacRole: parser.Strings(values, []string{}, "role"), + LastSeenAfter: parser.Time3339Nano(values, time.Time{}, "last_seen_after"), + LastSeenBefore: parser.Time3339Nano(values, time.Time{}, "last_seen_before"), } parser.ErrorExcessParams(values) return filter, parser.Errors diff --git a/coderd/users.go b/coderd/users.go index 04dbab057f..f9c55380c1 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -199,12 +199,14 @@ func (api *API) users(rw http.ResponseWriter, r *http.Request) { } userRows, err := api.Database.GetUsers(ctx, database.GetUsersParams{ - AfterID: paginationParams.AfterID, - OffsetOpt: int32(paginationParams.Offset), - LimitOpt: int32(paginationParams.Limit), - Search: params.Search, - Status: params.Status, - RbacRole: params.RbacRole, + AfterID: paginationParams.AfterID, + Search: params.Search, + Status: params.Status, + RbacRole: params.RbacRole, + LastSeenBefore: params.LastSeenBefore, + LastSeenAfter: params.LastSeenAfter, + OffsetOpt: int32(paginationParams.Offset), + LimitOpt: int32(paginationParams.Limit), }) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ diff --git a/coderd/users_test.go b/coderd/users_test.go index 3fc92fb05e..504b29a52f 100644 --- a/coderd/users_test.go +++ b/coderd/users_test.go @@ -18,6 +18,7 @@ import ( "github.com/coder/coder/coderd/audit" "github.com/coder/coder/coderd/coderdtest" "github.com/coder/coder/coderd/database" + "github.com/coder/coder/coderd/database/dbauthz" "github.com/coder/coder/coderd/rbac" "github.com/coder/coder/codersdk" "github.com/coder/coder/testutil" @@ -1102,7 +1103,7 @@ func TestGetUser(t *testing.T) { func TestUsersFilter(t *testing.T) { t.Parallel() - client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) + client, _, api := coderdtest.NewWithAPI(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) first := coderdtest.CreateFirstUser(t, client) ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) @@ -1111,6 +1112,13 @@ func TestUsersFilter(t *testing.T) { firstUser, err := client.User(ctx, codersdk.Me) require.NoError(t, err, "fetch me") + // Noon on Jan 18 is the "now" for this test for last_seen timestamps. + // All these values are equal + // 2023-01-18T12:00:00Z (UTC) + // 2023-01-18T07:00:00-05:00 (America/New_York) + // 2023-01-18T13:00:00+01:00 (Europe/Madrid) + // 2023-01-16T00:00:00+12:00 (Asia/Anadyr) + lastSeenNow := time.Date(2023, 1, 18, 12, 0, 0, 0, time.UTC) users := make([]codersdk.User, 0) users = append(users, firstUser) for i := 0; i < 15; i++ { @@ -1121,7 +1129,16 @@ func TestUsersFilter(t *testing.T) { if i%3 == 0 { roles = append(roles, "auditor") } - userClient, _ := coderdtest.CreateAnotherUser(t, client, first.OrganizationID, roles...) + userClient, userData := coderdtest.CreateAnotherUser(t, client, first.OrganizationID, roles...) + // Set the last seen for each user to a unique day + // nolint:gocritic // Unit test + _, err := api.Database.UpdateUserLastSeenAt(dbauthz.AsSystemRestricted(ctx), database.UpdateUserLastSeenAtParams{ + ID: userData.ID, + LastSeenAt: lastSeenNow.Add(-1 * time.Hour * 24 * time.Duration(i)), + UpdatedAt: time.Now(), + }) + require.NoError(t, err, "set a last seen") + user, err := userClient.User(ctx, codersdk.Me) require.NoError(t, err, "fetch me") @@ -1262,6 +1279,26 @@ func TestUsersFilter(t *testing.T) { return false }, }, + { + Name: "LastSeenBeforeNow", + Filter: codersdk.UsersRequest{ + SearchQuery: `last_seen_before:"2023-01-16T00:00:00+12:00"`, + }, + FilterF: func(_ codersdk.UsersRequest, u codersdk.User) bool { + return u.LastSeenAt.Before(lastSeenNow) + }, + }, + { + Name: "LastSeenLastWeek", + Filter: codersdk.UsersRequest{ + SearchQuery: `last_seen_before:"2023-01-14T23:59:59Z" last_seen_after:"2023-01-08T00:00:00Z"`, + }, + FilterF: func(_ codersdk.UsersRequest, u codersdk.User) bool { + start := time.Date(2023, 1, 8, 0, 0, 0, 0, time.UTC) + end := time.Date(2023, 1, 14, 23, 59, 59, 0, time.UTC) + return u.LastSeenAt.Before(end) && u.LastSeenAt.After(start) + }, + }, } for _, c := range testCases {