Skip to content

feat: digest post type#5599

Open
capJavert wants to merge 16 commits intomainfrom
feat/digest-post-type
Open

feat: digest post type#5599
capJavert wants to merge 16 commits intomainfrom
feat/digest-post-type

Conversation

@capJavert
Copy link
Contributor

@capJavert capJavert commented Feb 25, 2026

Changes

API: dailydotdev/daily-api#3635

Summary

  • Add PostType.Digest that renders a curated feed of posts at /posts/:id using DigestPostContent component
  • Inject stored Skadi ad snapshot into the digest feed at the backend-specified index via a new staticAd prop on useFeed/Feed
  • Add NotificationType.DigestReady with in-app notification toggle and cross-channel unsubscribe logic (only fully unsubscribes when both email and in-app are off)

Changes

  • Types & GraphQL: New PostType.Digest enum, DigestPostAd type, updated post flags fragment with digestPostIds and ad
  • DigestPostContent: New post content component rendering a feed of curated posts with header (date, stats, sources)
  • Ad injection: transformDigestAd maps backend ad shape to frontend Ad interface; staticAd param on useFeed inserts it at the correct index while keeping live ads disabled
  • Notification settings: DigestReady notification type, DigestNotification component for in-app tab, updated PersonalizedDigest email tab with cross-channel muting awareness
  • Tests: Unit tests for transformDigestAd and isMutingDigestCompletely

Test plan

  • Open a digest post at /posts/:digest-post-id and verify the curated feed renders with post cards
  • Verify the stored ad appears at the correct index in the digest feed
  • Verify Plus users see no ad when flags.ad is null
  • Toggle digest in-app notification on/off in notification settings
  • Verify unsubscribing both email and in-app channels fully unsubscribes from digest
  • Run pnpm --filter shared test — 13 new tests pass

Events

Did you introduce any new tracking events?

Experiment

Did you introduce any new experiments?

Manual Testing

Caution

Please make sure existing components are not breaking/affected by this PR

Preview domain

https://feat-digest-post-type.preview.app.daily.dev

capJavert and others added 3 commits February 25, 2026 16:58
Add digest post type support (Phase 1 & 2):
- Add PostType.Digest enum value and DIGEST_SOURCE constant
- Add digestPostIds and ad fields to PostFlags type and GraphQL fragments
- Create DigestPostContent component that renders a feed of curated posts
- Register in CONTENT_MAP and FeedItemComponent card type maps

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Inject stored Skadi ads into the digest feed via a new staticAd prop on
Feed/useFeed, and add in-app notification controls for digest posts.

Phase 3 – Ad injection:
- Add staticAd support to useFeed and FeedProps
- Transform DigestPostAd → Ad interface in DigestPostContent
- Fix missing PostType.Digest in PostModalMap

Phase 4 – Notification settings:
- Add NotificationType.DigestReady
- Create DigestNotification component for in-app toggle
- Add digest section to InAppNotificationsTab
- Update PersonalizedDigest unsubscribe logic to respect in-app channel

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…etely

Extract transformDigestAd to digest/utils.ts for testability and add
unit tests for the digest ad transformation and notification muting logic.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@capJavert capJavert self-assigned this Feb 25, 2026
@vercel
Copy link

vercel bot commented Feb 25, 2026

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

Project Deployment Actions Updated (UTC)
daily-webapp Ready Ready Preview Feb 27, 2026 7:41pm
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
storybook Ignored Ignored Feb 27, 2026 7:41pm

Request Review

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
API returns camelCase (companyName, companyLogo, callToAction) instead
of snake_case. Update type, fragments, transform utils, and tests.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@capJavert
Copy link
Contributor Author

@claude review

@claude
Copy link
Contributor

claude bot commented Feb 26, 2026

Claude finished @capJavert's task in 2m 53s —— View job


PR Review: Feat/digest post type

  • Gather context and read changed files
  • Review GraphQL types and fragments
  • Review DigestPostContent component
  • Review ad injection logic (useFeed, Feed)
  • Review notification components (DigestNotification, utils)
  • Review tests
  • Compile final review

Overall this is a solid implementation. The feature decomposition is clean — the PostType.Digest rendering, static ad injection, and notification cross-channel logic are all well-separated. A few things worth addressing:

Issues

1. Static ad injected among loading placeholders [packages/shared/src/hooks/useFeed.ts:388–397]

The static ad is inserted into newItems after placeholders have been pushed during the loading state. On initial render (no cached data), feedQuery.data is falsy so newItems is empty, but feedQuery.isFetching is true so placeholders are added — and then the static ad gets spliced into placeholder positions. This means an ad card renders among skeleton cards before the feed loads.

