From 88cc385e8a38201cb1fbb41d5b386bdf5091633c Mon Sep 17 00:00:00 2001 From: Vitor Hervatin <54643926+vhervatin@users.noreply.github.com> Date: Tue, 6 Oct 2026 14:17:10 -0300 Subject: [PATCH 1/3] feat(notification): include task/doc title in email event payload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The notification.{assigned,mentioned,doc_mentioned, task_description_mentioned} events published to plugins carried only actor_name and link_url, so a subscribing email plugin had no way to name the task/document — the email could only say "a task was assigned to you" with a bare link. Add an entity_title field to the payload, resolved best-effort via a new WithTitleLookup(taskRepo, docRepo) option: when no lookup is configured or the entity can't be loaded, the title is simply empty and behaviour is identical to before, so a subscribing plugin must tolerate an empty entity_title. For the mention topics (whose IDs come from client-supplied content) the title is only resolved after the existing project-membership check, preserving the current anti-leak guarantee. Co-Authored-By: Claude Opus 4.8 --- services/api/internal/bootstrap/app.go | 3 +- .../internal/service/notification/service.go | 75 +++++++- .../service/notification/service_test.go | 164 ++++++++++++++++++ 3 files changed, 236 insertions(+), 6 deletions(-) diff --git a/services/api/internal/bootstrap/app.go b/services/api/internal/bootstrap/app.go index a686f4644..544b5a712 100644 --- a/services/api/internal/bootstrap/app.go +++ b/services/api/internal/bootstrap/app.go @@ -172,7 +172,8 @@ func New(cfg *config.Config) (*App, error) { sprintService := sprintsvc.NewCachedSprintService(sprintsvc.New(sprintRepo, taskRepo, publisher), cacheStore, cfg.Cache.SprintTTL, log) viewService := sprintsvc.NewCachedViewService(sprintsvc.NewViewService(viewRepo, sprintRepo, taskRepo, publisher), cacheStore, cfg.Cache.SprintTTL, log) notificationService := notificationsvc.New(notificationRepo, projectRepo, publisher). - WithEventPublishing(userRepo, cfg.Server.PublicURL) + WithEventPublishing(userRepo, cfg.Server.PublicURL). + WithTitleLookup(taskRepo, docRepo) agentService := agentsvc.New(agentRepo, projectService, publisher, pluginRepo) // environmentService calls agent-runner via the same AI_AGENT_URL/ // AI_AGENT_INTERNAL_KEY pair AgentHandler already uses for its fast diff --git a/services/api/internal/service/notification/service.go b/services/api/internal/service/notification/service.go index c4e365d96..62880abaa 100644 --- a/services/api/internal/service/notification/service.go +++ b/services/api/internal/service/notification/service.go @@ -10,8 +10,10 @@ import ( "github.com/google/uuid" + docdom "github.com/Paca-AI/api/internal/domain/doc" notificationdom "github.com/Paca-AI/api/internal/domain/notification" projectdom "github.com/Paca-AI/api/internal/domain/project" + taskdom "github.com/Paca-AI/api/internal/domain/task" userdom "github.com/Paca-AI/api/internal/domain/user" "github.com/Paca-AI/api/internal/events" "github.com/Paca-AI/api/internal/platform/messaging" @@ -41,12 +43,30 @@ type memberLookup interface { ListMembers(ctx context.Context, projectID uuid.UUID) ([]*projectdom.ProjectMember, error) } +// taskLookup resolves a task's title for the entity_title field of the +// notification.{assigned,mentioned,task_description_mentioned} plugin-event +// payloads, so a subscribing plugin (e.g. an email sender) can name the task +// a notification is about instead of only linking to it. Minimal by design — +// the full task repository already satisfies it. +type taskLookup interface { + FindTaskByID(ctx context.Context, id uuid.UUID) (*taskdom.Task, error) +} + +// docLookup resolves a document's title for the entity_title field of the +// notification.doc_mentioned plugin-event payload. Minimal by design — the +// full document repository already satisfies it. +type docLookup interface { + FindDocumentByID(ctx context.Context, id uuid.UUID) (*docdom.Document, error) +} + // Svc implements notificationdom.Service. type Svc struct { repo notificationdom.Repository memberRepo memberLookup publisher *messaging.Publisher userRepo userLookup + taskRepo taskLookup + docRepo docLookup publicURL string } @@ -70,6 +90,17 @@ func (s *Svc) WithEventPublishing(userRepo userLookup, publicURL string) *Svc { return s } +// WithTitleLookup configures resolving the task/document title carried as +// entity_title in the notification plugin-event payloads. Both lookups are +// optional and best-effort: when a repo is nil (or the lookup fails) the +// title is simply empty in the payload — exactly the behaviour before this was +// added — so a subscribing plugin must tolerate an empty entity_title. +func (s *Svc) WithTitleLookup(taskRepo taskLookup, docRepo docLookup) *Svc { + s.taskRepo = taskRepo + s.docRepo = docRepo + return s +} + // publishNotificationEvent resolves recipientUserID's name/email and // publishes topic to the plugin event stream with recipient/actor names and // a direct link to whatever was mentioned/assigned — the same event fires @@ -80,7 +111,10 @@ func (s *Svc) WithEventPublishing(userRepo userLookup, publicURL string) *Svc { // errors are swallowed, matching publishCreated's existing convention, // since the in-app notification (already created by the caller, where one // exists) is this service's primary responsibility. -func (s *Svc) publishNotificationEvent(ctx context.Context, topic string, recipientUserID uuid.UUID, actorName, linkURL string) { +// entityTitle is the title of the task or document the notification is about +// (resolved via WithTitleLookup); it may be empty when no lookup is configured +// or the entity can't be loaded, and a subscribing plugin must tolerate that. +func (s *Svc) publishNotificationEvent(ctx context.Context, topic string, recipientUserID uuid.UUID, actorName, linkURL, entityTitle string) { if s.publisher == nil || s.userRepo == nil { return } @@ -99,10 +133,38 @@ func (s *Svc) publishNotificationEvent(ctx context.Context, topic string, recipi "recipient_name": recipient.FullName, "actor_name": actorName, "link_url": linkURL, + "entity_title": entityTitle, } _ = s.publisher.Append(ctx, events.StreamPluginEvents, topic, payload) } +// taskTitle resolves taskID's title for an event payload, returning "" when no +// task lookup is configured or the task can't be loaded — best-effort, like +// the rest of publishNotificationEvent. +func (s *Svc) taskTitle(ctx context.Context, taskID uuid.UUID) string { + if s.taskRepo == nil { + return "" + } + t, err := s.taskRepo.FindTaskByID(ctx, taskID) + if err != nil || t == nil { + return "" + } + return t.Title +} + +// docTitle resolves docID's title for an event payload, returning "" when no +// document lookup is configured or the document can't be loaded. +func (s *Svc) docTitle(ctx context.Context, docID uuid.UUID) string { + if s.docRepo == nil { + return "" + } + d, err := s.docRepo.FindDocumentByID(ctx, docID) + if err != nil || d == nil { + return "" + } + return d.Title +} + // resolveUserName returns userID's full name, or "" if it can't be resolved // (no userRepo configured, or the lookup fails). func (s *Svc) resolveUserName(ctx context.Context, userID uuid.UUID) string { @@ -176,7 +238,7 @@ func (s *Svc) NotifyAssigned(ctx context.Context, in notificationdom.NotifyAssig if actorMember != nil { actorName = actorMember.FullName } - s.publishNotificationEvent(ctx, events.TopicNotificationAssigned, assigneeMember.UserID, actorName, s.taskURL(in.ProjectID, taskID)) + s.publishNotificationEvent(ctx, events.TopicNotificationAssigned, assigneeMember.UserID, actorName, s.taskURL(in.ProjectID, taskID), s.taskTitle(ctx, taskID)) return nil } @@ -223,6 +285,9 @@ func (s *Svc) NotifyMentioned(ctx context.Context, in notificationdom.NotifyMent if actorMember, err := s.memberRepo.FindMemberByID(ctx, actorID); err == nil { actorName = actorMember.FullName } + // Resolved once (not per-mentioned-user) — the task is the same for every + // notification created below. + taskTitle := s.taskTitle(ctx, in.TaskID) seen := make(map[uuid.UUID]bool) // avoid duplicate notifications for _, mentionedUserID := range mentionedUserIDs { // Skip self-mention. @@ -255,7 +320,7 @@ func (s *Svc) NotifyMentioned(ctx context.Context, in notificationdom.NotifyMent continue // best-effort; don't abort for one failure } s.publishCreated(ctx, n, mentionedUserID) - s.publishNotificationEvent(ctx, events.TopicNotificationMentioned, mentionedUserID, actorName, s.taskURL(in.ProjectID, taskID)) + s.publishNotificationEvent(ctx, events.TopicNotificationMentioned, mentionedUserID, actorName, s.taskURL(in.ProjectID, taskID), taskTitle) } return nil } @@ -286,7 +351,7 @@ func (s *Svc) NotifyDocMentioned(ctx context.Context, mentionedUserID, actorUser return // mentioned user isn't a member of this project; nothing to notify } actorName := s.resolveUserName(ctx, actorUserID) - s.publishNotificationEvent(ctx, events.TopicNotificationDocMentioned, mentionedUserID, actorName, s.docURL(projectID, docID)) + s.publishNotificationEvent(ctx, events.TopicNotificationDocMentioned, mentionedUserID, actorName, s.docURL(projectID, docID), s.docTitle(ctx, docID)) } // NotifyTaskDescriptionMentioned publishes a @@ -303,7 +368,7 @@ func (s *Svc) NotifyTaskDescriptionMentioned(ctx context.Context, mentionedUserI return // mentioned user isn't a member of this project; nothing to notify } actorName := s.resolveUserName(ctx, actorUserID) - s.publishNotificationEvent(ctx, events.TopicNotificationTaskDescriptionMentioned, mentionedUserID, actorName, s.taskURL(projectID, taskID)) + s.publishNotificationEvent(ctx, events.TopicNotificationTaskDescriptionMentioned, mentionedUserID, actorName, s.taskURL(projectID, taskID), s.taskTitle(ctx, taskID)) } // ListNotifications returns up to limit notifications for the user, newest diff --git a/services/api/internal/service/notification/service_test.go b/services/api/internal/service/notification/service_test.go index 5eca89032..9dd677484 100644 --- a/services/api/internal/service/notification/service_test.go +++ b/services/api/internal/service/notification/service_test.go @@ -10,8 +10,10 @@ import ( "github.com/google/uuid" + docdom "github.com/Paca-AI/api/internal/domain/doc" notificationdom "github.com/Paca-AI/api/internal/domain/notification" projectdom "github.com/Paca-AI/api/internal/domain/project" + taskdom "github.com/Paca-AI/api/internal/domain/task" userdom "github.com/Paca-AI/api/internal/domain/user" ) @@ -223,6 +225,48 @@ func (r *fakeUserRepo) callCount() int { return len(r.calls) } +// fakeTaskRepo / fakeDocRepo back the optional WithTitleLookup dependency, +// tracking lookups so a test can prove the entity_title resolution runs (and, +// for the client-supplied-mention cases, that it runs only once the recipient +// has passed the project-membership check). +type fakeTaskRepo struct { + mu sync.Mutex + title string + calls int +} + +func (r *fakeTaskRepo) FindTaskByID(_ context.Context, id uuid.UUID) (*taskdom.Task, error) { + r.mu.Lock() + defer r.mu.Unlock() + r.calls++ + return &taskdom.Task{ID: id, Title: r.title}, nil +} + +func (r *fakeTaskRepo) callCount() int { + r.mu.Lock() + defer r.mu.Unlock() + return r.calls +} + +type fakeDocRepo struct { + mu sync.Mutex + title string + calls int +} + +func (r *fakeDocRepo) FindDocumentByID(_ context.Context, id uuid.UUID) (*docdom.Document, error) { + r.mu.Lock() + defer r.mu.Unlock() + r.calls++ + return &docdom.Document{ID: id, Title: r.title}, nil +} + +func (r *fakeDocRepo) callCount() int { + r.mu.Lock() + defer r.mu.Unlock() + return r.calls +} + type errCreateRepo struct { *fakeNotificationRepo } @@ -1112,3 +1156,123 @@ func TestNotifyTaskDescriptionMentioned_MemberNotified(t *testing.T) { t.Errorf("expected the actor-name lookup to proceed for a mentioned project member, got %d calls", got) } } + +// --- entity_title resolution (WithTitleLookup) ------------------------------ +// +// These tests run with a nil publisher as the rest of the file does: the title +// helpers are evaluated as arguments to publishNotificationEvent, so they run +// (and the lookup is observable) even though the publish itself is a no-op. + +func TestNotifyAssigned_ResolvesTaskTitle(t *testing.T) { + ctx := context.Background() + repo := newFakeNotificationRepo() + members := newFakeMemberRepo() + tasks := &fakeTaskRepo{title: "Ship the landing page"} + svc := New(repo, members, nil).WithTitleLookup(tasks, nil) + + projectID := uuid.New() + actorUserID := uuid.New() + assigneeUserID := uuid.New() + actorMemberID := uuid.New() + assigneeMemberID := uuid.New() + members.add(&projectdom.ProjectMember{ID: actorMemberID, ProjectID: projectID, UserID: actorUserID, Username: "actor"}) + members.add(&projectdom.ProjectMember{ID: assigneeMemberID, ProjectID: projectID, UserID: assigneeUserID, Username: "assignee"}) + + if err := svc.NotifyAssigned(ctx, notificationdom.NotifyAssignedInput{ + TaskID: uuid.New(), + ProjectID: projectID, + NewAssigneeMemberID: assigneeMemberID, + ActorUserID: actorUserID, + }); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if got := tasks.callCount(); got != 1 { + t.Errorf("expected the task title to be resolved once for the assignment event, got %d lookups", got) + } +} + +func TestNotifyMentioned_ResolvesTaskTitleOnce(t *testing.T) { + ctx := context.Background() + repo := newFakeNotificationRepo() + members := newFakeMemberRepo() + tasks := &fakeTaskRepo{title: "Ship the landing page"} + svc := New(repo, members, nil).WithTitleLookup(tasks, nil) + + projectID := uuid.New() + actorUserID := uuid.New() + actorMemberID := uuid.New() + alice := uuid.New() + bob := uuid.New() + members.add(&projectdom.ProjectMember{ID: actorMemberID, ProjectID: projectID, UserID: actorUserID, Username: "actor"}) + members.add(&projectdom.ProjectMember{ID: uuid.New(), ProjectID: projectID, UserID: alice, Username: "alice"}) + members.add(&projectdom.ProjectMember{ID: uuid.New(), ProjectID: projectID, UserID: bob, Username: "bob"}) + + if err := svc.NotifyMentioned(ctx, notificationdom.NotifyMentionedInput{ + TaskID: uuid.New(), + ProjectID: projectID, + CommentText: "hey @alice and @bob", + ActorMemberID: actorMemberID, + ActorUserID: actorUserID, + }); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Two members mentioned, but the task title is the same for both — it must + // be resolved once, not once per recipient. + if got := tasks.callCount(); got != 1 { + t.Errorf("expected the task title to be resolved once for the whole comment, got %d lookups", got) + } +} + +func TestNotifyDocMentioned_ResolvesTitleOnlyForMember(t *testing.T) { + ctx := context.Background() + projectID := uuid.New() + actorUserID := uuid.New() + + // Member path: title resolved exactly once. + repo := newFakeNotificationRepo() + members := newFakeMemberRepo() + users := newFakeUserRepo() + docs := &fakeDocRepo{title: "Q4 roadmap"} + svc := New(repo, members, nil).WithEventPublishing(users, "https://paca.example").WithTitleLookup(nil, docs) + mentioned := uuid.New() + members.add(&projectdom.ProjectMember{ID: uuid.New(), ProjectID: projectID, UserID: mentioned, Username: "alice"}) + svc.NotifyDocMentioned(ctx, mentioned, actorUserID, projectID, uuid.New()) + if got := docs.callCount(); got != 1 { + t.Errorf("expected the document title to be resolved once for a mentioned member, got %d lookups", got) + } + + // Non-member path: membership check short-circuits before any title lookup. + docs2 := &fakeDocRepo{title: "Q4 roadmap"} + svc2 := New(newFakeNotificationRepo(), newFakeMemberRepo(), nil).WithEventPublishing(newFakeUserRepo(), "https://paca.example").WithTitleLookup(nil, docs2) + svc2.NotifyDocMentioned(ctx, uuid.New(), actorUserID, projectID, uuid.New()) + if got := docs2.callCount(); got != 0 { + t.Errorf("expected no document title lookup for a mention naming a non-member, got %d lookups", got) + } +} + +func TestNotifyTaskDescriptionMentioned_ResolvesTitleOnlyForMember(t *testing.T) { + ctx := context.Background() + projectID := uuid.New() + actorUserID := uuid.New() + + repo := newFakeNotificationRepo() + members := newFakeMemberRepo() + users := newFakeUserRepo() + tasks := &fakeTaskRepo{title: "Ship the landing page"} + svc := New(repo, members, nil).WithEventPublishing(users, "https://paca.example").WithTitleLookup(tasks, nil) + mentioned := uuid.New() + members.add(&projectdom.ProjectMember{ID: uuid.New(), ProjectID: projectID, UserID: mentioned, Username: "alice"}) + svc.NotifyTaskDescriptionMentioned(ctx, mentioned, actorUserID, projectID, uuid.New()) + if got := tasks.callCount(); got != 1 { + t.Errorf("expected the task title to be resolved once for a mentioned member, got %d lookups", got) + } + + tasks2 := &fakeTaskRepo{title: "Ship the landing page"} + svc2 := New(newFakeNotificationRepo(), newFakeMemberRepo(), nil).WithEventPublishing(newFakeUserRepo(), "https://paca.example").WithTitleLookup(tasks2, nil) + svc2.NotifyTaskDescriptionMentioned(ctx, uuid.New(), actorUserID, projectID, uuid.New()) + if got := tasks2.callCount(); got != 0 { + t.Errorf("expected no task title lookup for a mention naming a non-member, got %d lookups", got) + } +} From 1ec9d2d765993dfe6fc86298260764a3ebb9fa8e Mon Sep 17 00:00:00 2001 From: Vitor Hervatin <54643926+vhervatin@users.noreply.github.com> Date: Tue, 6 Oct 2026 14:34:52 -0300 Subject: [PATCH 2/3] perf(notification): title-only lookup + payload assertion test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the PR review: - entity_title now resolves via dedicated FindTaskTitleByID / FindDocumentTitleByID (SELECT title) instead of FindTaskByID / FindDocumentByID, which pulled the full row (task description JSONB + a second assignee query; full document content) just to read a title on the synchronous comment/doc-update path. - The publisher is now an eventPublisher interface so a unit test can capture the published payload and assert entity_title carries the resolved title — not only that the lookup ran. Co-Authored-By: Claude Opus 4.8 --- .../postgres/document_repository.go | 16 ++++ .../repository/postgres/task_repository.go | 17 ++++ .../internal/service/notification/service.go | 42 +++++---- .../service/notification/service_test.go | 89 +++++++++++++++++-- 4 files changed, 141 insertions(+), 23 deletions(-) diff --git a/services/api/internal/repository/postgres/document_repository.go b/services/api/internal/repository/postgres/document_repository.go index 869636dd6..427660b8c 100644 --- a/services/api/internal/repository/postgres/document_repository.go +++ b/services/api/internal/repository/postgres/document_repository.go @@ -388,6 +388,22 @@ func (r *DocumentRepository) FindDocumentByID(_ context.Context, id uuid.UUID) ( return documentFromRecord(rec), nil } +// FindDocumentTitleByID returns only the title of a non-deleted document — a +// cheaper read than FindDocumentByID (which pulls the full row, including the +// content) for callers that need just the title, e.g. building a notification +// event payload. Returns ErrDocNotFound when the document doesn't exist or is +// deleted. +func (r *DocumentRepository) FindDocumentTitleByID(_ context.Context, id uuid.UUID) (string, error) { + var title string + if err := r.db.Get(&title, `SELECT title FROM documents WHERE id = $1 AND deleted_at IS NULL`, id.String()); err != nil { + if errors.Is(err, sql.ErrNoRows) { + return "", docdom.ErrDocNotFound + } + return "", err + } + return title, nil +} + // CreateDocument persists a new document. func (r *DocumentRepository) CreateDocument(ctx context.Context, d *docdom.Document) error { rec := documentToRecord(d) diff --git a/services/api/internal/repository/postgres/task_repository.go b/services/api/internal/repository/postgres/task_repository.go index 71a5feb6b..6c1ceda1d 100644 --- a/services/api/internal/repository/postgres/task_repository.go +++ b/services/api/internal/repository/postgres/task_repository.go @@ -1272,6 +1272,23 @@ func (r *TaskRepository) FindTaskByID(ctx context.Context, id uuid.UUID) (*taskd return t, nil } +// FindTaskTitleByID returns only the title of a non-deleted task — a cheaper +// read than FindTaskByID (which pulls the full row, including the description +// JSONB, plus a second assignee query) for callers that need just the title, +// e.g. building a notification event payload. Returns ErrTaskNotFound when the +// task doesn't exist or is deleted. +func (r *TaskRepository) FindTaskTitleByID(ctx context.Context, id uuid.UUID) (string, error) { + var title string + err := r.db.GetContext(ctx, &title, `SELECT title FROM tasks WHERE id = $1 AND deleted_at IS NULL`, id.String()) + if errors.Is(err, sql.ErrNoRows) { + return "", taskdom.ErrTaskNotFound + } + if err != nil { + return "", fmt.Errorf("task repo: find title by id: %w", err) + } + return title, nil +} + // FindTaskByNumber returns the task with the given project-scoped task number (non-deleted). func (r *TaskRepository) FindTaskByNumber(ctx context.Context, projectID uuid.UUID, taskNumber int64) (*taskdom.Task, error) { var rec taskRecord diff --git a/services/api/internal/service/notification/service.go b/services/api/internal/service/notification/service.go index 62880abaa..5b8ad8ed4 100644 --- a/services/api/internal/service/notification/service.go +++ b/services/api/internal/service/notification/service.go @@ -10,13 +10,10 @@ import ( "github.com/google/uuid" - docdom "github.com/Paca-AI/api/internal/domain/doc" notificationdom "github.com/Paca-AI/api/internal/domain/notification" projectdom "github.com/Paca-AI/api/internal/domain/project" - taskdom "github.com/Paca-AI/api/internal/domain/task" userdom "github.com/Paca-AI/api/internal/domain/user" "github.com/Paca-AI/api/internal/events" - "github.com/Paca-AI/api/internal/platform/messaging" ) // userLookup resolves a recipient's name/email for the notification.assigned/ @@ -46,24 +43,35 @@ type memberLookup interface { // taskLookup resolves a task's title for the entity_title field of the // notification.{assigned,mentioned,task_description_mentioned} plugin-event // payloads, so a subscribing plugin (e.g. an email sender) can name the task -// a notification is about instead of only linking to it. Minimal by design — -// the full task repository already satisfies it. +// a notification is about instead of only linking to it. Deliberately a +// title-only read, not FindTaskByID, so this doesn't pull the full task row +// (description JSONB + assignees) on the synchronous comment/update path. type taskLookup interface { - FindTaskByID(ctx context.Context, id uuid.UUID) (*taskdom.Task, error) + FindTaskTitleByID(ctx context.Context, id uuid.UUID) (string, error) } // docLookup resolves a document's title for the entity_title field of the -// notification.doc_mentioned plugin-event payload. Minimal by design — the -// full document repository already satisfies it. +// notification.doc_mentioned plugin-event payload. Title-only, for the same +// reason as taskLookup (avoids loading the full document content). type docLookup interface { - FindDocumentByID(ctx context.Context, id uuid.UUID) (*docdom.Document, error) + FindDocumentTitleByID(ctx context.Context, id uuid.UUID) (string, error) +} + +// eventPublisher is the subset of messaging.Publisher this service uses: +// Append for the durable plugin-event stream and Publish for the real-time +// channel. Declared as an interface (rather than the concrete +// *messaging.Publisher) so a test can capture what's published; +// *messaging.Publisher satisfies it. +type eventPublisher interface { + Append(ctx context.Context, stream, eventType string, payload any) error + Publish(ctx context.Context, channel string, payload any) error } // Svc implements notificationdom.Service. type Svc struct { repo notificationdom.Repository memberRepo memberLookup - publisher *messaging.Publisher + publisher eventPublisher userRepo userLookup taskRepo taskLookup docRepo docLookup @@ -72,7 +80,7 @@ type Svc struct { // New returns a configured Svc. // publisher may be nil; real-time events are then skipped silently. -func New(repo notificationdom.Repository, memberRepo memberLookup, publisher *messaging.Publisher) *Svc { +func New(repo notificationdom.Repository, memberRepo memberLookup, publisher eventPublisher) *Svc { return &Svc{repo: repo, memberRepo: memberRepo, publisher: publisher} } @@ -145,11 +153,11 @@ func (s *Svc) taskTitle(ctx context.Context, taskID uuid.UUID) string { if s.taskRepo == nil { return "" } - t, err := s.taskRepo.FindTaskByID(ctx, taskID) - if err != nil || t == nil { + title, err := s.taskRepo.FindTaskTitleByID(ctx, taskID) + if err != nil { return "" } - return t.Title + return title } // docTitle resolves docID's title for an event payload, returning "" when no @@ -158,11 +166,11 @@ func (s *Svc) docTitle(ctx context.Context, docID uuid.UUID) string { if s.docRepo == nil { return "" } - d, err := s.docRepo.FindDocumentByID(ctx, docID) - if err != nil || d == nil { + title, err := s.docRepo.FindDocumentTitleByID(ctx, docID) + if err != nil { return "" } - return d.Title + return title } // resolveUserName returns userID's full name, or "" if it can't be resolved diff --git a/services/api/internal/service/notification/service_test.go b/services/api/internal/service/notification/service_test.go index 9dd677484..32f1ff248 100644 --- a/services/api/internal/service/notification/service_test.go +++ b/services/api/internal/service/notification/service_test.go @@ -10,10 +10,8 @@ import ( "github.com/google/uuid" - docdom "github.com/Paca-AI/api/internal/domain/doc" notificationdom "github.com/Paca-AI/api/internal/domain/notification" projectdom "github.com/Paca-AI/api/internal/domain/project" - taskdom "github.com/Paca-AI/api/internal/domain/task" userdom "github.com/Paca-AI/api/internal/domain/user" ) @@ -235,11 +233,11 @@ type fakeTaskRepo struct { calls int } -func (r *fakeTaskRepo) FindTaskByID(_ context.Context, id uuid.UUID) (*taskdom.Task, error) { +func (r *fakeTaskRepo) FindTaskTitleByID(_ context.Context, _ uuid.UUID) (string, error) { r.mu.Lock() defer r.mu.Unlock() r.calls++ - return &taskdom.Task{ID: id, Title: r.title}, nil + return r.title, nil } func (r *fakeTaskRepo) callCount() int { @@ -254,11 +252,11 @@ type fakeDocRepo struct { calls int } -func (r *fakeDocRepo) FindDocumentByID(_ context.Context, id uuid.UUID) (*docdom.Document, error) { +func (r *fakeDocRepo) FindDocumentTitleByID(_ context.Context, _ uuid.UUID) (string, error) { r.mu.Lock() defer r.mu.Unlock() r.calls++ - return &docdom.Document{ID: id, Title: r.title}, nil + return r.title, nil } func (r *fakeDocRepo) callCount() int { @@ -267,6 +265,39 @@ func (r *fakeDocRepo) callCount() int { return r.calls } +// fakePublisher satisfies eventPublisher and captures every Append so a test +// can assert the exact payload published to the plugin event stream (the real +// *messaging.Publisher is a concrete Redis-backed struct that can't be +// observed in a unit test). +type capturedEvent struct { + stream string + eventType string + payload any +} + +type fakePublisher struct { + mu sync.Mutex + appends []capturedEvent +} + +func (p *fakePublisher) Append(_ context.Context, stream, eventType string, payload any) error { + p.mu.Lock() + defer p.mu.Unlock() + p.appends = append(p.appends, capturedEvent{stream: stream, eventType: eventType, payload: payload}) + return nil +} + +func (p *fakePublisher) Publish(_ context.Context, _ string, _ any) error { return nil } + +func (p *fakePublisher) lastAppend() (capturedEvent, bool) { + p.mu.Lock() + defer p.mu.Unlock() + if len(p.appends) == 0 { + return capturedEvent{}, false + } + return p.appends[len(p.appends)-1], true +} + type errCreateRepo struct { *fakeNotificationRepo } @@ -1276,3 +1307,49 @@ func TestNotifyTaskDescriptionMentioned_ResolvesTitleOnlyForMember(t *testing.T) t.Errorf("expected no task title lookup for a mention naming a non-member, got %d lookups", got) } } + +// TestNotifyAssigned_PublishesEntityTitle is the direct output test the rest +// of this group's count-based assertions can't give: it captures the payload +// handed to the publisher and proves entity_title carries the resolved title. +func TestNotifyAssigned_PublishesEntityTitle(t *testing.T) { + ctx := context.Background() + repo := newFakeNotificationRepo() + members := newFakeMemberRepo() + users := newFakeUserRepo() + pub := &fakePublisher{} + tasks := &fakeTaskRepo{title: "Ship the landing page"} + svc := New(repo, members, pub). + WithEventPublishing(users, "https://paca.example"). + WithTitleLookup(tasks, nil) + + projectID := uuid.New() + actorUserID := uuid.New() + assigneeUserID := uuid.New() + actorMemberID := uuid.New() + assigneeMemberID := uuid.New() + members.add(&projectdom.ProjectMember{ID: actorMemberID, ProjectID: projectID, UserID: actorUserID, Username: "actor"}) + members.add(&projectdom.ProjectMember{ID: assigneeMemberID, ProjectID: projectID, UserID: assigneeUserID, Username: "assignee"}) + email := "assignee@example.com" + users.byID[assigneeUserID] = &userdom.User{ID: assigneeUserID, FullName: "Assignee", Email: &email} + + if err := svc.NotifyAssigned(ctx, notificationdom.NotifyAssignedInput{ + TaskID: uuid.New(), + ProjectID: projectID, + NewAssigneeMemberID: assigneeMemberID, + ActorUserID: actorUserID, + }); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + evt, ok := pub.lastAppend() + if !ok { + t.Fatal("expected a plugin event to be published") + } + payload, ok := evt.payload.(map[string]any) + if !ok { + t.Fatalf("expected a map payload, got %T", evt.payload) + } + if got := payload["entity_title"]; got != "Ship the landing page" { + t.Errorf("expected entity_title=%q in the published payload, got %v", "Ship the landing page", got) + } +} From 88bca4fa753bf51373b254a93740856be874bf02 Mon Sep 17 00:00:00 2001 From: Vitor Hervatin <54643926+vhervatin@users.noreply.github.com> Date: Wed, 7 Oct 2026 09:21:28 -0300 Subject: [PATCH 3/3] fix(test): use net.ListenConfig.Listen in email_test.go (noctx lint) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The repo-wide `golangci-lint run ./...` (api-pr-ci's Lint job) fails on `net.Listen` in email_test.go, added by 27b1029d (the GHSA-f2mf-mg99-22w5 netguard fix) — a pre-existing violation unrelated to this PR's change, but it turns the Lint check red on every PR built against current master. Switch to (*net.ListenConfig).Listen with context, exactly as the noctx linter prescribes, so the Lint job passes. `golangci-lint run ./...` is now clean (0 issues). Co-Authored-By: Claude Opus 4.8 --- services/api/internal/platform/plugin/email_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/services/api/internal/platform/plugin/email_test.go b/services/api/internal/platform/plugin/email_test.go index 801f66bc1..822c3b2f8 100644 --- a/services/api/internal/platform/plugin/email_test.go +++ b/services/api/internal/platform/plugin/email_test.go @@ -9,7 +9,8 @@ import ( func TestDialAndSendSMTP_BlocksPrivateTargets(t *testing.T) { // A live loopback listener proves the block happens before any connect. - ln, err := net.Listen("tcp", "127.0.0.1:0") + var lc net.ListenConfig + ln, err := lc.Listen(context.Background(), "tcp", "127.0.0.1:0") if err != nil { t.Fatal(err) }