Fix unknown post media type handling#135
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (8)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (9)**/*.{vue,ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{vue,ts,tsx,js,jsx,css,json}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx,vue}📄 CodeRabbit inference engine (AGENTS.md)
Files:
app/components/**/*.vue📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.vue📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,vue}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.test.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.ts📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.test.ts📄 CodeRabbit inference engine (AGENTS.md)
Files:
🔇 Additional comments (13)
📝 WalkthroughWalkthroughAdds PostMediaType, IRenderablePost, and isRenderablePost; updates components and pages to use these narrowed types and filters fetched posts with isRenderablePost; adds a test ensuring saved-posts with missing/unknown media_type are not rendered and produce no runtime error. ChangesPost Renderability Filtering
Sequence DiagramsequenceDiagram
participant FetchAPI as Fetch API
participant Page as Posts Page
participant TypeGuard as isRenderablePost
participant Component as PostComponent
participant Render as Media Render
FetchAPI-->>Page: returns IPost[] (may include unknown/null media_type)
Page->>TypeGuard: filter each post
alt matches renderable types
TypeGuard-->>Page: narrows to IRenderablePost
Page->>Component: pass IRenderablePost
Component->>Render: render media using media_type
Render-->>Component: figure element
else unknown/null media_type
TypeGuard-->>Page: filtered out
end
Possibly Related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
✨ Simplify code
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. 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. Comment |
AlejandroAkbal
force-pushed
the
fix-unknown-media-type
branch
from
ebbd420 to
3460087
Compare
| // | ||
|
|
||
| return page.data.flatMap((post, index) => { | ||
| return page.data.filter(isRenderablePost).flatMap((post, index) => { |
There was a problem hiding this comment.
Is this one supposed to be replaced?
There was a problem hiding this comment.
Validated this one. The allRows renderability filter should stay because it narrows rows before they reach PostComponent; I did not extend it to live blocklist filtering because that breaks the existing “add tag to blocklist without closing the tag sheet” behavior.
| // | ||
|
|
||
| return page.data.flatMap((post, index) => { | ||
| return page.data.filter(isRenderablePost).flatMap((post, index) => { |
There was a problem hiding this comment.
This one should also stay. Premium saved posts do not have the normal posts-page select() filter, so the parent must filter non-renderable PocketBase records before passing rows into PostComponent.
| export type IRenderablePost = IPost & { media_type: PostMediaType } | ||
|
|
||
| export function isRenderablePost(post: IPost): post is IRenderablePost { | ||
| return post.media_type === 'image' || post.media_type === 'animated' || post.media_type === 'video' |
There was a problem hiding this comment.
Are there ways to make this more optimized?
There was a problem hiding this comment.
Checked this. The explicit comparisons are the cheapest/safest option here: no array allocation, no Set lookup overhead, and it keeps the type guard exhaustive for the currently allowed renderable values.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/pages/posts/[domain]/index.vue (1)
621-633:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep
allRowsaligned with the blocklist filter.Line 621 only reapplies
isRenderablePost, butallRowsis rebuilt fromdata.value.pages, while the blocklist filter inselect()only runs against the last fetched page. After loading page 2+, earlier pages are flattened from raw data again, so blocked posts can reappear in the rendered list. Apply the same visibility predicate here as well, or extract one shared helper and use it in both places.🤖 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 `@app/pages/posts/`[domain]/index.vue around lines 621 - 633, The allRows construction rebuilds from data.value.pages without applying the same blocklist/visibility predicate used in select(), so blocked posts reappear; fix by applying the same isRenderablePost predicate (or extract a shared helper used by select()) when flattening pages into allRows (i.e., when iterating data.value.pages and inside the page.data.filter(...) step) so both the last-fetched page and earlier pages use identical visibility logic (reference: allRows, isRenderablePost, select(), data.value.pages, page.data.filter).
🤖 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 `@test/pages/premium-cloud.test.ts`:
- Around line 151-188: The test "filters saved posts without a media type before
rendering post media" currently asserts pageErrors does not contain 'Unknown
media type: null' but misses the 'unknown' media_type branch; update the
assertion referencing the pageErrors variable to ensure no error message
contains the substring "Unknown media type" (e.g., replace the current
expect(pageErrors).not.toContain('Unknown media type: null') with an assertion
that verifies every entry in pageErrors does not include "Unknown media type" or
that pageErrors has no entries at all) so both invalid-media_type cases are
covered.
---
Outside diff comments:
In `@app/pages/posts/`[domain]/index.vue:
- Around line 621-633: The allRows construction rebuilds from data.value.pages
without applying the same blocklist/visibility predicate used in select(), so
blocked posts reappear; fix by applying the same isRenderablePost predicate (or
extract a shared helper used by select()) when flattening pages into allRows
(i.e., when iterating data.value.pages and inside the page.data.filter(...)
step) so both the last-fetched page and earlier pages use identical visibility
logic (reference: allRows, isRenderablePost, select(), data.value.pages,
page.data.filter).
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ee0a61c5-a15a-4cda-a307-58ece5f46f41
📒 Files selected for processing (8)
app/assets/js/post.dto.tsapp/components/pages/posts/post/PostChatWithAi.vueapp/components/pages/posts/post/PostComponent.vueapp/components/pages/posts/post/PostMedia.vueapp/pages/posts/[domain]/[tag].vueapp/pages/posts/[domain]/index.vueapp/pages/premium/saved-posts/[domain].vuetest/pages/premium-cloud.test.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Nuxt 4 with Vue 3 and TypeScript as the primary framework for SSR and Nitro server
Files:
test/pages/premium-cloud.test.tsapp/assets/js/post.dto.tsapp/components/pages/posts/post/PostChatWithAi.vueapp/pages/premium/saved-posts/[domain].vueapp/components/pages/posts/post/PostMedia.vueapp/pages/posts/[domain]/[tag].vueapp/pages/posts/[domain]/index.vueapp/components/pages/posts/post/PostComponent.vue
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Vitest with
@nuxt/test-utilsand Playwright browser mode for testing
Files:
test/pages/premium-cloud.test.ts
**/*.{vue,ts,tsx,js,jsx,css,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use Prettier for formatting with 120-char print width, no semicolons, single quotes, trailing commas removed, single attribute per line in Vue templates
Files:
test/pages/premium-cloud.test.tsapp/assets/js/post.dto.tsapp/components/pages/posts/post/PostChatWithAi.vueapp/pages/premium/saved-posts/[domain].vueapp/components/pages/posts/post/PostMedia.vueapp/pages/posts/[domain]/[tag].vueapp/pages/posts/[domain]/index.vueapp/components/pages/posts/post/PostComponent.vue
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
Use Nuxt flat ESLint via
@nuxt/eslintfor linting
Files:
test/pages/premium-cloud.test.tsapp/assets/js/post.dto.tsapp/components/pages/posts/post/PostChatWithAi.vueapp/pages/premium/saved-posts/[domain].vueapp/components/pages/posts/post/PostMedia.vueapp/pages/posts/[domain]/[tag].vueapp/pages/posts/[domain]/index.vueapp/components/pages/posts/post/PostComponent.vue
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: For URL validation/parsing, prefer URL.canParse() or URL.parse() over constructor try/catch; use URL.parse() when parsed URL object is needed
Use static URL.parse()/URL.canParse() from app/polyfills/url-static-methods.ts; do not re-add@vitejs/plugin-legacyURL polyfill config
Files:
test/pages/premium-cloud.test.tsapp/assets/js/post.dto.ts
**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.test.ts: Tests use@nuxt/test-utilswith Playwright inside describe blocks calling await setup({ browser: true })
For debug mode in tests, import debugBrowserOptions from test/helper.ts for headful playback with slowMo
Plain Vitest suites importing app modules directly do not get Nuxt alias resolution; keep modules importable through relative paths
@nuxt/test-utils$fetch has no .raw property; use fetch from@nuxt/test-utilswith { redirect: 'manual' } for redirect status and Location headers
Locale-related tests should import localeCodes, prefixedLocaleCodes, and removedLocaleCodes from config/i18n instead of hardcoding locale lists
Files:
test/pages/premium-cloud.test.ts
**/*.{ts,vue}
📄 CodeRabbit inference engine (AGENTS.md)
Use PocketBase batch writes for multi-record replacement/reorder operations instead of one HTTP write per changed row
Files:
test/pages/premium-cloud.test.tsapp/assets/js/post.dto.tsapp/components/pages/posts/post/PostChatWithAi.vueapp/pages/premium/saved-posts/[domain].vueapp/components/pages/posts/post/PostMedia.vueapp/pages/posts/[domain]/[tag].vueapp/pages/posts/[domain]/index.vueapp/components/pages/posts/post/PostComponent.vue
app/components/**/*.vue
📄 CodeRabbit inference engine (AGENTS.md)
Register components without path prefix using auto-imports; import them as not <Input/DomainSelector>
Files:
app/components/pages/posts/post/PostChatWithAi.vueapp/components/pages/posts/post/PostMedia.vueapp/components/pages/posts/post/PostComponent.vue
**/*.vue
📄 CodeRabbit inference engine (AGENTS.md)
**/*.vue: Schema.org breadcrumb item URLs should stay local/locale-relative; do not convert to project.urls.production
Use useLazyToast() to lazy-load vue-sonner and render ClientToaster; wait for mount before calling toast.* instead of relying on nextTick()
Keep interaction-gated loading for post UI features using Nuxt Lazy* components, dynamic imports, or deferred boundaries instead of first-load chunks
Keep@formkit/auto-animateroute-scoped unless used broadly; local vAutoAnimate imports on premium CSR pages saved ~3 KB gzip
For state immediately persisted, build reordered array synchronously instead of reading it before VueUse moveArrayElement() applies the move on nextTick
Use@nuxt/imagev2 module API for image preload priority with preload: { fetchPriority: 'high' } instead of patching rendered HTML in Nitro
PostMedia uses imgproxy for SSR post images; non-premium SPA navigations keep direct image path; validate image delivery where imgproxy resolves source URL
Files:
app/components/pages/posts/post/PostChatWithAi.vueapp/pages/premium/saved-posts/[domain].vueapp/components/pages/posts/post/PostMedia.vueapp/pages/posts/[domain]/[tag].vueapp/pages/posts/[domain]/index.vueapp/components/pages/posts/post/PostComponent.vue
🔇 Additional comments (6)
app/assets/js/post.dto.ts (1)
29-34: LGTM!app/components/pages/posts/post/PostComponent.vue (1)
4-4: LGTM!Also applies to: 12-12
app/components/pages/posts/post/PostMedia.vue (1)
2-2: LGTM!Also applies to: 21-21
app/components/pages/posts/post/PostChatWithAi.vue (1)
6-6: LGTM!Also applies to: 10-10
app/pages/posts/[domain]/[tag].vue (1)
5-5: LGTM!Also applies to: 77-81, 92-106
app/pages/premium/saved-posts/[domain].vue (1)
12-18: LGTM!Also applies to: 396-408
Fixes R34-APP-6KQ
AlejandroAkbal
force-pushed
the
fix-unknown-media-type
branch
from
3460087 to
8e6edf8
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
1 participant
Summary
Fixes R34-APP-6KQ
Verification
Summary by CodeRabbit
Bug Fixes
Tests