// current — ad can appear among placeholders
if (settings?.staticAd && newItems.length > 0) {

// fix — only inject ad when real posts exist
if (settings?.staticAd && feedQuery.data && newItems.length > 0) {

Fix this →


2. digestPostIds + ad added to SHARED_POST_INFO_FRAGMENT [packages/shared/src/graphql/fragments.ts:366–376]

SHARED_POST_INFO_FRAGMENT is used for shared/reposted posts — a digest post (which is a container) would never itself appear as a shared post. Adding these fields to this fragment adds unnecessary payload to every shared post query. They belong only in FEED_POST_FRAGMENT (where they're also correctly added).


3. DIGEST_SOURCE constant is unused [packages/shared/src/types.ts:47]

export const DIGEST_SOURCE = 'digest';

This was added (mirroring BRIEFING_SOURCE) but is not referenced anywhere in this PR. Either use it or remove it.

Fix this →


4. plans/.gitkeep shouldn't be committed [.gitignore, plans/.gitkeep]

The plans/ directory is a local AI planning workspace. Since plans/*.md is now gitignored, committing a .gitkeep to track the empty directory is unnecessary. Just add plans/ to .gitignore outright and drop the .gitkeep.


5. AdItem.index hardcoded to 0 for static ad [packages/shared/src/hooks/useFeed.ts:393]

newItems.splice(insertAt, 0, {
  type: FeedItemType.Ad,
  ad: settings.staticAd.ad,
  index: 0,  // ← always 0
  ...
} as AdItem);

The AdItem.index is used elsewhere (line 327) for ad-slot logic (adItem.index === 0). For the static ad case this likely doesn't matter since disableAds: true prevents dynamic ads from running, but it's semantically wrong — the ad's actual position in the feed is insertAt, not 0. Low-risk for now but could bite if this code path is reused.


Minor Notes

  • Both stats icons are AnalyticsIcon [DigestPostContent.tsx:122–136]: The posts count and sources count both use AnalyticsIcon. Intentional or placeholder?

  • setState during render [DigestNotification.tsx:41–43]: The setDigestTimeIndex call during the render phase (same pattern as PersonalizedDigest.tsx) is a pre-existing anti-pattern but works in practice. Not introduced by this PR.

  • Cross-channel notification dependency: DigestNotification.tsx checks BriefingReady.email to decide full unsubscribe (line 83–86), while PersonalizedDigest.tsx checks DigestReady.inApp. The asymmetry is correct given the email/in-app split but the coupling between these two files is fragile. A comment explaining the relationship would help future readers.


Tests

Good coverage on transformDigestAd and isMutingDigestCompletely. The DigestPostContent.spec.ts tests pass. The edge case for isMutingDigestCompletely with undefined status (line 111, as any cast) is a nice defensive test.

- Only inject static ad when real feed data exists, not during loading
- Remove digest fields from SHARED_POST_INFO_FRAGMENT (only needed in
  FEED_POST_FRAGMENT)
- Remove unused DIGEST_SOURCE constant

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@capJavert capJavert changed the title Feat/digest post type feat: digest post type Feb 26, 2026
Override insaneMode via SettingsContext.Provider to force list layout
in digest post feed. Add disableAdRefresh prop to hide reload button
on static ads. Add digestPostIds and ad fields to SHARED_POST_INFO_FRAGMENT.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add Digest to NotificationAvatarType enum and render BriefIcon
for digest notifications so the avatar displays correctly.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Show BriefUpgradeAlert below digest feed for non-Plus users.
Add text prop to BriefUpgradeAlert for customizable copy.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Map companyLogo and callToAction from digest ad data to Ad interface.
Use companyLogo as favicon when available. Render callToAction as a
primary button in AdList/AdGrid with refresh+remove grouped on the right.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@capJavert
Copy link
Contributor Author

@claude review one more time, especially focus on notification settings page changes in regards to how brief works and now digest

@claude
Copy link
Contributor

claude bot commented Feb 27, 2026

Claude finished @capJavert's task in 4m 15s —— View job


PR Review: Notification Settings — Digest vs. Brief

  • Gather context
  • Read notification components (DigestNotification, PersonalizedDigest, InAppNotificationsTab)
  • Read notification utils and tests
  • Read existing Brief/Briefing notification handling for comparison
  • Compile focused review

Here's a focused review of the notification settings changes, tracing the full cross-channel logic across the three interacting components.

Architecture Overview

The settings page now has this structure:

Tab Component Controls
Email PersonalizedDigest BriefingReady.email (covers both Digest and Brief email)
In-app DigestNotification DigestReady.inApp
In-app PresidentialBriefingNotification BriefingReady.inApp

Issues

1. PresidentialBriefingNotification + DigestNotification now coexist on the same page — and Brief's toggle silently destroys Digest subscription [PresidentialBriefingNotification.tsx:112–119]

PresidentialBriefingNotification.onToggleBriefing has this block that runs when no Brief subscription exists and user enables Brief in-app:

} else if (!briefDigest) {
  unsubscribePersonalizedDigest({ type: UserPersonalizedDigestType.Digest }); // ← destroys Digest
  subscribePersonalizedDigest({ type: UserPersonalizedDigestType.Brief, sendType: SendType.Daily });
}

With DigestNotification now placed directly above PresidentialBriefingNotification on the same page, a Plus user could:

  1. Enable Digest in-app → Digest subscription created, DigestReady.inApp = subscribed
  2. Enable Brief in-app (right below) → unsubscribePersonalizedDigest(Digest) is called, destroying the subscription

After step 2, the Digest in-app switch still shows on (because DigestReady.inApp = subscribed was not cleared), but there's no longer a Digest subscription backing it. This is a UX inconsistency that becomes a real problem now that both controls appear side-by-side.

This is pre-existing code in PresidentialBriefingNotification, but placing it adjacent to DigestNotification exposes the interaction. PresidentialBriefingNotification likely predates the idea of Digest and Brief having separate in-app toggles — when it was written, enabling Brief from the in-app tab needed to clean up Digest because there was only one subscription slot. That assumption no longer holds.

The fix would be to remove the unsubscribePersonalizedDigest(Digest) from PresidentialBriefingNotification.onToggleBriefing, since the Brief and Digest subscriptions are now managed independently. But since PresidentialBriefingNotification is out of scope for this PR, at minimum consider whether the ordering (DigestNotification first, then PresidentialBriefingNotification) mitigates or worsens the risk, and whether a follow-up task should address the cleanup.


2. DigestNotification.tsx — cross-channel email check uses BriefingReady.email, which could be subscribed for Brief (not Digest) [DigestNotification.tsx:83–86]

const emailActive = ns?.[NotificationType.BriefingReady]?.email === 'subscribed';

if (isChecked && !emailActive) {
  unsubscribePersonalizedDigest({ type: UserPersonalizedDigestType.Digest });
}

For a non-Plus user this is correct: BriefingReady.email being subscribed means the user has email delivery set up for Digest (because they can only have Digest). For a Plus user with Brief email active, BriefingReady.email = subscribed — but this is for Brief delivery, not Digest. In that case, when the user turns off Digest in-app, emailActive = true (Brief email is subscribed), so unsubscribePersonalizedDigest(Digest) is skipped. This leaves an orphaned Digest subscription alongside the Brief subscription.

Whether this causes a real problem depends on whether the backend delivers digest emails based on the subscription record or based on BriefingReady.email. But it's worth auditing whether a Plus user with Brief email and Digest in-app gets both a digest and a briefing email. At minimum, the intent should be documented.


3. PersonalizedDigest.tsx change — correct and intentional [diff: lines 138–148]

The change from "always unsubscribe on email toggle off" to "only unsubscribe when in-app is also off" is the right cross-channel preservation logic:

// Before
unsubscribePersonalizedDigest({ type: UserPersonalizedDigestType.Digest });

// After
const digestInAppActive = ns?.[NotificationType.DigestReady]?.inApp === 'subscribed';
if (!digestInAppActive) {
  unsubscribePersonalizedDigest({ type: UserPersonalizedDigestType.Digest });
}

This mirrors the symmetric logic in DigestNotification.tsx (check email before unsubscribing from in-app). The cross-channel logic is consistent across both components. ✓


4. DigestNotification.tsxonLogToggle(isChecked, ...) logs the wrong event [DigestNotification.tsx:80]

const onToggleDigest = () => {
  toggleSetting(NotificationType.DigestReady, 'inApp');
  onLogToggle(isChecked, NotificationCategory.Digest); // ← passes current (pre-toggle) state

onLogToggle interprets its first arg as "is this now enabled":

event_name: isEnabled ? LogEvent.EnableNotification : LogEvent.DisableNotification

When user is disabling (isChecked = true), this fires EnableNotification. The correct call is onLogToggle(!isChecked, ...). This same bug exists in the pre-existing PersonalizedDigest.tsx:137 and the new component copies the pattern.

Fix this →


Summary

The core logic — mutual cross-channel preservation between DigestNotification (in-app) and PersonalizedDigest (email) — is sound. The main risk is the interaction between DigestNotification and the pre-existing PresidentialBriefingNotification now that they share the same page: Brief's toggle can silently destroy a Digest subscription. For the Plus-user segment this is a real issue worth addressing, either in this PR (modify PresidentialBriefingNotification.onToggleBriefing to not unsubscribe Digest when Digest in-app is enabled) or as an immediate follow-up.

Add missing companyLogo and callToAction fields to transformDigestAd
test expectations. Remove trailing blank line in types.ts.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Prevent the upgrade alert from flashing before the digest feed
content is ready by gating it on query completion state.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…cription toggles

DigestNotification could create a Digest subscription alongside an existing
Brief subscription for Plus users, causing double emails and posts. Now
enabling one type properly replaces the other and clears the stale in-app
preference across all three toggle surfaces (DigestNotification,
PresidentialBriefingNotification, PersonalizedDigest email radio).

Also fixes onLogToggle passing pre-toggle state in DigestNotification and
PersonalizedDigest, which logged the wrong event name.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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.

1 participant