feat: show tickets page#170
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughReplaces Epic references with a Tickets feature: adds ticket store, pages, components, server API and DB ordering; updates navigation and startapp routing; switches app refresh from epic to ticket store and reduces interval from 30000ms to 20000ms; adds RU i18n keys and a small landing-page icon. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant A as App (Nuxt)
participant TS as ticketStore
participant API as /api/ticket/list
participant Repo as repository.ticket
U->>A: Open Tickets UI
A->>TS: TS.update()
TS->>API: GET /api/ticket/list (Authorization: tma ...)
API->>Repo: listOpened()
Repo-->>API: Tickets ordered (updatedAt desc, messages createdAt desc)
API-->>TS: JSON tickets[]
TS-->>A: tickets state updated
A-->>U: Render Ticket list / details
Note over A,TS: Periodic refresh every 20s (was 30s)
sequenceDiagram
autonumber
actor U as User
participant R as startapp.vue
participant Nav as Router
U->>R: Open via tgWebAppStartParam
R->>R: switch(startParam.key)
alt key == "ticket"
R->>Nav: navigate to /ticket/{value}?query
else
R->>Nav: existing route handling (other keys)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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 |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (7)
apps/atrium-telegram/app/app.vue (1)
96-96: Consider the impact of reduced refresh interval.The refresh interval has been reduced from 30 seconds to 20 seconds. While this provides more frequent updates, it increases server load and client battery consumption.
- }, 20000) + }, 30000)Consider whether the 10-second reduction is necessary, or if adaptive polling based on user activity would be more efficient.
apps/atrium-telegram/app/components/TicketCard.vue (2)
17-17: Fix potential runtime error with optional chainingThe optional chaining on line 17 is inconsistent with the prop type. Since
ticketis a required prop of typeTicketWithDatawhich guaranteesmessagesis an array, the optional chaining is unnecessary. However, ifmessagescould be undefined in practice, you should useticket.messages?.length ?? 0to avoid displayingundefined.- <p>{{ ticket?.messages.length }}</p> + <p>{{ ticket.messages?.length ?? 0 }}</p>
24-24: Consider extracting date format string as a constantThe hardcoded Russian text "обновлен" in the format string makes the component less maintainable and not internationalization-friendly. Consider using the i18n system for this label.
- v-text="format(new Date(ticket.updatedAt), 'обновлен d MMMM yyyy', { locale: ru })" + v-text="`${$t('common.updated')} ${format(new Date(ticket.updatedAt), 'd MMMM yyyy', { locale: ru })}`"Also add the translation key to your i18n files:
{ "common": { "updated": "обновлен" } }apps/atrium-telegram/app/stores/ticket.ts (1)
22-24: Consider more explicit handling of empty responsesThe current check
if (!data)only handles falsy values but doesn't distinguish between an empty array (valid response with no tickets) and an actual error condition. Consider being more explicit about what constitutes a valid response.- if (!data) { - return - } + if (!Array.isArray(data)) { + console.warn('Unexpected response format from /api/ticket/list') + return + }apps/atrium-telegram/app/components/TicketMessage.vue (1)
119-130: Add proper TypeScript type safety for file type parameterThe
getFileIconfunction accepts a union type includingnull, but the implementation doesn't explicitly handle thenullcase (it falls through to default). While this works, it's better to be explicit for clarity and type safety.-function getFileIcon(type: 'image' | 'video' | 'document' | null) { +function getFileIcon(type: 'image' | 'video' | 'document' | null): string { switch (type) { case 'image': return 'i-lucide-image' case 'video': return 'i-lucide-video' case 'document': return 'i-lucide-file' + case null: default: return 'i-lucide-file' } }apps/atrium-telegram/app/pages/ticket/[ticketId]/index.vue (2)
20-20: Handle edge case for undefined ticket in pluralizationThe pluralization function receives
ticket?.messages.length ?? 0but the template also referencesticket?.messages.lengthdirectly without fallback, which could displayundefined.- {{ ticket?.messages.length }} {{ pluralizationRu(ticket?.messages.length ?? 0, ['сообщение', 'сообщения', 'сообщений']) }} + {{ ticket?.messages.length ?? 0 }} {{ pluralizationRu(ticket?.messages.length ?? 0, ['сообщение', 'сообщения', 'сообщений']) }}
44-58: Clean up commented codeThere's a large block of commented code for a drawer component. If this feature is planned for future implementation, consider adding a TODO comment explaining when it will be implemented. Otherwise, remove it to keep the codebase clean.
Either remove the commented code entirely or replace with a clear TODO:
- <!-- <UDrawer v-model:open="isDrawerOpened"> - <CreateCard - v-if="epic?.id" - :label="$t('app.create.epic-comment.button')" - icon="i-lucide-message-circle" - /> - - <template #body> - <FormCreateEpicComment - :epic-id="epic?.id ?? ''" - @submitted="isDrawerOpened = false" - @success="isDrawerOpened = false" - /> - </template> - </UDrawer> --> + <!-- TODO: Implement comment creation drawer when FormCreateTicketComment is ready -->
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
apps/atrium-telegram/app/app.vue(3 hunks)apps/atrium-telegram/app/components/TicketCard.vue(1 hunks)apps/atrium-telegram/app/components/TicketMessage.vue(1 hunks)apps/atrium-telegram/app/composables/useNavigation.ts(1 hunks)apps/atrium-telegram/app/pages/index.vue(1 hunks)apps/atrium-telegram/app/pages/startapp.vue(1 hunks)apps/atrium-telegram/app/pages/ticket/[ticketId]/index.vue(1 hunks)apps/atrium-telegram/app/pages/ticket/index.vue(1 hunks)apps/atrium-telegram/app/stores/ticket.ts(1 hunks)apps/atrium-telegram/i18n/locales/ru-RU.json(1 hunks)apps/atrium-telegram/server/api/ticket/list.get.ts(1 hunks)packages/database/src/repository/ticket.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
packages/database/src/repository/ticket.ts (1)
packages/database/src/tables.ts (2)
tickets(689-701)ticketMessages(703-719)
apps/atrium-telegram/app/stores/ticket.ts (3)
packages/database/src/repository/ticket.ts (2)
Ticket(6-76)update(61-71)packages/database/src/types.ts (1)
TicketMessage(182-182)packages/database/src/tables.ts (1)
tickets(689-701)
⏰ 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: build
🔇 Additional comments (11)
apps/atrium-telegram/app/pages/startapp.vue (1)
30-32: LGTM! The routing implementation is consistent.The new
ticketcase follows the established pattern and correctly routes to/ticket/${value}with the proper query parameter, maintaining consistency with existingflowandepicroutes.apps/atrium-telegram/app/pages/index.vue (1)
38-40: Stylistic addition for visual consistency.The centered icon matches the design pattern from the tickets page, providing visual consistency across the application.
apps/atrium-telegram/app/composables/useNavigation.ts (1)
19-23: Navigation update aligns with the tickets feature.The route replacement from
/epicto/ticketis consistent with the overall migration to a ticket-based system. The icon choicei-lucide-mail-question-markappropriately represents a ticketing system.packages/database/src/repository/ticket.ts (2)
20-20: Proper ordering for recent tickets first.Adding
desc(tickets.updatedAt)ensures tickets are displayed with most recently updated ones first, which is appropriate for a ticketing system.
22-24: Consistent message ordering.Sorting ticket messages by
createdAtin descending order maintains chronological consistency, showing the most recent messages first within each ticket.apps/atrium-telegram/i18n/locales/ru-RU.json (2)
21-21: Translation added for UI functionality.The "Показать еще" translation supports the show-more functionality mentioned in the AI summary.
26-26: Navigation label translation added.The "Тикеты" translation provides proper Russian localization for the tickets navigation item.
apps/atrium-telegram/app/app.vue (2)
63-63: Store migration from epic to ticket.The replacement of
useEpicStore()withuseTicketStore()aligns with the feature migration to tickets.
82-82: Store update calls consistent with migration.Both initial and periodic update calls now use
ticket.update()instead ofepic.update(), maintaining data consistency.Also applies to: 92-92
apps/atrium-telegram/app/pages/ticket/index.vue (1)
1-22: Well-structured ticket listing page.The implementation follows Vue 3 best practices with proper key binding, consistent styling with
motion-preset-slide-left, and appropriate component organization.apps/atrium-telegram/app/components/TicketMessage.vue (1)
132-134: Validate file URLs before openingSearches didn't reveal a shared URL sanitizer; add runtime validation to prevent javascript:, data:, or other unsafe schemes.
Location: apps/atrium-telegram/app/components/TicketMessage.vue lines 132–134
function handleFileClick(fileUrl: string) { + // Validate URL to prevent javascript: or data: URIs + try { + const url = new URL(fileUrl) + if (!['http:', 'https:'].includes(url.protocol)) { + console.error('Invalid file URL protocol') + return + } + } catch { + console.error('Invalid file URL') + return + } window.open(fileUrl, '_blank') }
| color: 'neutral', | ||
| disabled: false, | ||
| onSelect: () => navigator.clipboard.writeText(message.value?.text ?? ''), | ||
| condition: true, |
There was a problem hiding this comment.
🧩 Analysis chain
Ensure proper handling of undefined condition property
The TypeScript type DropdownMenuItem might not include a condition property based on the Nuxt UI types. This could cause TypeScript errors or runtime issues.
🌐 Web query:
Does the DropdownMenuItem type from @nuxt/ui include a condition property?
💡 Result:
No. The official DropdownMenuItem type in @nuxt/ui does not include a "condition" property — the documented item properties (label, icon, avatar, kbds, type, color, checked, disabled, slot, onSelect, onUpdateChecked, children, class, ui, etc.) do not list "condition". [1][2]
Remove/guard unknown "condition" on DropdownMenuItem objects
The @nuxt/ui DropdownMenuItem type does not include a "condition" property; keeping it in an object literal will cause TypeScript errors.
- Fix: apps/atrium-telegram/app/components/TicketMessage.vue — remove
condition: true(line 82) and compute/filter visibility before creating DropdownMenuItem objects. - Alternative: if you need the field, map items to the library's shape or explicitly cast/augment the type (e.g.,
as DropdownMenuItem & { condition?: boolean }or module augmentation).
🤖 Prompt for AI Agents
In apps/atrium-telegram/app/components/TicketMessage.vue around line 82, the
DropdownMenuItem objects include an unsupported "condition" property which
causes TypeScript errors; remove the `condition: true` entry from the object
literal and instead compute visibility before constructing the DropdownMenuItem
array (filter items based on your condition), or if you must keep the field for
internal logic map your items into the library's expected shape or explicitly
cast/augment the type (e.g., map to DropdownMenuItem & { condition?: boolean })
so the final objects passed to the DropdownMenu accept the correct type.
| const shownMessages = ref(10) | ||
| const messages = computed(() => ticket.value?.messages.slice(0, shownMessages.value)) | ||
| const isShowMore = computed<boolean>(() => messages.value?.length && ticket.value?.messages.length ? messages.value.length < ticket.value.messages.length : false) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify computed property logic and fix potential edge cases
The messages computed property could return undefined if ticket is not found, and the isShowMore logic is unnecessarily complex with redundant checks.
-const messages = computed(() => ticket.value?.messages.slice(0, shownMessages.value))
-const isShowMore = computed<boolean>(() => messages.value?.length && ticket.value?.messages.length ? messages.value.length < ticket.value.messages.length : false)
+const messages = computed(() => ticket.value?.messages.slice(0, shownMessages.value) ?? [])
+const isShowMore = computed<boolean>(() => {
+ if (!ticket.value?.messages.length) return false
+ return messages.value.length < ticket.value.messages.length
+})Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/atrium-telegram/app/pages/ticket/[ticketId]/index.vue around lines
74-76, the messages computed may return undefined when ticket or ticket.messages
is missing and isShowMore has redundant checks; change messages to always return
an array (e.g., use (ticket.value?.messages ?? []).slice(0,
shownMessages.value)) and simplify isShowMore to compare lengths against a safe
number (e.g., messages.value.length < (ticket.value?.messages?.length ?? 0)) so
it handles missing data and removes unnecessary conditional logic.
| if (error instanceof Error) { | ||
| if (error.message.includes('401')) { | ||
| // No session | ||
| } | ||
| if (error.message.includes('404')) { | ||
| // Not found | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Improve error handling implementation
The current error handling checks for status codes via string matching in error messages, which is fragile and non-standard. Additionally, the error handlers don't perform any actions, making them ineffective.
- } catch (error) {
- if (error instanceof Error) {
- if (error.message.includes('401')) {
- // No session
- }
- if (error.message.includes('404')) {
- // Not found
- }
- }
+ } catch (error: any) {
+ // Handle FetchError from $fetch
+ if (error?.data?.statusCode === 401 || error?.statusCode === 401) {
+ // No session - could redirect to login or show auth prompt
+ console.error('Authentication required for ticket access')
+ tickets.value = []
+ } else if (error?.data?.statusCode === 404 || error?.statusCode === 404) {
+ // Not found - likely means no tickets endpoint
+ console.error('Ticket endpoint not found')
+ tickets.value = []
+ } else {
+ // Log unexpected errors
+ console.error('Failed to fetch tickets:', error)
+ throw error // Re-throw to let caller handle if needed
+ }
}📝 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.
| if (error instanceof Error) { | |
| if (error.message.includes('401')) { | |
| // No session | |
| } | |
| if (error.message.includes('404')) { | |
| // Not found | |
| } | |
| } | |
| } | |
| } catch (error: any) { | |
| // Handle FetchError from $fetch | |
| if (error?.data?.statusCode === 401 || error?.statusCode === 401) { | |
| // No session - could redirect to login or show auth prompt | |
| console.error('Authentication required for ticket access') | |
| tickets.value = [] | |
| } else if (error?.data?.statusCode === 404 || error?.statusCode === 404) { | |
| // Not found - likely means no tickets endpoint | |
| console.error('Ticket endpoint not found') | |
| tickets.value = [] | |
| } else { | |
| // Log unexpected errors | |
| console.error('Failed to fetch tickets:', error) | |
| throw error // Re-throw to let caller handle if needed | |
| } | |
| } |
🤖 Prompt for AI Agents
In apps/atrium-telegram/app/stores/ticket.ts around lines 28-36, the code is
doing fragile string-matching on error.message for status codes and no-ops
inside those branches; replace this with robust type-aware checks (e.g., if the
error is an AxiosError or has error.response?.status) and switch on
error.response.status (401, 404) instead of message.includes; inside the 401
branch clear any auth/session state and trigger a logout or session-refresh flow
and log the event, and inside the 404 branch set a notFound flag or return a
specific error value so callers can handle it, while for other errors rethrow or
log the full error to preserve diagnostics.
| export default defineEventHandler(async () => { | ||
| return repository.ticket.listOpened() | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
Critical security concern: API endpoint lacks authentication.
This API endpoint is unprotected and exposes ticket data without any authentication checks. In Nuxt 3, you should implement authentication using server middleware or wrapper functions to validate user sessions before allowing access to sensitive data.
Based on the web search results showing Nuxt 3 authentication patterns, implement authentication protection:
+import { serverSupabaseUser } from '#supabase/server' // or your auth method
import { repository } from '@roll-stack/database'
-export default defineEventHandler(async () => {
+export default defineEventHandler(async (event) => {
+ // Verify authentication
+ const user = await serverSupabaseUser(event) // or your auth validation
+ if (!user) {
+ throw createError({
+ statusCode: 401,
+ statusMessage: 'Unauthorized'
+ })
+ }
+
return repository.ticket.listOpened()
})Alternatively, create a reusable protectRoute utility and apply it to endpoints that require authentication.
🏁 Script executed:
#!/bin/bash
# Check if other API endpoints in the project have authentication
rg -n --type=ts -C3 'defineEventHandler' apps/atrium-telegram/server/api/Length of output: 15387
Critical — protect ticket list endpoint (unauthenticated data exposure).
apps/atrium-telegram/server/api/ticket/list.get.ts returns repository.ticket.listOpened() with no auth check; other handlers rely on event.context.user (e.g. apps/atrium-telegram/server/api/notification/my.get.ts, apps/atrium-telegram/server/api/auth/me.get.ts). Require authentication — either make the route explicitly public and document/tests that decision, or enforce auth by switching to defineEventHandler(async (event) => { if (!event.context?.user) throw createError({ statusCode: 401, statusMessage: 'Unauthorized' }); return repository.ticket.listOpened(); }) or by applying the project’s existing protectRoute/middleware.
🤖 Prompt for AI Agents
In apps/atrium-telegram/server/api/ticket/list.get.ts around lines 3 to 5, the
handler returns repository.ticket.listOpened() with no authentication check
exposing ticket data; update the handler to require auth by accepting the event
parameter and either call the project’s existing protectRoute/middleware or
manually verify event.context?.user and if missing throw createError({
statusCode: 401, statusMessage: 'Unauthorized' }) before returning
repository.ticket.listOpened(); ensure tests or route docs are updated if you
intend to make the route explicitly public instead.
1 participant
Summary by CodeRabbit
New Features
Improvements
Localization