From 4068129b230fd29a94c4131e2c5e14865f6a9a01 Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Wed, 14 May 2025 09:17:29 -0500 Subject: [PATCH 01/13] feat: add private column to user_contact_methods table --- migrate/migrations/20250514091650-cm-private.sql | 8 ++++++++ migrate/schema.sql | 7 ++++--- 2 files changed, 12 insertions(+), 3 deletions(-) create mode 100644 migrate/migrations/20250514091650-cm-private.sql diff --git a/migrate/migrations/20250514091650-cm-private.sql b/migrate/migrations/20250514091650-cm-private.sql new file mode 100644 index 0000000000..c4a9112c04 --- /dev/null +++ b/migrate/migrations/20250514091650-cm-private.sql @@ -0,0 +1,8 @@ +-- +migrate Up +ALTER TABLE user_contact_methods + ADD COLUMN private boolean DEFAULT FALSE; + +-- +migrate Down +ALTER TABLE user_contact_methods + DROP COLUMN private; + diff --git a/migrate/schema.sql b/migrate/schema.sql index 48416c04e4..c46eb6d072 100644 --- a/migrate/schema.sql +++ b/migrate/schema.sql @@ -1,7 +1,7 @@ -- This file is auto-generated by "make db-schema"; DO NOT EDIT --- DATA=50f009ab810d42724027d31ec49f2584a1c8ecd647194c2455de853975d40bd0 - --- DISK=2f8c154346b509ddac6e6d36a7a4f1bc86c047dcd1f98eb3ee5846bdf4bcf687 - --- PSQL=2f8c154346b509ddac6e6d36a7a4f1bc86c047dcd1f98eb3ee5846bdf4bcf687 - +-- DATA=a6bc6457557fa5c5d4b971a25798b6ffb24156f1e608b66066320e372c4dc86c - +-- DISK=22479577daa4266fcaccbcf25675b21862e38fb630b49da2a5e5875344a27c3a - +-- PSQL=22479577daa4266fcaccbcf25675b21862e38fb630b49da2a5e5875344a27c3a - -- -- pgdump-lite database dump -- @@ -2554,6 +2554,7 @@ CREATE TABLE user_contact_methods ( metadata jsonb, name text NOT NULL, pending boolean DEFAULT true NOT NULL, + private boolean DEFAULT false, type enum_user_contact_method_type NOT NULL, user_id uuid NOT NULL, value text NOT NULL, From cb3e09105a6bef2508e56b38f1abd10bca484b5b Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Wed, 14 May 2025 09:28:05 -0500 Subject: [PATCH 02/13] feat: add private field to user_contact_methods and update related queries --- gadb/models.go | 1 + gadb/queries.sql.go | 23 +++-- graphql2/generated.go | 83 ++++++++++++++++++- graphql2/models_gen.go | 4 + graphql2/schema.graphql | 11 +++ .../migrations/20250514091650-cm-private.sql | 2 +- migrate/schema.sql | 4 +- user/contactmethod/contactmethod.go | 1 + user/contactmethod/queries.sql | 7 +- user/contactmethod/store.go | 3 +- 10 files changed, 123 insertions(+), 16 deletions(-) diff --git a/gadb/models.go b/gadb/models.go index 9458fc1a87..82d6222a0c 100644 --- a/gadb/models.go +++ b/gadb/models.go @@ -1337,6 +1337,7 @@ type UserContactMethod struct { Metadata pqtype.NullRawMessage Name string Pending bool + Private bool Type EnumUserContactMethodType UserID uuid.UUID Value string diff --git a/gadb/queries.sql.go b/gadb/queries.sql.go index 417198f707..fca2bacf3e 100644 --- a/gadb/queries.sql.go +++ b/gadb/queries.sql.go @@ -1691,8 +1691,8 @@ func (q *Queries) ConnectionInfo(ctx context.Context) ([]ConnectionInfoRow, erro } const contactMethodAdd = `-- name: ContactMethodAdd :exec -INSERT INTO user_contact_methods(id, name, dest, disabled, user_id, enable_status_updates) - VALUES ($1, $2, $3, $4, $5, $6) +INSERT INTO user_contact_methods(id, name, dest, disabled, user_id, enable_status_updates, private) + VALUES ($1, $2, $3, $4, $5, $6, $7) ` type ContactMethodAddParams struct { @@ -1702,6 +1702,7 @@ type ContactMethodAddParams struct { Disabled bool UserID uuid.UUID EnableStatusUpdates bool + Private bool } func (q *Queries) ContactMethodAdd(ctx context.Context, arg ContactMethodAddParams) error { @@ -1712,6 +1713,7 @@ func (q *Queries) ContactMethodAdd(ctx context.Context, arg ContactMethodAddPara arg.Disabled, arg.UserID, arg.EnableStatusUpdates, + arg.Private, ) return err } @@ -1741,7 +1743,7 @@ func (q *Queries) ContactMethodEnableDisable(ctx context.Context, arg ContactMet const contactMethodFindAll = `-- name: ContactMethodFindAll :many SELECT - dest, disabled, enable_status_updates, id, last_test_verify_at, metadata, name, pending, type, user_id, value + dest, disabled, enable_status_updates, id, last_test_verify_at, metadata, name, pending, private, type, user_id, value FROM user_contact_methods WHERE @@ -1766,6 +1768,7 @@ func (q *Queries) ContactMethodFindAll(ctx context.Context, userID uuid.UUID) ([ &i.Metadata, &i.Name, &i.Pending, + &i.Private, &i.Type, &i.UserID, &i.Value, @@ -1785,7 +1788,7 @@ func (q *Queries) ContactMethodFindAll(ctx context.Context, userID uuid.UUID) ([ const contactMethodFindMany = `-- name: ContactMethodFindMany :many SELECT - dest, disabled, enable_status_updates, id, last_test_verify_at, metadata, name, pending, type, user_id, value + dest, disabled, enable_status_updates, id, last_test_verify_at, metadata, name, pending, private, type, user_id, value FROM user_contact_methods WHERE @@ -1810,6 +1813,7 @@ func (q *Queries) ContactMethodFindMany(ctx context.Context, dollar_1 []uuid.UUI &i.Metadata, &i.Name, &i.Pending, + &i.Private, &i.Type, &i.UserID, &i.Value, @@ -1829,7 +1833,7 @@ func (q *Queries) ContactMethodFindMany(ctx context.Context, dollar_1 []uuid.UUI const contactMethodFindOneUpdate = `-- name: ContactMethodFindOneUpdate :one SELECT - dest, disabled, enable_status_updates, id, last_test_verify_at, metadata, name, pending, type, user_id, value + dest, disabled, enable_status_updates, id, last_test_verify_at, metadata, name, pending, private, type, user_id, value FROM user_contact_methods WHERE @@ -1849,6 +1853,7 @@ func (q *Queries) ContactMethodFindOneUpdate(ctx context.Context, id uuid.UUID) &i.Metadata, &i.Name, &i.Pending, + &i.Private, &i.Type, &i.UserID, &i.Value, @@ -1858,7 +1863,7 @@ func (q *Queries) ContactMethodFindOneUpdate(ctx context.Context, id uuid.UUID) const contactMethodFineOne = `-- name: ContactMethodFineOne :one SELECT - dest, disabled, enable_status_updates, id, last_test_verify_at, metadata, name, pending, type, user_id, value + dest, disabled, enable_status_updates, id, last_test_verify_at, metadata, name, pending, private, type, user_id, value FROM user_contact_methods WHERE @@ -1877,6 +1882,7 @@ func (q *Queries) ContactMethodFineOne(ctx context.Context, id uuid.UUID) (UserC &i.Metadata, &i.Name, &i.Pending, + &i.Private, &i.Type, &i.UserID, &i.Value, @@ -1944,7 +1950,8 @@ UPDATE SET name = $2, disabled = $3, - enable_status_updates = $4 + enable_status_updates = $4, + private = $5 WHERE id = $1 ` @@ -1954,6 +1961,7 @@ type ContactMethodUpdateParams struct { Name string Disabled bool EnableStatusUpdates bool + Private bool } func (q *Queries) ContactMethodUpdate(ctx context.Context, arg ContactMethodUpdateParams) error { @@ -1962,6 +1970,7 @@ func (q *Queries) ContactMethodUpdate(ctx context.Context, arg ContactMethodUpda arg.Name, arg.Disabled, arg.EnableStatusUpdates, + arg.Private, ) return err } diff --git a/graphql2/generated.go b/graphql2/generated.go index 7ff49ddcc3..0b1a4b178b 100644 --- a/graphql2/generated.go +++ b/graphql2/generated.go @@ -853,6 +853,7 @@ type ComplexityRoot struct { LastVerifyMessageState func(childComplexity int) int Name func(childComplexity int) int Pending func(childComplexity int) int + Private func(childComplexity int) int StatusUpdates func(childComplexity int) int Type func(childComplexity int) int Value func(childComplexity int) int @@ -5121,6 +5122,13 @@ func (e *executableSchema) Complexity(ctx context.Context, typeName, field strin return e.complexity.UserContactMethod.Pending(childComplexity), true + case "UserContactMethod.private": + if e.complexity.UserContactMethod.Private == nil { + break + } + + return e.complexity.UserContactMethod.Private(childComplexity), true + case "UserContactMethod.statusUpdates": if e.complexity.UserContactMethod.StatusUpdates == nil { break @@ -21092,6 +21100,8 @@ func (ec *executionContext) fieldContext_Mutation_createUserContactMethod(ctx co return ec.fieldContext_UserContactMethod_lastVerifyMessageState(ctx, field) case "statusUpdates": return ec.fieldContext_UserContactMethod_statusUpdates(ctx, field) + case "private": + return ec.fieldContext_UserContactMethod_private(ctx, field) } return nil, fmt.Errorf("no field named %q was found under type UserContactMethod", field.Name) }, @@ -25893,6 +25903,8 @@ func (ec *executionContext) fieldContext_Query_userContactMethod(ctx context.Con return ec.fieldContext_UserContactMethod_lastVerifyMessageState(ctx, field) case "statusUpdates": return ec.fieldContext_UserContactMethod_statusUpdates(ctx, field) + case "private": + return ec.fieldContext_UserContactMethod_private(ctx, field) } return nil, fmt.Errorf("no field named %q was found under type UserContactMethod", field.Name) }, @@ -32208,6 +32220,8 @@ func (ec *executionContext) fieldContext_User_contactMethods(_ context.Context, return ec.fieldContext_UserContactMethod_lastVerifyMessageState(ctx, field) case "statusUpdates": return ec.fieldContext_UserContactMethod_statusUpdates(ctx, field) + case "private": + return ec.fieldContext_UserContactMethod_private(ctx, field) } return nil, fmt.Errorf("no field named %q was found under type UserContactMethod", field.Name) }, @@ -33785,6 +33799,50 @@ func (ec *executionContext) fieldContext_UserContactMethod_statusUpdates(_ conte return fc, nil } +func (ec *executionContext) _UserContactMethod_private(ctx context.Context, field graphql.CollectedField, obj *contactmethod.ContactMethod) (ret graphql.Marshaler) { + fc, err := ec.fieldContext_UserContactMethod_private(ctx, field) + if err != nil { + return graphql.Null + } + ctx = graphql.WithFieldContext(ctx, fc) + defer func() { + if r := recover(); r != nil { + ec.Error(ctx, ec.Recover(ctx, r)) + ret = graphql.Null + } + }() + resTmp, err := ec.ResolverMiddleware(ctx, func(rctx context.Context) (any, error) { + ctx = rctx // use context from middleware stack in children + return obj.Private, nil + }) + if err != nil { + ec.Error(ctx, err) + return graphql.Null + } + if resTmp == nil { + if !graphql.HasFieldError(ctx, fc) { + ec.Errorf(ctx, "must not be null") + } + return graphql.Null + } + res := resTmp.(bool) + fc.Result = res + return ec.marshalNBoolean2bool(ctx, field.Selections, res) +} + +func (ec *executionContext) fieldContext_UserContactMethod_private(_ context.Context, field graphql.CollectedField) (fc *graphql.FieldContext, err error) { + fc = &graphql.FieldContext{ + Object: "UserContactMethod", + Field: field, + IsMethod: false, + IsResolver: false, + Child: func(ctx context.Context, field graphql.CollectedField) (*graphql.FieldContext, error) { + return nil, errors.New("field of type Boolean does not have child fields") + }, + } + return fc, nil +} + func (ec *executionContext) _UserNotificationRule_id(ctx context.Context, field graphql.CollectedField, obj *notificationrule.NotificationRule) (ret graphql.Marshaler) { fc, err := ec.fieldContext_UserNotificationRule_id(ctx, field) if err != nil { @@ -33977,6 +34035,8 @@ func (ec *executionContext) fieldContext_UserNotificationRule_contactMethod(_ co return ec.fieldContext_UserContactMethod_lastVerifyMessageState(ctx, field) case "statusUpdates": return ec.fieldContext_UserContactMethod_statusUpdates(ctx, field) + case "private": + return ec.fieldContext_UserContactMethod_private(ctx, field) } return nil, fmt.Errorf("no field named %q was found under type UserContactMethod", field.Name) }, @@ -37950,7 +38010,7 @@ func (ec *executionContext) unmarshalInputCreateUserContactMethodInput(ctx conte asMap[k] = v } - fieldsInOrder := [...]string{"userID", "type", "dest", "name", "value", "newUserNotificationRule", "enableStatusUpdates"} + fieldsInOrder := [...]string{"userID", "type", "dest", "name", "value", "newUserNotificationRule", "enableStatusUpdates", "private"} for _, k := range fieldsInOrder { v, ok := asMap[k] if !ok { @@ -38006,6 +38066,13 @@ func (ec *executionContext) unmarshalInputCreateUserContactMethodInput(ctx conte return it, err } it.EnableStatusUpdates = data + case "private": + ctx := graphql.WithPathContext(ctx, graphql.NewPathWithField("private")) + data, err := ec.unmarshalOBoolean2ᚖbool(ctx, v) + if err != nil { + return it, err + } + it.Private = data } } @@ -40627,7 +40694,7 @@ func (ec *executionContext) unmarshalInputUpdateUserContactMethodInput(ctx conte asMap[k] = v } - fieldsInOrder := [...]string{"id", "name", "value", "enableStatusUpdates"} + fieldsInOrder := [...]string{"id", "name", "value", "enableStatusUpdates", "private"} for _, k := range fieldsInOrder { v, ok := asMap[k] if !ok { @@ -40662,6 +40729,13 @@ func (ec *executionContext) unmarshalInputUpdateUserContactMethodInput(ctx conte return it, err } it.EnableStatusUpdates = data + case "private": + ctx := graphql.WithPathContext(ctx, graphql.NewPathWithField("private")) + data, err := ec.unmarshalOBoolean2ᚖbool(ctx, v) + if err != nil { + return it, err + } + it.Private = data } } @@ -49922,6 +49996,11 @@ func (ec *executionContext) _UserContactMethod(ctx context.Context, sel ast.Sele } out.Concurrently(i, func(ctx context.Context) graphql.Marshaler { return innerFunc(ctx, out) }) + case "private": + out.Values[i] = ec._UserContactMethod_private(ctx, field, obj) + if out.Values[i] == graphql.Null { + atomic.AddUint32(&out.Invalids, 1) + } default: panic("unknown field " + strconv.Quote(field.Name)) } diff --git a/graphql2/models_gen.go b/graphql2/models_gen.go index db08cc2a4d..82e53a8d76 100644 --- a/graphql2/models_gen.go +++ b/graphql2/models_gen.go @@ -298,6 +298,8 @@ type CreateUserContactMethodInput struct { // // Note: Some contact method types, like Slack, will always receive status updates and this value is ignored. EnableStatusUpdates *bool `json:"enableStatusUpdates,omitempty"` + // If true, this contact method will be private and only visible to the user. + Private *bool `json:"private,omitempty"` } type CreateUserInput struct { @@ -924,6 +926,8 @@ type UpdateUserContactMethodInput struct { // // Note: Some contact method types, like Slack, will always receive status updates and this value is ignored. EnableStatusUpdates *bool `json:"enableStatusUpdates,omitempty"` + // If true, this contact method will be private and only visible to the user. + Private *bool `json:"private,omitempty"` } type UpdateUserInput struct { diff --git a/graphql2/schema.graphql b/graphql2/schema.graphql index d668ba4fa3..a92711f9fa 100644 --- a/graphql2/schema.graphql +++ b/graphql2/schema.graphql @@ -1395,6 +1395,7 @@ type UserContactMethod { lastVerifyMessageState: NotificationState statusUpdates: StatusUpdateState! + private: Boolean! } enum StatusUpdateState { @@ -1426,6 +1427,11 @@ input CreateUserContactMethodInput { Note: Some contact method types, like Slack, will always receive status updates and this value is ignored. """ enableStatusUpdates: Boolean + + """ + If true, this contact method will be private and only visible to the user. + """ + private: Boolean } input CreateUserNotificationRuleInput { @@ -1448,6 +1454,11 @@ input UpdateUserContactMethodInput { Note: Some contact method types, like Slack, will always receive status updates and this value is ignored. """ enableStatusUpdates: Boolean + + """ + If true, this contact method will be private and only visible to the user. + """ + private: Boolean } input SendContactMethodVerificationInput { diff --git a/migrate/migrations/20250514091650-cm-private.sql b/migrate/migrations/20250514091650-cm-private.sql index c4a9112c04..68b3de34f8 100644 --- a/migrate/migrations/20250514091650-cm-private.sql +++ b/migrate/migrations/20250514091650-cm-private.sql @@ -1,6 +1,6 @@ -- +migrate Up ALTER TABLE user_contact_methods - ADD COLUMN private boolean DEFAULT FALSE; + ADD COLUMN private boolean NOT NULL DEFAULT FALSE; -- +migrate Down ALTER TABLE user_contact_methods diff --git a/migrate/schema.sql b/migrate/schema.sql index c46eb6d072..9d817ff7b8 100644 --- a/migrate/schema.sql +++ b/migrate/schema.sql @@ -1,5 +1,5 @@ -- This file is auto-generated by "make db-schema"; DO NOT EDIT --- DATA=a6bc6457557fa5c5d4b971a25798b6ffb24156f1e608b66066320e372c4dc86c - +-- DATA=c274594d00459967cd85c939f0b0712baa81e041eee02a383371eadc06d549cd - -- DISK=22479577daa4266fcaccbcf25675b21862e38fb630b49da2a5e5875344a27c3a - -- PSQL=22479577daa4266fcaccbcf25675b21862e38fb630b49da2a5e5875344a27c3a - -- @@ -2554,7 +2554,7 @@ CREATE TABLE user_contact_methods ( metadata jsonb, name text NOT NULL, pending boolean DEFAULT true NOT NULL, - private boolean DEFAULT false, + private boolean DEFAULT false NOT NULL, type enum_user_contact_method_type NOT NULL, user_id uuid NOT NULL, value text NOT NULL, diff --git a/user/contactmethod/contactmethod.go b/user/contactmethod/contactmethod.go index e3ca37dc4e..70334a8067 100644 --- a/user/contactmethod/contactmethod.go +++ b/user/contactmethod/contactmethod.go @@ -20,6 +20,7 @@ type ContactMethod struct { Disabled bool UserID string Pending bool + Private bool StatusUpdates bool diff --git a/user/contactmethod/queries.sql b/user/contactmethod/queries.sql index d62660195b..0ed6e70e11 100644 --- a/user/contactmethod/queries.sql +++ b/user/contactmethod/queries.sql @@ -1,6 +1,6 @@ -- name: ContactMethodAdd :exec -INSERT INTO user_contact_methods(id, name, dest, disabled, user_id, enable_status_updates) - VALUES ($1, $2, $3, $4, $5, $6); +INSERT INTO user_contact_methods(id, name, dest, disabled, user_id, enable_status_updates, private) + VALUES ($1, $2, $3, $4, $5, $6, $7); -- name: ContactMethodUpdate :exec UPDATE @@ -8,7 +8,8 @@ UPDATE SET name = $2, disabled = $3, - enable_status_updates = $4 + enable_status_updates = $4, + private = $5 WHERE id = $1; diff --git a/user/contactmethod/store.go b/user/contactmethod/store.go index f050f70bda..82c2963ba6 100644 --- a/user/contactmethod/store.go +++ b/user/contactmethod/store.go @@ -141,6 +141,7 @@ func (s *Store) Create(ctx context.Context, dbtx gadb.DBTX, c *ContactMethod) (* Disabled: n.Disabled, UserID: uuid.MustParse(n.UserID), EnableStatusUpdates: n.StatusUpdates, + Private: n.Private, }) if err != nil { return nil, err @@ -248,7 +249,7 @@ func (s *Store) Update(ctx context.Context, dbtx gadb.DBTX, c *ContactMethod) er return err } - err = gadb.New(dbtx).ContactMethodUpdate(ctx, gadb.ContactMethodUpdateParams{ID: n.ID, Name: n.Name, Disabled: n.Disabled, EnableStatusUpdates: n.StatusUpdates}) + err = gadb.New(dbtx).ContactMethodUpdate(ctx, gadb.ContactMethodUpdateParams{ID: n.ID, Name: n.Name, Disabled: n.Disabled, EnableStatusUpdates: n.StatusUpdates, Private: n.Private}) return err } From f86c083a2ffb0f2918a800c7a00f83025879c6da Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Wed, 14 May 2025 09:35:18 -0500 Subject: [PATCH 03/13] feat: update contact method queries to support requester and owner parameters --- gadb/queries.sql.go | 11 +++++++++-- graphql2/graphqlapp/dataloaders.go | 18 +++++++++++++++--- user/contactmethod/queries.sql | 4 +++- user/contactmethod/store.go | 5 ++++- 4 files changed, 31 insertions(+), 7 deletions(-) diff --git a/gadb/queries.sql.go b/gadb/queries.sql.go index fca2bacf3e..4f03ac40be 100644 --- a/gadb/queries.sql.go +++ b/gadb/queries.sql.go @@ -1748,10 +1748,17 @@ FROM user_contact_methods WHERE user_id = $1 + AND (user_id = $2 + OR NOT private) ` -func (q *Queries) ContactMethodFindAll(ctx context.Context, userID uuid.UUID) ([]UserContactMethod, error) { - rows, err := q.db.QueryContext(ctx, contactMethodFindAll, userID) +type ContactMethodFindAllParams struct { + Owner uuid.UUID + Requester uuid.UUID +} + +func (q *Queries) ContactMethodFindAll(ctx context.Context, arg ContactMethodFindAllParams) ([]UserContactMethod, error) { + rows, err := q.db.QueryContext(ctx, contactMethodFindAll, arg.Owner, arg.Requester) if err != nil { return nil, err } diff --git a/graphql2/graphqlapp/dataloaders.go b/graphql2/graphqlapp/dataloaders.go index a44ea6824c..508dfca1a8 100644 --- a/graphql2/graphqlapp/dataloaders.go +++ b/graphql2/graphqlapp/dataloaders.go @@ -12,6 +12,7 @@ import ( "github.com/target/goalert/heartbeat" "github.com/target/goalert/notification" "github.com/target/goalert/notificationchannel" + "github.com/target/goalert/permission" "github.com/target/goalert/schedule" "github.com/target/goalert/schedule/rotation" "github.com/target/goalert/service" @@ -164,13 +165,24 @@ func (app *App) FindOneAlertMetric(ctx context.Context, id int) (*alertmetrics.M } // FindOneCM will return a single contact method for the given id, using the contexts dataloader if enabled. -func (app *App) FindOneCM(ctx context.Context, id uuid.UUID) (*contactmethod.ContactMethod, error) { +func (app *App) FindOneCM(ctx context.Context, id uuid.UUID) (cm *contactmethod.ContactMethod, err error) { loader, ok := ctx.Value(dataLoaderKeyCM).(*dataloader.Loader[uuid.UUID, contactmethod.ContactMethod]) if !ok { - return app.CMStore.FindOne(ctx, app.DB, id) + cm, err = app.CMStore.FindOne(ctx, app.DB, id) + } else { + cm, err = loader.FetchOne(ctx, id) + } + if err != nil { + return nil, err + } + if cm == nil { + return nil, nil + } + if cm.Private && cm.UserID != permission.UserID(ctx) { + return nil, nil } - return loader.FetchOne(ctx, id) + return cm, nil } // FindOneNC will return a single notification channel for the given id, using the contexts dataloader if enabled. diff --git a/user/contactmethod/queries.sql b/user/contactmethod/queries.sql index 0ed6e70e11..59445a77d8 100644 --- a/user/contactmethod/queries.sql +++ b/user/contactmethod/queries.sql @@ -48,7 +48,9 @@ SELECT FROM user_contact_methods WHERE - user_id = $1; + user_id = @owner + AND (user_id = @requester + OR NOT private); -- name: ContactMethodLookupUserID :many SELECT DISTINCT diff --git a/user/contactmethod/store.go b/user/contactmethod/store.go index 82c2963ba6..851f1ea9c6 100644 --- a/user/contactmethod/store.go +++ b/user/contactmethod/store.go @@ -300,7 +300,10 @@ func (s *Store) FindAll(ctx context.Context, dbtx gadb.DBTX, userID string) ([]C return nil, err } - rows, err := gadb.New(dbtx).ContactMethodFindAll(ctx, uid) + rows, err := gadb.New(dbtx).ContactMethodFindAll(ctx, gadb.ContactMethodFindAllParams{ + Owner: uid, + Requester: permission.UserNullUUID(ctx).UUID, + }) if err != nil { return nil, err } From 3248f2de9abb8380464f8bbee3a5dd70be7e30eb Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Wed, 14 May 2025 09:40:14 -0500 Subject: [PATCH 04/13] feat: refactor contact methods queries to simplify parameters and include omitted count --- gadb/queries.sql.go | 11 ++--------- graphql2/graphqlapp/user.go | 3 ++- user/contactmethod/queries.sql | 4 +--- user/contactmethod/store.go | 33 +++++++++++++++++++-------------- 4 files changed, 24 insertions(+), 27 deletions(-) diff --git a/gadb/queries.sql.go b/gadb/queries.sql.go index 4f03ac40be..fca2bacf3e 100644 --- a/gadb/queries.sql.go +++ b/gadb/queries.sql.go @@ -1748,17 +1748,10 @@ FROM user_contact_methods WHERE user_id = $1 - AND (user_id = $2 - OR NOT private) ` -type ContactMethodFindAllParams struct { - Owner uuid.UUID - Requester uuid.UUID -} - -func (q *Queries) ContactMethodFindAll(ctx context.Context, arg ContactMethodFindAllParams) ([]UserContactMethod, error) { - rows, err := q.db.QueryContext(ctx, contactMethodFindAll, arg.Owner, arg.Requester) +func (q *Queries) ContactMethodFindAll(ctx context.Context, userID uuid.UUID) ([]UserContactMethod, error) { + rows, err := q.db.QueryContext(ctx, contactMethodFindAll, userID) if err != nil { return nil, err } diff --git a/graphql2/graphqlapp/user.go b/graphql2/graphqlapp/user.go index 23382f13b1..9675b83b50 100644 --- a/graphql2/graphqlapp/user.go +++ b/graphql2/graphqlapp/user.go @@ -119,7 +119,8 @@ func (a *User) Role(ctx context.Context, usr *user.User) (graphql2.UserRole, err } func (a *User) ContactMethods(ctx context.Context, obj *user.User) ([]contactmethod.ContactMethod, error) { - return a.CMStore.FindAll(ctx, a.DB, obj.ID) + cm, _, err := a.CMStore.FindAll(ctx, a.DB, obj.ID) + return cm, err } func (a *User) NotificationRules(ctx context.Context, obj *user.User) ([]notificationrule.NotificationRule, error) { diff --git a/user/contactmethod/queries.sql b/user/contactmethod/queries.sql index 59445a77d8..0ed6e70e11 100644 --- a/user/contactmethod/queries.sql +++ b/user/contactmethod/queries.sql @@ -48,9 +48,7 @@ SELECT FROM user_contact_methods WHERE - user_id = @owner - AND (user_id = @requester - OR NOT private); + user_id = $1; -- name: ContactMethodLookupUserID :many SELECT DISTINCT diff --git a/user/contactmethod/store.go b/user/contactmethod/store.go index 851f1ea9c6..d60b01e7e5 100644 --- a/user/contactmethod/store.go +++ b/user/contactmethod/store.go @@ -288,29 +288,33 @@ func (s *Store) FindMany(ctx context.Context, dbtx gadb.DBTX, ids []string) ([]C return cms, nil } -// FindAll finds all contact methods from the database associated with the given user ID. -func (s *Store) FindAll(ctx context.Context, dbtx gadb.DBTX, userID string) ([]ContactMethod, error) { +// FindAll finds all contact methods from the database associated with the given user ID along with the number of omitted (private) entries. +func (s *Store) FindAll(ctx context.Context, dbtx gadb.DBTX, userID string) ([]ContactMethod, int, error) { uid, err := validate.ParseUUID("ContactMethodID", userID) if err != nil { - return nil, err + return nil, 0, err } err = permission.LimitCheckAny(ctx, permission.All) if err != nil { - return nil, err + return nil, 0, err } - rows, err := gadb.New(dbtx).ContactMethodFindAll(ctx, gadb.ContactMethodFindAllParams{ - Owner: uid, - Requester: permission.UserNullUUID(ctx).UUID, - }) + rows, err := gadb.New(dbtx).ContactMethodFindAll(ctx, uid) if err != nil { - return nil, err + return nil, 0, err } + authID := permission.UserNullUUID(ctx).UUID - cms := make([]ContactMethod, len(rows)) - for i, row := range rows { - cms[i] = ContactMethod{ + result := make([]ContactMethod, 0, len(rows)) + var omitted int + for _, row := range rows { + if row.Private && row.UserID != authID { + omitted++ + continue + } + + result = append(result, ContactMethod{ ID: row.ID, Name: row.Name, Dest: row.Dest.DestV1, @@ -319,8 +323,9 @@ func (s *Store) FindAll(ctx context.Context, dbtx gadb.DBTX, userID string) ([]C Pending: row.Pending, StatusUpdates: row.EnableStatusUpdates, lastTestVerifyAt: row.LastTestVerifyAt, - } + Private: row.Private, + }) } - return cms, nil + return result, omitted, nil } From 8b5ced1369dcf99c895dceb17200e320445579a2 Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Wed, 14 May 2025 09:48:49 -0500 Subject: [PATCH 05/13] feat: add private field to contact methods and update related queries --- devtools/resetdb/datagen.go | 3 +++ devtools/resetdb/main.go | 4 ++-- user/contactmethod/store.go | 2 ++ web/src/schema.d.ts | 3 +++ 4 files changed, 10 insertions(+), 2 deletions(-) diff --git a/devtools/resetdb/datagen.go b/devtools/resetdb/datagen.go index 36b5f60d41..640253346e 100644 --- a/devtools/resetdb/datagen.go +++ b/devtools/resetdb/datagen.go @@ -158,6 +158,9 @@ func (d *datagen) NewCM(userID string) { if d.Bool() { cm.Dest.Type = twilio.DestTypeTwilioVoice } + if d.Intn(4) == 0 { + cm.Private = true + } cm.Dest.SetArg(twilio.FieldPhoneNumber, d.ids.Gen(d.genPhone, cm.Dest.Type)) d.ContactMethods = append(d.ContactMethods, cm) diff --git a/devtools/resetdb/main.go b/devtools/resetdb/main.go index b7d50057f3..6dff1343b1 100644 --- a/devtools/resetdb/main.go +++ b/devtools/resetdb/main.go @@ -140,9 +140,9 @@ func fillDB(ctx context.Context, dataCfg *datagenConfig, url string) error { u := data.Users[n] return []interface{}{asUUID(u.ID), u.Name, u.Role, u.Email} }) - copyFrom("user_contact_methods", []string{"id", "user_id", "name", "dest", "disabled", "pending"}, len(data.ContactMethods), func(n int) []interface{} { + copyFrom("user_contact_methods", []string{"id", "user_id", "name", "dest", "disabled", "pending", "private"}, len(data.ContactMethods), func(n int) []interface{} { cm := data.ContactMethods[n] - return []interface{}{cm.ID, asUUID(cm.UserID), cm.Name, cm.Dest, cm.Disabled, cm.Pending} + return []interface{}{cm.ID, asUUID(cm.UserID), cm.Name, cm.Dest, cm.Disabled, cm.Pending, cm.Private} }, "users") copyFrom("user_notification_rules", []string{"id", "user_id", "contact_method_id", "delay_minutes"}, len(data.NotificationRules), func(n int) []interface{} { nr := data.NotificationRules[n] diff --git a/user/contactmethod/store.go b/user/contactmethod/store.go index d60b01e7e5..57370a2328 100644 --- a/user/contactmethod/store.go +++ b/user/contactmethod/store.go @@ -210,6 +210,7 @@ func (s *Store) FindOne(ctx context.Context, dbtx gadb.DBTX, id uuid.UUID) (*Con UserID: row.UserID.String(), Pending: row.Pending, StatusUpdates: row.EnableStatusUpdates, + Private: row.Private, lastTestVerifyAt: row.LastTestVerifyAt, } @@ -281,6 +282,7 @@ func (s *Store) FindMany(ctx context.Context, dbtx gadb.DBTX, ids []string) ([]C UserID: row.UserID.String(), Pending: row.Pending, StatusUpdates: row.EnableStatusUpdates, + Private: row.Private, lastTestVerifyAt: row.LastTestVerifyAt, } } diff --git a/web/src/schema.d.ts b/web/src/schema.d.ts index 577465e7d9..43a54ae740 100644 --- a/web/src/schema.d.ts +++ b/web/src/schema.d.ts @@ -320,6 +320,7 @@ export interface CreateUserContactMethodInput { enableStatusUpdates?: null | boolean name: string newUserNotificationRule?: null | CreateUserNotificationRuleInput + private?: null | boolean type?: null | ContactMethodType userID: string value?: null | string @@ -1382,6 +1383,7 @@ export interface UpdateUserContactMethodInput { enableStatusUpdates?: null | boolean id: string name?: null | string + private?: null | boolean value?: null | string } @@ -1445,6 +1447,7 @@ export interface UserContactMethod { lastVerifyMessageState?: null | NotificationState name: string pending: boolean + private: boolean statusUpdates: StatusUpdateState type?: null | ContactMethodType value: string From 3d74430ab7d56a21e4785f376c0d8281e2e16547 Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Wed, 14 May 2025 09:54:03 -0500 Subject: [PATCH 06/13] feat: enhance notification rule formatting to handle missing contact method info --- .../app/users/UserNotificationRuleList.tsx | 24 +++++++++++-------- web/src/app/users/util.js | 10 ++++---- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/web/src/app/users/UserNotificationRuleList.tsx b/web/src/app/users/UserNotificationRuleList.tsx index 57e0d91c0c..021b4b22fa 100644 --- a/web/src/app/users/UserNotificationRuleList.tsx +++ b/web/src/app/users/UserNotificationRuleList.tsx @@ -104,19 +104,23 @@ export default function UserNotificationRuleList(props: { emptyMessage='No notification rules' > {sortNotificationRules(user?.notificationRules ?? []).map((nr) => { - const formattedValue = - nr.contactMethod.dest.displayInfo.text || 'Unknown Label' - const name = nr.contactMethod.name || 'Unknown User' - const type = - nr.contactMethod.dest.displayInfo.iconAltText || 'Unknown Type' + let info = null + if (nr.contactMethod) { + const formattedValue = + nr.contactMethod.dest.displayInfo.text || 'Unknown Label' + const name = nr.contactMethod.name || 'Unknown User' + const type = + nr.contactMethod.dest.displayInfo.iconAltText || 'Unknown Type' + info = { + type, + name, + formattedValue, + } + } return ( Date: Wed, 14 May 2025 10:03:50 -0500 Subject: [PATCH 07/13] feat: add private field to user contact methods and update related forms and queries --- graphql2/graphqlapp/contactmethod.go | 4 +++ user/contactmethod/store.go | 2 +- .../users/UserContactMethodCreateDialog.tsx | 3 +++ .../app/users/UserContactMethodEditDialog.tsx | 3 +++ web/src/app/users/UserContactMethodForm.tsx | 25 +++++++++++++++++++ web/src/app/users/UserContactMethodList.tsx | 3 ++- 6 files changed, 38 insertions(+), 2 deletions(-) diff --git a/graphql2/graphqlapp/contactmethod.go b/graphql2/graphqlapp/contactmethod.go index bae487368e..a5f272926a 100644 --- a/graphql2/graphqlapp/contactmethod.go +++ b/graphql2/graphqlapp/contactmethod.go @@ -121,6 +121,7 @@ func (m *Mutation) CreateUserContactMethod(ctx context.Context, input graphql2.C UserID: input.UserID, Disabled: true, StatusUpdates: input.EnableStatusUpdates != nil && *input.EnableStatusUpdates, + Private: input.Private != nil && *input.Private, } if input.Dest != nil { @@ -189,6 +190,9 @@ func (m *Mutation) UpdateUserContactMethod(ctx context.Context, input graphql2.U } cm.Name = *input.Name } + if input.Private != nil { + cm.Private = *input.Private + } if input.EnableStatusUpdates != nil { cm.StatusUpdates = *input.EnableStatusUpdates diff --git a/user/contactmethod/store.go b/user/contactmethod/store.go index 57370a2328..04ca5a7302 100644 --- a/user/contactmethod/store.go +++ b/user/contactmethod/store.go @@ -241,7 +241,7 @@ func (s *Store) Update(ctx context.Context, dbtx gadb.DBTX, c *ContactMethod) er } if permission.Admin(ctx) { - err = gadb.New(dbtx).ContactMethodUpdate(ctx, gadb.ContactMethodUpdateParams{ID: n.ID, Name: n.Name, Disabled: n.Disabled, EnableStatusUpdates: n.StatusUpdates}) + err = gadb.New(dbtx).ContactMethodUpdate(ctx, gadb.ContactMethodUpdateParams{ID: n.ID, Name: n.Name, Disabled: n.Disabled, EnableStatusUpdates: n.StatusUpdates, Private: n.Private}) return err } diff --git a/web/src/app/users/UserContactMethodCreateDialog.tsx b/web/src/app/users/UserContactMethodCreateDialog.tsx index fed8d6865b..c334710bb4 100644 --- a/web/src/app/users/UserContactMethodCreateDialog.tsx +++ b/web/src/app/users/UserContactMethodCreateDialog.tsx @@ -13,6 +13,7 @@ type Value = { name: string dest: DestinationInput statusUpdates: boolean + private: boolean } const createMutation = gql` @@ -41,6 +42,7 @@ export default function UserContactMethodCreateDialog(props: { args: {}, }, statusUpdates: false, + private: false, }) const [createErr, setCreateErr] = useState(null) const setCMValue = (newValue: Value): void => { @@ -112,6 +114,7 @@ export default function UserContactMethodCreateDialog(props: { dest: CMValue.dest, enableStatusUpdates: CMValue.statusUpdates, userID: props.userID, + private: CMValue.private, newUserNotificationRule: { delayMinutes: 0, }, diff --git a/web/src/app/users/UserContactMethodEditDialog.tsx b/web/src/app/users/UserContactMethodEditDialog.tsx index 2c1f54a332..0bbfc94722 100644 --- a/web/src/app/users/UserContactMethodEditDialog.tsx +++ b/web/src/app/users/UserContactMethodEditDialog.tsx @@ -10,6 +10,7 @@ type Value = { name: string dest: DestinationInput statusUpdates: boolean + private: boolean } const query = gql` @@ -22,6 +23,7 @@ const query = gql` args } statusUpdates + private } } ` @@ -89,6 +91,7 @@ export default function UserContactMethodEditDialog(props: { id: props.contactMethodID, name: CMValue.name, enableStatusUpdates: Boolean(CMValue.statusUpdates), + private: CMValue.private, }, }, { additionalTypenames: ['UserContactMethod'] }, diff --git a/web/src/app/users/UserContactMethodForm.tsx b/web/src/app/users/UserContactMethodForm.tsx index 0d237bbaff..65e31fded7 100644 --- a/web/src/app/users/UserContactMethodForm.tsx +++ b/web/src/app/users/UserContactMethodForm.tsx @@ -12,6 +12,7 @@ export type Value = { name: string dest: DestinationInput statusUpdates: boolean + private: boolean } export type UserContactMethodFormProps = { @@ -168,6 +169,30 @@ export default function UserContactMethodForm( } /> + + + props.onChange && + props.onChange({ + ...value, + private: v.target.checked, + }) + } + /> + } + /> + ) diff --git a/web/src/app/users/UserContactMethodList.tsx b/web/src/app/users/UserContactMethodList.tsx index 757986b046..a8d77c76f6 100644 --- a/web/src/app/users/UserContactMethodList.tsx +++ b/web/src/app/users/UserContactMethodList.tsx @@ -44,6 +44,7 @@ const query = gql` } disabled pending + private } } } @@ -220,7 +221,7 @@ export default function UserContactMethodList( return ( Date: Tue, 20 May 2025 13:06:01 -0500 Subject: [PATCH 08/13] feat: update UserContactMethodForm to use 'private' field and simplify status updates checkbox --- web/src/app/users/UserContactMethodForm.tsx | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/web/src/app/users/UserContactMethodForm.tsx b/web/src/app/users/UserContactMethodForm.tsx index 65e31fded7..9e1d41cefb 100644 --- a/web/src/app/users/UserContactMethodForm.tsx +++ b/web/src/app/users/UserContactMethodForm.tsx @@ -175,12 +175,8 @@ export default function UserContactMethodForm( title='Private contact methods are not visible to other users.' control={ props.onChange && From 1c999cc60f560d9052e6ea6176f3079a5d7f90bb Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Thu, 22 May 2025 16:25:13 -0500 Subject: [PATCH 09/13] feat: update permission check in FindAll to use 'All' instead of specific roles --- user/notificationrule/store.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/user/notificationrule/store.go b/user/notificationrule/store.go index 918c5dc73f..51583af895 100644 --- a/user/notificationrule/store.go +++ b/user/notificationrule/store.go @@ -124,7 +124,7 @@ func (s *Store) FindAll(ctx context.Context, userID string) ([]NotificationRule, return nil, err } - err = permission.LimitCheckAny(ctx, permission.System, permission.User, permission.Admin) + err = permission.LimitCheckAny(ctx, permission.All) if err != nil { return nil, err } From fb5709a652a32c98cdc19cc9092740f95bc95593 Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Thu, 22 May 2025 16:25:20 -0500 Subject: [PATCH 10/13] feat: add tests for private contact methods visibility in GraphQL queries --- test/smoke/privatecm_test.go | 198 +++++++++++++++++++++++++++++++++++ 1 file changed, 198 insertions(+) create mode 100644 test/smoke/privatecm_test.go diff --git a/test/smoke/privatecm_test.go b/test/smoke/privatecm_test.go new file mode 100644 index 0000000000..7f70176288 --- /dev/null +++ b/test/smoke/privatecm_test.go @@ -0,0 +1,198 @@ +package smoke + +import ( + "encoding/json" + "fmt" + "sort" + "testing" + + "github.com/stretchr/testify/require" + "github.com/target/goalert/test/smoke/harness" +) + +const privCMQuery = ` +mutation NewCM($input: CreateUserContactMethodInput!) { + createUserContactMethod(input: $input) { + id + } +} + +mutation UpdateCM($input: UpdateUserContactMethodInput!) { + updateUserContactMethod(input: $input) +} + +query GetCM($id: ID!) { + userContactMethod(id: $id) { + id + name + private + } +} + + +query ListUserCM($userID: ID!) { + user(id: $userID) { + id + contactMethods { + id + name + private + } + notificationRules { + id + contactMethodID + contactMethod { + id + } + } + } +} +` + +// TestPrivateCM checks that private contact methods are not visible in GraphQL calls from any user that is not the owner. +func TestPrivateCM(t *testing.T) { + t.Parallel() + + const sql = ` + insert into users (id, name, email) + values + ({{uuid "user1"}}, 'bob', 'joe'), + ({{uuid "user2"}}, 'bob2', 'joe2'); + ` + + h := harness.NewHarness(t, sql, "") + defer h.Close() + + var newCM struct { + CreateUserContactMethod struct{ ID string } + } + + resp := h.GraphQLQueryUserVarsT(t, h.UUID("user1"), privCMQuery, "NewCM", + json.RawMessage(fmt.Sprintf( + `{"input":{ + "userID": "%s", + "name": "cm1", + "dest":{ + "type":"builtin-smtp-email", + "args":{"email_address":"foobar@example.com"} + }, + "newUserNotificationRule": {"delayMinutes": 0} + }}`, h.UUID("user1")))) + require.Empty(t, resp.Errors, "expected no errors") + require.NoError(t, json.Unmarshal(resp.Data, &newCM)) + cmID1 := newCM.CreateUserContactMethod.ID + + resp = h.GraphQLQueryUserVarsT(t, h.UUID("user1"), privCMQuery, "NewCM", json.RawMessage(fmt.Sprintf( + `{"input":{ + "userID": "%s", + "name": "cm2", + "private": true, + "dest":{ + "type":"builtin-smtp-email", + "args":{"email_address":"foobar-priv@example.com"} + }, + "newUserNotificationRule": {"delayMinutes": 0} + }}`, h.UUID("user1")))) + require.Empty(t, resp.Errors, "expected no errors") + require.NoError(t, json.Unmarshal(resp.Data, &newCM)) + cmID2 := newCM.CreateUserContactMethod.ID + + userCanSeeCM := func(userID, cmID string, isPrivate, expectAccess bool) { + t.Helper() + + // check direct access to the contact method + resp := h.GraphQLQueryUserVarsT(t, userID, privCMQuery, "GetCM", json.RawMessage(fmt.Sprintf(`{"id":"%s"}`, cmID))) + require.Empty(t, resp.Errors, "expected no errors") + t.Log("resp", string(resp.Data)) + var cm struct { + UserContactMethod *struct { + ID string + Name string + Private bool + } + } + require.NoError(t, json.Unmarshal(resp.Data, &cm)) + if !expectAccess { + require.Nil(t, cm.UserContactMethod, "expected to not see private contact method") + return + } + require.NotNil(t, cm.UserContactMethod, "expected to see contact method") + require.Equal(t, cmID, cm.UserContactMethod.ID, "expected to see contact method with ID %s", cmID) + require.Equal(t, isPrivate, cm.UserContactMethod.Private, "expected to see contact method with private=%t", isPrivate) + } + + userCanSeeCM(h.UUID("user1"), cmID1, false, true) + userCanSeeCM(h.UUID("user1"), cmID2, true, true) + userCanSeeCM(h.UUID("user2"), cmID1, false, true) + userCanSeeCM(h.UUID("user2"), cmID2, true, false) + + // validate listing + type listCMResp struct { + User struct { + ID string + ContactMethods []struct { + ID string + Name string + Private bool + } + NotificationRules []struct { + ID string + ContactMethodID string + ContactMethod *struct { + ID string + } + } + } + } + + // as user 1 + var listCM listCMResp + resp = h.GraphQLQueryUserVarsT(t, h.UUID("user1"), privCMQuery, "ListUserCM", json.RawMessage(fmt.Sprintf(`{"userID":"%s"}`, h.UUID("user1")))) + require.Empty(t, resp.Errors, "expected no errors") + + require.NoError(t, json.Unmarshal(resp.Data, &listCM)) + + require.Len(t, listCM.User.ContactMethods, 2, "expected to see both contact methods") + sort.Slice(listCM.User.ContactMethods, func(i, j int) bool { + return listCM.User.ContactMethods[i].Name < listCM.User.ContactMethods[j].Name + }) + require.Equal(t, cmID1, listCM.User.ContactMethods[0].ID, "expected to see contact method 1") + require.False(t, listCM.User.ContactMethods[0].Private, "expected to see contact method 1 as not private") + require.True(t, listCM.User.ContactMethods[1].Private, "expected to see contact method 2 as private") + require.Len(t, listCM.User.NotificationRules, 2, "expected to see two notification rules") + for _, rule := range listCM.User.NotificationRules { + switch rule.ContactMethodID { + case cmID1: + require.NotNil(t, rule.ContactMethod, "expected to see contact method 1") + case cmID2: + require.NotNil(t, rule.ContactMethod, "expected to see contact method 2") + default: + t.Fatalf("unexpected contact method ID %s", rule.ContactMethodID) + } + } + + // as user 2 + listCM = listCMResp{} + resp = h.GraphQLQueryUserVarsT(t, h.UUID("user2"), privCMQuery, "ListUserCM", json.RawMessage(fmt.Sprintf(`{"userID":"%s"}`, h.UUID("user1")))) + require.Empty(t, resp.Errors, "expected no errors") + + require.NoError(t, json.Unmarshal(resp.Data, &listCM)) + + require.Len(t, listCM.User.ContactMethods, 1, "expected to see only contact method 1") + sort.Slice(listCM.User.ContactMethods, func(i, j int) bool { + return listCM.User.ContactMethods[i].Name < listCM.User.ContactMethods[j].Name + }) + require.Equal(t, cmID1, listCM.User.ContactMethods[0].ID, "expected to see contact method 1") + require.False(t, listCM.User.ContactMethods[0].Private, "expected to see contact method 1 as not private") + require.Len(t, listCM.User.NotificationRules, 2, "expected to see two notification rules") + for _, rule := range listCM.User.NotificationRules { + switch rule.ContactMethodID { + case cmID1: + require.NotNil(t, rule.ContactMethod, "expected to see contact method 1") + case cmID2: + require.Nil(t, rule.ContactMethod, "expected to not see contact method 2") + default: + t.Fatalf("unexpected contact method ID %s", rule.ContactMethodID) + } + } +} From d16bab6b7389c6a37c81317f429f1bc5b0bba431 Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Fri, 23 May 2025 10:17:07 -0500 Subject: [PATCH 11/13] feat: set 'private' field to false in user contact method stories --- web/src/app/users/UserContactMethodForm.stories.tsx | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/web/src/app/users/UserContactMethodForm.stories.tsx b/web/src/app/users/UserContactMethodForm.stories.tsx index 0dfad216e1..45121863f3 100644 --- a/web/src/app/users/UserContactMethodForm.stories.tsx +++ b/web/src/app/users/UserContactMethodForm.stories.tsx @@ -41,6 +41,7 @@ export const SupportStatusUpdates: Story = { args: { value: { name: 'supports status', + private: false, dest: { type: 'supports-status', args: { phone_number: '+15555555555' }, @@ -63,6 +64,7 @@ export const RequiredStatusUpdates: Story = { args: { value: { name: 'required status', + private: false, dest: { type: 'required-status', args: { phone_number: '+15555555555' }, @@ -87,6 +89,7 @@ export const ErrorSingleField: Story = { args: { value: { name: '-notvalid', + private: false, dest: { type: 'single-field', args: { phone_number: '+15555555555' }, @@ -112,6 +115,7 @@ export const ErrorMultiField: Story = { args: { value: { name: '-notvalid', + private: false, dest: { type: 'triple-field', args: { @@ -144,6 +148,7 @@ export const Disabled: Story = { args: { value: { name: 'disabled dest', + private: false, dest: { type: 'triple-field', args: {}, From 49c5f1388b1e31a76ee87b5c72031a22293e3532 Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Mon, 29 Sep 2025 14:18:39 -0500 Subject: [PATCH 12/13] update form test --- web/src/app/users/UserContactMethodEditDialog.stories.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/web/src/app/users/UserContactMethodEditDialog.stories.tsx b/web/src/app/users/UserContactMethodEditDialog.stories.tsx index a123c32308..664d079eb5 100644 --- a/web/src/app/users/UserContactMethodEditDialog.stories.tsx +++ b/web/src/app/users/UserContactMethodEditDialog.stories.tsx @@ -171,7 +171,9 @@ export const SingleField: Story = { const single = await canvas.findByLabelText('Destination Type') expect(single).toHaveTextContent('Single With Status') - await canvas.findByTestId('CheckBoxOutlineBlankIcon') + + await canvas.findByLabelText('Send alert status updates') + await canvas.findByLabelText('This is a private contact method') }, } From aaa9b012329f324d497394864161f2715ceb1235 Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Mon, 29 Sep 2025 14:18:48 -0500 Subject: [PATCH 13/13] fix storybook ui --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 84cf6ea902..e7ee346ee1 100644 --- a/Makefile +++ b/Makefile @@ -270,7 +270,7 @@ test-components: $(NODE_DEPS) "$(WAITFOR) tcp://localhost:6008 && $(BIN_DIR)/tools/bun run test-storybook --ci --url http://127.0.0.1:6008 --maxWorkers 2" storybook: $(NODE_DEPS) # Start the Storybook UI - $(BIN_DIR)/tools/bun run storybook + $(BIN_DIR)/tools/bun -b run storybook playwright-run: $(NODE_DEPS) web/src/build/static/app.js bin/goalert.cover web/src/schema.d.ts $(BIN_DIR)/tools/prometheus $(BIN_DIR)/tools/mailpit reset-integration ## Start playwright tests in headless mode rm -rf test/coverage/integration/playwright