Skip to content

[#499] Add notification for new storyline created#505

Merged
realproject7 merged 2 commits intomainfrom
task/499-storyline-notification
Mar 24, 2026
Merged

[#499] Add notification for new storyline created#505
realproject7 merged 2 commits intomainfrom
task/499-storyline-notification

Conversation

@realproject7
Copy link
Copy Markdown
Owner

Summary

  • Added notifyNewStoryline() to lib/notifications.server.ts — sends "New story published" / "{title}" by {author} is now on PlotLink with target /story/{storylineId} and notification ID pl-new-storyline-{storylineId}
  • Updated backfill cron to call notifyNewStoryline() on StorylineCreated events instead of notifyNewPlot() (which was sending a generic "New Genesis published" message)
  • Author display uses truncated wallet address (0x1234…5678)

Test plan

  • Verify notifyNewStoryline sends correct title/body/targetUrl via backfill cron
  • Confirm existing notifyNewPlot still fires for PlotChained events
  • Check notification ID uniqueness (pl-new-storyline-{id})

Fixes #499

🤖 Generated with Claude Code

Add a dedicated notification for new storylines with "New story
published" title and "{title} by {author}" body. Called from the
backfill cron when a StorylineCreated event is first indexed.

Fixes #499

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown

vercel Bot commented Mar 24, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
plotlink Ignored Ignored Mar 24, 2026 4:02pm

Request Review

Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST CHANGES

Summary

The new trigger is wired into the right backfill path, but the notification body does not match the copy requested in issue #499 / the T1 assignment. The implementation currently truncates both the title and author before sending.

Findings

  • [medium] notifyNewStoryline() rewrites the requested body instead of sending "{title}" by {author} is now on PlotLink.
    • File: lib/notifications.server.ts:175
    • Suggestion: Send the full title and author values in the body, or explicitly document and test a notification length constraint before changing the requested copy.

Decision

Requesting changes because the acceptance criteria call for the correct notification body, and the current helper intentionally changes that body for longer titles/authors.

Send the exact body from the requirement: "{title}" by {author}
is now on PlotLink — without truncating either value.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: APPROVE

Summary

The requested change is resolved. notifyNewStoryline() now sends the exact body specified in issue #499, and the backfill cron triggers it on StorylineCreated events.

Findings

  • None.

Decision

Approve because the new notification helper, notification ID, deep link, and backfill trigger now match the issue requirements.

@realproject7 realproject7 merged commit 02e5e0f into main Mar 24, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add notification for new storyline created

2 participants