Skip to content

Commit 3b1e647

Browse files
committed
Simplify feed creation review follow-ups
1 parent 90c4bfe commit 3b1e647

19 files changed

Lines changed: 128 additions & 605 deletions

app/web/api/v1/create_feed.rb

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ module V1
1212
module CreateFeed
1313
FEED_ATTRIBUTE_KEYS =
1414
%i[id name url feed_token public_url json_public_url created_at updated_at].freeze
15+
FEED_METADATA_KEYS =
16+
%i[id name url username feed_token public_url json_public_url].freeze
1517

1618
class << self
1719
# Creates a feed and returns a normalized API success payload.
@@ -138,7 +140,13 @@ def create_feed(params, account)
138140
feed_data = AutoSource.create_stable_feed(params.name, params.url, account)
139141
raise Html2rss::Web::InternalServerError, 'Failed to create feed' unless feed_data
140142

141-
feed_data.is_a?(FeedMetadata::Metadata) ? feed_data : FeedMetadata::Metadata.new(**feed_data)
143+
feed_data.is_a?(FeedMetadata::Metadata) ? feed_data : feed_metadata(feed_data)
144+
end
145+
146+
# @param feed_data [Hash]
147+
# @return [Html2rss::Web::Api::V1::FeedMetadata::Metadata]
148+
def feed_metadata(feed_data)
149+
FeedMetadata::Metadata.new(**feed_data.slice(*FEED_METADATA_KEYS))
142150
end
143151

144152
# @param feed_data [Html2rss::Web::Api::V1::FeedMetadata::Metadata]

app/web/api/v1/feed_metadata.rb

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ def metadata_attributes(attributes)
3434
name: attributes[:name],
3535
url: attributes[:url],
3636
username: attributes[:username],
37-
strategy: attributes[:strategy],
3837
feed_token: attributes[:feed_token],
3938
public_url: public_url(attributes[:feed_token]),
4039
json_public_url: json_public_url(attributes[:feed_token])
@@ -73,15 +72,14 @@ def to_h
7372

