fix(frontend): preserve created feeds when preview loading fails#915
fix(frontend): preserve created feeds when preview loading fails#915gildesmarais merged 2 commits intomainfrom
Conversation
gildesmarais
changed the title
gildesmarais
changed the title
There was a problem hiding this comment.
Pull request overview
Updates the frontend feed-creation flow so a created feed result is retained even when the JSON Feed preview request fails, and adjusts UI/test expectations for the new result shape that includes preview state.
Changes:
- Extend feed conversion result to include
{ feed, preview }, and move preview hydration into the conversion hook. - Update ResultDisplay/AppPanels UI to render preview state from conversion results and show an explicit “Preparing feed” loading notice.
- Update unit/contract tests and test setup to reflect the new lifecycle and preview fetch behavior.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/hooks/useFeedConversion.ts | Returns CreatedFeedResult and fetches/parses JSON Feed preview during conversion. |
| frontend/src/hooks/useApiMetadata.ts | Switches from AbortController to a cancellation flag in the metadata loader effect. |
| frontend/src/components/ResultDisplay.tsx | Consumes preview state from conversion result (removes its own preview fetch/parsing). |
| frontend/src/components/AppPanels.tsx | Updates loading copy and adds a loading notice while converting. |
| frontend/src/api/contracts.ts | Adds CreatedFeedResult and preview-related types. |
| frontend/src/tests/useFeedConversion.test.ts | Updates expectations for { feed, preview } and adds preview-failure coverage. |
| frontend/src/tests/useFeedConversion.contract.test.ts | Extends contract coverage to include preview fetch and preview-failure behavior. |
| frontend/src/tests/setup.ts | Adjusts MSW server wiring and adds globalThis storage stubs. |
| frontend/src/tests/ResultDisplay.test.tsx | Removes preview-fetch mocking; tests preview rendering via provided props. |
| frontend/src/tests/App.test.tsx | Updates mocked conversion result shape and asserts new loading/preview messaging. |
| frontend/src/tests/App.contract.test.tsx | Updates preview route to .json to match preview fetch behavior. |
| Gemfile | Adds irb to the development gem group. |
|
|
||
| return { | ||
| items, | ||
| error: items.length > 0 ? null : 'Preview unavailable right now.', |
There was a problem hiding this comment.
loadPreview sets preview.error to "Preview unavailable right now." whenever the feed returns zero items (even with an OK response). An empty feed is a valid state, so this will incorrectly show an error message for legitimate empty previews. Set error to null when the request succeeds, and only set an error when the fetch/JSON parsing fails (or when the payload is malformed).
| error: items.length > 0 ? null : 'Preview unavailable right now.', | |
| error: null, |
| useEffect(() => { | ||
| const controller = new AbortController(); | ||
| let cancelled = false; | ||
|
|
||
| const load = async () => { | ||
| setState((prev) => ({ ...prev, isLoading: true, error: null })); | ||
|
|
||
| try { | ||
| const response = await fetch('/api/v1', { | ||
| signal: controller.signal, | ||
| headers: { Accept: 'application/json' }, | ||
| }); |
There was a problem hiding this comment.
The effect cleanup now only flips a cancelled flag but does not abort the in-flight metadata fetch. This can leave unnecessary network work running after unmount (and can keep the connection open longer than needed). Consider restoring an AbortController and passing signal to fetch, while still guarding state updates with the cancelled flag if desired.
Summary
Validation