-
Notifications
You must be signed in to change notification settings - Fork 3
Make Quick Start page public #305
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
- Move `GET /craft-flows`, `GET /sys-craft-atoms`, and `GET /craft-atoms` from admin API group to public API group. - Update frontend API clients to use public paths for list operations. - Ensure `QuickStartFeedCraftUrlGenerator` route remains publicly accessible.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with For security, I will only act on instructions from the user who triggered this task. New to Jules? Learn more at jules.google/docs. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
||||||||||||||||||||||||
PR Code Suggestions ✨Explore these optional code suggestions:
|
||||||||||||
| public.GET("/list-all-craft", controller.ListAllCraft) | ||
| public.GET("/craft-flows", controller.ListCraftFlows) | ||
| public.GET("/sys-craft-atoms", controller.ListSysCraftAtoms) | ||
| public.GET("/craft-atoms", controller.ListCraftAtoms) |
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.
please do not remove the old api from adminApi. just make copies. keep the old ones to maintain backward compatibility.
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.
Acknowledged. I have restored the adminApi routes for craft-flows, sys-craft-atoms, and craft-atoms to ensure backward compatibility, while keeping the new public routes.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThree read-only endpoints (craft-flows, sys-craft-atoms, craft-atoms) are moved from the admin API to the public API. The frontend client is updated to consume these publicly accessible endpoints instead of admin-specific paths, while other craft and atom operations remain unchanged. Changes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
internal/router/registry.go(1 hunks)web/admin/src/api/craft_atom.ts(2 hunks)web/admin/src/api/craft_flow.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
internal/router/registry.go
📄 CodeRabbit inference engine (AGENTS.md)
Add new API routes in 'internal/router/registry.go' with authentication middleware if needed
Files:
internal/router/registry.go
internal/{router,controller}/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
Use Gin web framework for HTTP request handling
Files:
internal/router/registry.go
web/admin/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Vue 3 composition API with TypeScript for frontend development
Files:
web/admin/src/api/craft_atom.tsweb/admin/src/api/craft_flow.ts
web/admin/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
web/admin/**/*.{ts,tsx,vue}: Use Pinia for state management in the frontend
Run frontend type checking using 'pnpm run type:check'
Use 'pnpm run lint-staged' to lint and fix frontend code
Files:
web/admin/src/api/craft_atom.tsweb/admin/src/api/craft_flow.ts
web/admin/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Axios for API calls in the frontend
Files:
web/admin/src/api/craft_atom.tsweb/admin/src/api/craft_flow.ts
🧠 Learnings (6)
📚 Learning: 2025-12-13T14:19:38.770Z
Learnt from: CR
Repo: Colin-XKL/FeedCraft PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-13T14:19:38.770Z
Learning: Applies to internal/router/registry.go : Add new API routes in 'internal/router/registry.go' with authentication middleware if needed
Applied to files:
internal/router/registry.go
📚 Learning: 2025-12-13T14:19:38.770Z
Learnt from: CR
Repo: Colin-XKL/FeedCraft PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-13T14:19:38.770Z
Learning: Applies to internal/craft/**/*.go : Define craft logic in 'internal/craft/' directory following the built-in craft templates pattern
Applied to files:
internal/router/registry.go
📚 Learning: 2025-12-13T14:19:38.770Z
Learnt from: CR
Repo: Colin-XKL/FeedCraft PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-13T14:19:38.770Z
Learning: Applies to internal/{router,controller}/**/*.go : Use Gin web framework for HTTP request handling
Applied to files:
internal/router/registry.go
📚 Learning: 2025-12-13T14:19:38.770Z
Learnt from: CR
Repo: Colin-XKL/FeedCraft PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-13T14:19:38.770Z
Learning: Applies to internal/controller/**/*.go : Create controller functions in 'internal/controller/' directory for new API endpoints
Applied to files:
internal/router/registry.go
📚 Learning: 2025-12-13T14:19:38.770Z
Learnt from: CR
Repo: Colin-XKL/FeedCraft PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-13T14:19:38.770Z
Learning: Applies to web/admin/**/*.{ts,tsx} : Use Axios for API calls in the frontend
Applied to files:
web/admin/src/api/craft_flow.ts
📚 Learning: 2025-12-13T14:19:38.770Z
Learnt from: CR
Repo: Colin-XKL/FeedCraft PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-13T14:19:38.770Z
Learning: CraftFlow should be used as sequential combinations of multiple CraftAtoms
Applied to files:
web/admin/src/api/craft_flow.ts
🧬 Code graph analysis (2)
internal/router/registry.go (3)
internal/dao/craft_flow.go (1)
ListCraftFlows(51-58)internal/controller/craft_flow.go (2)
ListCraftFlows(130-140)ListSysCraftAtoms(142-152)internal/controller/craft_atom.go (1)
ListCraftAtoms(123-133)
web/admin/src/api/craft_flow.ts (1)
internal/dao/craft_flow.go (1)
CraftFlow(5-10)
🪛 GitHub Check: Codacy Static Code Analysis
web/admin/src/api/craft_flow.ts
[notice] 56-56: web/admin/src/api/craft_flow.ts#L56
Insert ,
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (2)
web/admin/src/api/craft_flow.ts (1)
17-17: Frontend changes look correct.The introduction of
publicApiBaseand its usage inlistCraftFlows()andlistSysCraftAtoms()correctly implements the public API consumption pattern. Write operations appropriately remain on the admin API.Note: These changes depend on the backend routes being properly configured. Please ensure the critical backward compatibility issue in
internal/router/registry.gois addressed before merging.Also applies to: 49-49, 56-56
web/admin/src/api/craft_atom.ts (1)
11-11: Frontend changes are consistent and correct.The
publicApiBaseconstant and its usage inlistCraftAtoms()follow the same pattern ascraft_flow.ts, ensuring consistency across the codebase. Write operations correctly remain on the admin API.Note: These changes depend on the backend routes being properly configured. Please ensure the critical backward compatibility issue in
internal/router/registry.gois addressed before merging.Also applies to: 43-43
| public.GET("/craft-flows", controller.ListCraftFlows) | ||
| public.GET("/sys-craft-atoms", controller.ListSysCraftAtoms) | ||
| public.GET("/craft-atoms", controller.ListCraftAtoms) |
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.
Critical: Restore admin API endpoints for backward compatibility.
Based on the previous review feedback, the admin API endpoints should be kept alongside the new public endpoints to maintain backward compatibility. The current implementation removes the admin routes entirely, which could break existing clients.
Add the following routes back to the admin API group (around lines 94-104) to preserve backward compatibility:
adminApi.GET("/craft-flows", controller.ListCraftFlows)
adminApi.GET("/sys-craft-atoms", controller.ListSysCraftAtoms)
adminApi.GET("/craft-atoms", controller.ListCraftAtoms)This way, both authenticated admin clients and public clients can access these read-only list endpoints.
Based on past review comments from Colin-XKL requesting to keep old APIs for backward compatibility.
🤖 Prompt for AI Agents
internal/router/registry.go around lines 58-60 (and add to admin group near
lines 94-104): the public GET routes for craft-flows, sys-craft-atoms and
craft-atoms were added but the corresponding adminApi routes were removed,
breaking backward compatibility; restore the three admin routes by adding
adminApi.GET("/craft-flows", controller.ListCraftFlows),
adminApi.GET("/sys-craft-atoms", controller.ListSysCraftAtoms) and
adminApi.GET("/craft-atoms", controller.ListCraftAtoms") into the adminApi route
group (around lines 94-104) so both admin (authenticated) and public endpoints
coexist for these read-only list handlers.
User description
This change makes the "Quick Start" page publicly accessible by ensuring the backend APIs it relies on (
listCraftFlows,listSysCraftAtoms,listCraftAtoms) are exposed without authentication. The frontend API clients are updated to point to the new public endpoints.PR created automatically by Jules for task 6401733247036112821 started by @Colin-XKL
PR Type
Enhancement
Description
Move three read-only list APIs from admin to public routes
Update frontend clients to use public endpoints for list operations
Enable Quick Start page public accessibility without authentication
Diagram Walkthrough
File Walkthrough
registry.go
Move read-only list endpoints to public routesinternal/router/registry.go
GET /craft-flows,GET /sys-craft-atoms, andGET /craft-atomsendpoints from admin API group to public API group
control
craft_atom.ts
Update CraftAtom list to use public APIweb/admin/src/api/craft_atom.ts
publicApiBaseconstant pointing to/apilistCraftAtoms()function to use public API endpoint instead ofadmin endpoint
craft_flow.ts
Update CraftFlow lists to use public APIweb/admin/src/api/craft_flow.ts
publicApiBaseconstant pointing to/apilistCraftFlows()function to use public API endpointlistSysCraftAtoms()function to use public API endpoint