Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 관리자가 홈 배너를 효율적으로 관리할 수 있도록 새로운 배너 관리 시스템을 도입합니다. 관리자는 별도의 코드 배포 없이 배너의 생성, 수정, 삭제, 노출 상태 변경 및 순서 조정을 직접 수행할 수 있습니다. 또한, 홈 화면의 캐러셀은 이제 서버에서 동적으로 배너 데이터를 가져와 표시함으로써 운영 유연성을 크게 향상시킵니다. 이를 통해 운영 변경 사항이 즉시 홈 화면에 반영될 수 있는 구조를 구축했습니다. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3bef495b98
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces banner management functionality to the admin page and integrates home screen banners with the API. While the overall implementation is solid, particularly the clean use of react-query and dnd-kit for complex UI elements like drag-and-drop reordering and inline editing on the admin page, a critical security vulnerability was identified. The application is susceptible to Stored Cross-Site Scripting (XSS) because banner redirect URLs from the API are rendered directly into the href attribute of anchor tags without proper protocol validation. This could allow a malicious or compromised administrator to execute arbitrary JavaScript in users' browsers. I've also left specific review comments regarding UX improvements and accessibility enhancements.
Address PR review feedback so banner links only use safe external URLs and the banner admin flow feels more responsive and consistent for operators.
📝 WalkthroughWalkthroughThis PR introduces a banner management system with API clients, TypeScript types, an admin page for CRUD and reordering operations, integration with the home page to display public banners, and supporting utilities for URL validation and alt text generation. Changes
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Keep browser and Playwright Node contexts aligned so CI can boot the app and tests even when workflow-level VITE variables are unset or blank.
Ensure Playwright setup workers populate base URL variables before importing user login helpers so CI auth bootstrap does not fail with empty workflow env values.
Provide a safe default backend base URL so Playwright auth setup can reach the API even when workflow-level VITE variables are blank in CI.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/ARouter.tsx`:
- Around line 115-120: The admin route registration for paths.admin.manageBanner
in ARouter.tsx currently exposes the page directly; wrap the lazy-loaded
Component with the existing PrivateRoute (or an Admin-only gate) so only
authenticated/admin users can access it: import and use PrivateRoute (or
AdminRoute) to guard the Component returned in the lazy loader (the Module
default exported Component) for the manageBanner route; ensure the guard checks
admin role/permissions consistent with PrivateRoute's API and mirrors how other
admin routes are protected.
In `@src/pages/Admin/ManageBanner/Page.tsx`:
- Around line 571-579: The UI currently treats a failed getAdminBanners fetch as
an empty list because the render branch checks only displayBanners.length and
editingRowId, causing the NoData component to show on load errors; add an
explicit fetch/error state (e.g., isLoading and loadError) driven by
getAdminBanners in the component that populates displayBanners, and change the
conditional around the NoData render to only show when loadError is falsy and
displayBanners.length === 0 (otherwise render an error message or retry action
when loadError is truthy). Update the data-loading code path that calls
getAdminBanners to set loadError on failure and ensure startCreate remains
available as the action for real empty lists while an error UI offers retry that
re-invokes getAdminBanners.
In `@src/pages/Home/Page.tsx`:
- Around line 51-93: The current JSX treats any non-loading falsy banners as an
empty-state, which hides load failures; update the render logic in Page.tsx to
distinguish an explicit banner load error from a true empty array by introducing
or using an existing error flag (e.g., bannerError) alongside isBannerLoading
and banners, and change the conditional that now uses "isBannerLoading ? ... :
banners && banners.length > 0 ? ..." so that when bannerError is truthy you
render an error state (or a retry UI) instead of NoData; ensure components
referenced (WaveLoading, EmblaCarousel, NoData) and helpers (getSafeExternalUrl,
getBannerAltText) remain unchanged and keep key usage on banner.id.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 49a1faaf-3f64-441e-b234-0805a019bfb9
📒 Files selected for processing (9)
src/apis/banner.tssrc/components/ARouter.tsxsrc/components/SideBar.tsxsrc/const/paths.tssrc/interface/banner.tssrc/pages/Admin/ManageBanner/Page.tsxsrc/pages/Home/Page.tsxsrc/pages/Home/components/EmblaCarousel.tsxsrc/utils/banner.ts
Keep the banner review fixes intact while dropping the follow-up changes that were added only to chase the unrelated E2E workflow failures.
Keep the new banner surfaces safe for operators and users by protecting the admin route and separating true empty states from failed banner fetches.
2 participants
관리자에서 홈 배너를 생성·수정·삭제·정렬할 수 있는 관리 화면을 추가했습니다
홈 캐러셀을 공개 배너 API 기반으로 전환했습니다
배너 링크 보안과 관리자 상호작용을 보강했습니다