Tighten blog admin auth and validation handling - #52
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughRefactors ChangesAuth Logout: Response Return Type
Admin Auth Inline (blog-next)
Validation, Codegen, and Minor Features
Test Suite Expansion (blog-next)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/blog-next/app/admin/actions.ts (1)
26-30: ⚡ Quick winConsider extracting repeated auth check pattern into a helper.
The same 5-line authentication check and redirect pattern is repeated across all 9 actions. While the PR intentionally inlines these checks, a small helper could reduce duplication without losing clarity:
async function requireAuth() { const currentAuth = await auth() if (!currentAuth.authenticated || !currentAuth.user) { redirect('/login') } return currentAuth }Each action could then use
const currentAuth = await requireAuth().Also applies to: 58-62, 89-93, 101-105, 116-120, 132-136, 145-149, 157-161, 170-174
🤖 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 `@apps/blog-next/app/admin/actions.ts` around lines 26 - 30, Extract the repeated 5-line authentication check pattern that appears at lines 26-30, 58-62, 89-93, 101-105, 116-120, 132-136, 145-149, 157-161, and 170-174 in apps/blog-next/app/admin/actions.ts into a new helper function called requireAuth(). The helper should call auth(), verify currentAuth.authenticated and currentAuth.user are truthy (redirecting to '/login' if not), and return the currentAuth object. Replace all 9 occurrences of the repeated pattern with a single call to await requireAuth() and assign the result to currentAuth.
🤖 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 `@apps/blog-next/tests/edit-tag-page.test.mjs`:
- Around line 22-24: The vi.mock() call is using an incorrect path that does not
match the actual import path used by the page being tested. The page imports
updateTagAction from app/admin/actions.ts, but the mock is targeting
../app/admin/tags/actions.ts. Update the path in the vi.mock() call to match the
actual import path used by the page so that the mock for updateTagAction is
properly applied during the test.
In `@apps/blog-nuxt/app/pages/categories/`[slug].vue:
- Around line 3-5: The route slug parameter is being interpolated directly into
the fetch URL without proper encoding, which allows reserved characters like
forward slashes to break the URL structure. Fix this by wrapping
route.params.slug with encodeURIComponent() when constructing the URL in the
useFetch call, so that special characters are properly escaped before being used
in the API path.
---
Nitpick comments:
In `@apps/blog-next/app/admin/actions.ts`:
- Around line 26-30: Extract the repeated 5-line authentication check pattern
that appears at lines 26-30, 58-62, 89-93, 101-105, 116-120, 132-136, 145-149,
157-161, and 170-174 in apps/blog-next/app/admin/actions.ts into a new helper
function called requireAuth(). The helper should call auth(), verify
currentAuth.authenticated and currentAuth.user are truthy (redirecting to
'/login' if not), and return the currentAuth object. Replace all 9 occurrences
of the repeated pattern with a single call to await requireAuth() and assign the
result to currentAuth.
🪄 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: CHILL
Plan: Pro
Run ID: 7b49aee5-5283-4cdd-b857-b2423c3b9de8
📒 Files selected for processing (71)
apps/blog-next/app/admin/actions.tsapps/blog-next/app/admin/auth.tsapps/blog-next/app/admin/categories/[id]/edit/page.tsxapps/blog-next/app/admin/layout.tsxapps/blog-next/app/admin/page.tsxapps/blog-next/app/admin/posts/[id]/edit/page.tsxapps/blog-next/app/admin/posts/new/page.tsxapps/blog-next/app/admin/tags/[id]/edit/page.tsxapps/blog-next/app/api/auth/clerk/logout/route.tsapps/blog-next/app/api/auth/workos/logout/route.tsapps/blog-next/app/api/holo/health/route.tsapps/blog-next/app/api/verify-email/resend/route.tsapps/blog-next/app/register/page.tsxapps/blog-next/app/storage/[[...path]]/route.tsapps/blog-next/server/holo.tsapps/blog-next/server/lib/blog.tsapps/blog-next/tests/admin-actions.test.mjsapps/blog-next/tests/admin-categories-page.test.mjsapps/blog-next/tests/admin-dashboard-page.test.mjsapps/blog-next/tests/admin-posts-page.test.mjsapps/blog-next/tests/admin-tags-page.test.mjsapps/blog-next/tests/api-v1-routes.test.mjsapps/blog-next/tests/broadcast-auth-route.test.mjsapps/blog-next/tests/edit-category-page.test.mjsapps/blog-next/tests/edit-post-page.test.mjsapps/blog-next/tests/edit-tag-page.test.mjsapps/blog-next/tests/health-route.test.mjsapps/blog-next/tests/home-page.test.mjsapps/blog-next/tests/hosted-callback-routes.test.mjsapps/blog-next/tests/hosted-login-routes.test.mjsapps/blog-next/tests/hosted-logout-routes.test.mjsapps/blog-next/tests/hosted-register-routes.test.mjsapps/blog-next/tests/login-route.test.mjsapps/blog-next/tests/new-post-page.test.mjsapps/blog-next/tests/post-detail-page.test.mjsapps/blog-next/tests/register-page.test.mjsapps/blog-next/tests/reset-password-page.test.mjsapps/blog-next/tests/reset-password-route.test.mjsapps/blog-next/tests/run.mjsapps/blog-next/tests/super-admin-logout-route.test.mjsapps/blog-next/tests/super-admin-page.test.mjsapps/blog-next/tests/verify-email-resend-route.test.mjsapps/blog-next/tests/verify-email-route.test.mjsapps/blog-nuxt/app/pages/admin/index.vueapps/blog-nuxt/app/pages/admin/posts/[id]/edit.vueapps/blog-nuxt/app/pages/admin/posts/index.vueapps/blog-nuxt/app/pages/categories/[slug].vueapps/blog-nuxt/app/pages/index.vueapps/blog-nuxt/app/pages/tags/[slug].vueapps/blog-nuxt/server/api/auth/clerk/logout.post.tsapps/blog-nuxt/server/api/auth/workos/logout.post.tsapps/blog-nuxt/server/api/verify-email/resend.post.tsapps/blog-nuxt/server/lib/blog.tsapps/blog-sveltekit/src/routes/api/auth/clerk/logout/+server.tsapps/blog-sveltekit/src/routes/api/auth/workos/logout/+server.tsapps/blog-sveltekit/src/routes/api/verify-email/resend/+server.tspackages/auth-clerk/src/contracts.tspackages/auth-clerk/src/index.tspackages/auth-clerk/tests/package.test.tspackages/auth-workos/src/contracts.tspackages/auth-workos/src/index.tspackages/auth-workos/tests/package.test.tspackages/authorization/src/contracts.tspackages/cli/src/project/registry-svelte.tspackages/cli/src/project/registry.tspackages/cli/src/project/scaffold/framework-renderers.tspackages/cli/tests/authorization-registry.test.tspackages/core/tests/authorization-feature.test.tspackages/validation/src/contracts.tspackages/validation/tests/contracts.test.tstests/example-app-auth-flow.mjs
💤 Files with no reviewable changes (3)
- apps/blog-next/app/admin/auth.ts
- apps/blog-next/server/holo.ts
- apps/blog-nuxt/server/lib/blog.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests