点呼ページの追加 #115
点呼ページの追加 #115
Conversation
|
Preview (prod) → https://115-prod.rucq-ui-preview.trapti.tech/ |
There was a problem hiding this comment.
Pull Request Overview
This PR implements a dedicated roll call page for camps, allowing camp organizers to share a direct link for participants to respond to attendance checks. The implementation includes real-time updates via Server-Sent Events (SSE) and a responsive UI for viewing and responding to roll calls.
Key changes:
- Added a new route
/:campname/rollcall/:rollcallIdfor dedicated roll call pages - Implemented real-time roll call functionality with SSE streams
- Enhanced user interface components with improved tooltip behavior and styling
Reviewed Changes
Copilot reviewed 12 out of 17 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/views/RollCallView.vue | Main roll call page component with real-time updates and user interaction |
| src/router/index.ts | Added new route for roll call pages |
| src/lib/rollCallStream.ts | Composable for managing SSE connections and roll call state |
| src/components/rollcall/UserResponse.vue | Component for displaying user responses with expandable user lists |
| src/components/generic/UserIcon.vue | Enhanced with improved tooltip behavior and timing controls |
| src/mocks/handlers/rollCalls.ts | Mock API handlers for roll call endpoints |
| src/components/generic/BackgroundPattern.vue | Fixed positioning with !important declaration |
| package.json | Added eslint-plugin-security dependency |
| eslint.config.js | Added security plugin and Vue template security rules |
| .env files | Added environment configuration for traQ channel paths |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| :href="channelUrl" | ||
| target="_blank" | ||
| rel="noopener noreferrer" |
There was a problem hiding this comment.
The rel attribute should include both noopener and noreferrer for security when opening external links, but the ESLint rule vue/no-template-target-blank should enforce this. Consider using rel="noopener noreferrer" explicitly or ensure the ESLint rule is properly configured.
There was a problem hiding this comment.
eslint.config.js にて確かに 'vue/no-template-target-blank' ルールが有効になっています
| const listenRollCallReactions = ( | ||
| rollCallId: number, | ||
| onEvent: (ev: RollCallReactionEvent) => void, | ||
| ) => { |
There was a problem hiding this comment.
The EventSource URL is constructed using string interpolation with rollCallId parameter. Consider validating or sanitizing the rollCallId parameter to prevent potential injection attacks, especially since this value comes from route parameters.
| ) => { | |
| ) => { | |
| // Validate rollCallId to prevent injection attacks | |
| if ( | |
| typeof rollCallId !== 'number' || | |
| !Number.isFinite(rollCallId) || | |
| !Number.isInteger(rollCallId) || | |
| rollCallId <= 0 | |
| ) { | |
| throw new Error('Invalid rollCallId'); | |
| } |
src/components/generic/UserIcon.vue
Outdated
| hoverTimer.value = window.setTimeout(() => { | ||
| showTooltip.value = true | ||
| }, 1000) |
There was a problem hiding this comment.
The hardcoded 1000ms delay should be extracted as a named constant to improve readability and maintainability. Consider defining const TOOLTIP_HOVER_DELAY = 1000 at the top of the script section.
| textColor?: string | ||
| }>() | ||
| const displayCount = 6 |
There was a problem hiding this comment.
The hardcoded display count should be extracted as a named constant with a descriptive name. Consider defining const MAX_VISIBLE_USER_COUNT = 6 to clarify its purpose.
| const displayCount = 6 | |
| const MAX_VISIBLE_USER_COUNT = 6 |
|
eslint.config.js の変更に関しては影響範囲が大きそうだったのでここでの導入を取りやめて #116 に分けました。合宿後にマージしても良さそう |
kitsne241
force-pushed
the
feat/add-rollcall-page
branch
from
56edf37 to
57216c2
Compare
closes #103