From f1afe2c11035815d2df112076e4a51c4be573e07 Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Tue, 9 Jul 2024 09:17:36 -0500 Subject: [PATCH 01/15] Add signal management module and related updates - Introduced SignalManager to handle outgoing status updates - Added new processing lock type and enum 'signals' - Updated database schema with new signal module migrations - Implemented signal message types and related notification changes - Added SQL queries for handling pending signals operations --- engine/engine.go | 10 +++ engine/processinglock/type.go | 1 + engine/signalmgr/db.go | 31 +++++++ engine/signalmgr/queries.sql | 30 +++++++ engine/signalmgr/update.go | 54 +++++++++++ gadb/queries.sql.go | 89 +++++++++++++++++++ .../20240709091828-signal-module.sql | 13 +++ migrate/schema.sql | 7 +- notification/messagetype.go | 6 ++ notification/messagetype_string.go | 5 +- notification/signal.go | 18 ++++ sqlc.yaml | 1 + 12 files changed, 260 insertions(+), 5 deletions(-) create mode 100644 engine/signalmgr/db.go create mode 100644 engine/signalmgr/queries.sql create mode 100644 engine/signalmgr/update.go create mode 100644 migrate/migrations/20240709091828-signal-module.sql create mode 100644 notification/signal.go diff --git a/engine/engine.go b/engine/engine.go index 18284e7c49..707c2e54cd 100644 --- a/engine/engine.go +++ b/engine/engine.go @@ -21,8 +21,10 @@ import ( "github.com/target/goalert/engine/processinglock" "github.com/target/goalert/engine/rotationmanager" "github.com/target/goalert/engine/schedulemanager" + "github.com/target/goalert/engine/signalmgr" "github.com/target/goalert/engine/statusmgr" "github.com/target/goalert/engine/verifymanager" + "github.com/target/goalert/expflag" "github.com/target/goalert/notification" "github.com/target/goalert/permission" "github.com/target/goalert/user" @@ -147,6 +149,14 @@ func NewEngine(ctx context.Context, db *sql.DB, c *Config) (*Engine, error) { metricsMgr, } + if expflag.ContextHas(ctx, expflag.UnivKeys) { + signalMgr, err := signalmgr.NewDB(ctx, db) + if err != nil { + return nil, errors.Wrap(err, "signal manager backend") + } + p.modules = append(p.modules, signalMgr) + } + p.msg, err = message.NewDB(ctx, db, c.AlertLogStore, p.mgr) if err != nil { return nil, errors.Wrap(err, "messaging backend") diff --git a/engine/processinglock/type.go b/engine/processinglock/type.go index 235b6cc7b9..b998404cca 100644 --- a/engine/processinglock/type.go +++ b/engine/processinglock/type.go @@ -16,4 +16,5 @@ const ( TypeCleanup Type = "cleanup" TypeMetrics Type = "metrics" TypeCompat Type = "compat" + TypeSignals Type = "signals" ) diff --git a/engine/signalmgr/db.go b/engine/signalmgr/db.go new file mode 100644 index 0000000000..d02ad2225a --- /dev/null +++ b/engine/signalmgr/db.go @@ -0,0 +1,31 @@ +package signalmgr + +import ( + "context" + "database/sql" + + "github.com/target/goalert/engine/processinglock" +) + +// DB manages outgoing status updates. +type DB struct { + lock *processinglock.Lock +} + +// Name returns the name of the module. +func (db *DB) Name() string { return "Engine.SignalManager" } + +// NewDB creates a new DB. +func NewDB(ctx context.Context, db *sql.DB) (*DB, error) { + lock, err := processinglock.NewLock(ctx, db, processinglock.Config{ + Type: processinglock.TypeSignals, + Version: 1, + }) + if err != nil { + return nil, err + } + + return &DB{ + lock: lock, + }, nil +} diff --git a/engine/signalmgr/queries.sql b/engine/signalmgr/queries.sql new file mode 100644 index 0000000000..4837dea46c --- /dev/null +++ b/engine/signalmgr/queries.sql @@ -0,0 +1,30 @@ +-- name: SignalMgrGetPending :many +SELECT + id, + dest_id, + service_id +FROM + pending_signals +WHERE + message_id IS NULL +FOR UPDATE + SKIP LOCKED +LIMIT 100; + +-- name: SignalMgrInsertMessage :exec +INSERT INTO outgoing_messages(id, message_type, service_id, channel_id) + VALUES ($1, 'signal_message', $2, $3); + +-- name: SignalMgrUpdateSignal :exec +UPDATE + pending_signals +SET + message_id = $2 +WHERE + id = $1; + +-- name: SignalMgrDeleteStale :exec +DELETE FROM pending_signals +WHERE message_id IS NULL + AND created_at < NOW() - INTERVAL '1 hour'; + diff --git a/engine/signalmgr/update.go b/engine/signalmgr/update.go new file mode 100644 index 0000000000..0bee3e7787 --- /dev/null +++ b/engine/signalmgr/update.go @@ -0,0 +1,54 @@ +package signalmgr + +import ( + "context" + "database/sql" + "fmt" + + "github.com/google/uuid" + "github.com/target/goalert/gadb" + "github.com/target/goalert/permission" +) + +func (db *DB) UpdateAll(ctx context.Context) error { + err := permission.LimitCheckAny(ctx, permission.System) + if err != nil { + return err + } + + return db.lock.WithTx(ctx, func(ctx context.Context, tx *sql.Tx) error { + q := gadb.New(tx) + + err := q.SignalMgrDeleteStale(ctx) + if err != nil { + return fmt.Errorf("delete stale signals: %w", err) + } + + work, err := q.SignalMgrGetPending(ctx) + if err != nil { + return fmt.Errorf("get pending signals: %w", err) + } + + for _, w := range work { + id := uuid.New() + err = q.SignalMgrInsertMessage(ctx, gadb.SignalMgrInsertMessageParams{ + ID: id, + ServiceID: uuid.NullUUID{Valid: true, UUID: w.ServiceID}, + ChannelID: uuid.NullUUID{Valid: true, UUID: w.DestID}, + }) + if err != nil { + return fmt.Errorf("insert message: %w", err) + } + + err = q.SignalMgrUpdateSignal(ctx, gadb.SignalMgrUpdateSignalParams{ + ID: w.ID, + MessageID: uuid.NullUUID{Valid: true, UUID: id}, + }) + if err != nil { + return fmt.Errorf("update signal: %w", err) + } + } + + return nil + }) +} diff --git a/gadb/queries.sql.go b/gadb/queries.sql.go index 0c76c09a76..53c369e400 100644 --- a/gadb/queries.sql.go +++ b/gadb/queries.sql.go @@ -2607,6 +2607,95 @@ func (q *Queries) SetManyAlertFeedback(ctx context.Context, arg SetManyAlertFeed return items, nil } +const signalMgrDeleteStale = `-- name: SignalMgrDeleteStale :exec +DELETE FROM pending_signals +WHERE message_id IS NULL + AND created_at < NOW() - INTERVAL '1 hour' +` + +func (q *Queries) SignalMgrDeleteStale(ctx context.Context) error { + _, err := q.db.ExecContext(ctx, signalMgrDeleteStale) + return err +} + +const signalMgrGetPending = `-- name: SignalMgrGetPending :many +SELECT + id, + dest_id, + service_id +FROM + pending_signals +WHERE + message_id IS NULL +FOR UPDATE + SKIP LOCKED +LIMIT 100 +` + +type SignalMgrGetPendingRow struct { + ID int32 + DestID uuid.UUID + ServiceID uuid.UUID +} + +func (q *Queries) SignalMgrGetPending(ctx context.Context) ([]SignalMgrGetPendingRow, error) { + rows, err := q.db.QueryContext(ctx, signalMgrGetPending) + if err != nil { + return nil, err + } + defer rows.Close() + var items []SignalMgrGetPendingRow + for rows.Next() { + var i SignalMgrGetPendingRow + if err := rows.Scan(&i.ID, &i.DestID, &i.ServiceID); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + +const signalMgrInsertMessage = `-- name: SignalMgrInsertMessage :exec +INSERT INTO outgoing_messages(id, message_type, service_id, channel_id) + VALUES ($1, 'signal_message', $2, $3) +` + +type SignalMgrInsertMessageParams struct { + ID uuid.UUID + ServiceID uuid.NullUUID + ChannelID uuid.NullUUID +} + +func (q *Queries) SignalMgrInsertMessage(ctx context.Context, arg SignalMgrInsertMessageParams) error { + _, err := q.db.ExecContext(ctx, signalMgrInsertMessage, arg.ID, arg.ServiceID, arg.ChannelID) + return err +} + +const signalMgrUpdateSignal = `-- name: SignalMgrUpdateSignal :exec +UPDATE + pending_signals +SET + message_id = $2 +WHERE + id = $1 +` + +type SignalMgrUpdateSignalParams struct { + ID int32 + MessageID uuid.NullUUID +} + +func (q *Queries) SignalMgrUpdateSignal(ctx context.Context, arg SignalMgrUpdateSignalParams) error { + _, err := q.db.ExecContext(ctx, signalMgrUpdateSignal, arg.ID, arg.MessageID) + return err +} + const statusMgrCMInfo = `-- name: StatusMgrCMInfo :one SELECT user_id, diff --git a/migrate/migrations/20240709091828-signal-module.sql b/migrate/migrations/20240709091828-signal-module.sql new file mode 100644 index 0000000000..7b18da43be --- /dev/null +++ b/migrate/migrations/20240709091828-signal-module.sql @@ -0,0 +1,13 @@ +-- +migrate Up notransaction +ALTER TYPE engine_processing_type + ADD VALUE IF NOT EXISTS 'signals'; + +INSERT INTO engine_processing_versions(type_id, version) + VALUES ('signals', 1) +ON CONFLICT + DO NOTHING; + +-- +migrate Down +DELETE FROM engine_processing_versions +WHERE type_id = 'signals'; + diff --git a/migrate/schema.sql b/migrate/schema.sql index ab762951a5..0ef3829be1 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=511bce0e238c0b0c7e243a86dfae98228e1af8ea3139f4f53b32b39a148e1f94 - --- DISK=34744f4f69294b7adc245d61ead9871e8bfd1c469b7db407507fd641cc4e204f - --- PSQL=34744f4f69294b7adc245d61ead9871e8bfd1c469b7db407507fd641cc4e204f - +-- DATA=6f5f5fbf52efe1aa2427784129bf061e63e4e21f12f2d3a32db7db204c83c430 - +-- DISK=f010d74fe4a91163a422a5d37b15d6ba47164f67ae86a6eadab769913a7f6117 - +-- PSQL=f010d74fe4a91163a422a5d37b15d6ba47164f67ae86a6eadab769913a7f6117 - -- -- pgdump-lite database dump -- @@ -22,6 +22,7 @@ CREATE TYPE engine_processing_type AS ENUM ( 'np_cycle', 'rotation', 'schedule', + 'signals', 'status_update', 'verify' ); diff --git a/notification/messagetype.go b/notification/messagetype.go index 8d8031b456..ba4cf7c868 100644 --- a/notification/messagetype.go +++ b/notification/messagetype.go @@ -27,6 +27,8 @@ const ( // messages are now dropped. MessageTypeAlertStatusBundle MessageTypeScheduleOnCallUsers + + MessageTypeSignalMessage ) func (s MessageType) Value() (driver.Value, error) { @@ -45,6 +47,8 @@ func (s MessageType) Value() (driver.Value, error) { return "alert_status_update_bundle", nil case MessageTypeScheduleOnCallUsers: return "schedule_on_call_notification", nil + case MessageTypeSignalMessage: + return "signal_message", nil } return nil, fmt.Errorf("could not process unknown type for MessageType %s", s) } @@ -77,6 +81,8 @@ func (s *MessageType) Scan(value interface{}) error { *s = MessageTypeAlertStatusBundle case "schedule_on_call_notification": *s = MessageTypeScheduleOnCallUsers + case "signal_message": + *s = MessageTypeSignalMessage default: return fmt.Errorf("could not process unknown type for MessageType %str", str) } diff --git a/notification/messagetype_string.go b/notification/messagetype_string.go index b689af71e2..a930261fb1 100644 --- a/notification/messagetype_string.go +++ b/notification/messagetype_string.go @@ -16,11 +16,12 @@ func _() { _ = x[MessageTypeAlertBundle-5] _ = x[MessageTypeAlertStatusBundle-6] _ = x[MessageTypeScheduleOnCallUsers-7] + _ = x[MessageTypeSignalMessage-8] } -const _MessageType_name = "MessageTypeUnknownMessageTypeAlertMessageTypeAlertStatusMessageTypeTestMessageTypeVerificationMessageTypeAlertBundleMessageTypeAlertStatusBundleMessageTypeScheduleOnCallUsers" +const _MessageType_name = "MessageTypeUnknownMessageTypeAlertMessageTypeAlertStatusMessageTypeTestMessageTypeVerificationMessageTypeAlertBundleMessageTypeAlertStatusBundleMessageTypeScheduleOnCallUsersMessageTypeSignalMessage" -var _MessageType_index = [...]uint8{0, 18, 34, 56, 71, 94, 116, 144, 174} +var _MessageType_index = [...]uint8{0, 18, 34, 56, 71, 94, 116, 144, 174, 198} func (i MessageType) String() string { if i < 0 || i >= MessageType(len(_MessageType_index)-1) { diff --git a/notification/signal.go b/notification/signal.go new file mode 100644 index 0000000000..10814bfc19 --- /dev/null +++ b/notification/signal.go @@ -0,0 +1,18 @@ +package notification + +// Test represents outgoing test notification. +type SignalMessage struct { + Dest Dest + CallbackID string // CallbackID is the identifier used to communicate a response to the notification + + Params map[string]string +} + +var _ Message = &Test{} + +func (t SignalMessage) Type() MessageType { return MessageTypeSignalMessage } +func (t SignalMessage) ID() string { return t.CallbackID } +func (t SignalMessage) Destination() Dest { return t.Dest } +func (SignalMessage) Body() string { return "" } +func (SignalMessage) ExtendedBody() string { return "" } +func (SignalMessage) SubjectID() int { return -1 } diff --git a/sqlc.yaml b/sqlc.yaml index f2e92e4050..577b07c073 100644 --- a/sqlc.yaml +++ b/sqlc.yaml @@ -24,6 +24,7 @@ sql: - engine/statusmgr/queries.sql - engine/message/queries.sql - engine/schedulemanager/queries.sql + - engine/signalmgr/queries.sql - auth/authlink/queries.sql - alert/alertlog/queries.sql - user/favorite/queries.sql From f96df25106244ece3d2e7f270a6836ab706476df Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Tue, 9 Jul 2024 11:15:33 -0500 Subject: [PATCH 02/15] Standardize JSON key naming conventions - Updated `cm_type_val_to_dest` and `nc_type_val_to_dest` functions to use underscore naming for JSON keys. - Added migrations to update existing records to the new format. - Created triggers for backward compatibility during insert operations. --- .../20240709094902-underscore-fields-cm.sql | 130 ++++++++++++++++++ .../20240709094903-underscore-fields-nr.sql | 110 +++++++++++++++ 2 files changed, 240 insertions(+) create mode 100644 migrate/migrations/20240709094902-underscore-fields-cm.sql create mode 100644 migrate/migrations/20240709094903-underscore-fields-nr.sql diff --git a/migrate/migrations/20240709094902-underscore-fields-cm.sql b/migrate/migrations/20240709094902-underscore-fields-cm.sql new file mode 100644 index 0000000000..4cb13e7047 --- /dev/null +++ b/migrate/migrations/20240709094902-underscore-fields-cm.sql @@ -0,0 +1,130 @@ +-- +migrate Up +LOCK user_contact_methods; + +-- +migrate StatementBegin +CREATE OR REPLACE FUNCTION cm_type_val_to_dest(typeName enum_user_contact_method_type, value text) + RETURNS jsonb + AS $$ +BEGIN + IF typeName = 'EMAIL' THEN + RETURN jsonb_build_object('Type', 'builtin-smtp-email', 'Args', jsonb_build_object('email_address', value)); + ELSIF typeName = 'VOICE' THEN + RETURN jsonb_build_object('Type', 'builtin-twilio-voice', 'Args', jsonb_build_object('phone_number', value)); + ELSIF typeName = 'SMS' THEN + RETURN jsonb_build_object('Type', 'builtin-twilio-sms', 'Args', jsonb_build_object('phone_number', value)); + ELSIF typeName = 'WEBHOOK' THEN + RETURN jsonb_build_object('Type', 'builtin-webhook', 'Args', jsonb_build_object('webhook_url', value)); + ELSIF typeName = 'SLACK_DM' THEN + RETURN jsonb_build_object('Type', 'builtin-slack-dm', 'Args', jsonb_build_object('slack_user_id', value)); + ELSE + -- throw an error + RAISE EXCEPTION 'Unknown contact method type: %', typeName; + END IF; +END; +$$ +LANGUAGE plpgsql; + +-- +migrate StatementEnd +-- +migrate StatementBegin +CREATE OR REPLACE FUNCTION fn_cm_compat_set_type_val_on_insert() + RETURNS TRIGGER + AS $$ +BEGIN + IF NEW.dest ->> 'Type' = 'builtin-smtp-email' THEN + NEW.type = 'EMAIL'; + NEW.value = NEW.dest -> 'Args' ->> 'email_address'; + ELSIF NEW.dest ->> 'Type' = 'builtin-twilio-voice' THEN + NEW.type = 'VOICE'; + NEW.value = NEW.dest -> 'Args' ->> 'phone_number'; + ELSIF NEW.dest ->> 'Type' = 'builtin-twilio-sms' THEN + NEW.type = 'SMS'; + NEW.value = NEW.dest -> 'Args' ->> 'phone_number'; + ELSIF NEW.dest ->> 'Type' = 'builtin-webhook' THEN + NEW.type = 'WEBHOOK'; + NEW.value = NEW.dest -> 'Args' ->> 'webhook_url'; + ELSIF NEW.dest ->> 'Type' = 'builtin-slack-dm' THEN + NEW.type = 'SLACK_DM'; + NEW.value = NEW.dest -> 'Args' ->> 'slack_user_id'; + ELSE + NEW.type = 'DEST'; + NEW.value = gen_random_uuid()::text; + END IF; + RETURN new; +END; +$$ +LANGUAGE plpgsql; + +-- +migrate StatementEnd +UPDATE + user_contact_methods +SET + dest = jsonb_set(dest, '{Args}',( + SELECT + jsonb_object_agg(replace(key, '-', '_'), value) + FROM jsonb_each_text(dest -> 'Args'))); + +-- +migrate Down +LOCK user_contact_methods; + +-- +migrate StatementBegin +CREATE OR REPLACE FUNCTION cm_type_val_to_dest(typeName enum_user_contact_method_type, value text) + RETURNS jsonb + AS $$ +BEGIN + IF typeName = 'EMAIL' THEN + RETURN jsonb_build_object('Type', 'builtin-smtp-email', 'Args', jsonb_build_object('email-address', value)); + ELSIF typeName = 'VOICE' THEN + RETURN jsonb_build_object('Type', 'builtin-twilio-voice', 'Args', jsonb_build_object('phone-number', value)); + ELSIF typeName = 'SMS' THEN + RETURN jsonb_build_object('Type', 'builtin-twilio-sms', 'Args', jsonb_build_object('phone-number', value)); + ELSIF typeName = 'WEBHOOK' THEN + RETURN jsonb_build_object('Type', 'builtin-webhook', 'Args', jsonb_build_object('webhook-url', value)); + ELSIF typeName = 'SLACK_DM' THEN + RETURN jsonb_build_object('Type', 'builtin-slack-dm', 'Args', jsonb_build_object('slack-user-id', value)); + ELSE + -- throw an error + RAISE EXCEPTION 'Unknown contact method type: %', typeName; + END IF; +END; +$$ +LANGUAGE plpgsql; + +-- +migrate StatementEnd +-- +migrate StatementBegin +CREATE OR REPLACE FUNCTION fn_cm_compat_set_type_val_on_insert() + RETURNS TRIGGER + AS $$ +BEGIN + IF NEW.dest ->> 'Type' = 'builtin-smtp-email' THEN + NEW.type = 'EMAIL'; + NEW.value = NEW.dest -> 'Args' ->> 'email-address'; + ELSIF NEW.dest ->> 'Type' = 'builtin-twilio-voice' THEN + NEW.type = 'VOICE'; + NEW.value = NEW.dest -> 'Args' ->> 'phone-number'; + ELSIF NEW.dest ->> 'Type' = 'builtin-twilio-sms' THEN + NEW.type = 'SMS'; + NEW.value = NEW.dest -> 'Args' ->> 'phone-number'; + ELSIF NEW.dest ->> 'Type' = 'builtin-webhook' THEN + NEW.type = 'WEBHOOK'; + NEW.value = NEW.dest -> 'Args' ->> 'webhook-url'; + ELSIF NEW.dest ->> 'Type' = 'builtin-slack-dm' THEN + NEW.type = 'SLACK_DM'; + NEW.value = NEW.dest -> 'Args' ->> 'slack-user-id'; + ELSE + NEW.type = 'DEST'; + NEW.value = gen_random_uuid()::text; + END IF; + RETURN new; +END; +$$ +LANGUAGE plpgsql; + +-- +migrate StatementEnd +UPDATE + user_contact_methods +SET + dest = jsonb_set(dest, '{Args}',( + SELECT + jsonb_object_agg(replace(key, '_', '-'), value) + FROM jsonb_each_text(dest -> 'Args'))); + diff --git a/migrate/migrations/20240709094903-underscore-fields-nr.sql b/migrate/migrations/20240709094903-underscore-fields-nr.sql new file mode 100644 index 0000000000..9742b68e80 --- /dev/null +++ b/migrate/migrations/20240709094903-underscore-fields-nr.sql @@ -0,0 +1,110 @@ +-- +migrate Up +LOCK notification_channels; + +-- +migrate StatementBegin +CREATE OR REPLACE FUNCTION nc_type_val_to_dest(typeName enum_notif_channel_type, value text) + RETURNS jsonb + AS $$ +BEGIN + IF typeName = 'SLACK' THEN + RETURN jsonb_build_object('Type', 'builtin-slack-channel', 'Args', jsonb_build_object('slack_channel_id', value)); + ELSIF typeName = 'WEBHOOK' THEN + RETURN jsonb_build_object('Type', 'builtin-webhook', 'Args', jsonb_build_object('webhook_url', value)); + ELSIF typeName = 'SLACK_USER_GROUP' THEN + RETURN jsonb_build_object('Type', 'builtin-slack-usergroup', 'Args', jsonb_build_object('slack_usergroup_id', split_part(value, ':', 1), 'slack_channel_id', split_part(value, ':', 2))); + ELSE + -- throw an error + RAISE EXCEPTION 'Unknown notification channel type: %', typeName; + END IF; +END; +$$ +LANGUAGE plpgsql; + +-- +migrate StatementEnd +-- +migrate StatementBegin +CREATE OR REPLACE FUNCTION fn_nc_compat_set_type_val_on_insert() + RETURNS TRIGGER + AS $$ +BEGIN + IF NEW.dest ->> 'Type' = 'builtin-slack-channel' THEN + NEW.type = 'SLACK'; + NEW.value = NEW.dest -> 'Args' ->> 'slack_channel_id'; + ELSIF NEW.dest ->> 'Type' = 'builtin-slack-usergroup' THEN + NEW.type = 'SLACK_USER_GROUP'; + NEW.value = NEW.dest -> 'Args' ->> 'slack_usergroup_id' || ':' || NEW.dest -> 'Args' ->> 'slack_channel_id'; + ELSIF NEW.dest ->> 'Type' = 'builtin-webhook' THEN + NEW.type = 'WEBHOOK'; + NEW.value = NEW.dest -> 'Args' ->> 'webhook_url'; + ELSE + NEW.type = 'DEST'; + NEW.value = gen_random_uuid()::text; + END IF; + RETURN new; +END; +$$ +LANGUAGE plpgsql; + +-- +migrate StatementEnd +UPDATE + notification_channels +SET + dest = jsonb_set(dest, '{Args}',( + SELECT + jsonb_object_agg(replace(key, '-', '_'), value) + FROM jsonb_each_text(dest -> 'Args'))); + +-- +migrate Down +LOCK notification_channels; + +-- +migrate StatementBegin +CREATE OR REPLACE FUNCTION nc_type_val_to_dest(typeName enum_notif_channel_type, value text) + RETURNS jsonb + AS $$ +BEGIN + IF typeName = 'SLACK' THEN + RETURN jsonb_build_object('Type', 'builtin-slack-channel', 'Args', jsonb_build_object('slack-channel-id', value)); + ELSIF typeName = 'WEBHOOK' THEN + RETURN jsonb_build_object('Type', 'builtin-webhook', 'Args', jsonb_build_object('webhook-url', value)); + ELSIF typeName = 'SLACK_USER_GROUP' THEN + RETURN jsonb_build_object('Type', 'builtin-slack-usergroup', 'Args', jsonb_build_object('slack-usergroup-id', split_part(value, ':', 1), 'slack-channel-id', split_part(value, ':', 2))); + ELSE + -- throw an error + RAISE EXCEPTION 'Unknown notification channel type: %', typeName; + END IF; +END; +$$ +LANGUAGE plpgsql; + +-- +migrate StatementEnd +-- +migrate StatementBegin +CREATE OR REPLACE FUNCTION fn_nc_compat_set_type_val_on_insert() + RETURNS TRIGGER + AS $$ +BEGIN + IF NEW.dest ->> 'Type' = 'builtin-slack-channel' THEN + NEW.type = 'SLACK'; + NEW.value = NEW.dest -> 'Args' ->> 'slack-channel-id'; + ELSIF NEW.dest ->> 'Type' = 'builtin-slack-usergroup' THEN + NEW.type = 'SLACK_USER_GROUP'; + NEW.value = NEW.dest -> 'Args' ->> 'slack-usergroup-id' || ':' || NEW.dest -> 'Args' ->> 'slack-channel-id'; + ELSIF NEW.dest ->> 'Type' = 'builtin-webhook' THEN + NEW.type = 'WEBHOOK'; + NEW.value = NEW.dest -> 'Args' ->> 'webhook-url'; + ELSE + NEW.type = 'DEST'; + NEW.value = gen_random_uuid()::text; + END IF; + RETURN new; +END; +$$ +LANGUAGE plpgsql; + +-- +migrate StatementEnd +UPDATE + notification_channels +SET + dest = jsonb_set(dest, '{Args}',( + SELECT + jsonb_object_agg(replace(key, '_', '-'), value) + FROM jsonb_each_text(dest -> 'Args'))); + From f0f15174b4203a4a8c5d5506df5676a7c3c99e9d Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Tue, 9 Jul 2024 11:15:51 -0500 Subject: [PATCH 03/15] Add support for Signal messages - Implement fetching signal message parameters from the database - Update sendMessage to handle SignalMessage type with UUID parsing and JSON unmarshalling - Add migration to enum type for signal_message - Update field naming conventions to use underscores for better consistency - Create smoke tests for Signal message functionality --- engine/queries.sql | 9 ++ engine/sendmessage.go | 26 +++- gadb/models.go | 1 + gadb/queries.sql.go | 17 +++ graphql2/graphqlapp/destinationtypes.go | 19 +-- integrationkey/uik/handler.go | 16 ++- .../20240709110419-signal-message-type.sql | 5 + notification/slack/channel.go | 2 + sqlc.yaml | 1 + test/smoke/ncdedup_test.go | 2 + test/smoke/signal_test.go | 117 ++++++++++++++++++ 11 files changed, 200 insertions(+), 15 deletions(-) create mode 100644 engine/queries.sql create mode 100644 migrate/migrations/20240709110419-signal-message-type.sql create mode 100644 test/smoke/signal_test.go diff --git a/engine/queries.sql b/engine/queries.sql new file mode 100644 index 0000000000..17dcd8dfb8 --- /dev/null +++ b/engine/queries.sql @@ -0,0 +1,9 @@ +-- name: EngineGetSignalParams :one +-- Get a pending signal's rendered params. +SELECT + params +FROM + pending_signals +WHERE + message_id = $1; + diff --git a/engine/sendmessage.go b/engine/sendmessage.go index 008e732e1c..344748a4a3 100644 --- a/engine/sendmessage.go +++ b/engine/sendmessage.go @@ -3,11 +3,14 @@ package engine import ( "context" "database/sql" + "encoding/json" "fmt" + "github.com/google/uuid" "github.com/pkg/errors" "github.com/target/goalert/alert/alertlog" "github.com/target/goalert/engine/message" + "github.com/target/goalert/gadb" "github.com/target/goalert/notification" "github.com/target/goalert/permission" "github.com/target/goalert/util/log" @@ -168,8 +171,29 @@ func (p *Engine) sendMessage(ctx context.Context, msg *message.Message) (*notifi ScheduleID: msg.ScheduleID, Users: onCallUsers, } + case notification.MessageTypeSignalMessage: + id, err := uuid.Parse(msg.ID) + if err != nil { + return nil, errors.Wrap(err, "parse signal message id") + } + rawParams, err := gadb.New(p.b.db).EngineGetSignalParams(ctx, uuid.NullUUID{Valid: true, UUID: id}) + if err != nil { + return nil, errors.Wrap(err, "get signal message params") + } + + var params map[string]string + err = json.Unmarshal(rawParams, ¶ms) + if err != nil { + return nil, errors.Wrap(err, "parse signal message params") + } + + notifMsg = notification.SignalMessage{ + Dest: msg.Dest, + CallbackID: msg.ID, + Params: params, + } default: - log.Log(ctx, errors.New("SEND NOT IMPLEMENTED FOR MESSAGE TYPE")) + log.Log(ctx, errors.New("SEND NOT IMPLEMENTED FOR MESSAGE TYPE "+msg.Type.String())) return ¬ification.SendResult{ID: msg.ID, Status: notification.Status{State: notification.StateFailedPerm}}, nil } diff --git a/gadb/models.go b/gadb/models.go index 409b671c3d..2640443963 100644 --- a/gadb/models.go +++ b/gadb/models.go @@ -27,6 +27,7 @@ const ( EngineProcessingTypeNpCycle EngineProcessingType = "np_cycle" EngineProcessingTypeRotation EngineProcessingType = "rotation" EngineProcessingTypeSchedule EngineProcessingType = "schedule" + EngineProcessingTypeSignals EngineProcessingType = "signals" EngineProcessingTypeStatusUpdate EngineProcessingType = "status_update" EngineProcessingTypeVerify EngineProcessingType = "verify" ) diff --git a/gadb/queries.sql.go b/gadb/queries.sql.go index 53c369e400..5f0917d141 100644 --- a/gadb/queries.sql.go +++ b/gadb/queries.sql.go @@ -1259,6 +1259,23 @@ func (q *Queries) DeleteManyCalSub(ctx context.Context, arg DeleteManyCalSubPara return err } +const engineGetSignalParams = `-- name: EngineGetSignalParams :one +SELECT + params +FROM + pending_signals +WHERE + message_id = $1 +` + +// Get a pending signal's rendered params. +func (q *Queries) EngineGetSignalParams(ctx context.Context, messageID uuid.NullUUID) (json.RawMessage, error) { + row := q.db.QueryRowContext(ctx, engineGetSignalParams, messageID) + var params json.RawMessage + err := row.Scan(¶ms) + return params, err +} + const findManyCalSubByUser = `-- name: FindManyCalSubByUser :many SELECT id, diff --git a/graphql2/graphqlapp/destinationtypes.go b/graphql2/graphqlapp/destinationtypes.go index e295bd0123..106aa01335 100644 --- a/graphql2/graphqlapp/destinationtypes.go +++ b/graphql2/graphqlapp/destinationtypes.go @@ -25,15 +25,15 @@ const ( destSchedule = "builtin-schedule" destAlert = "builtin-alert" - fieldPhoneNumber = "phone-number" - fieldEmailAddress = "email-address" - fieldWebhookURL = "webhook-url" - fieldSlackUserID = "slack-user-id" - fieldSlackChanID = "slack-channel-id" - fieldSlackUGID = "slack-usergroup-id" - fieldUserID = "user-id" - fieldRotationID = "rotation-id" - fieldScheduleID = "schedule-id" + fieldPhoneNumber = "phone_number" + fieldEmailAddress = "email_address" + fieldWebhookURL = "webhook_url" + fieldSlackUserID = "slack_user_id" + fieldSlackChanID = "slack_channel_id" + fieldSlackUGID = "slack_usergroup_id" + fieldUserID = "user_id" + fieldRotationID = "rotation_id" + fieldScheduleID = "schedule_id" ) type ( @@ -384,6 +384,7 @@ func (q *Query) DestinationTypes(ctx context.Context, isDynamicAction *bool) ([] Enabled: cfg.Slack.Enable, IsEPTarget: true, IsSchedOnCallNotify: true, + IsDynamicAction: true, SupportsStatusUpdates: true, StatusUpdatesRequired: true, RequiredFields: []graphql2.DestinationFieldConfig{{ diff --git a/integrationkey/uik/handler.go b/integrationkey/uik/handler.go index ae808678c4..29a2e18b9b 100644 --- a/integrationkey/uik/handler.go +++ b/integrationkey/uik/handler.go @@ -37,14 +37,20 @@ func NewHandler(db TxAble, intStore *integrationkey.Store, aStore *alert.Store) func (h *Handler) handleAction(ctx context.Context, act integrationkey.Action, _params any) error { params := _params.(map[string]any) + param := func(name string) string { + if v, ok := params[name]; ok { + return v.(string) + } + return "" + } switch act.Type { case "builtin-webhook": - req, err := http.NewRequest("POST", act.StaticParams["webhook-url"], strings.NewReader(params["body"].(string))) + req, err := http.NewRequest("POST", act.StaticParams["webhook_url"], strings.NewReader(param("body"))) if err != nil { return err } - req.Header.Set("Content-Type", params["content-type"].(string)) + req.Header.Set("Content-Type", param("content-type")) _, err = http.DefaultClient.Do(req.WithContext(ctx)) if err != nil { @@ -53,14 +59,14 @@ func (h *Handler) handleAction(ctx context.Context, act integrationkey.Action, _ case "builtin-alert": status := alert.StatusTriggered - if params["close"] == "true" { + if param("close") == "true" { status = alert.StatusClosed } _, _, err := h.alertStore.CreateOrUpdate(ctx, &alert.Alert{ ServiceID: permission.ServiceID(ctx), - Summary: params["summary"].(string), - Details: params["details"].(string), + Summary: param("summary"), + Details: param("details"), Source: alert.SourceUniversal, Status: status, }) diff --git a/migrate/migrations/20240709110419-signal-message-type.sql b/migrate/migrations/20240709110419-signal-message-type.sql new file mode 100644 index 0000000000..9d5b7a85df --- /dev/null +++ b/migrate/migrations/20240709110419-signal-message-type.sql @@ -0,0 +1,5 @@ +-- +migrate Up notransaction +ALTER TYPE enum_outgoing_messages_type + ADD VALUE IF NOT EXISTS 'signal_message'; + +-- +migrate Down diff --git a/notification/slack/channel.go b/notification/slack/channel.go index b8c16a4a98..3be5edd3ff 100644 --- a/notification/slack/channel.go +++ b/notification/slack/channel.go @@ -474,6 +474,8 @@ func (s *ChannelSender) Send(ctx context.Context, msg notification.Message) (*no opts = append(opts, slack.MsgOptionText( fmt.Sprintf("Service '%s' has %d unacknowledged alerts.\n\n<%s>", slackutilsx.EscapeMessage(t.ServiceName), t.Count, cfg.CallbackURL("/services/"+t.ServiceID+"/alerts")), false)) + case notification.SignalMessage: + opts = append(opts, slack.MsgOptionText(t.Params["message"], false)) case notification.ScheduleOnCallUsers: opts = append(opts, slack.MsgOptionText(s.onCallNotificationText(ctx, t), false)) default: diff --git a/sqlc.yaml b/sqlc.yaml index 577b07c073..f38a691d5c 100644 --- a/sqlc.yaml +++ b/sqlc.yaml @@ -25,6 +25,7 @@ sql: - engine/message/queries.sql - engine/schedulemanager/queries.sql - engine/signalmgr/queries.sql + - engine/queries.sql - auth/authlink/queries.sql - alert/alertlog/queries.sql - user/favorite/queries.sql diff --git a/test/smoke/ncdedup_test.go b/test/smoke/ncdedup_test.go index 9060a8a1f8..d381f0325f 100644 --- a/test/smoke/ncdedup_test.go +++ b/test/smoke/ncdedup_test.go @@ -8,6 +8,8 @@ import ( // TestNCDedup tests that deduplicated notification_channels continue to function in schedule_data. func TestNCDedup(t *testing.T) { + t.Parallel() + const sql = ` insert into users (id, name, email) values ({{uuid "uid"}}, 'bob', ''); diff --git a/test/smoke/signal_test.go b/test/smoke/signal_test.go new file mode 100644 index 0000000000..616d9e2dec --- /dev/null +++ b/test/smoke/signal_test.go @@ -0,0 +1,117 @@ +package smoke + +import ( + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/target/goalert/expflag" + "github.com/target/goalert/gadb" + "github.com/target/goalert/test/smoke/harness" +) + +// TestSignal tests that signal messages are sent correctly. +func TestSignal(t *testing.T) { + t.Parallel() + + const sql = ` + insert into users (id, name, email) values + ({{uuid "user"}}, 'bob', 'joe'); + insert into user_contact_methods (id, user_id, name, type, value) values + ({{uuid "cm1"}}, {{uuid "user"}}, 'personal', 'SMS', {{phone "1"}}); + insert into user_notification_rules (user_id, contact_method_id, delay_minutes) values + ({{uuid "user"}}, {{uuid "cm1"}}, 0); + insert into escalation_policies (id, name) values + ({{uuid "ep"}}, 'esc policy'); + insert into escalation_policy_steps (id, escalation_policy_id, delay) values + ({{uuid "step"}}, {{uuid "ep"}}, 5); + insert into escalation_policy_actions (escalation_policy_step_id, user_id) values + ({{uuid "step"}}, {{uuid "user"}}); + insert into services (id, name, escalation_policy_id) values + ({{uuid "svc"}}, 'service', {{uuid "ep"}}); + ` + + h := harness.NewHarnessWithFlags(t, sql, "signal-module", expflag.FlagSet{expflag.UnivKeys}) + defer h.Close() + + var dest gadb.DestV1 + err := h.App().DB().QueryRowContext(context.Background(), `select dest from user_contact_methods where id = $1`, h.UUID("cm1")).Scan(&dest) + require.NoError(t, err) + + // validate fields + assert.Equal(t, "builtin-twilio-sms", dest.Type, "unexpected type") + assert.Equal(t, h.Phone("1"), dest.Args["phone_number"], "unexpected arg") + + // create key + resp := h.GraphQLQuery2(fmt.Sprintf(`mutation{ createIntegrationKey(input: {name: "key", type: universal, serviceID: "%s"}){ id, href } }`, h.UUID("svc"))) + require.Empty(t, resp.Errors) + + var respData struct { + CreateIntegrationKey struct { + ID uuid.UUID + Href string + } + } + err = json.Unmarshal(resp.Data, &respData) + require.NoError(t, err) + + var gotTestMessage bool + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "POST", r.Method, "unexpected method") + assert.Equal(t, "/test-path", r.URL.Path, "unexpected webhook path") + data, err := io.ReadAll(r.Body) + if !assert.NoError(t, err) { + return + } + + assert.Equal(t, "webhook-body-data", string(data), "unexpected webhook body") + gotTestMessage = true + })) + defer srv.Close() + + // configure key + resp = h.GraphQLQuery2(fmt.Sprintf(` + mutation{ + updateKeyConfig(input: { + keyID: "%s", + defaultActions: [ + {dest: {type: "builtin-alert"}, + params: {summary: "req.body['summary']"}}, + {dest: {type: "builtin-webhook", args: {webhook_url: "%s"}}, + params: {body: "req.body['webhook-body']"}}, + {dest: {type: "builtin-slack-channel", args: {slack_channel_id: "%s"}}, + params: {message: "req.body['slack-text']"}} + ] + }) + }`, respData.CreateIntegrationKey.ID, srv.URL+"/test-path", h.Slack().Channel("chan1").ID())) + require.Empty(t, resp.Errors) + + // generate token + resp = h.GraphQLQuery2(fmt.Sprintf(`mutation{ generateKeyToken(id: "%s")}`, respData.CreateIntegrationKey.ID)) + require.Empty(t, resp.Errors) + var gen struct { + GenerateKeyToken string + } + err = json.Unmarshal(resp.Data, &gen) + require.NoError(t, err) + + req, err := http.NewRequest("POST", respData.CreateIntegrationKey.Href, strings.NewReader(`{"summary": "test-summary", "webhook-body": "webhook-body-data", "slack-text": "slack-text-data"}`)) + require.NoError(t, err) + req.Header.Set("Authorization", "Bearer "+gen.GenerateKeyToken) + req.Header.Set("Content-Type", "application/json") + r, err := http.DefaultClient.Do(req) + require.NoError(t, err) + require.Equal(t, http.StatusNoContent, r.StatusCode) + + assert.True(t, gotTestMessage, "expected webhook test message") + h.Slack().Channel("chan1").ExpectMessage("slack-text-data") + h.Twilio(t).Device(h.Phone("1")).ExpectSMS("test-summary") +} From d7ca07811049a610f302fb01587f192d37fd9a81 Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Tue, 9 Jul 2024 11:21:59 -0500 Subject: [PATCH 04/15] Standardize field naming conventions in forms - Renamed `phone-number` to `phone_number` for consistency - Updated `webhook-url` to `webhook_url` --- .../PolicyStepCreateDialog.stories.tsx | 6 +++--- .../PolicyStepEditDialog.stories.tsx | 8 ++++---- .../app/escalation-policies/PolicyStepForm.stories.tsx | 10 +++++----- ...ScheduleOnCallNotificationsCreateDialog.stories.tsx | 4 ++-- .../ScheduleOnCallNotificationsEditDialog.stories.tsx | 6 +++--- .../ScheduleOnCallNotificationsForm.stories.tsx | 2 +- web/src/app/selection/DestinationField.stories.tsx | 2 +- .../app/selection/DestinationInputDirect.stories.tsx | 6 +++--- .../UniversalKey/UniversalKeysActionsForm.stories.tsx | 2 +- web/src/app/storybook/defaultDestTypes.ts | 6 +++--- .../users/UserContactMethodCreateDialog.stories.tsx | 2 +- .../app/users/UserContactMethodEditDialog.stories.tsx | 6 +++--- web/src/app/users/UserContactMethodForm.stories.tsx | 8 ++++---- web/src/app/users/UserContactMethodList.stories.tsx | 8 ++++---- web/src/cypress/e2e/escalationPolicySteps.cy.ts | 4 ++-- web/src/cypress/e2e/schedules.cy.ts | 4 ++-- 16 files changed, 42 insertions(+), 42 deletions(-) diff --git a/web/src/app/escalation-policies/PolicyStepCreateDialog.stories.tsx b/web/src/app/escalation-policies/PolicyStepCreateDialog.stories.tsx index 8421b3be05..a766a46d31 100644 --- a/web/src/app/escalation-policies/PolicyStepCreateDialog.stories.tsx +++ b/web/src/app/escalation-policies/PolicyStepCreateDialog.stories.tsx @@ -34,7 +34,7 @@ const meta = { }) }), graphql.query('DestDisplayInfo', ({ variables: vars }) => { - if (vars.input.args['phone-number'].length !== 12) { + if (vars.input.args['phone_number'].length !== 12) { return HttpResponse.json({ errors: [ { message: 'generic error' }, @@ -43,7 +43,7 @@ const meta = { path: ['destinationDisplayInfo', 'input'], extensions: { code: 'INVALID_DEST_FIELD_VALUE', - fieldID: 'phone-number', + fieldID: 'phone_number', }, } satisfies DestFieldValueError, ], @@ -53,7 +53,7 @@ const meta = { return HttpResponse.json({ data: { destinationDisplayInfo: { - text: vars.input.args['phone-number'], + text: vars.input.args['phone_number'], iconURL: 'builtin://phone-voice', iconAltText: 'Voice Call', }, diff --git a/web/src/app/escalation-policies/PolicyStepEditDialog.stories.tsx b/web/src/app/escalation-policies/PolicyStepEditDialog.stories.tsx index e60fa47ad3..cc89722b14 100644 --- a/web/src/app/escalation-policies/PolicyStepEditDialog.stories.tsx +++ b/web/src/app/escalation-policies/PolicyStepEditDialog.stories.tsx @@ -37,7 +37,7 @@ const meta = { }) }), graphql.query('DestDisplayInfo', ({ variables: vars }) => { - if (vars.input.args['phone-number'].length !== 12) { + if (vars.input.args['phone_number'].length !== 12) { return HttpResponse.json({ errors: [ { message: 'generic error' }, @@ -46,7 +46,7 @@ const meta = { path: ['destinationDisplayInfo', 'input'], extensions: { code: 'INVALID_DEST_FIELD_VALUE', - fieldID: 'phone-number', + fieldID: 'phone_number', }, } satisfies DestFieldValueError, ], @@ -56,7 +56,7 @@ const meta = { return HttpResponse.json({ data: { destinationDisplayInfo: { - text: vars.input.args['phone-number'], + text: vars.input.args['phone_number'], iconURL: 'builtin://phone-voice', iconAltText: 'Voice Call', }, @@ -76,7 +76,7 @@ const meta = { actions: [ { type: 'single-field', - args: { 'phone-number': '+19995550123' }, + args: { phone_number: '+19995550123' }, } as Partial as Destination, ], } as EscalationPolicyStep, diff --git a/web/src/app/escalation-policies/PolicyStepForm.stories.tsx b/web/src/app/escalation-policies/PolicyStepForm.stories.tsx index 96e5cdb5e8..b6087f4ee2 100644 --- a/web/src/app/escalation-policies/PolicyStepForm.stories.tsx +++ b/web/src/app/escalation-policies/PolicyStepForm.stories.tsx @@ -38,14 +38,14 @@ const meta = { }) }), graphql.query('DestDisplayInfo', ({ variables: vars }) => { - switch (vars.input.args['phone-number']) { + switch (vars.input.args['phone_number']) { case VALID_PHONE: case VALID_PHONE2: return HttpResponse.json({ data: { destinationDisplayInfo: { text: - vars.input.args['phone-number'] === VALID_PHONE + vars.input.args['phone_number'] === VALID_PHONE ? 'VALID_CHIP_1' : 'VALID_CHIP_2', iconURL: 'builtin://phone-voice', @@ -64,7 +64,7 @@ const meta = { message: 'invalid phone number', extensions: { code: 'INVALID_DEST_FIELD_VALUE', - fieldID: 'phone-number', + fieldID: 'phone_number', }, } satisfies DestFieldValueError, ], @@ -94,11 +94,11 @@ export const WithExistingActions: Story = { actions: [ { type: 'single-field', - args: { 'phone-number': VALID_PHONE }, + args: { phone_number: VALID_PHONE }, }, { type: 'single-field', - args: { 'phone-number': VALID_PHONE2 }, + args: { phone_number: VALID_PHONE2 }, }, ], }, diff --git a/web/src/app/schedules/on-call-notifications/ScheduleOnCallNotificationsCreateDialog.stories.tsx b/web/src/app/schedules/on-call-notifications/ScheduleOnCallNotificationsCreateDialog.stories.tsx index 12d3b4c057..759de85729 100644 --- a/web/src/app/schedules/on-call-notifications/ScheduleOnCallNotificationsCreateDialog.stories.tsx +++ b/web/src/app/schedules/on-call-notifications/ScheduleOnCallNotificationsCreateDialog.stories.tsx @@ -38,7 +38,7 @@ const meta = { HttpResponse.json({ data: { destinationFieldValidate: true } }), ), graphql.mutation('SetRules', ({ variables }) => { - switch (variables.input.rules[0].dest.args['phone-number']) { + switch (variables.input.rules[0].dest.args['phone_number']) { case '+123': return HttpResponse.json({ errors: [ @@ -56,7 +56,7 @@ const meta = { ], extensions: { code: 'INVALID_DEST_FIELD_VALUE', - fieldID: 'phone-number', + fieldID: 'phone_number', }, } satisfies DestFieldValueError, ], diff --git a/web/src/app/schedules/on-call-notifications/ScheduleOnCallNotificationsEditDialog.stories.tsx b/web/src/app/schedules/on-call-notifications/ScheduleOnCallNotificationsEditDialog.stories.tsx index c110931b66..1afb8e4a31 100644 --- a/web/src/app/schedules/on-call-notifications/ScheduleOnCallNotificationsEditDialog.stories.tsx +++ b/web/src/app/schedules/on-call-notifications/ScheduleOnCallNotificationsEditDialog.stories.tsx @@ -39,7 +39,7 @@ const meta = { HttpResponse.json({ data: { destinationFieldValidate: true } }), ), graphql.mutation('SetRules', ({ variables }) => { - switch (variables.input.rules[0].dest.args['phone-number']) { + switch (variables.input.rules[0].dest.args['phone_number']) { case '+123': return HttpResponse.json({ errors: [ @@ -57,7 +57,7 @@ const meta = { ], extensions: { code: 'INVALID_DEST_FIELD_VALUE', - fieldID: 'phone-number', + fieldID: 'phone_number', }, } satisfies DestFieldValueError, ], @@ -82,7 +82,7 @@ const meta = { id: 'existing-id', dest: { type: 'single-field', - args: { 'phone-number': '+1234567890' }, + args: { phone_number: '+1234567890' }, }, }, ], diff --git a/web/src/app/schedules/on-call-notifications/ScheduleOnCallNotificationsForm.stories.tsx b/web/src/app/schedules/on-call-notifications/ScheduleOnCallNotificationsForm.stories.tsx index de1f280587..39eeabf802 100644 --- a/web/src/app/schedules/on-call-notifications/ScheduleOnCallNotificationsForm.stories.tsx +++ b/web/src/app/schedules/on-call-notifications/ScheduleOnCallNotificationsForm.stories.tsx @@ -44,7 +44,7 @@ export const ValidationErrors: Story = { args: { destTypeError: 'error with dest type', destFieldErrors: { - 'phone-number': 'error with dest field', + phone_number: 'error with dest field', }, }, play: async ({ canvasElement }) => { diff --git a/web/src/app/selection/DestinationField.stories.tsx b/web/src/app/selection/DestinationField.stories.tsx index 7afec821e8..c5298a1152 100644 --- a/web/src/app/selection/DestinationField.stories.tsx +++ b/web/src/app/selection/DestinationField.stories.tsx @@ -32,7 +32,7 @@ type Story = StoryObj export const SingleField: Story = { args: { destType: 'single-field', - value: { 'phone-number': '' }, + value: { phone_number: '' }, disabled: false, }, play: async ({ canvasElement }) => { diff --git a/web/src/app/selection/DestinationInputDirect.stories.tsx b/web/src/app/selection/DestinationInputDirect.stories.tsx index 8e4be6eef0..ac33133f10 100644 --- a/web/src/app/selection/DestinationInputDirect.stories.tsx +++ b/web/src/app/selection/DestinationInputDirect.stories.tsx @@ -51,7 +51,7 @@ export const WebookWithDocLink: Story = { args: { value: '', - fieldID: 'webhook-url', + fieldID: 'webhook_url', hint: 'Webhook Documentation', hintURL: '/docs#webhooks', inputType: 'url', @@ -98,7 +98,7 @@ export const PhoneNumbers: Story = { args: { value: '', - fieldID: 'phone-number', + fieldID: 'phone_number', hint: 'Include country code e.g. +1 (USA), +91 (India), +44 (UK)', hintURL: '', inputType: 'tel', @@ -146,7 +146,7 @@ export const Email: Story = { args: { value: '', - fieldID: 'email-address', + fieldID: 'email_address', hint: '', hintURL: '', inputType: 'email', diff --git a/web/src/app/services/UniversalKey/UniversalKeysActionsForm.stories.tsx b/web/src/app/services/UniversalKey/UniversalKeysActionsForm.stories.tsx index 1219b9eb9c..8b2ae91ef4 100644 --- a/web/src/app/services/UniversalKey/UniversalKeysActionsForm.stories.tsx +++ b/web/src/app/services/UniversalKey/UniversalKeysActionsForm.stories.tsx @@ -70,7 +70,7 @@ export const ValidationError: Story = { { path: ['actionInputValidate', 'input', 'dest', 'args'], extensions: { - key: 'phone-number', + key: 'phone_number', }, message: 'invalid number', }, diff --git a/web/src/app/storybook/defaultDestTypes.ts b/web/src/app/storybook/defaultDestTypes.ts index bebdcd3aea..52c5b744cc 100644 --- a/web/src/app/storybook/defaultDestTypes.ts +++ b/web/src/app/storybook/defaultDestTypes.ts @@ -24,7 +24,7 @@ export const destTypes: DestinationTypeInfo[] = [ ], requiredFields: [ { - fieldID: 'phone-number', + fieldID: 'phone_number', label: 'Phone Number', hint: 'Include country code e.g. +1 (USA), +91 (India), +44 (UK)', hintURL: '', @@ -102,7 +102,7 @@ export const destTypes: DestinationTypeInfo[] = [ dynamicParams: [], requiredFields: [ { - fieldID: 'phone-number', + fieldID: 'phone_number', label: 'Phone Number', hint: 'Include country code e.g. +1 (USA), +91 (India), +44 (UK)', hintURL: '', @@ -130,7 +130,7 @@ export const destTypes: DestinationTypeInfo[] = [ dynamicParams: [], requiredFields: [ { - fieldID: 'phone-number', + fieldID: 'phone_number', label: 'Phone Number', hint: 'Include country code e.g. +1 (USA), +91 (India), +44 (UK)', hintURL: '', diff --git a/web/src/app/users/UserContactMethodCreateDialog.stories.tsx b/web/src/app/users/UserContactMethodCreateDialog.stories.tsx index 137da422e6..9d755d4bb4 100644 --- a/web/src/app/users/UserContactMethodCreateDialog.stories.tsx +++ b/web/src/app/users/UserContactMethodCreateDialog.stories.tsx @@ -36,7 +36,7 @@ const meta = { path: ['createUserContactMethod', 'input', 'dest'], extensions: { code: 'INVALID_DEST_FIELD_VALUE', - fieldID: 'phone-number', + fieldID: 'phone_number', }, } satisfies DestFieldValueError, { diff --git a/web/src/app/users/UserContactMethodEditDialog.stories.tsx b/web/src/app/users/UserContactMethodEditDialog.stories.tsx index 3fc17919ea..2f6d18cba3 100644 --- a/web/src/app/users/UserContactMethodEditDialog.stories.tsx +++ b/web/src/app/users/UserContactMethodEditDialog.stories.tsx @@ -37,11 +37,11 @@ const meta = { type: 'supports-status', values: [ { - fieldID: 'phone-number', + fieldID: 'phone_number', value: '+15555555555', }, ], - args: { 'phone-number': '+15555555555' }, + args: { phone_number: '+15555555555' }, displayInfo: { text: '+1 555-555-5555', iconAltText: 'Voice Call', @@ -102,7 +102,7 @@ const meta = { path: ['updateUserContactMethod', 'input', 'dest'], extensions: { code: 'INVALID_DEST_FIELD_VALUE', - fieldID: 'phone-number', + fieldID: 'phone_number', }, } satisfies DestFieldValueError, { diff --git a/web/src/app/users/UserContactMethodForm.stories.tsx b/web/src/app/users/UserContactMethodForm.stories.tsx index 685866a937..0dfad216e1 100644 --- a/web/src/app/users/UserContactMethodForm.stories.tsx +++ b/web/src/app/users/UserContactMethodForm.stories.tsx @@ -43,7 +43,7 @@ export const SupportStatusUpdates: Story = { name: 'supports status', dest: { type: 'supports-status', - args: { 'phone-number': '+15555555555' }, + args: { phone_number: '+15555555555' }, }, statusUpdates: false, }, @@ -65,7 +65,7 @@ export const RequiredStatusUpdates: Story = { name: 'required status', dest: { type: 'required-status', - args: { 'phone-number': '+15555555555' }, + args: { phone_number: '+15555555555' }, }, statusUpdates: false, }, @@ -89,12 +89,12 @@ export const ErrorSingleField: Story = { name: '-notvalid', dest: { type: 'single-field', - args: { 'phone-number': '+15555555555' }, + args: { phone_number: '+15555555555' }, }, statusUpdates: false, }, disabled: false, - destFieldErrors: { 'phone-number': 'number is too short' }, + destFieldErrors: { phone_number: 'number is too short' }, }, play: async ({ canvasElement }) => { const canvas = within(canvasElement) diff --git a/web/src/app/users/UserContactMethodList.stories.tsx b/web/src/app/users/UserContactMethodList.stories.tsx index c1417d91e4..200d8e8083 100644 --- a/web/src/app/users/UserContactMethodList.stories.tsx +++ b/web/src/app/users/UserContactMethodList.stories.tsx @@ -29,11 +29,11 @@ const meta = { type: 'single-field', values: [ { - fieldID: 'phone-number', + fieldID: 'phone_number', value: '+15555555555', }, ], - args: { 'phone-number': '+15555555555' }, + args: { phone_number: '+15555555555' }, displayInfo: { text: '+1 555-555-5555', iconAltText: 'Voice Call', @@ -92,11 +92,11 @@ const meta = { type: 'single-field', values: [ { - fieldID: 'phone-number', + fieldID: 'phone_number', value: '+15555555556', }, ], - args: { 'phone-number': '+15555555556' }, + args: { phone_number: '+15555555556' }, displayInfo: { text: '+1 555-555-5556', iconAltText: 'Voice Call', diff --git a/web/src/cypress/e2e/escalationPolicySteps.cy.ts b/web/src/cypress/e2e/escalationPolicySteps.cy.ts index 47c20f748c..a087a05736 100644 --- a/web/src/cypress/e2e/escalationPolicySteps.cy.ts +++ b/web/src/cypress/e2e/escalationPolicySteps.cy.ts @@ -330,7 +330,7 @@ testScreen('Webhook Support', (screen: ScreenFormat) => { // add webhooks cy.dialogForm({ 'dest.type': 'Webhook', - 'webhook-url': 'https://webhook.site', + webhook_url: 'https://webhook.site', }) cy.dialogClick('Add Destination') cy.get('div[role="dialog"] [data-testid=destination-chip]').should( @@ -339,7 +339,7 @@ testScreen('Webhook Support', (screen: ScreenFormat) => { ) cy.dialogForm({ - 'webhook-url': 'https://example.com', + webhook_url: 'https://example.com', }) cy.dialogClick('Add Destination') cy.get('div[role="dialog"] [data-testid=destination-chip]').should( diff --git a/web/src/cypress/e2e/schedules.cy.ts b/web/src/cypress/e2e/schedules.cy.ts index 407e1ff816..97e70f2cfc 100644 --- a/web/src/cypress/e2e/schedules.cy.ts +++ b/web/src/cypress/e2e/schedules.cy.ts @@ -688,7 +688,7 @@ testScreen('Slack User Group Support', (screen: ScreenFormat) => { cy.dialogForm({ ruleType: 'on-change', 'dest.type': 'Webhook', - 'webhook-url': 'http://www.example.com', + webhook_url: 'http://www.example.com', }) cy.dialogFinish('Submit') @@ -705,7 +705,7 @@ testScreen('Slack User Group Support', (screen: ScreenFormat) => { cy.dialogForm({ ruleType: 'time-of-day', 'dest.type': 'Webhook', - 'webhook-url': 'http://www.example.com', + webhook_url: 'http://www.example.com', time: '00:00', 'weekdayFilter[0]': false, 'weekdayFilter[1]': true, From 18f8cf3535eb2f09505f23a3c20684da13630161 Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Tue, 9 Jul 2024 13:44:02 -0500 Subject: [PATCH 05/15] Update message DB version to 10 - Incremented message processing lock version to 10 - Added migration to update processing version in DB - Renamed existing migrations for clarity --- engine/message/db.go | 2 +- .../20240709134232-incr-message-vers.sql | 16 ++++++++++++++++ ...dule.sql => 20240709134305-signal-module.sql} | 0 ...ql => 20240709134405-signal-message-type.sql} | 0 migrate/schema.sql | 6 +++--- 5 files changed, 20 insertions(+), 4 deletions(-) create mode 100644 migrate/migrations/20240709134232-incr-message-vers.sql rename migrate/migrations/{20240709091828-signal-module.sql => 20240709134305-signal-module.sql} (100%) rename migrate/migrations/{20240709110419-signal-message-type.sql => 20240709134405-signal-message-type.sql} (100%) diff --git a/engine/message/db.go b/engine/message/db.go index e68fb74995..58434bfd49 100644 --- a/engine/message/db.go +++ b/engine/message/db.go @@ -70,7 +70,7 @@ type DB struct { func NewDB(ctx context.Context, db *sql.DB, a *alertlog.Store, pausable lifecycle.Pausable) (*DB, error) { lock, err := processinglock.NewLock(ctx, db, processinglock.Config{ Type: processinglock.TypeMessage, - Version: 9, + Version: 10, }) if err != nil { return nil, err diff --git a/migrate/migrations/20240709134232-incr-message-vers.sql b/migrate/migrations/20240709134232-incr-message-vers.sql new file mode 100644 index 0000000000..870a2f346e --- /dev/null +++ b/migrate/migrations/20240709134232-incr-message-vers.sql @@ -0,0 +1,16 @@ +-- +migrate Up +UPDATE + engine_processing_versions +SET + version = 10 +WHERE + type_id = 'message'; + +-- +migrate Down +UPDATE + engine_processing_versions +SET + version = 9 +WHERE + type_id = 'message'; + diff --git a/migrate/migrations/20240709091828-signal-module.sql b/migrate/migrations/20240709134305-signal-module.sql similarity index 100% rename from migrate/migrations/20240709091828-signal-module.sql rename to migrate/migrations/20240709134305-signal-module.sql diff --git a/migrate/migrations/20240709110419-signal-message-type.sql b/migrate/migrations/20240709134405-signal-message-type.sql similarity index 100% rename from migrate/migrations/20240709110419-signal-message-type.sql rename to migrate/migrations/20240709134405-signal-message-type.sql diff --git a/migrate/schema.sql b/migrate/schema.sql index a43937a7e7..fe410580da 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=fa218447de763811bd8369aaa3956f3b9b2fcd27c601b7d6e4224daced56e985 - --- DISK=cae3359d2e6c758e50843bb37f002cd3637f27be012ba6fd314e194808037c30 - --- PSQL=cae3359d2e6c758e50843bb37f002cd3637f27be012ba6fd314e194808037c30 - +-- DATA=cb593143774e3a1ccbe0b3cce558621ba93273c2331a604468908c5651d0f526 - +-- DISK=a741eb2fce8f461e15999d68af6bbd8625ef37f5aa668b2e258ebe53bdcd3ad2 - +-- PSQL=a741eb2fce8f461e15999d68af6bbd8625ef37f5aa668b2e258ebe53bdcd3ad2 - -- -- pgdump-lite database dump -- From 1286eaecd6439125207852b6821899ffb21bf40f Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Tue, 9 Jul 2024 13:47:55 -0500 Subject: [PATCH 06/15] Refactor comment for better clarity - Updated comment to better describe the purpose of SignalMessage struct. --- notification/signal.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/notification/signal.go b/notification/signal.go index 10814bfc19..4cc5dc4969 100644 --- a/notification/signal.go +++ b/notification/signal.go @@ -1,6 +1,6 @@ package notification -// Test represents outgoing test notification. +// SignalMessage is a dynamic message that is sent to a notification destination. type SignalMessage struct { Dest Dest CallbackID string // CallbackID is the identifier used to communicate a response to the notification From 6dfdc37cfc74a0cd77978c8f9bb38514aa0db1ee Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Tue, 9 Jul 2024 13:48:31 -0500 Subject: [PATCH 07/15] Add lowest priority signal messages to queue - Added `MessageTypeSignalMessage` with the lowest priority (99) in the message priority map. --- engine/message/queue.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/engine/message/queue.go b/engine/message/queue.go index f9a018082d..3e8aea94d7 100644 --- a/engine/message/queue.go +++ b/engine/message/queue.go @@ -21,6 +21,8 @@ var typePriority = map[notification.MessageType]int{ notification.MessageTypeAlertBundle: 4, notification.MessageTypeAlertStatus: 5, + + notification.MessageTypeSignalMessage: 99, // lowest priority } type queue struct { @@ -69,6 +71,7 @@ func newQueue(msgs []Message, now time.Time) *queue { return q } + func (q *queue) addSent(m Message) { if m.SentAt.IsZero() { m.SentAt = q.now From 7d3301871b8a5124e27a3a7b1be58c1941c5e2ac Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Tue, 9 Jul 2024 13:49:45 -0500 Subject: [PATCH 08/15] Update comment to clarify DB's function Renamed to better reflect the role of scheduling outgoing signals. --- engine/signalmgr/db.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/signalmgr/db.go b/engine/signalmgr/db.go index d02ad2225a..c01f26fa11 100644 --- a/engine/signalmgr/db.go +++ b/engine/signalmgr/db.go @@ -7,7 +7,7 @@ import ( "github.com/target/goalert/engine/processinglock" ) -// DB manages outgoing status updates. +// DB schedules outgoing signal messages. type DB struct { lock *processinglock.Lock } From e4d848fcb55ea5b77c3776fa7d99969248f11945 Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Tue, 9 Jul 2024 13:52:09 -0500 Subject: [PATCH 09/15] Add comments for better query documentation - Added descriptions to SQL queries to enhance readability - Introduced 'signal_message' type to message enums - Provided documentation for query-related Go functions --- engine/signalmgr/queries.sql | 4 ++++ gadb/models.go | 1 + gadb/queries.sql.go | 4 ++++ 3 files changed, 9 insertions(+) diff --git a/engine/signalmgr/queries.sql b/engine/signalmgr/queries.sql index 4837dea46c..8228d502ad 100644 --- a/engine/signalmgr/queries.sql +++ b/engine/signalmgr/queries.sql @@ -1,4 +1,5 @@ -- name: SignalMgrGetPending :many +-- Get a batch of pending signals to process. SELECT id, dest_id, @@ -12,10 +13,12 @@ FOR UPDATE LIMIT 100; -- name: SignalMgrInsertMessage :exec +-- Insert a new message into the outgoing_messages table. INSERT INTO outgoing_messages(id, message_type, service_id, channel_id) VALUES ($1, 'signal_message', $2, $3); -- name: SignalMgrUpdateSignal :exec +-- Update a pending signal with the message_id. UPDATE pending_signals SET @@ -24,6 +27,7 @@ WHERE id = $1; -- name: SignalMgrDeleteStale :exec +-- Delete stale pending signals. DELETE FROM pending_signals WHERE message_id IS NULL AND created_at < NOW() - INTERVAL '1 hour'; diff --git a/gadb/models.go b/gadb/models.go index 2640443963..cf2b3645cc 100644 --- a/gadb/models.go +++ b/gadb/models.go @@ -496,6 +496,7 @@ const ( EnumOutgoingMessagesTypeAlertStatusUpdate EnumOutgoingMessagesType = "alert_status_update" EnumOutgoingMessagesTypeAlertStatusUpdateBundle EnumOutgoingMessagesType = "alert_status_update_bundle" EnumOutgoingMessagesTypeScheduleOnCallNotification EnumOutgoingMessagesType = "schedule_on_call_notification" + EnumOutgoingMessagesTypeSignalMessage EnumOutgoingMessagesType = "signal_message" EnumOutgoingMessagesTypeTestNotification EnumOutgoingMessagesType = "test_notification" EnumOutgoingMessagesTypeVerificationMessage EnumOutgoingMessagesType = "verification_message" ) diff --git a/gadb/queries.sql.go b/gadb/queries.sql.go index 5f0917d141..c6d69cc50b 100644 --- a/gadb/queries.sql.go +++ b/gadb/queries.sql.go @@ -2630,6 +2630,7 @@ WHERE message_id IS NULL AND created_at < NOW() - INTERVAL '1 hour' ` +// Delete stale pending signals. func (q *Queries) SignalMgrDeleteStale(ctx context.Context) error { _, err := q.db.ExecContext(ctx, signalMgrDeleteStale) return err @@ -2655,6 +2656,7 @@ type SignalMgrGetPendingRow struct { ServiceID uuid.UUID } +// Get a batch of pending signals to process. func (q *Queries) SignalMgrGetPending(ctx context.Context) ([]SignalMgrGetPendingRow, error) { rows, err := q.db.QueryContext(ctx, signalMgrGetPending) if err != nil { @@ -2689,6 +2691,7 @@ type SignalMgrInsertMessageParams struct { ChannelID uuid.NullUUID } +// Insert a new message into the outgoing_messages table. func (q *Queries) SignalMgrInsertMessage(ctx context.Context, arg SignalMgrInsertMessageParams) error { _, err := q.db.ExecContext(ctx, signalMgrInsertMessage, arg.ID, arg.ServiceID, arg.ChannelID) return err @@ -2708,6 +2711,7 @@ type SignalMgrUpdateSignalParams struct { MessageID uuid.NullUUID } +// Update a pending signal with the message_id. func (q *Queries) SignalMgrUpdateSignal(ctx context.Context, arg SignalMgrUpdateSignalParams) error { _, err := q.db.ExecContext(ctx, signalMgrUpdateSignal, arg.ID, arg.MessageID) return err From ccc75f15e3ca8ae24c6ca9d4096793246748a843 Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Tue, 9 Jul 2024 14:35:50 -0500 Subject: [PATCH 10/15] add dynamic param for slack --- graphql2/graphqlapp/destinationtypes.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/graphql2/graphqlapp/destinationtypes.go b/graphql2/graphqlapp/destinationtypes.go index 106aa01335..2a78885207 100644 --- a/graphql2/graphqlapp/destinationtypes.go +++ b/graphql2/graphqlapp/destinationtypes.go @@ -393,6 +393,11 @@ func (q *Query) DestinationTypes(ctx context.Context, isDynamicAction *bool) ([] InputType: "text", SupportsSearch: true, }}, + DynamicParams: []graphql2.DynamicParamConfig{{ + ParamID: "message", + Label: "Message", + Hint: "The text of the message to send.", + }}, }, { Type: destSlackUG, From ebfb3d40e61c3bae5a23d983c5548074328dd16a Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Tue, 9 Jul 2024 14:42:40 -0500 Subject: [PATCH 11/15] disable email dynamic action (requires more work) --- graphql2/graphqlapp/destinationtypes.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/graphql2/graphqlapp/destinationtypes.go b/graphql2/graphqlapp/destinationtypes.go index 2a78885207..63971a4860 100644 --- a/graphql2/graphqlapp/destinationtypes.go +++ b/graphql2/graphqlapp/destinationtypes.go @@ -312,7 +312,7 @@ func (q *Query) DestinationTypes(ctx context.Context, isDynamicAction *bool) ([] Enabled: cfg.SMTP.Enable, IsContactMethod: true, SupportsStatusUpdates: true, - IsDynamicAction: true, + IsDynamicAction: false, RequiredFields: []graphql2.DestinationFieldConfig{{ FieldID: fieldEmailAddress, Label: "Email Address", From f9a376fb7c0d577b5728cb9b8bf5bdfa36710145 Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Tue, 9 Jul 2024 15:02:52 -0500 Subject: [PATCH 12/15] map continue field --- graphql2/graphqlapp/integrationkey.go | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/graphql2/graphqlapp/integrationkey.go b/graphql2/graphqlapp/integrationkey.go index f09d51ea88..ab4fa1e874 100644 --- a/graphql2/graphqlapp/integrationkey.go +++ b/graphql2/graphqlapp/integrationkey.go @@ -136,22 +136,24 @@ func (m *Mutation) UpdateKeyConfig(ctx context.Context, input graphql2.UpdateKey if input.SetRule.ID == nil { // Since we don't have a rule ID, we're need to create a new rule. cfg.Rules = append(cfg.Rules, integrationkey.Rule{ - ID: uuid.New(), - Name: input.SetRule.Name, - Description: input.SetRule.Description, - ConditionExpr: input.SetRule.ConditionExpr, - Actions: actionsGQLToGo(input.SetRule.Actions), + ID: uuid.New(), + Name: input.SetRule.Name, + Description: input.SetRule.Description, + ConditionExpr: input.SetRule.ConditionExpr, + Actions: actionsGQLToGo(input.SetRule.Actions), + ContinueAfterMatch: input.SetRule.ContinueAfterMatch, }) } else { var found bool for i, r := range cfg.Rules { if r.ID.String() == *input.SetRule.ID { cfg.Rules[i] = integrationkey.Rule{ - ID: r.ID, - Name: input.SetRule.Name, - Description: input.SetRule.Description, - ConditionExpr: input.SetRule.ConditionExpr, - Actions: actionsGQLToGo(input.SetRule.Actions), + ID: r.ID, + Name: input.SetRule.Name, + Description: input.SetRule.Description, + ConditionExpr: input.SetRule.ConditionExpr, + Actions: actionsGQLToGo(input.SetRule.Actions), + ContinueAfterMatch: input.SetRule.ContinueAfterMatch, } found = true break From 171042b66317283a6b79bf4994a2b022b64002ad Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Tue, 9 Jul 2024 15:02:58 -0500 Subject: [PATCH 13/15] remove unused field --- integrationkey/config.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/integrationkey/config.go b/integrationkey/config.go index 0cc681112d..887540ad77 100644 --- a/integrationkey/config.go +++ b/integrationkey/config.go @@ -31,8 +31,6 @@ type Config struct { // DefaultActions are the actions to take if no rules match. DefaultActions []Action - - StopAfterFirstMatchingRule bool } // A Rule is a set of conditions and actions to take if those conditions are met. From 18117e5b3dbb4b0538cd24e5ccf56c12aa7602e3 Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Tue, 9 Jul 2024 15:12:19 -0500 Subject: [PATCH 14/15] Add support for default values in DynamicParamConfig - Updated schema to include `defaultValue` - Adjusted GraphQL server to handle `defaultValue` fields - Modified UI to prioritize `defaultValue` for dynamic parameters --- graphql2/generated.go | 82 ++++++++++++++++++++- graphql2/graph/destinations.graphqls | 5 ++ graphql2/graphqlapp/destinationtypes.go | 7 +- graphql2/models_gen.go | 2 + web/src/app/selection/DynamicActionForm.tsx | 2 +- web/src/app/util/RequireConfig.tsx | 1 + web/src/schema.d.ts | 1 + 7 files changed, 92 insertions(+), 8 deletions(-) diff --git a/graphql2/generated.go b/graphql2/generated.go index 0975487f83..1ce0b9f9bb 100644 --- a/graphql2/generated.go +++ b/graphql2/generated.go @@ -293,10 +293,11 @@ type ComplexityRoot struct { } DynamicParamConfig struct { - Hint func(childComplexity int) int - HintURL func(childComplexity int) int - Label func(childComplexity int) int - ParamID func(childComplexity int) int + DefaultValue func(childComplexity int) int + Hint func(childComplexity int) int + HintURL func(childComplexity int) int + Label func(childComplexity int) int + ParamID func(childComplexity int) int } EscalationPolicy struct { @@ -1948,6 +1949,13 @@ func (e *executableSchema) Complexity(typeName, field string, childComplexity in return e.complexity.DestinationTypeInfo.UserDisclaimer(childComplexity), true + case "DynamicParamConfig.defaultValue": + if e.complexity.DynamicParamConfig.DefaultValue == nil { + break + } + + return e.complexity.DynamicParamConfig.DefaultValue(childComplexity), true + case "DynamicParamConfig.hint": if e.complexity.DynamicParamConfig.Hint == nil { break @@ -11815,6 +11823,8 @@ func (ec *executionContext) fieldContext_DestinationTypeInfo_dynamicParams(_ con return ec.fieldContext_DynamicParamConfig_hint(ctx, field) case "hintURL": return ec.fieldContext_DynamicParamConfig_hintURL(ctx, field) + case "defaultValue": + return ec.fieldContext_DynamicParamConfig_defaultValue(ctx, field) } return nil, fmt.Errorf("no field named %q was found under type DynamicParamConfig", field.Name) }, @@ -12306,6 +12316,50 @@ func (ec *executionContext) fieldContext_DynamicParamConfig_hintURL(_ context.Co return fc, nil } +func (ec *executionContext) _DynamicParamConfig_defaultValue(ctx context.Context, field graphql.CollectedField, obj *DynamicParamConfig) (ret graphql.Marshaler) { + fc, err := ec.fieldContext_DynamicParamConfig_defaultValue(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) (interface{}, error) { + ctx = rctx // use context from middleware stack in children + return obj.DefaultValue, 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.(string) + fc.Result = res + return ec.marshalNExprStringExpression2string(ctx, field.Selections, res) +} + +func (ec *executionContext) fieldContext_DynamicParamConfig_defaultValue(_ context.Context, field graphql.CollectedField) (fc *graphql.FieldContext, err error) { + fc = &graphql.FieldContext{ + Object: "DynamicParamConfig", + Field: field, + IsMethod: false, + IsResolver: false, + Child: func(ctx context.Context, field graphql.CollectedField) (*graphql.FieldContext, error) { + return nil, errors.New("field of type ExprStringExpression does not have child fields") + }, + } + return fc, nil +} + func (ec *executionContext) _EscalationPolicy_id(ctx context.Context, field graphql.CollectedField, obj *escalation.Policy) (ret graphql.Marshaler) { fc, err := ec.fieldContext_EscalationPolicy_id(ctx, field) if err != nil { @@ -40012,6 +40066,11 @@ func (ec *executionContext) _DynamicParamConfig(ctx context.Context, sel ast.Sel if out.Values[i] == graphql.Null { out.Invalids++ } + case "defaultValue": + out.Values[i] = ec._DynamicParamConfig_defaultValue(ctx, field, obj) + if out.Values[i] == graphql.Null { + out.Invalids++ + } default: panic("unknown field " + strconv.Quote(field.Name)) } @@ -48476,6 +48535,21 @@ func (ec *executionContext) marshalNExprOperator2string(ctx context.Context, sel return res } +func (ec *executionContext) unmarshalNExprStringExpression2string(ctx context.Context, v interface{}) (string, error) { + res, err := UnmarshalExprStringExpression(v) + return res, graphql.ErrorOnPath(ctx, err) +} + +func (ec *executionContext) marshalNExprStringExpression2string(ctx context.Context, sel ast.SelectionSet, v string) graphql.Marshaler { + res := MarshalExprStringExpression(v) + if res == graphql.Null { + if !graphql.HasFieldError(ctx, graphql.GetFieldContext(ctx)) { + ec.Errorf(ctx, "the requested element is null which the schema does not allow") + } + } + return res +} + func (ec *executionContext) unmarshalNExprStringMap2map(ctx context.Context, v interface{}) (map[string]string, error) { res, err := UnmarshalExprStringMap(v) return res, graphql.ErrorOnPath(ctx, err) diff --git a/graphql2/graph/destinations.graphqls b/graphql2/graph/destinations.graphqls index 89935a2508..17d43303c4 100644 --- a/graphql2/graph/destinations.graphqls +++ b/graphql2/graph/destinations.graphqls @@ -316,4 +316,9 @@ type DynamicParamConfig { URL to link to for more information about the destination type """ hintURL: String! + + """ + default value for the input field + """ + defaultValue: ExprStringExpression! } diff --git a/graphql2/graphqlapp/destinationtypes.go b/graphql2/graphqlapp/destinationtypes.go index 63971a4860..26834bd607 100644 --- a/graphql2/graphqlapp/destinationtypes.go +++ b/graphql2/graphqlapp/destinationtypes.go @@ -356,9 +356,10 @@ func (q *Query) DestinationTypes(ctx context.Context, isDynamicAction *bool) ([] Hint: "The body of the request.", }, { - ParamID: "content-type", - Label: "Content Type", - Hint: "The content type (usually application/json).", + ParamID: "content-type", + Label: "Content Type", + Hint: "The content type (e.g., application/json).", + DefaultValue: `"application/json"`, // Because this is an expression, it needs the double quotes. }, }, }, diff --git a/graphql2/models_gen.go b/graphql2/models_gen.go index ac67ff160e..4fadea623f 100644 --- a/graphql2/models_gen.go +++ b/graphql2/models_gen.go @@ -470,6 +470,8 @@ type DynamicParamConfig struct { Hint string `json:"hint"` // URL to link to for more information about the destination type HintURL string `json:"hintURL"` + // default value for the input field + DefaultValue string `json:"defaultValue"` } type EscalationPolicyConnection struct { diff --git a/web/src/app/selection/DynamicActionForm.tsx b/web/src/app/selection/DynamicActionForm.tsx index bd913b7f49..e5f386cb1a 100644 --- a/web/src/app/selection/DynamicActionForm.tsx +++ b/web/src/app/selection/DynamicActionForm.tsx @@ -68,7 +68,7 @@ export function defaults(destTypeInfo: DestinationTypeInfo): Value { const dynamicParams = Object.fromEntries( destTypeInfo.dynamicParams.map((p) => [ p.paramID, - `req.body['${p.paramID}']`, + p.defaultValue || `req.body['${p.paramID}']`, ]), ) diff --git a/web/src/app/util/RequireConfig.tsx b/web/src/app/util/RequireConfig.tsx index 212314be53..0dcf53355a 100644 --- a/web/src/app/util/RequireConfig.tsx +++ b/web/src/app/util/RequireConfig.tsx @@ -70,6 +70,7 @@ const query = gql` label hint hintURL + defaultValue } } } diff --git a/web/src/schema.d.ts b/web/src/schema.d.ts index a8a3cae0a5..9acb6dc4b2 100644 --- a/web/src/schema.d.ts +++ b/web/src/schema.d.ts @@ -464,6 +464,7 @@ export interface DestinationTypeInfo { } export interface DynamicParamConfig { + defaultValue: ExprStringExpression hint: string hintURL: string label: string From 5e2ecb5de929c65fd98298ff6f3453986d7fc56a Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Wed, 10 Jul 2024 09:41:01 -0500 Subject: [PATCH 15/15] fix test data --- web/src/app/selection/DynamicActionForm.stories.tsx | 2 ++ web/src/app/storybook/defaultDestTypes.ts | 1 + 2 files changed, 3 insertions(+) diff --git a/web/src/app/selection/DynamicActionForm.stories.tsx b/web/src/app/selection/DynamicActionForm.stories.tsx index 3a61e50275..23beb37794 100644 --- a/web/src/app/selection/DynamicActionForm.stories.tsx +++ b/web/src/app/selection/DynamicActionForm.stories.tsx @@ -36,6 +36,7 @@ const meta = { label: 'Dynamic Param', hint: 'Param Hint', hintURL: 'http://example.com/hint', + defaultValue: '', }, ], requiredFields: [ @@ -71,6 +72,7 @@ const meta = { label: 'Dynamic Param 2', hint: 'Param Hint 2', hintURL: 'http://example.com/hint2', + defaultValue: '', }, ], requiredFields: [ diff --git a/web/src/app/storybook/defaultDestTypes.ts b/web/src/app/storybook/defaultDestTypes.ts index 52c5b744cc..4fee38ccbc 100644 --- a/web/src/app/storybook/defaultDestTypes.ts +++ b/web/src/app/storybook/defaultDestTypes.ts @@ -20,6 +20,7 @@ export const destTypes: DestinationTypeInfo[] = [ label: 'Example Param', hint: 'Some hint text', hintURL: '', + defaultValue: '', }, ], requiredFields: [