7473
##
7574
# Feed metadata contract used between creation services and API responses.
76-
Metadata = Data.define(:id, :name, :url, :username, :strategy, :feed_token, :public_url, :json_public_url) do
75+
Metadata = Data.define(:id, :name, :url, :username, :feed_token, :public_url, :json_public_url) do
7776
# @return [Hash{Symbol=>Object}]
7877
def to_h
7978
{
8079
id: id,
8180
name: name,
8281
url: url,
8382
username: username,
84-
strategy: strategy,
8583
feed_token: feed_token,
8684
public_url: public_url,
8785
json_public_url: json_public_url

app/web/feeds/source_resolver.rb

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,10 @@ def token_generator_input(url, strategy)
133133

134134
# @return [String]
135135
def default_strategy_name
136-
return Html2rss::Config.default_strategy_name.to_s if Html2rss::Config.respond_to?(:default_strategy_name)
136+
if Html2rss::Config.respond_to?(:default_strategy_name)
137+
configured = Html2rss::Config.default_strategy_name.to_s
138+
end
139+
return configured unless configured.to_s.strip.empty?
137140

138141
'auto'
139142
end

docs/README.md

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ To change or add API endpoints, follow this sequence:
7575
1. **Ruby Request Spec**: Define the new behavior or endpoint in `spec/html2rss/web/app_integration_spec.rb` or a dedicated request spec.
7676
2. **OpenAPI Generation**: Run `make openapi` inside the Dev Container to regenerate `public/openapi.yaml` from the spec metadata.
7777
3. **Verify Contract**: Run `make openapi-verify` and `make openapi-lint` to ensure the generated file matches the specs and is valid.
78-
4. **Frontend Client**: The frontend generated client in `frontend/src/api/generated` is updated by the build process.
78+
4. **Frontend Client**: Keep generated client artifacts in `frontend/src/api/generated` aligned with `public/openapi.yaml`.
7979

8080
Always verify the contract before committing API changes.
8181

@@ -218,19 +218,11 @@ Critical-path event families: auth, feed create, feed render, request errors.
218218
When `SENTRY_DSN` is present, Sentry is enabled. `BUILD_TAG` and `GIT_SHA` become the release identifier, and
219219
`RACK_ENV` becomes the environment tag.
220220

221-
First-15-minute triage checklist:
221+
Triage starts with the newest `feed.create`, `feed.render`, and `request.error` events. Confirm the release tag,
222+
route group, strategy, and outcome before deciding whether the failure is retryable, terminal, or user-facing.
222223

223-
1. Open the newest `feed.create`, `feed.render`, and `request.error` events.
224-
2. Confirm the release tag matches the deployed build.
225-
3. Check the breadcrumb trail for the failing path, strategy, and outcome.
226-
4. Decide whether the failure is retryable or terminal before paging the user-facing incident path.
227-
228-
Alert baseline:
229-
230-
1. Page on sustained `request.error` spikes in production.
231-
2. Page on a repeated `feed.render` failure burst, especially when success drops to zero for a route or strategy.
232-
3. Track the first recovery signal after fallback or retry succeeds so the incident can be closed quickly.
233-
4. Keep the initial threshold simple; tune after a few real incidents instead of pre-optimizing for every edge case.
224+
Alert on sustained production `request.error` spikes or repeated `feed.render` failures, then tune thresholds from
225+
real incidents.
234226

235227
---
236228

frontend/e2e/smoke.spec.ts

Lines changed: 41 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,11 @@ test.describe('frontend smoke', () => {
4242
await page.getByRole('button', { name: 'Back' }).click();
4343
await expect(page).toHaveURL(/#\/create(?:\?.*)?$/);
4444
await expect(page.getByRole('button', { name: 'Generate feed URL' })).toBeVisible();
45-
await expect(page.locator('.form-shell')).toHaveAttribute('data-state', 'idle');
45+
await expect(page.locator('.form-shell')).toHaveAttribute('data-state', 'create');
46+
await expect(page.getByLabel('Utilities')).toBeVisible();
4647
});
4748

48-
test('restores result deep links and shows a recovery state when snapshot is missing', async ({ page }) => {
49+
test('shows result after successful feed creation without snapshot recovery', async ({ page }) => {
4950
await page.route(/\/api\/v1$/, async (route) => {
5051
await route.fulfill({
5152
status: 200,
@@ -70,12 +71,13 @@ test.describe('frontend smoke', () => {
7071
});
7172
});
7273

73-
await page.addInitScript(() => {
74-
localStorage.setItem(
75-
'html2rss_feed_result_snapshot:generated-token',
76-
JSON.stringify({
77-
savedAt: '2026-04-05T09:00:00.000Z',
78-
result: {
74+
await page.route(/\/api\/v1\/feeds$/, async (route) => {
75+
await route.fulfill({
76+
status: 201,
77+
contentType: 'application/json',
78+
body: JSON.stringify({
79+
success: true,
80+
data: {
7981
feed: {
8082
id: 'feed-123',
8183
name: 'Example Feed',
@@ -86,45 +88,50 @@ test.describe('frontend smoke', () => {
8688
created_at: '2026-04-05T08:59:00.000Z',
8789
updated_at: '2026-04-05T09:00:00.000Z',
8890
},
89-
preview: {
90-
items: [
91-
{
92-
title: 'Sample preview item',
93-
excerpt: 'Current restore snapshots include preview content.',
94-
publishedLabel: 'April 5, 2026',
95-
url: 'https://example.com/articles/sample-preview-item',
96-
},
97-
],
98-
isLoading: false,
99-
},
100-
workflowState: 'preview_ready' as const,
101-
warnings: [],
10291
},
103-
})
104-
);
92+
}),
93+
});
94+
});
95+
96+
await page.route(/\/api\/v1\/feeds\/generated-token\.json$/, async (route) => {
97+
await route.fulfill({
98+
status: 200,
99+
contentType: 'application/feed+json',
100+
body: JSON.stringify({
101+
items: [
102+
{
103+
title: 'Sample preview item',
104+
content_text: 'Current preview fetch includes rendered content.',
105+
date_published: '2026-04-05T09:00:00.000Z',
106+
url: 'https://example.com/articles/sample-preview-item',
107+
},
108+
],
109+
}),
110+
});
105111
});
106112

107-
await page.goto('/#/result/generated-token');
113+
await page.addInitScript(() => {
114+
sessionStorage.setItem('html2rss_access_token', 'token-123');
115+
});
116+
117+
await page.goto('/');
118+
await page.getByLabel('Page URL').fill('https://example.com/articles');
119+
await page.getByRole('button', { name: 'Generate feed URL' }).click();
108120

109121
await expect(page.getByRole('heading', { name: 'Feed ready' })).toBeVisible();
110-
await expect(page.locator('.result-shell')).toHaveAttribute('data-state', 'ready');
122+
await expect(page.locator('.result-shell')).toHaveAttribute('data-state', 'result');
111123
await expect(page.getByText('Example Feed')).toBeVisible();
112124
await expect(page.getByRole('link', { name: 'Open feed' })).toBeVisible();
113125
await expect(page.getByRole('link', { name: 'Open JSON Feed' })).toBeVisible();
114126
await expect(page.getByRole('link', { name: 'Open in feed reader' })).toBeVisible();
115127
await expect(page.getByRole('button', { name: 'Create another feed' })).toBeVisible();
116128
await expect(page.getByText('Sample preview item')).toBeVisible();
117-
await expect(page.getByText('Current restore snapshots include preview content.')).toBeVisible();
118-
119-
await page.evaluate(() => {
120-
localStorage.removeItem('html2rss_feed_result_snapshot:missing-token');
121-
});
129+
await expect(page.getByText('Current preview fetch includes rendered content.')).toBeVisible();
122130

123131
await page.goto('/#/result/missing-token');
124132

125-
await expect(page.getByText('Saved result unavailable')).toBeVisible();
126-
await expect(page.getByText('Create a new feed link to continue.')).toBeVisible();
127-
await expect(page.getByRole('button', { name: 'Go to create' })).toBeVisible();
128-
await expect(page.locator('.result-recovery')).toHaveAttribute('data-state', 'failed');
133+
await expect(page.getByLabel('Page URL')).toBeVisible();
134+
await expect(page.getByText('Saved result unavailable')).toHaveCount(0);
135+
await expect(page.locator('.result-recovery')).toHaveCount(0);
129136
});
130137
});

frontend/src/__tests__/App.contract.test.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ describe('App contract', () => {
105105
await waitFor(() => {
106106
expect(screen.getByText('Feed ready')).toBeInTheDocument();
107107
expect(screen.getByText('Example Feed')).toBeInTheDocument();
108-
expect(document.querySelector('.result-shell')).toHaveAttribute('data-state', 'ready');
108+
expect(document.querySelector('.result-shell')).toHaveAttribute('data-state', 'result');
109109
expect(screen.getByLabelText('Feed URL')).toBeInTheDocument();
110110
expect(screen.getByRole('button', { name: 'Copy feed URL' })).toBeInTheDocument();
111111
expect(screen.getByRole('button', { name: 'Create another feed' })).toBeInTheDocument();

frontend/src/__tests__/App.test.tsx

Lines changed: 7 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@ describe('App', () => {
4747
const mockClearConversionError = vi.fn();
4848
const mockClearResult = vi.fn();
4949
const mockRetryPreviewFetch = vi.fn();
50-
const mockRestoreResult = vi.fn();
5150

5251
beforeEach(() => {
5352
vi.clearAllMocks();
@@ -91,7 +90,6 @@ describe('App', () => {
9190
clearError: mockClearConversionError,
9291
clearResult: mockClearResult,
9392
retryPreviewFetch: mockRetryPreviewFetch,
94-
restoreResult: mockRestoreResult,
9593
});
9694
});
9795

@@ -104,7 +102,7 @@ describe('App', () => {
104102
expect(screen.queryByRole('combobox')).not.toBeInTheDocument();
105103
expect(screen.getByLabelText('Utilities')).toBeInTheDocument();
106104
expect(screen.getByRole('link', { name: 'Bookmarklet' })).toBeInTheDocument();
107-
expect(document.querySelector('.form-shell')).toHaveAttribute('data-state', 'idle');
105+
expect(document.querySelector('.form-shell')).toHaveAttribute('data-state', 'create');
108106
});
109107

110108
it('keeps the page url field permissive enough for hostname-only input', () => {
@@ -170,61 +168,13 @@ describe('App', () => {
170168
});
171169
});
172170

173-
it('restores result state from local snapshot when opening a result deep link', async () => {
174-
globalThis.localStorage.setItem(
175-
'html2rss_feed_result_snapshot:generated-token',
176-
JSON.stringify({
177-
savedAt: '2026-04-05T09:00:00.000Z',
178-
result: {
179-
feed: {
180-
id: 'feed-123',
181-
name: 'Example Feed',
182-
url: 'https://example.com/articles',
183-
feed_token: 'generated-token',
184-
public_url: '/api/v1/feeds/generated-token',
185-
json_public_url: '/api/v1/feeds/generated-token.json',
186-
created_at: '2026-04-05T08:59:00.000Z',
187-
updated_at: '2026-04-05T09:00:00.000Z',
188-
},
189-
preview: {
190-
items: [],
191-
isLoading: false,
192-
},
193-
workflowState: 'preview_ready' as const,
194-
warnings: [],
195-
},
196-
})
197-
);
171+
it('shows the create flow when opening a stale result deep link', async () => {
198172
globalThis.history.replaceState({}, '', 'http://localhost:3000/#/result/generated-token');
199173

200174
render(<App />);
201175

202-
await waitFor(() => {
203-
expect(mockRestoreResult).toHaveBeenCalledTimes(1);
204-
expect(mockRestoreResult).toHaveBeenCalledWith(
205-
expect.objectContaining({
206-
feed: expect.objectContaining({ feed_token: 'generated-token' }),
207-
workflowState: 'preview_ready',
208-
})
209-
);
210-
});
211-
});
212-
213-
it('shows a recovery notice when result deep link has no snapshot to restore', async () => {
214-
globalThis.history.replaceState({}, '', 'http://localhost:3000/#/result/missing-token');
215-
216-
render(<App />);
217-
218-
await waitFor(() => {
219-
expect(screen.getByRole('heading', { name: 'Saved result unavailable' })).toBeInTheDocument();
220-
});
221-
222-
expect(screen.getByRole('alert')).toHaveClass('result-shell', 'result-recovery');
223-
expect(screen.getByRole('alert')).toHaveAttribute('data-state', 'failed');
224-
expect(document.querySelector('.result-recovery .ui-hero')).toBeInTheDocument();
225-
expect(document.querySelector('.result-recovery .layout-rail-reading')).toBeInTheDocument();
226-
expect(screen.getAllByRole('button')).toHaveLength(1);
227-
expect(screen.getByRole('button', { name: 'Go to create' })).toHaveClass('btn--primary');
176+
expect(screen.getByLabelText('Page URL')).toBeInTheDocument();
177+
expect(screen.queryByRole('heading', { name: 'Saved result unavailable' })).not.toBeInTheDocument();
228178
});
229179

230180
it('shows inline token prompt when submitting without a token', async () => {
@@ -237,10 +187,10 @@ describe('App', () => {
237187

238188
expect(screen.getByText('Enter access token')).toBeInTheDocument();
239189
expect(globalThis.location.hash).toMatch(/^#\/token/);
240-
expect(document.querySelector('.form-shell')).toHaveAttribute('data-state', 'token_required');
190+
expect(document.querySelector('.form-shell')).toHaveAttribute('data-state', 'token_prompt');
241191
expect(screen.getByLabelText('Page URL')).toBeDisabled();
242192
expect(screen.queryByRole('combobox')).not.toBeInTheDocument();
243-
expect(screen.queryByLabelText('Utilities')).not.toBeInTheDocument();
193+
expect(screen.getByLabelText('Utilities')).toBeInTheDocument();
244194
expect(screen.getByRole('link', { name: 'Set up your own instance with Docker.' })).toBeInTheDocument();
245195
expect(screen.getByText('Required by this instance.')).toBeInTheDocument();
246196
expect(screen.queryByText('Paste an access token to keep going.')).not.toBeInTheDocument();
@@ -319,12 +269,11 @@ describe('App', () => {
319269
clearError: mockClearConversionError,
320270
clearResult: mockClearResult,
321271
retryPreviewFetch: mockRetryPreviewFetch,
322-
restoreResult: mockRestoreResult,
323272
});
324273

325274
render(<App />);
326275

327-
expect(document.querySelector('.result-shell')).toHaveAttribute('data-state', 'failed');
276+
expect(document.querySelector('.result-shell')).toHaveAttribute('data-state', 'result');
328277
expect(screen.getByRole('button', { name: 'Create another feed' })).toBeInTheDocument();
329278
expect(screen.getByRole('link', { name: 'Open feed' })).toBeInTheDocument();
330279
expect(screen.getByRole('link', { name: 'Bookmarklet' })).toBeInTheDocument();
@@ -354,7 +303,6 @@ describe('App', () => {
354303
clearError: mockClearConversionError,
355304
clearResult: mockClearResult,
356305
retryPreviewFetch: mockRetryPreviewFetch,
357-
restoreResult: mockRestoreResult,
358306
});
359307

360308
render(<App />);
@@ -372,7 +320,6 @@ describe('App', () => {
372320
clearError: mockClearConversionError,
373321
clearResult: mockClearResult,
374322
retryPreviewFetch: mockRetryPreviewFetch,
375-
restoreResult: mockRestoreResult,
376323
});
377324

378325
render(<App />);
@@ -563,7 +510,6 @@ describe('App', () => {
563510
clearError: mockClearConversionError,
564511
clearResult: mockClearResult,
565512
retryPreviewFetch: mockRetryPreviewFetch,
566-
restoreResult: mockRestoreResult,
567513
});
568514
mockUseAccessToken.mockReturnValue({
569515
token: 'saved-token',
@@ -604,7 +550,6 @@ describe('App', () => {
604550
clearError: mockClearConversionError,
605551
clearResult: mockClearResult,
606552
retryPreviewFetch: mockRetryPreviewFetch,
607-
restoreResult: mockRestoreResult,
608553
});
609554
mockUseAccessToken.mockReturnValue({
610555
token: 'saved-token',
@@ -643,7 +588,6 @@ describe('App', () => {
643588
clearError: mockClearConversionError,
644589
clearResult: mockClearResult,
645590
retryPreviewFetch: mockRetryPreviewFetch,
646-
restoreResult: mockRestoreResult,
647591
});
648592
mockUseAccessToken.mockReturnValue({
649593
token: 'saved-token',
@@ -682,7 +626,6 @@ describe('App', () => {
682626
clearError: mockClearConversionError,
683627
clearResult: mockClearResult,
684628
retryPreviewFetch: mockRetryPreviewFetch,
685-
restoreResult: mockRestoreResult,
686629
});
687630
mockUseAccessToken.mockReturnValue({
688631
token: 'saved-token',

0 commit comments

Comments
 (0)