Flake: make starring non-optimistic

.rubocop_todo.yml

@@ -1,12 +1,12 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config --auto-gen-only-exclude --exclude-limit 400`
-# on 2026-04-07 03:24:43 UTC using RuboCop version 1.86.0.
+# on 2026-06-21 23:25:43 UTC using RuboCop version 1.87.0.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
 # versions of RuboCop, may require this file to be generated again.
 
-# Offense count: 3
+# Offense count: 4
 Capybara/RSpec/NegationMatcherAfterVisit:
   Exclude:
     - 'spec/system/account_setup_spec.rb'
@@ -50,7 +50,7 @@ Lint/NumberConversion:
     - 'spec/models/story_spec.rb'
 
 # Offense count: 1
-# Configuration parameters: CountComments, Max, CountAsOne, AllowedMethods, AllowedPatterns.
+# Configuration parameters: CountComments, Max, CountAsOne.
 Metrics/ClassLength:
   Exclude:
     - 'app/repositories/story_repository.rb'
@@ -116,7 +116,7 @@ RSpec/DescribeClass:
   Exclude:
     - 'spec/integration/feed_importing_spec.rb'
 
-# Offense count: 54
+# Offense count: 68
 # Configuration parameters: Max, CountAsOne.
 RSpec/ExampleLength:
   Exclude:
@@ -138,6 +138,7 @@ RSpec/ExampleLength:
     - 'spec/system/feed_show_spec.rb'
     - 'spec/system/feeds_index_spec.rb'
     - 'spec/system/good_job_spec.rb'
+    - 'spec/system/keyboard_shortcuts_spec.rb'
     - 'spec/system/starred_spec.rb'
     - 'spec/system/stories_index_spec.rb'
     - 'spec/tasks/remove_old_stories_spec.rb'
@@ -154,7 +155,7 @@ RSpec/LeakyLocalVariable:
     - 'spec/requests/stories_controller_spec.rb'
     - 'spec/utils/feed_discovery_spec.rb'
 
-# Offense count: 17
+# Offense count: 18
 # Configuration parameters: EnforcedStyle.
 # SupportedStyles: allow, expect
 RSpec/MessageExpectation:
@@ -166,7 +167,7 @@ RSpec/MessageExpectation:
     - 'spec/tasks/remove_old_stories_spec.rb'
     - 'spec/utils/i18n_support_spec.rb'
 
-# Offense count: 28
+# Offense count: 34
 # Configuration parameters: Max.
 RSpec/MultipleExpectations:
   Exclude:
@@ -176,7 +177,9 @@ RSpec/MultipleExpectations:
     - 'spec/repositories/feed_repository_spec.rb'
     - 'spec/repositories/story_repository_spec.rb'
     - 'spec/system/add_feed_spec.rb'
+    - 'spec/system/keyboard_shortcuts_spec.rb'
     - 'spec/system/starred_spec.rb'
+    - 'spec/system/stories_index_spec.rb'
     - 'spec/tasks/remove_old_stories_spec.rb'
     - 'spec/utils/feed_discovery_spec.rb'
     - 'spec/utils/i18n_support_spec.rb'

app/javascript/controllers/star_toggle_controller.ts

@@ -13,19 +13,22 @@ export default class extends Controller {
 
   iconTargets!: HTMLElement[];
 
-  toggle(): void {
-    this.starredValue = !this.starredValue;
+  async toggle(): Promise<void> {
+    const starred = !this.starredValue;
+
+    // eslint-disable-next-line camelcase
+    const response = await updateStory(this.idValue, {is_starred: starred});
+    if (!response.ok) {
+      throw new Error(`Failed to star story ${this.idValue}`);
+    }
+
+    this.starredValue = starred;
 
     let icon = "fa fa-star-o";
-    if (this.starredValue) { icon = "fa fa-star"; }
+    if (starred) { icon = "fa fa-star"; }
 
     for (const target of this.iconTargets) {
       target.className = icon;
     }
-
-    // eslint-disable-next-line camelcase
-    updateStory(this.idValue, {is_starred: this.starredValue}).catch(() => {
-      // Optimistic UI — ignore server errors
-    });
   }
 }

spec/javascript/controllers/star_toggle_controller_spec.ts

@@ -1,3 +1,4 @@
+import type {MockInstance} from "vitest";
 import {bootStimulus, getController} from "support/stimulus";
 import StarToggleController from "controllers/star_toggle_controller";
 import {assert} from "helpers/assert";
@@ -51,6 +52,17 @@ function iconTargets(): HTMLElement[] {
   return Array.from(document.querySelectorAll<HTMLElement>(iconSel));
 }
 
+function expectIcons(className: string): void {
+  for (const icon of iconTargets()) {
+    expect(icon.className).toBe(className);
+  }
+}
+
+function mockFetch(status = 200): MockInstance {
+  return vi.spyOn(globalThis, "fetch").
+    mockResolvedValue(new Response(null, {status}));
+}
+
 describe("toggle", () => {
   // eslint-disable-next-line vitest/no-hooks
   afterEach(() => {
@@ -59,32 +71,36 @@ describe("toggle", () => {
 
   it("flips icons from unstarred to starred", async () => {
     await setupController(false);
-    vi.spyOn(globalThis, "fetch").mockResolvedValue(new Response());
+    mockFetch();
 
-    controller().toggle();
+    await controller().toggle();
 
-    for (const icon of iconTargets()) {
-      expect(icon.className).toBe("fa fa-star");
-    }
+    expectIcons("fa fa-star");
   });
 
   it("flips icons from starred to unstarred", async () => {
     await setupController(true);
-    vi.spyOn(globalThis, "fetch").mockResolvedValue(new Response());
+    mockFetch();
+
+    await controller().toggle();
+
+    expectIcons("fa fa-star-o");
+  });
+
+  it("does not flip icons when the request fails", async () => {
+    await setupController(false);
+    mockFetch(500);
 
-    controller().toggle();
+    await expect(controller().toggle()).rejects.toThrow("Failed to star");
 
-    for (const icon of iconTargets()) {
-      expect(icon.className).toBe("fa fa-star-o");
-    }
+    expectIcons("fa fa-star-o");
   });
 
   it("calls fetch with the correct payload", async () => {
     await setupController(false);
-    const fetchSpy = vi.spyOn(globalThis, "fetch").
-      mockResolvedValue(new Response());
+    const fetchSpy = mockFetch();
 
-    controller().toggle();
+    await controller().toggle();
 
     expect(fetchSpy).toHaveBeenCalledWith(
       "/stories/42",

spec/system/keyboard_shortcuts_spec.rb

@@ -93,6 +93,7 @@ def create_story_and_visit(title:)
     login_as(default_user)
     create_story_and_visit(title: "My Story")
     send_keys("j", "s")
+    expect(page).to have_css(".story.cursor .story-starred .fa-star")
 
     visit(starred_path)
 

spec/system/stories_index_spec.rb

@@ -61,6 +61,7 @@ def star_story(story_title)
     login_as(default_user)
 
     star_story("My Story")
+    expect(page).to have_css(".story-actions .story-starred .fa-star")
 
     visit(starred_path)
     expect(page).to have_text("My Story")
@@ -71,6 +72,7 @@ def star_story(story_title)
     login_as(default_user)
 
     star_story("My Story")
+    expect(page).to have_css(".story-actions .story-starred .fa-star-o")
 
     visit(starred_path)
     expect(page).to have_no_text("My Story")