Skip to content

feat: stories#2172

Open
stackingsaunter wants to merge 18 commits into
masterfrom
chore/stories-widget-dummy-preview
Open

feat: stories#2172
stackingsaunter wants to merge 18 commits into
masterfrom
chore/stories-widget-dummy-preview

Conversation

@stackingsaunter
Copy link
Copy Markdown
Member

@stackingsaunter stackingsaunter commented Mar 25, 2026

Summary

Adds backend plumbing and a StoriesWidget on Home that fetches a curated list of stories from the Alby API (getAlby/getalby.com#2247).

  • New GET /api/alby/stories for HTTP and Wails — proxies the Alby /api/stories endpoint.
  • StoriesWidget shows avatars in a horizontal strip at the top of the right Home column; viewed state is persisted in localStorage.
  • Clicking opens a modal with the YouTube embed and a CTA driven entirely by the API (label, url, openInNewTab).
  • Avatars: 64px ring with a small white gap before the logo, primary-yellow border when unviewed, muted when viewed.
  • CSP: img-src allows cdn.getalby-assets.com for the story logos; frame-src allows youtube-nocookie.com for the video embed.
  • Data fetching via useSWR + swrFetcher (project convention).
  • API sends a videoId; the hub composes the canonical youtube-nocookie.com/embed/<id>?autoplay=1&rel=0 URL locally.

Related PRs

  • getAlby/getalby.com#2247 — backend endpoint (merged & deployed)
  • getAlby/getalby.com#2568 — switches the API to expose videoId instead of a full videoUrl. Independent merge order; if either side ships first, the story modal degrades gracefully (no iframe).
  • chore: remove Home Alby Go and extension cards #2175 — removes the now-duplicate Alby Go and Alby Extension cards from Home. Should merge right after this PR (within the same hub release) so users never lose those affordances.

Test plan

  • go test ./alby ./api ./http ./wails
  • cd frontend && yarn tsc:compile
  • Stories renders at the top of Home's right column; opening each story plays the YouTube embed and the CTA opens the right destination (in-app vs new tab)

Summary by CodeRabbit

  • New Features

    • Stories widget on Home: horizontally scrollable stories, modal playback for video stories, and CTA actions (external or in-app).
    • Viewed stories are tracked and persisted locally so seen items remain marked.
  • Backend / API

    • New API method/endpoint to fetch stories and expose them to the frontend.
  • Security / Config

    • Content Security Policy updated to allow YouTube privacy-enhanced embeds and added image CDN for story assets.

Review Change Stack

Add stories endpoint plumbing for HTTP and Wails, wire the Home Stories card
to fetch from /api/alby/stories, and keep it first in the right column.

Made-with: Cursor
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 25, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds Story models and a GetStories service call, exposes it via API/HTTP routes, updates CSP for YouTube embeds, and implements a frontend StoriesWidget with localStorage persistence and YouTube embed normalization.

Changes

Stories feature

Layer / File(s) Summary
Models & interfaces
alby/models.go, api/models.go
Added exported StoryCta and Story structs and GetStories(ctx context.Context) ([]Story, error) to AlbyOAuthService and API interfaces.
Alby OAuth service implementation
alby/alby_oauth_service.go
Implemented albyOAuthService.GetStories performing HTTP GET to albyInternalAPIURL + "/stories" with 10s timeout, default headers, non-2xx error handling (includes response body), and JSON decode into []Story.
API layer method
api/api.go
Added api.GetStories(ctx) delegating to albyOAuthSvc.GetStories.
HTTP routing & handler
http/alby_http_service.go
Registered /api/alby/stories route and handler that calls service, returns 500 on error or 200 with stories.
CSP frame-src updates
http/http_service.go, frontend/vite.config.ts
Extended CSP frame-src to include https://www.youtube-nocookie.com in both middleware and dev-only vite injection; added dev img-src origin.
Frontend helpers
frontend/src/components/home/widgets/StoriesWidget.tsx, frontend/src/constants.ts
Added localStorage helpers, TypeScript shapes, YouTube embed URL helper, and localStorageKeys.homeStoriesViewed constant used by the widget.
Frontend StoriesWidget UI
frontend/src/components/home/widgets/StoriesWidget.tsx
New StoriesWidget: fetches /api/alby/stories, tracks viewed IDs, renders horizontal carousel and modal with conditional YouTube iframe and CTA rendering.
Frontend integration
frontend/src/screens/Home.tsx
Imported and inserted StoriesWidget into Home widget stack.

Sequence Diagram

sequenceDiagram
  participant User
  participant Frontend as StoriesWidget
  participant HTTP as Alby_HTTP_Handler
  participant API as API_Layer
  participant Service as AlbyOAuthService
  participant Alby as Alby_Internal_API

  User->>Frontend: mount / click story
  Frontend->>HTTP: GET /api/alby/stories
  HTTP->>API: GetStories(ctx)
  API->>Service: GetStories(ctx)
  Service->>Alby: HTTP GET /stories (10s timeout)
  Alby-->>Service: JSON []Story
  Service-->>API: []Story
  API-->>HTTP: []Story
  HTTP-->>Frontend: 200 OK + stories
  Frontend->>Frontend: read/write localStorage, normalize YouTube URL, open modal
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

Suggested reviewers

  • reneaaron

Poem

🐰✨ I hop to fetch the tales anew,
Little frames that hum and view,
I mark the seen and stash each dot,
A modal blooms where videos plot,
Enjoy the stories — bunny-approved!

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'feat: stories' is vague and generic, using a non-descriptive term that does not convey meaningful information about the changeset. Enhance the title to be more specific, such as 'feat: add StoriesWidget with backend integration' or 'feat: integrate Stories widget with Alby API endpoint' to better describe the main changes.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/stories-widget-dummy-preview

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (3)
frontend/src/components/home/widgets/StoriesWidget.tsx (2)

229-238: Consider restricting iframe to YouTube-only URLs.

getYouTubeEmbedUrl falls back to the original URL when parsing fails or the URL isn't recognized as YouTube. If videoUrl contains a non-YouTube URL, it will be embedded directly. Since the story data comes from the backend, verify that the CSP frame-src directive appropriately restricts allowed iframe sources.

💡 Alternative: Only render iframe for valid YouTube URLs
+function isYouTubeUrl(url: string): boolean {
+  try {
+    const host = new URL(url).hostname.replace(/^www\./, "");
+    return host === "youtu.be" || host.endsWith("youtube.com");
+  } catch {
+    return false;
+  }
+}

-              {activeStory.videoUrl && (
+              {activeStory.videoUrl && isYouTubeUrl(activeStory.videoUrl) && (
                 <div className="aspect-video w-full bg-black">
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/src/components/home/widgets/StoriesWidget.tsx` around lines 229 -
238, The iframe currently renders any activeStory.videoUrl because
getYouTubeEmbedUrl falls back to the original URL; update StoriesWidget to only
render the iframe when the URL is recognized as YouTube by using the output of
getYouTubeEmbedUrl (or a new isYouTubeUrl helper) and verify embedUrl is
non-empty before creating the iframe; reference activeStory.videoUrl and
getYouTubeEmbedUrl (or a new isYouTubeUrl) and ensure the component skips
rendering the iframe for non-YouTube URLs so only validated YouTube embeds are
used.

107-135: Use SWR and the typed request() helper for data fetching.

The coding guidelines specify using SWR for server state management and the typed request() helper for HTTP requests. This would provide automatic caching, revalidation, and type safety.

♻️ Suggested refactor using SWR
+import useSWR from "swr";
+import { request } from "src/utils/request";
+
+type StoryResponse = {
+  id: number;
+  title: string;
+  avatar: string;
+  videoUrl?: string;
+};

 export function StoriesWidget() {
-  const [stories, setStories] = React.useState<Story[]>([]);
-  const [isLoading, setIsLoading] = React.useState(true);
+  const { data, isLoading } = useSWR<StoryResponse[]>(
+    "/api/alby/stories",
+    request
+  );
+
+  const stories: Story[] = React.useMemo(
+    () =>
+      (data ?? []).map((story) => ({
+        id: String(story.id),
+        title: story.title,
+        avatar: story.avatar,
+        videoUrl: story.videoUrl,
+      })),
+    [data]
+  );
   const [activeStory, setActiveStory] = React.useState<Story | null>(null);
   const [viewedIds, setViewedIds] =
     React.useState<Set<string>>(loadViewedStoryIds);
-
-  React.useEffect(() => {
-    const loadStories = async () => {
-      try {
-        const response = await fetch("/api/alby/stories");
-        if (!response.ok) {
-          throw new Error(`Failed to fetch stories: ${response.status}`);
-        }
-        const payload = (await response.json()) as Array<{
-          id: number;
-          title: string;
-          avatar: string;
-          videoUrl?: string;
-        }>;
-        const mappedStories = payload.map((story) => ({
-          id: String(story.id),
-          title: story.title,
-          avatar: story.avatar,
-          videoUrl: story.videoUrl,
-        }));
-        setStories(mappedStories);
-      } catch {
-        setStories([]);
-      } finally {
-        setIsLoading(false);
-      }
-    };
-
-    void loadStories();
-  }, []);

As per coding guidelines: "Use SWR for server state management" and "Use the typed request() helper in frontend/src/utils/request.ts for HTTP requests".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/src/components/home/widgets/StoriesWidget.tsx` around lines 107 -
135, Replace the manual fetch inside the useEffect/loadStories with SWR and the
typed request() helper: import useSWR and request, call
useSWR('/api/alby/stories', () =>
request<Array<{id:number;title:string;avatar:string;videoUrl?:string}>>('/api/alby/stories')),
derive isLoading from the SWR status instead of setIsLoading, map the returned
payload (convert id to string and preserve title/avatar/videoUrl) and call
setStories or initialise stories from swr.data, and remove the loadStories async
function and its try/catch/finally block; ensure types match and handle the
empty or error case using SWR's error value.
alby/alby_oauth_service.go (1)

1374-1411: Wrap returned errors with context in GetStories.

Several returns propagate raw errors, which makes upstream debugging harder. Prefer contextual wrapping in this method.

♻️ Suggested change
  req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
  if err != nil {
  	logger.Logger.WithError(err).Error("Error creating request to stories endpoint")
- 	return nil, err
+ 	return nil, fmt.Errorf("create stories request: %w", err)
  }
@@
  body, err := io.ReadAll(res.Body)
  if err != nil {
  	logger.Logger.WithError(err).WithFields(logrus.Fields{
  		"url": url,
  	}).Error("Failed to read response body")
- 	return nil, errors.New("failed to read response body")
+ 	return nil, fmt.Errorf("read stories response body: %w", err)
  }
@@
  if err := json.Unmarshal(body, &stories); err != nil {
  	logger.Logger.WithFields(logrus.Fields{
  		"body":  string(body),
  		"error": err,
  	}).Error("Failed to decode stories API response")
- 	return nil, err
+ 	return nil, fmt.Errorf("decode stories response: %w", err)
  }

As per coding guidelines, **/*.go: Use error wrapping with fmt.Errorf("context: %w", err) for debugging.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@alby/alby_oauth_service.go` around lines 1374 - 1411, GetStories currently
returns raw errors from http.NewRequestWithContext, client.Do, io.ReadAll and
json.Unmarshal which loses context; update each return to wrap the underlying
error with fmt.Errorf including a short contextual prefix (e.g.
fmt.Errorf("GetStories: creating request: %w", err), fmt.Errorf("GetStories:
performing request: %w", err), fmt.Errorf("GetStories: reading response body:
%w", err), fmt.Errorf("GetStories: decoding stories response: %w", err)). Also
replace the plain errors.New("failed to read response body") with a wrapped
error that includes the original read error, and keep the non-success status
error message descriptive (include status code and body) using fmt.Errorf.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@alby/alby_oauth_service.go`:
- Around line 1374-1411: GetStories currently returns raw errors from
http.NewRequestWithContext, client.Do, io.ReadAll and json.Unmarshal which loses
context; update each return to wrap the underlying error with fmt.Errorf
including a short contextual prefix (e.g. fmt.Errorf("GetStories: creating
request: %w", err), fmt.Errorf("GetStories: performing request: %w", err),
fmt.Errorf("GetStories: reading response body: %w", err),
fmt.Errorf("GetStories: decoding stories response: %w", err)). Also replace the
plain errors.New("failed to read response body") with a wrapped error that
includes the original read error, and keep the non-success status error message
descriptive (include status code and body) using fmt.Errorf.

In `@frontend/src/components/home/widgets/StoriesWidget.tsx`:
- Around line 229-238: The iframe currently renders any activeStory.videoUrl
because getYouTubeEmbedUrl falls back to the original URL; update StoriesWidget
to only render the iframe when the URL is recognized as YouTube by using the
output of getYouTubeEmbedUrl (or a new isYouTubeUrl helper) and verify embedUrl
is non-empty before creating the iframe; reference activeStory.videoUrl and
getYouTubeEmbedUrl (or a new isYouTubeUrl) and ensure the component skips
rendering the iframe for non-YouTube URLs so only validated YouTube embeds are
used.
- Around line 107-135: Replace the manual fetch inside the useEffect/loadStories
with SWR and the typed request() helper: import useSWR and request, call
useSWR('/api/alby/stories', () =>
request<Array<{id:number;title:string;avatar:string;videoUrl?:string}>>('/api/alby/stories')),
derive isLoading from the SWR status instead of setIsLoading, map the returned
payload (convert id to string and preserve title/avatar/videoUrl) and call
setStories or initialise stories from swr.data, and remove the loadStories async
function and its try/catch/finally block; ensure types match and handle the
empty or error case using SWR's error value.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3b5a8727-71ed-4ecf-a346-d0d0eee3a766

📥 Commits

Reviewing files that changed from the base of the PR and between c0e8df6 and f4e5311.

📒 Files selected for processing (11)
  • alby/alby_oauth_service.go
  • alby/models.go
  • api/api.go
  • api/models.go
  • frontend/src/components/home/widgets/StoriesWidget.tsx
  • frontend/src/screens/Home.tsx
  • frontend/vite.config.ts
  • http/alby_http_service.go
  • http/http_service.go
  • tests/mocks/AlbyOAuthService.go
  • wails/wails_handlers.go

- Add contextual actions in the story dialog (update hub with version,
  open Alby Go in-app, install extension) keyed by kind or title
- Use preview stories when the stories API request fails
- Pass hub version from useInfo into the update link

Made-with: Cursor
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
frontend/src/components/home/widgets/StoriesWidget.tsx (1)

341-348: Use proper router navigation for internal links.

The internal route /appstore/alby-go uses a raw <a> tag which causes a full page reload instead of client-side navigation. Consider using React Router's Link component for SPA navigation.

♻️ Suggested fix
+import { Link } from "react-router-dom";

                     return (
-                      <a
+                      <Link
-                        href={action.url}
+                        to={action.url}
                         className="inline-flex h-9 items-center rounded-md border border-white/15 px-4 text-sm font-medium text-white transition-colors hover:bg-white/10"
                       >
                         {action.label}
-                      </a>
+                      </Link>
                     );
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/src/components/home/widgets/StoriesWidget.tsx` around lines 341 -
348, In StoriesWidget replace the raw anchor used for internal navigation:
detect when action.url is an internal path (e.g. startsWith('/')) and render
React Router's Link instead of the <a> so navigation is client-side; preserve
the existing className, children (action.label) and other props, and import Link
from 'react-router-dom'; keep using a normal <a> for external URLs (and add
target="_blank" rel="noopener noreferrer" if appropriate) to avoid breaking
external behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@frontend/src/components/home/widgets/StoriesWidget.tsx`:
- Around line 189-218: The component currently uses manual fetch inside
loadStories and local useEffect/useState (setStories, isLoading) — replace that
with SWR and the typed request() helper: remove loadStories and effect, call
useSWR('/api/alby/stories', () =>
request<Array<{id:number;title:string;avatar:string;videoUrl?:string;kind?:string}>>('/api/alby/stories'))
in StoriesWidget, map the returned data to the existing story shape (id as
string, title, avatar, videoUrl, kind) before passing to setStories or directly
render from the mapped data, use fallbackData or onError to fall back to
previewStories, and derive loading state from SWR (e.g., !data && !error)
instead of setIsLoading.

---

Nitpick comments:
In `@frontend/src/components/home/widgets/StoriesWidget.tsx`:
- Around line 341-348: In StoriesWidget replace the raw anchor used for internal
navigation: detect when action.url is an internal path (e.g. startsWith('/'))
and render React Router's Link instead of the <a> so navigation is client-side;
preserve the existing className, children (action.label) and other props, and
import Link from 'react-router-dom'; keep using a normal <a> for external URLs
(and add target="_blank" rel="noopener noreferrer" if appropriate) to avoid
breaking external behavior.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 87ef3fc3-53be-42b1-b0e8-e9593b94b1d2

📥 Commits

Reviewing files that changed from the base of the PR and between f4e5311 and 27160e0.

📒 Files selected for processing (1)
  • frontend/src/components/home/widgets/StoriesWidget.tsx

Comment thread frontend/src/components/home/widgets/StoriesWidget.tsx Outdated
@reneaaron reneaaron marked this pull request as draft April 14, 2026 09:10
@reneaaron reneaaron force-pushed the chore/stories-widget-dummy-preview branch from f2e740b to dbd2569 Compare May 21, 2026 14:14
reneaaron and others added 4 commits May 21, 2026 16:14
- Widen modal and put video edge-to-edge with overlay close button
- Drop verbose header and 'Watch on YouTube' button
- Remove previewStories fallback so widget hides until upstream API ships
- Tighten title line-height
Move CTA copy and URLs into the API response. Hub renders story.cta
directly, so adding new story types no longer requires a hub release.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@reneaaron reneaaron marked this pull request as ready for review May 26, 2026 11:37
- Use react-router Link for in-tab CTA instead of plain <a>.
- Drop redundant www.youtube.com from frame-src (embeds always go through nocookie).
- Tighten stories endpoint status check from >= 300 to >= 400.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@reneaaron reneaaron force-pushed the chore/stories-widget-dummy-preview branch from 70ea053 to da3fd36 Compare May 26, 2026 11:56
reneaaron and others added 5 commits May 26, 2026 20:57
- Switch StoriesWidget to useSWR + swrFetcher (project convention).
- Guard story iframe with isYouTubeUrl so non-YouTube urls never embed.
- Wrap GetStories errors with fmt.Errorf("...: %w", err).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Stories are curated and always YouTube; the runtime check was
redundant. CSP frame-src still constrains the iframe source.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Alby API now sends canonical youtube-nocookie embed URLs with
autoplay/rel query strings (getAlby/getalby.com#2568), so the
runtime normalization is no longer needed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Match the avatar's size token; no magic numbers.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pairs with getAlby/getalby.com#2568. The API now sends just the
YouTube videoId; the hub composes the canonical embed URL so the
domain/query-string format stays in one place.

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

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
frontend/src/components/home/widgets/StoriesWidget.tsx (1)

135-135: ⚡ Quick win

Use theme/token classes instead of hardcoded style values.

rounded-[14px] and bg-zinc-950 hardcode design values. Prefer theme radius/color tokens (e.g., rounded-xl/rounded-2xl and semantic background tokens) to keep consistency with the rest of the UI system.

As per coding guidelines: "Prefer Tailwind utility classes over custom px definitions..." and "Use the theme system for colors, border-radius, shadows, and design tokens instead of hardcoding values".

Also applies to: 183-183

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/components/home/widgets/StoriesWidget.tsx` at line 135, The Card
component uses hardcoded design tokens (`rounded-[14px]` and `bg-zinc-950`)
which violates the theme token guideline; update the Card's className in
StoriesWidget (and the other occurrence noted) to use the theme/radius and
semantic color tokens (e.g., replace `rounded-[14px]` with `rounded-xl` or
`rounded-2xl` as appropriate and replace `bg-zinc-950` with the app's semantic
background token class like `bg-surface-xxx` or `bg-neutral-xxx`), keeping
`overflow-hidden` and `shadow-none` behavior intact so styling remains
consistent with the UI system.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@alby/alby_oauth_service.go`:
- Around line 1398-1404: The current status-code check in GetStories treats 3xx
redirects as success and leads to JSON parse errors; update the conditional that
currently checks if res.StatusCode >= 400 to instead fail for any non-2xx by
using if res.StatusCode < 200 || res.StatusCode >= 300, keep the same logging
via logger.Logger.WithFields (include "body" and "status_code") and return the
same formatted error from GetStories so callers get clear failure semantics for
non-2xx responses.

---

Nitpick comments:
In `@frontend/src/components/home/widgets/StoriesWidget.tsx`:
- Line 135: The Card component uses hardcoded design tokens (`rounded-[14px]`
and `bg-zinc-950`) which violates the theme token guideline; update the Card's
className in StoriesWidget (and the other occurrence noted) to use the
theme/radius and semantic color tokens (e.g., replace `rounded-[14px]` with
`rounded-xl` or `rounded-2xl` as appropriate and replace `bg-zinc-950` with the
app's semantic background token class like `bg-surface-xxx` or
`bg-neutral-xxx`), keeping `overflow-hidden` and `shadow-none` behavior intact
so styling remains consistent with the UI system.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: dcd03404-5eb7-4492-98de-9cb959b186fd

📥 Commits

Reviewing files that changed from the base of the PR and between 27160e0 and dc13ca8.

📒 Files selected for processing (9)
  • alby/alby_oauth_service.go
  • alby/models.go
  • api/api.go
  • api/models.go
  • frontend/src/components/home/widgets/StoriesWidget.tsx
  • frontend/src/screens/Home.tsx
  • frontend/vite.config.ts
  • http/alby_http_service.go
  • http/http_service.go
💤 Files with no reviewable changes (3)
  • http/http_service.go
  • frontend/vite.config.ts
  • http/alby_http_service.go

Comment thread alby/alby_oauth_service.go Outdated
The other status checks in alby_oauth_service.go all use >= 300;
align GetStories so redirects don't slip through.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@reneaaron reneaaron requested a review from im-adithya May 26, 2026 21:33
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
alby/alby_oauth_service.go (1)

1398-1398: 💤 Low value

Consider adding check for status codes below 200.

The status check now correctly catches 3xx redirects, but doesn't handle 1xx informational responses. While 1xx responses are rare in practice, checking res.StatusCode < 200 || res.StatusCode >= 300 would provide complete non-2xx coverage and align with the previous review suggestion.

♻️ Complete non-2xx check
-	if res.StatusCode >= 300 {
+	if res.StatusCode < 200 || res.StatusCode >= 300 {
 		logger.Logger.WithFields(logrus.Fields{
 			"body":        string(body),
 			"status_code": res.StatusCode,
 		}).Error("stories endpoint returned non-success code")
 		return nil, fmt.Errorf("stories endpoint returned %d: %s", res.StatusCode, string(body))
 	}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@alby/alby_oauth_service.go` at line 1398, Update the HTTP status validation
where the response `res` is checked: currently it only tests `if res.StatusCode
>= 300`; change it to reject all non-2xx responses by using `if res.StatusCode <
200 || res.StatusCode >= 300`. Locate the check around the `res.StatusCode`
usage in alby_oauth_service.go (the block that handles `res` after the request)
and adjust the condition so informational 1xx responses are treated as errors
alongside 3xx+ codes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@alby/alby_oauth_service.go`:
- Line 1398: Update the HTTP status validation where the response `res` is
checked: currently it only tests `if res.StatusCode >= 300`; change it to reject
all non-2xx responses by using `if res.StatusCode < 200 || res.StatusCode >=
300`. Locate the check around the `res.StatusCode` usage in
alby_oauth_service.go (the block that handles `res` after the request) and
adjust the condition so informational 1xx responses are treated as errors
alongside 3xx+ codes.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7e4a2c2f-b299-46d3-a5f6-e25bc01c7fcb

📥 Commits

Reviewing files that changed from the base of the PR and between dc13ca8 and 7720bd1.

📒 Files selected for processing (1)
  • alby/alby_oauth_service.go

};

function youTubeEmbedUrl(videoId: string) {
return `https://www.youtube-nocookie.com/embed/${videoId}?autoplay=1&rel=0`;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

are we supposed to be using youtube-nocookie instead of the official domain?

Comment thread frontend/src/components/home/widgets/StoriesWidget.tsx Outdated
<div className="relative flex size-full items-center justify-center overflow-hidden rounded-full bg-white dark:bg-muted">
<img
src={story.avatar}
alt={`${story.title} story`}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

for one of them the title wraps onto 2 lines and doesn't look so good / doesn't fit the others

@rolznz rolznz added this to the v1.23.0 milestone May 27, 2026
@reneaaron reneaaron requested a review from rolznz May 28, 2026 20:38
Address review feedback:
- Centralize the localStorage key for viewed stories in localStorageKeys
  alongside the other keys.
- Widen the story button from w-16 to w-20 so "Alby Extension" fits on
  one line and matches the other titles.

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

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
frontend/src/components/home/widgets/StoriesWidget.tsx (2)

217-244: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Title and CTA are hidden when videoId is absent, leaving an empty modal.

The whole title + CTA block at Line 217 is gated on activeStory.videoId. Since the backend Story.VideoID is omitempty (i.e., optional), clicking a story that has only a cta (or just a title) opens a modal with no iframe, no title, and no CTA. The PR notes the modal should "degrade gracefully" without a video, but currently it degrades to blank content.

Decouple the title/CTA section from the video presence so videoless stories still render meaningful content.

🐛 Proposed fix
-              {activeStory.videoId && (
-                <div className="flex items-center justify-between gap-3 px-6 py-4">
+              {(activeStory.title || activeStory.cta) && (
+                <div className="flex items-center justify-between gap-3 px-6 py-4">
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/components/home/widgets/StoriesWidget.tsx` around lines 217 -
244, The title/CTA block in StoriesWidget (the JSX using activeStory.videoId to
gate rendering) should not be hidden when a story lacks videoId; change the
condition so the title and CTA render whenever activeStory.title or
activeStory.cta exists (e.g., use a conditional on activeStory.title ||
activeStory.cta) instead of activeStory.videoId, keeping ExternalLink and Link
usage for cta.openInNewTab as-is; keep the video iframe rendering separately
gated by activeStory.videoId so videoless stories still show title/CTA while
stories with video show both.

233-240: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Avoid react-router Link for absolute external CTA URLs

React Router v7 Link is intended for in-app/client-side navigation and isn’t meant for absolute cross-origin URLs; if activeStory.cta.url is an external absolute URL and openInNewTab is false, passing it to Link to={...} can produce incorrect URL resolution. Render an <a href={activeStory.cta.url}> for absolute external destinations instead (and use rel="noopener noreferrer" when opening in a new tab).

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/components/home/widgets/StoriesWidget.tsx` around lines 233 -
240, In StoriesWidget (the rendering block using Link to={activeStory.cta.url}),
detect whether activeStory.cta.url is an absolute external URL and, for external
destinations, render a plain anchor element (<a href=...>) instead of
react-router's Link to avoid client-side routing issues; preserve the existing
className, target behavior (use target="_blank" when
activeStory.cta.openInNewTab is true), and add rel="noopener noreferrer" when
opening in a new tab. Ensure internal/relative URLs still use Link for client
navigation and that the CTA label and styling remain unchanged.
🧹 Nitpick comments (1)
frontend/src/components/home/widgets/StoriesWidget.tsx (1)

137-137: 💤 Low value

Hardcoded colors/border-radius instead of theme tokens.

rounded-[14px] (Line 137) and the hardcoded colors bg-zinc-950/text-white (Line 185), bg-black (Line 195), and bg-black/60 (Line 208) bypass the design system. Prefer theme radius tokens (e.g., rounded-xl) and theme color tokens/CSS variables. A consistently dark video surface can be expressed via theme tokens rather than literal zinc/black values.

As per coding guidelines: "Use the theme system for colors, border-radius, shadows, and design tokens; reference CSS variables and Tailwind theme tokens instead of hardcoding values".

Also applies to: 183-208

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/components/home/widgets/StoriesWidget.tsx` at line 137, The
component uses hardcoded Tailwind classes in StoriesWidget.tsx (e.g., Card with
class "rounded-[14px]" and elements using "bg-zinc-950", "text-white",
"bg-black", "bg-black/60") which bypasses the design system; replace these
literal values with theme tokens/CSS variables (for example use rounded-xl or
the project's radius token, and the theme color tokens or CSS variables like
var(--color-surface-dark), var(--color-text-on-dark), var(--color-overlay)) so
the Card and the video surface/styles reference the centralized tokens instead
of hardcoded colors and radii; update className usages on the Card and the
elements around lines showing bg-zinc-950/text-white/bg-black/bg-black/60 to use
the token names or Tailwind theme utility classes defined by the design system.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@frontend/src/components/home/widgets/StoriesWidget.tsx`:
- Around line 217-244: The title/CTA block in StoriesWidget (the JSX using
activeStory.videoId to gate rendering) should not be hidden when a story lacks
videoId; change the condition so the title and CTA render whenever
activeStory.title or activeStory.cta exists (e.g., use a conditional on
activeStory.title || activeStory.cta) instead of activeStory.videoId, keeping
ExternalLink and Link usage for cta.openInNewTab as-is; keep the video iframe
rendering separately gated by activeStory.videoId so videoless stories still
show title/CTA while stories with video show both.
- Around line 233-240: In StoriesWidget (the rendering block using Link
to={activeStory.cta.url}), detect whether activeStory.cta.url is an absolute
external URL and, for external destinations, render a plain anchor element (<a
href=...>) instead of react-router's Link to avoid client-side routing issues;
preserve the existing className, target behavior (use target="_blank" when
activeStory.cta.openInNewTab is true), and add rel="noopener noreferrer" when
opening in a new tab. Ensure internal/relative URLs still use Link for client
navigation and that the CTA label and styling remain unchanged.

---

Nitpick comments:
In `@frontend/src/components/home/widgets/StoriesWidget.tsx`:
- Line 137: The component uses hardcoded Tailwind classes in StoriesWidget.tsx
(e.g., Card with class "rounded-[14px]" and elements using "bg-zinc-950",
"text-white", "bg-black", "bg-black/60") which bypasses the design system;
replace these literal values with theme tokens/CSS variables (for example use
rounded-xl or the project's radius token, and the theme color tokens or CSS
variables like var(--color-surface-dark), var(--color-text-on-dark),
var(--color-overlay)) so the Card and the video surface/styles reference the
centralized tokens instead of hardcoded colors and radii; update className
usages on the Card and the elements around lines showing
bg-zinc-950/text-white/bg-black/bg-black/60 to use the token names or Tailwind
theme utility classes defined by the design system.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5cac3f4e-4824-4fb8-8c6e-2199d1aa2769

📥 Commits

Reviewing files that changed from the base of the PR and between 7720bd1 and a959276.

📒 Files selected for processing (2)
  • frontend/src/components/home/widgets/StoriesWidget.tsx
  • frontend/src/constants.ts

@reneaaron reneaaron changed the title feat: integrate Stories widget with backend endpoint feat: stories May 29, 2026
reneaaron and others added 2 commits May 29, 2026 09:00
w-20 still wrapped "Alby Extension"; w-24 fits all current titles
without truncation.

Co-Authored-By: Claude Opus 4.7 (1M context) <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.

3 participants