-
Notifications
You must be signed in to change notification settings - Fork 0
docs: improve document content with ai #103
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
|
📝 WalkthroughWalkthroughAdds CodeRabbit config and ESLint ignores, registers several AI agent skill references, updates Storybook static dirs, tweaks a realtime query invalidate call, and performs large documentation rewrites across multiple guides including public return-shape changes for several composables ( Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
@ginjou/core
@ginjou/nuxt
@ginjou/vue
@ginjou/with-directus
@ginjou/with-rest-api
@ginjou/with-supabase
@ginjou/with-vue-i18n
@ginjou/with-vue-router
commit: |
Deploying ginjou with
|
| Latest commit: |
3f214e3
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://119b03aa.ginjou.pages.dev |
| Branch Preview URL: | https://docs-improve-document-conten.ginjou.pages.dev |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #103 +/- ##
=======================================
Coverage 55.09% 55.09%
=======================================
Files 128 128
Lines 3142 3142
Branches 771 769 -2
=======================================
Hits 1731 1731
Misses 1252 1252
Partials 159 159 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Deploying ginjou-storybook with
|
| Latest commit: |
3f214e3
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://114b8f99.ginjou-storybook.pages.dev |
| Branch Preview URL: | https://docs-improve-document-conten.ginjou-storybook.pages.dev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@docs/content/1.guides/5.authorization.md`:
- Around line 165-178: In authz.access, the parameter named resource is being
shadowed by the local const resource = await getResource(params.id); rename one
of them (e.g., rename the parameter to resourceParam or the local variable to
fetchedResource) and update all uses in the function (including the getResource
call and the can check) so the function uses distinct, descriptive names; ensure
authz.access still calls getCurrentUser() and returns the same can expression
comparing user.department to the correctly named resource field and checking
action in ['read','edit'].
In `@docs/content/1.guides/8.resources.md`:
- Around line 136-163: The example references an undefined resourceMap inside
buildBreadcrumbs causing runtime errors; fix by providing a source for parent
lookups: either define a resourceMap (e.g., populate a Map keyed by resource id
from resolved.value.resources) or replace resourceMap.get(...) with a
composable/helper that resolves a parent by id (create and import a function
like findResourceById) and use it inside buildBreadcrumbs; ensure you reference
useResource/resolved and update buildBreadcrumbs to use the chosen parent-lookup
mechanism so current = <resourceMap or findResourceById>(current.meta?.parent)
is valid.
| const authz = { | ||
| access: async ({ resource, action, params }) => { | ||
| const user = await getCurrentUser() | ||
| const resource = await getResource(params.id) | ||
|
|
||
| return { | ||
| can: ( | ||
| user.department === resource.department | ||
| && ['read', 'edit'].includes(action) | ||
| ), | ||
| } | ||
| }, | ||
| } | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Variable shadowing: resource parameter conflicts with local variable.
At line 168, the local variable const resource = await getResource(params.id) shadows the resource parameter from the function signature (line 166). This creates confusion and potential bugs.
🔧 Suggested fix
const authz = {
access: async ({ resource, action, params }) => {
const user = await getCurrentUser()
- const resource = await getResource(params.id)
+ const resourceData = await getResource(params.id)
return {
can: (
- user.department === resource.department
+ user.department === resourceData.department
&& ['read', 'edit'].includes(action)
),
}
},
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const authz = { | |
| access: async ({ resource, action, params }) => { | |
| const user = await getCurrentUser() | |
| const resource = await getResource(params.id) | |
| return { | |
| can: ( | |
| user.department === resource.department | |
| && ['read', 'edit'].includes(action) | |
| ), | |
| } | |
| }, | |
| } | |
| ``` | |
| const authz = { | |
| access: async ({ resource, action, params }) => { | |
| const user = await getCurrentUser() | |
| const resourceData = await getResource(params.id) | |
| return { | |
| can: ( | |
| user.department === resourceData.department | |
| && ['read', 'edit'].includes(action) | |
| ), | |
| } | |
| }, | |
| } |
🤖 Prompt for AI Agents
In `@docs/content/1.guides/5.authorization.md` around lines 165 - 178, In
authz.access, the parameter named resource is being shadowed by the local const
resource = await getResource(params.id); rename one of them (e.g., rename the
parameter to resourceParam or the local variable to fetchedResource) and update
all uses in the function (including the getResource call and the can check) so
the function uses distinct, descriptive names; ensure authz.access still calls
getCurrentUser() and returns the same can expression comparing user.department
to the correctly named resource field and checking action in ['read','edit'].
| ```vue | ||
| <script setup lang="ts"> | ||
| import { useResource } from '@ginjou/vue' | ||
|
|
||
| const resolved = useResource() | ||
|
|
||
| // resolved.value will contain: | ||
| // - resource: The matched ResourceDefinition | ||
| // - action: 'list' | 'create' | 'edit' | 'show' | ||
| // - id: The record ID (for 'edit' or 'show' actions) | ||
| function buildBreadcrumbs() { | ||
| const crumbs = [] | ||
| let current = resolved.value?.resource | ||
|
|
||
| while (current) { | ||
| crumbs.unshift(current.name) | ||
| // Look up parent resource | ||
| current = resourceMap.get(current.meta?.parent) | ||
| } | ||
|
|
||
| return crumbs | ||
| } | ||
| </script> | ||
|
|
||
| <template> | ||
| <div> | ||
| <h1>Current Resource: {{ resolved?.resource.name }}</h1> | ||
| <p>Action: {{ resolved?.action }}</p> | ||
| <p v-if="resolved?.id"> | ||
| ID: {{ resolved.id }} | ||
| </p> | ||
| </div> | ||
| <nav class="breadcrumbs"> | ||
| <span v-for="crumb in buildBreadcrumbs()" :key="crumb"> | ||
| {{ crumb }} | ||
| </span> | ||
| </nav> | ||
| </template> | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Undefined variable resourceMap in breadcrumb example.
At line 149, the code references resourceMap.get(current.meta?.parent), but resourceMap is never defined, imported, or explained. Developers following this example will encounter a runtime error.
Consider one of these approaches:
💡 Option 1: Define resourceMap in the component
<script setup lang="ts">
import { useResource } from '@ginjou/vue'
const resolved = useResource()
+// Assuming resources are available from context or props
+const resourceMap = new Map(resources.map(r => [r.name, r]))
function buildBreadcrumbs() {💡 Option 2: Use a composable
<script setup lang="ts">
-import { useResource } from '@ginjou/vue'
+import { useResource, useResourceMap } from '@ginjou/vue'
const resolved = useResource()
+const resourceMap = useResourceMap()
function buildBreadcrumbs() {🤖 Prompt for AI Agents
In `@docs/content/1.guides/8.resources.md` around lines 136 - 163, The example
references an undefined resourceMap inside buildBreadcrumbs causing runtime
errors; fix by providing a source for parent lookups: either define a
resourceMap (e.g., populate a Map keyed by resource id from
resolved.value.resources) or replace resourceMap.get(...) with a
composable/helper that resolves a parent by id (create and import a function
like findResourceById) and use it inside buildBreadcrumbs; ensure you reference
useResource/resolved and update buildBreadcrumbs to use the chosen parent-lookup
mechanism so current = <resourceMap or findResourceById>(current.meta?.parent)
is valid.
Summary by CodeRabbit
Documentation
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.