-
Notifications
You must be signed in to change notification settings - Fork 33
release: 8.2536.104 #2023
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
release: 8.2536.104 #2023
Conversation
…1956) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…1979) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
WalkthroughAdds S3 presigned-download support (backend endpoint/service and DTOs), extends image config with enablePresignedUrlDownload and bucket validation, switches bucket-listing to object-level checks, refactors feedback JSON update SQL, deep-clones AI feedback input to avoid mutation, updates web UI with presigned-aware FeedbackImage, and bumps Node/tooling images and deps. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Page as Feedback UI
participant Comp as FeedbackImage
participant API as Web API
participant Ctl as ChannelController
participant Srv as ChannelService
participant S3 as AWS S3
User->>Page: Open feedback with images
Page->>Comp: Render with url
Comp->>API: GET /api/admin/projects/{pid}/channels/{cid}
API-->>Comp: 200 { imageConfig{ enablePresignedUrlDownload, bucket, ... } }
alt presigned enabled & URL is S3
Comp->>API: GET /api/admin/projects/{pid}/channels/{cid}/image-download-url?imageKey=...
API->>Ctl: route
Ctl->>Srv: createImageDownloadUrl({ creds,bucket,imageKey })
Srv->>S3: Presign GetObject(bucket, key) (expires 60s)
S3-->>Srv: signed URL
Srv-->>Ctl: signed URL
Ctl-->>Comp: 200 "https://...signed..."
Comp-->>User: render image using presigned URL
else fallback
Comp-->>User: render image using original URL
end
sequenceDiagram
autonumber
participant Admin as Admin UI
participant API as Web API
participant Ctl as ChannelController
participant Srv as ChannelService
participant S3 as AWS S3
Admin->>API: Test/Save image config (includes bucket)
API->>Ctl: validate config
Ctl->>Srv: isValidImageConfig({ endpoint, region, bucket, ... })
Srv->>S3: ListObjects(bucket) (object-level check)
S3-->>Srv: 200 / error
Srv-->>Ctl: valid / invalid
Ctl-->>Admin: result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (2)
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/web/src/entities/feedback/ui/feedback-detail-sheet.ui.tsx (2)
87-122
: Fix: loading never set to true; edit mode resets on failure
- isLoading is never set to true before awaiting updateFeedback, so the spinner never shows and double-submits are possible.
- setMode('view') in finally closes edit mode even when the update fails, discarding user input.
Apply:
const onClickSubmit = async () => { - const editedFeedback = fields + setIsLoading(true); + const editedFeedback = fields .filter((v) => v.property === 'EDITABLE' && v.status === 'ACTIVE') .reduce((acc, cur) => { if (cur.key === 'issues') return acc; if (cur.format === 'date') { return { ...acc, [cur.key]: currentFeedback[cur.key] ? dayjs(currentFeedback[cur.key] as string).toISOString() : null, }; } if (cur.format === 'multiSelect') { return { ...acc, [cur.key]: currentFeedback[cur.key] ?? [], }; } if (cur.format === 'select') { return { ...acc, [cur.key]: currentFeedback[cur.key] ?? null, }; } return { ...acc, [cur.key]: currentFeedback[cur.key] }; - }, {} as Feedback); + }, {} as Partial<Feedback>); try { await updateFeedback?.(editedFeedback); - } finally { - setIsLoading(false); - setMode('view'); - } + setMode('view'); + } finally { + setIsLoading(false); + } };
49-58
: Type accuracy: updateFeedback should accept PartialeditedFeedback is a partial payload. Align the prop type.
interface Props { isOpen: boolean; close: () => void; fields: FieldInfo[]; feedback: Feedback; onClickDelete?: () => Promise<unknown>; - updateFeedback?: (feedback: Feedback) => Promise<unknown>; + updateFeedback?: (feedback: Partial<Feedback>) => Promise<unknown>; channelId: number; refetchFeedback?: () => Promise<unknown>; }apps/web/src/entities/channel/ui/image-config-form.ui.tsx (1)
86-87
: Remove hard-coded projectId=0; derive runtime projectId
- In apps/web/src/entities/channel/ui/image-config-form.ui.tsx (lines 86–87), change
- pathParams: { projectId: 0 }, + pathParams: { projectId },- In apps/web/src/entities/user/ui/invite-user-dialog.ui.tsx (line 53), change
- projectId: 0, + projectId,Ensure both components obtain
projectId
(e.g., viaconst { projectId } = useParams<{ projectId: string }>();
) before passing it touseOAIMutation
or default values.apps/api/src/domains/admin/channel/channel/channel.controller.ts (1)
16-28
: Enforce server-side flag and tighten error semantics for image download.
- Block presigned downloads when the channel has the feature disabled.
- Return NotFound when channel doesn’t belong to the given project to avoid leaking validity semantics via 400.
Apply this diff:
@@ -import { - BadRequestException, - Body, - Controller, - Delete, - Get, - Param, - ParseIntPipe, - Post, - Put, - Query, - UseGuards, -} from '@nestjs/common'; +import { + BadRequestException, + ForbiddenException, + NotFoundException, + Body, + Controller, + Delete, + Get, + Param, + ParseIntPipe, + Post, + Put, + Query, + UseGuards, +} from '@nestjs/common'; @@ async getImageDownloadUrl( @Param('projectId', ParseIntPipe) projectId: number, @Param('channelId', ParseIntPipe) channelId: number, @Query('imageKey') imageKey: string, ) { if (!imageKey) { throw new BadRequestException('imageKey is required in query parameter'); } const channel = await this.channelService.findById({ channelId }); - if (channel.project.id !== projectId) { - throw new BadRequestException('Invalid channel id'); - } - if (!channel.imageConfig) { + if (channel.project.id !== projectId) { + throw new NotFoundException('Channel not found'); + } + if (!channel.imageConfig) { throw new BadRequestException('No image config in this channel'); } + if (!channel.imageConfig.enablePresignedUrlDownload) { + throw new ForbiddenException('Presigned download is disabled for this channel'); + } - return await this.channelService.createImageDownloadUrl({ + return this.channelService.createImageDownloadUrl({ accessKeyId: channel.imageConfig.accessKeyId, secretAccessKey: channel.imageConfig.secretAccessKey, endpoint: channel.imageConfig.endpoint, region: channel.imageConfig.region, bucket: channel.imageConfig.bucket, imageKey, }); }Also applies to: 176-200
🧹 Nitpick comments (28)
apps/web/src/pages/tenant/create.tsx (3)
59-64
: Avoid hard reload after route replace
router.reload()
right afterrouter.replace(Path.SIGN_IN)
can cause unnecessary flicker and loses client state;replace
should suffice if the sign-in page fetches fresh data.Apply:
- async onSuccess() { - await router.replace(Path.SIGN_IN); - router.reload(); - toast.success(t('v2.toast.success')); - }, + async onSuccess() { + toast.success(t('v2.toast.success')); + await router.replace(Path.SIGN_IN); + },
55-65
: Surface failures to the userAdd
onError
to show a toast when tenant creation fails.queryOptions: { async onSuccess() { toast.success(t('v2.toast.success')); await router.replace(Path.SIGN_IN); }, + onError() { + toast.error(t('v2.toast.error')); + }, },
76-80
: Effect deps nitConsider including
router
in deps or explicitly disabling the lint rule to avoid noisy warnings.-}, [tenant]); +}, [tenant, router]);Or add an eslint-disable comment if intentional.
apps/web/src/shared/styles/global.css (2)
128-131
: Thumbnail track height increased to 70px — verify container spacing and overflow.70px aligns with 70px slide width (square thumbs), but check layouts where the thumbnail rail sits below .main-swiper to avoid overflow or clipping, especially in narrow sidepanels. Consider adding explicit gap/margin between main and thumbnail swipers if any overlap appears.
133-138
: Add keyboard focus styling and ensure contained images cover the slide area.Opacity alone is a weak affordance for selection/focus. Add a visible focus ring for accessibility and ensure images fill the 70×70 cell.
.thumbnail-swiper .swiper-slide { width: 70px; height: 100%; opacity: 0.4; @apply bg-neutral-tertiary; } + +/* Make thumbnails fully fill the cell when using <img> */ +.thumbnail-swiper .swiper-slide img { + width: 100%; + height: 100%; + object-fit: cover; + display: block; +} + +/* Visible keyboard focus */ +.thumbnail-swiper .swiper-slide:focus-visible { + outline: 2px solid currentColor; + outline-offset: 2px; +}docker/api.dockerfile (1)
3-3
: Version bump LGTM.
- No
docs/handbook/deploying-with-docker
directory found—update any relevant documentation as needed.- Optional: pin
node:22.19.0-alpine
with an image digest to prevent supply-chain drift.GUIDE.md (1)
26-26
: Clarify presigned-download behavior (TTL, privacy, and IAM).Add a short note that presigned URLs are time-limited, intended for private buckets, and require minimal IAM (GetObject) permissions; document where TTL is configured to prevent surprises.
apps/web/src/entities/feedback/ui/feedback-detail-sheet.ui.tsx (1)
148-151
: Nit: avoid double slash in copied URLUsing
${origin}/${pathname}
can create//
before the path.- `${window.location.origin}/${window.location.pathname}?queries=${JSON.stringify([...])}&channelId=${channelId}`, + `${window.location.origin}${window.location.pathname}?queries=${JSON.stringify([...])}&channelId=${channelId}`,apps/api/src/domains/admin/channel/channel/dtos/requests/image-config-request.dto.ts (1)
40-42
: Consider marking domainWhiteList optional tooSwagger marks it nullable, but without @IsOptional() validation may still fail when missing. Consider:
@ApiProperty({ nullable: true, type: [String] }) - @IsString({ each: true }) - domainWhiteList: string[]; + @IsOptional() + @IsString({ each: true }) + domainWhiteList?: string[];apps/api/src/domains/admin/project/ai/ai.service.ts (1)
539-540
: Clone only the mutable part to reduce overheadstructuredClone on the entire entity can be costly and may strip ORM internals. Only clone the
data
field instead:- feedback = structuredClone(feedback); + feedback = { + ...feedback, + data: structuredClone(feedback.data), + } as FeedbackEntity;apps/api/src/domains/admin/channel/channel/dtos/responses/image-config-response.dto.ts (1)
44-46
: Make the field truly optional and use ApiPropertyOptional for SwaggerPrefer an optional property plus ApiPropertyOptional for clearer typing/docs.
-import { ApiProperty } from '@nestjs/swagger'; +import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger'; @@ - @Expose() - @ApiProperty({ required: false, type: 'boolean' }) - enablePresignedUrlDownload: boolean | undefined; + @Expose() + @ApiPropertyOptional({ type: Boolean, description: 'Enable S3 presigned URL downloads' }) + enablePresignedUrlDownload?: boolean;apps/web/src/entities/channel/channel.schema.tsx (1)
26-26
: Default the toggle to false to simplify UI handlingA default avoids undefined checks across forms and consumers.
- enablePresignedUrlDownload: z.boolean().optional(), + enablePresignedUrlDownload: z.boolean().default(false),apps/web/src/entities/channel/ui/image-config-form.ui.tsx (1)
183-200
: Presigned URL toggle: add readOnly handling and localize option labels.
- Disable the control when readOnly.
- Prefer concise value derivation.
- Use i18n for "Enable/Disable".
Apply this diff:
<SelectInput - options={[ - { label: 'Enable', value: 'true' }, - { label: 'Disable', value: 'false' }, - ]} - value={ - typeof watch('enablePresignedUrlDownload') !== 'undefined' ? - String(watch('enablePresignedUrlDownload')) - : '' - } + options={[ + { label: t('v2.option.enable'), value: 'true' }, + { label: t('v2.option.disable'), value: 'false' }, + ]} + value={watch('enablePresignedUrlDownload')?.toString() ?? ''} placeholder={t('v2.placeholder.select')} onChange={(v) => setValue('enablePresignedUrlDownload', v === 'true', { shouldDirty: true, }) } + disabled={readOnly} label="Presigned URL Download" />apps/api/src/domains/admin/feedback/dtos/create-image-download-url.dto.ts (1)
17-24
: Add validation decorators to DTO fields.
Explicit validation helps avoid undefined/empty credentials and bad keys.Apply this diff:
+import { IsNotEmpty, IsString } from 'class-validator'; + export class CreateImageDownloadUrlDto { - accessKeyId: string; - secretAccessKey: string; - endpoint: string; - region: string; - bucket: string; - imageKey: string; + @IsString() @IsNotEmpty() + accessKeyId!: string; + @IsString() @IsNotEmpty() + secretAccessKey!: string; + @IsString() @IsNotEmpty() + endpoint!: string; + @IsString() @IsNotEmpty() + region!: string; + @IsString() @IsNotEmpty() + bucket!: string; + @IsString() @IsNotEmpty() + imageKey!: string; }packages/ufb-react/src/components/dialog.tsx (1)
104-124
: Overlay click handler may block expected click-to-close; also fix className composition.
- Stopping propagation on the Overlay can prevent closing on backdrop click (Radix closes on pointerDownOutside).
- Compose className outside cva variant call.
Apply this diff:
- <DialogOverlay onClick={(e) => e.stopPropagation()} /> + <DialogOverlay /> <DialogPrimitive.Content ref={ref} - className={cn( - dialogContentVariants({ radius: radius ?? themeRadius, className }), - )} + className={cn( + dialogContentVariants({ radius: radius ?? themeRadius }), + className, + )} {...props} >Verify desired behavior:
- Clicking the overlay should [close | not close] the dialog per your UX spec.
- If preventing close is intended, prefer handling
onPointerDownOutside={(e) => e.preventDefault()}
on Content instead of overlay click.apps/api/src/domains/admin/channel/channel/dtos/responses/find-channel-by-id-response.dto.ts (1)
40-41
: Make TS type optional (and nullable) to match Swagger required:false.Decorator marks it optional, but the TS type is still required. Align both to prevent confusion in callers/tests and reflect possible nulls.
- @ApiProperty({ required: false }) - imageConfig: ImageConfigResponseDto; + @ApiProperty({ required: false, nullable: true }) + imageConfig?: ImageConfigResponseDto | null;apps/api/src/domains/admin/channel/channel/channel.service.ts (1)
154-163
: Use HeadBucket for config validation instead of ListObjects.ListObjects requires s3:ListBucket and fails on buckets that disallow listing even when access is valid. HeadBucket is the conventional, least-privileged probe.
-import { - GetObjectCommand, - ListObjectsCommand, - PutObjectCommand, - S3Client, -} from '@aws-sdk/client-s3'; +import { + GetObjectCommand, + HeadBucketCommand, + PutObjectCommand, + S3Client, +} from '@aws-sdk/client-s3'; @@ - const command = new ListObjectsCommand({ Bucket: bucket }); + const command = new HeadBucketCommand({ Bucket: bucket });apps/web/src/shared/ui/sheet-detail-table.ui.tsx (1)
192-223
: Prefer stable keys for image list.Use URL as the key to avoid index-key pitfalls on reordering.
- {urls.map((v, index) => ( + {urls.map((v, index) => ( <div className="bg-neutral-tertiary relative h-16 w-16 overflow-hidden rounded" - key={index} + key={v} >apps/api/src/domains/admin/channel/channel/channel.controller.ts (1)
192-200
: Micro: avoid return-await.
return await
adds an extra promise tick without value here.Apply this diff (already included above, just calling out intent):
- return await this.channelService.createImageDownloadUrl({ + return this.channelService.createImageDownloadUrl({apps/web/src/shared/ui/feedback-image.tsx (3)
64-72
: Enable fetch only when router is ready and IDs are finite.Guard second query with the same router checks to prevent accidental early fires.
Apply this diff:
- queryOptions: { - enabled: Boolean( - imageKey && channelData?.imageConfig?.enablePresignedUrlDownload, - ), - }, + queryOptions: { + enabled: + router.isReady && + Number.isFinite(projectId) && + Number.isFinite(channelId) && + Boolean(imageKey && channelData?.imageConfig?.enablePresignedUrlDownload), + },
75-81
: Accessibility: avoid leaking URLs in alt; add sizes for fill images.Use an empty alt for decorative images and set
sizes
to prevent Next.js warnings withfill
.Apply this diff:
- <Image - src={presignedUrl ?? url} - alt={presignedUrl ?? url} + <Image + src={presignedUrl ?? url} + alt="" className="cursor-pointer object-cover" fill + sizes="100vw" onClick={() => onClick?.(presignedUrl ?? url)} />
43-62
: Optional: support non-AWS S3-compatible endpoints.If you use MinIO/other S3-compatible endpoints, compare
hostname
tochannelData.imageConfig.endpoint
and strip${bucket}/
from the path accordingly when hosts match.apps/web/src/shared/ui/image-preview-button.tsx (3)
66-74
: Avoid looping a single image.Looping with one slide creates duplicates; disable when
urls.length === 1
.Apply this diff:
- <Swiper - loop={true} + <Swiper + loop={urls.length > 1} spaceBetween={10}
75-83
: Use stable keys even when URLs repeat.Duplicate URLs will collide on key; include index.
Apply this diff:
- {urls.map((url) => ( - <SwiperSlide key={url} className="relative"> + {urls.map((url, idx) => ( + <SwiperSlide key={`${url}-${idx}`} className="relative">
96-101
: Same key fix for thumbnails.Apply this diff:
- {urls.map((url) => ( + {urls.map((url, idx) => ( <SwiperSlide - key={url} + key={`${url}-${idx}`} className="rounded-8 bg-neutral-secondary relative overflow-hidden" > <FeedbackImage url={url} />apps/web/src/shared/types/api.type.ts (3)
1807-1807
: Return a stable shape: make enablePresignedUrlDownload non-optional in responsePrefer always returning a boolean to avoid downstream optional checks. Update the spec and regenerate.
- enablePresignedUrlDownload?: boolean; + enablePresignedUrlDownload: boolean;
3744-3768
: Prefer structured response over bare string and define error responsesReturning a raw JSON string limits extensibility. Recommend a DTO (e.g., { url, expiresAt }) and adding 400/403 responses in the spec.
responses: { headers: { [name: string]: unknown; }; content: { - 'application/json': string; + 'application/json': components['schemas']['ImageDownloadUrlResponseDto']; }; }; + 400: { headers: { [name: string]: unknown }; content?: never; }; + 403: { headers: { [name: string]: unknown }; content?: never; }; };Add schema (in components.schemas) and regenerate:
ImageDownloadUrlResponseDto: { url: string; /** ISO timestamp when the URL expires */ expiresAt: string; };
1862-1862
: Align list DTO imageConfig optionalityIn apps/web/src/shared/types/api.type.ts (line 1814),
FindChannelsByProjectDto.imageConfig
is required while the single‐item DTO (FindChannelByIdResponseDto
at line 1862) marks it optional. Update the backend/OpenAPI spec for the list response (addrequired: false
on theimageConfig
property) and regenerate the client types.- imageConfig: components['schemas']['ImageConfigResponseDto']; + imageConfig?: components['schemas']['ImageConfigResponseDto'];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (34)
.nvmrc
(1 hunks)GUIDE.md
(1 hunks)apps/api/package.json
(4 hunks)apps/api/src/domains/admin/channel/channel/channel.controller.ts
(4 hunks)apps/api/src/domains/admin/channel/channel/channel.service.ts
(3 hunks)apps/api/src/domains/admin/channel/channel/dtos/requests/image-config-request.dto.ts
(2 hunks)apps/api/src/domains/admin/channel/channel/dtos/responses/find-channel-by-id-response.dto.ts
(1 hunks)apps/api/src/domains/admin/channel/channel/dtos/responses/image-config-response.dto.ts
(1 hunks)apps/api/src/domains/admin/feedback/dtos/create-image-download-url.dto.ts
(1 hunks)apps/api/src/domains/admin/feedback/dtos/image-upload-url-test.dto.ts
(1 hunks)apps/api/src/domains/admin/feedback/dtos/index.ts
(1 hunks)apps/api/src/domains/admin/feedback/feedback.mysql.service.ts
(1 hunks)apps/api/src/domains/admin/project/ai/ai.service.ts
(1 hunks)apps/e2e/package.json
(1 hunks)apps/web/package.json
(2 hunks)apps/web/src/entities/channel/channel.schema.tsx
(2 hunks)apps/web/src/entities/channel/ui/image-config-form.ui.tsx
(2 hunks)apps/web/src/entities/feedback/ui/feedback-cell.tsx
(2 hunks)apps/web/src/entities/feedback/ui/feedback-detail-sheet.ui.tsx
(1 hunks)apps/web/src/pages/tenant/create.tsx
(1 hunks)apps/web/src/shared/styles/global.css
(1 hunks)apps/web/src/shared/types/api.type.ts
(5 hunks)apps/web/src/shared/ui/feedback-image.tsx
(1 hunks)apps/web/src/shared/ui/image-preview-button.tsx
(3 hunks)apps/web/src/shared/ui/index.ts
(1 hunks)apps/web/src/shared/ui/sheet-detail-table.ui.tsx
(3 hunks)docker/api.dockerfile
(1 hunks)docker/web.dockerfile
(1 hunks)package.json
(2 hunks)packages/ufb-react/package.json
(1 hunks)packages/ufb-react/src/components/dialog.tsx
(1 hunks)pnpm-workspace.yaml
(1 hunks)tooling/eslint/package.json
(1 hunks)tooling/prettier/package.json
(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
apps/web/src/entities/feedback/ui/feedback-cell.tsx (2)
packages/ufb-react/src/components/tag.tsx (1)
Tag
(94-94)packages/ufb-react/src/components/icon.tsx (1)
Icon
(52-52)
apps/api/src/domains/admin/feedback/dtos/create-image-download-url.dto.ts (1)
apps/api/src/domains/admin/feedback/dtos/index.ts (1)
CreateImageDownloadUrlDto
(22-22)
apps/api/src/domains/admin/channel/channel/channel.service.ts (2)
apps/api/src/domains/admin/feedback/dtos/create-image-download-url.dto.ts (1)
CreateImageDownloadUrlDto
(17-24)apps/api/src/domains/admin/feedback/dtos/image-upload-url-test.dto.ts (1)
ImageUploadUrlTestDto
(17-23)
apps/web/src/shared/ui/image-preview-button.tsx (1)
packages/ufb-react/src/components/dialog.tsx (3)
Dialog
(235-235)DialogTrigger
(239-239)DialogContent
(240-240)
⏰ 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). (2)
- GitHub Check: publish-api-docs
- GitHub Check: integration-test
🔇 Additional comments (25)
apps/web/src/pages/tenant/create.tsx (1)
48-52
: LGTM: init state formatting-onlyOne-line initializer is fine; types remain intact.
.nvmrc (1)
1-1
: Verify CI workflows node-version pin and Docker bases
- package.json “engines.node” is “>=22.19.0” (aligned)
- No Dockerfiles detected in repo
- Ripgrep skipped hidden dirs; manually confirm in .github/workflows/* that actions/setup-node and any container images use node 22.19.0
docker/web.dockerfile (1)
3-3
: Node base image tags are aligned
Verified that both docker/api.dockerfile and docker/web.dockerfile usenode:22.19.0-alpine
. Confirm your CI build matrix is also using this exact tag to prevent cross-stage cache invalidation.apps/e2e/package.json (1)
10-13
: Playwright 1.55.0 & mysql2 3.14.4 bumps — verify CI runs browser install
Minor bumps are safe; ensure the CI/e2e pipeline runsnpx playwright install
to fetch browsers matching Playwright v1.55. I couldn’t locate any CI workflow definitions in the repo—please verify manually.pnpm-workspace.yaml (1)
7-7
: Verify ESLint plugins support ESLint 9.34.x
The workspace catalog bumps ESLint to^9.34.0
, but tooling/eslint/package.json pins these plugins:
• eslint-plugin-import
• eslint-plugin-jsx-a11y
• eslint-plugin-react
• eslint-plugin-react-compiler
• eslint-plugin-react-hooks
• eslint-plugin-turbo
Ensure each plugin’s peerDependencies accept ESLint>=9.34.0
to avoid peer warnings.tooling/eslint/package.json (1)
19-27
: ESLint toolchain bumps are consistent.typescript-eslint plugin and parser versions align at ^8.41.0 across all workspace manifests.
package.json (1)
4-8
: Approve packageManager and engines bump
Root package.json is the sole source ofpackageManager
/engines
,.nvmrc
matchesnode >=22.19.0
, and no workspace conflicts were found.tooling/prettier/package.json (2)
15-15
: Dependency bump looks good@ianvs/prettier-plugin-sort-imports -> ^4.6.3 is fine; no known breaking changes for our setup.
20-20
: Dev types bump is fine@types/node 22.18.0 aligns with our Node 22.x toolchain.
packages/ufb-react/package.json (2)
39-39
: LGTM: lucide-react bumpVersion bump to ^0.542.0 is non-breaking for icon names we use.
46-47
: LGTM: React type bumps@types/react and @types/react-dom minor bumps are safe.
apps/web/src/entities/channel/channel.schema.tsx (1)
43-43
: imageConfig now optional: verify downstream uses handle undefinedAudit places that access channel.imageConfig to ensure optional chaining or guards exist (e.g., form initial values, preview buttons).
apps/api/src/domains/admin/feedback/feedback.mysql.service.ts (1)
554-594
: Quote JSON path tokens and preserve JSON types
- Escape and quote keys in JSON_SET paths (e.g.
'$."escapedKey"'
) to handle dots, quotes, and other special characters and avoid ER_INVALID_JSON_PATH (dev.mysql.com, slingacademy.com).- Serialize each value with JSON.stringify and use
JSON_EXTRACT(:param, '$')
so booleans, nulls, arrays, and objects retain their JSON types instead of being coerced to numbers or strings (dev.mysql.com).If dotted notation is intended for nested updates, continue using
$.parent.child
; otherwise treat each fieldKey verbatim as shown.apps/web/src/entities/channel/ui/image-config-form.ui.tsx (1)
22-27
: Import of SelectInput looks correct.
No issues with the shared import additions.apps/web/package.json (1)
31-71
: Verify Next.js 15.4.7 and SWC core 1.13.5 upgrades
Your apps/web/package.json now pins next@^15.4.7 and @swc/core@^1.13.5—run your CI steps (pnpm -w install && pnpm --filter web typecheck && pnpm --filter web test && pnpm --filter web build
) to confirm no regressions from these version bumps.apps/web/src/shared/ui/index.ts (1)
54-55
: Expose FeedbackImage via shared UI index – looks good.
Unblocks downstream imports.apps/api/src/domains/admin/feedback/dtos/index.ts (1)
22-22
: Barrel export looks good.Re-export enables service/controller wiring for presigned downloads. No issues.
apps/web/src/shared/ui/sheet-detail-table.ui.tsx (1)
354-398
: Prevent preview/row clicks when deleting an image.Guard delete click with stopPropagation to avoid bubbling into surrounding handlers.
- <div className="flex h-5 w-5 cursor-pointer items-center justify-center rounded-full bg-[#FFFFFF80]"> + <div + className="flex h-5 w-5 cursor-pointer items-center justify-center rounded-full bg-[#FFFFFF80]" + onClick={(e) => e.stopPropagation()} + > <Icon name="RiDeleteBinLine" size={10} className="cursor-pointer text-white" onClick={() => { onChange?.( row.key, urls.filter((_, i) => i !== index), ); }} /> </div>apps/api/src/domains/admin/channel/channel/channel.controller.ts (2)
150-161
: LGTM: bucket is now validated in upload-url test.Propagating
bucket
intoisValidImageConfig
aligns the DTO and server validation path.
184-190
: No changes required
ChannelMySQLService.findById
includesrelations: { project: true }
, ensuringchannel.project.id
is always populated.apps/web/src/shared/ui/feedback-image.tsx (1)
32-42
: No action needed: wildcardremotePatterns
covers all hosts
Your Next.js config at apps/web/next.config.js already usesimages: { remotePatterns: [{ hostname: '*' }] }which permits images from any hostname—including
amazonaws.com
or custom S3 endpoints.apps/web/src/shared/ui/image-preview-button.tsx (2)
36-39
: Public API change looks good; initialIndex support is straightforward.
60-62
: All ImagePreviewButton usages include children; no self-closing calls found.apps/web/src/shared/types/api.type.ts (2)
695-710
: New GET /image-download-url path added — LGTMPath entry is consistent with existing patterns and correctly maps to the operation.
1756-1756
: New flag in ImageConfigRequestDto — verify default behaviorIf omitted, should this default to false? Please confirm server-side defaulting and update the OpenAPI spec with a default value to keep UI/BE behavior predictable.
apps/api/package.json
Outdated
"@aws-sdk/client-s3": "^3.876.0", | ||
"@aws-sdk/s3-request-presigner": "^3.876.0", | ||
"@fastify/multipart": "^9.0.3", |
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.
💡 Verification agent
🧩 Analysis chain
Jest 30 vs ts-jest 29: potential incompatibility; verify and align tooling
If ts-jest is still referenced, Jest 30.x will likely break. Either (a) upgrade ts-jest to a Jest 30–compatible version or (b) drop ts-jest and use @swc-node/jest exclusively, or (c) pin Jest back to 29.x. Also consider removing ts-jest if unused to avoid confusion.
Run to confirm whether ts-jest is configured anywhere:
Also applies to: 61-61, 64-64, 68-68, 79-79, 89-92, 97-99, 101-101, 106-106, 111-111
🏁 Script executed:
#!/bin/bash
# Search for ts-jest usage in jest configs and package.json scripts
rg -n -C2 -iP '(preset\s*:\s*[\'"]ts-jest[\'"]|transform\s*:\s*\{[^}]*ts-jest|ts-jest)' --glob '!**/node_modules/**'
# Show any Jest config files
fd -HI "^jest(\.config\.(js|cjs|mjs|ts|json)|\.json)?$" -t f
Length of output: 299
🏁 Script executed:
#!/bin/bash
set -e
# 1. Check for ts-jest in apps/api package.json
echo "Searching for ts-jest in apps/api/package.json..."
grep -R '"ts-jest"' -n apps/api/package.json || echo "No ts-jest entry in apps/api/package.json"
# 2. Search for any ts-jest references across the repo
echo "Searching for ts-jest references in repository..."
rg -F 'ts-jest' -n . --glob '!**/node_modules/**' || echo "No ts-jest references found"
# 3. List Jest config files
echo "Listing Jest configuration files..."
fd -HI "^jest(config(\\.cjs|\\.js|\\.mjs|\\.ts)|\\.json)$" -t f
Length of output: 1108
🏁 Script executed:
#!/bin/bash
set -e
echo "Extracting Jest-related devDependencies from apps/api/package.json..."
python3 - << 'EOF'
import json
with open('apps/api/package.json') as f:
data = json.load(f)
dev = data.get('devDependencies', {})
for name, version in dev.items():
if 'jest' in name.lower():
print(f"{name}: {version}")
EOF
Length of output: 244
Align ts-jest with Jest 30.x or remove ts-jest
ts-jest@29.4.1 in apps/api/package.json (line 110) mismatches jest@30.1.2; either upgrade ts-jest to a 30-compatible release, drop ts-jest (and update the transform in apps/api/test/jest-e2e.json and apps/api/integration-test/jest-integration.json to use @swc-node/jest), or pin Jest back to 29.x.
async getImageDownloadUrl( | ||
@Param('projectId', ParseIntPipe) projectId: number, | ||
@Param('channelId', ParseIntPipe) channelId: number, | ||
@Query('imageKey') imageKey: string, | ||
) { | ||
if (!imageKey) { | ||
throw new BadRequestException('imageKey is required in query parameter'); | ||
} | ||
const channel = await this.channelService.findById({ channelId }); | ||
if (channel.project.id !== projectId) { | ||
throw new BadRequestException('Invalid channel id'); | ||
} | ||
if (!channel.imageConfig) { | ||
throw new BadRequestException('No image config in this channel'); | ||
} | ||
|
||
return await this.channelService.createImageDownloadUrl({ | ||
accessKeyId: channel.imageConfig.accessKeyId, | ||
secretAccessKey: channel.imageConfig.secretAccessKey, | ||
endpoint: channel.imageConfig.endpoint, | ||
region: channel.imageConfig.region, | ||
bucket: channel.imageConfig.bucket, | ||
imageKey, | ||
}); | ||
} |
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.
Scope imageKey to a per-channel prefix to prevent cross-object access within shared buckets.
If the bucket is shared across channels/projects, allowing arbitrary imageKey
enables presigning unrelated objects. Validate that imageKey
starts with a channel/project-scoped prefix (e.g., projects/${projectId}/channels/${channelId}/
). If you don’t have a prefix today, consider adding one to your image config and enforce it here.
I can propose a small schema addition (e.g., downloadKeyPrefix
) to imageConfig
and wire validation here—want me to open a follow-up?
🤖 Prompt for AI Agents
In apps/api/src/domains/admin/channel/channel/channel.controller.ts around lines
176-200, the current implementation accepts any imageKey and can presign objects
outside the channel scope when buckets are shared; validate and scope imageKey
before generating a download URL by requiring it to start with an explicit
channel/project prefix (e.g., `projects/${projectId}/channels/${channelId}/`) or
with a configured imageConfig.downloadKeyPrefix if present, and throw
BadRequestException when the key does not match; if your imageConfig lacks a
downloadKeyPrefix, add that field to the schema and persist appropriate prefixes
for existing channels, then use the validated/scoped key when calling
createImageDownloadUrl so only channel-scoped objects can be presigned.
async createImageDownloadUrl(dto: CreateImageDownloadUrlDto) { | ||
const { accessKeyId, secretAccessKey, endpoint, region, bucket, imageKey } = | ||
dto; | ||
|
||
const s3 = new S3Client({ | ||
credentials: { accessKeyId, secretAccessKey }, | ||
endpoint, | ||
region, | ||
}); | ||
|
||
const command = new GetObjectCommand({ | ||
Bucket: bucket, | ||
Key: imageKey, | ||
}); | ||
|
||
return await getSignedUrl(s3, command, { expiresIn: 60 }); | ||
} |
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.
Presigned download URL OK; consider configurable expiry and endpoint path-style.
Make expiry configurable and support path-style for MinIO/compatible endpoints via config.
- const s3 = new S3Client({
- credentials: { accessKeyId, secretAccessKey },
- endpoint,
- region,
- });
+ const s3 = new S3Client({
+ credentials: { accessKeyId, secretAccessKey },
+ endpoint,
+ region,
+ // e.g., set via env S3_FORCE_PATH_STYLE=true for non-AWS endpoints
+ forcePathStyle: Boolean(this.configService.get('s3.forcePathStyle')),
+ });
@@
- return await getSignedUrl(s3, command, { expiresIn: 60 });
+ const expiresIn =
+ Number(this.configService.get('s3.downloadUrlExpiresInSeconds')) || 60;
+ return await getSignedUrl(s3, command, { expiresIn });
Additionally, the existing upload presign above includes ContentType and ACL headers that can cause issues/security exposure:
- ContentType='image/*' will not match clients sending 'image/png' and will break the signature.
- ACL='public-read' makes uploaded images public.
Recommend removing both unless explicitly required by policy.
- const command = new PutObjectCommand({
- Bucket: bucket,
- Key: `${projectId}_${channelId}_${Date.now()}.${extension}`,
- ContentType: 'image/*',
- ACL: 'public-read',
- });
+ const command = new PutObjectCommand({
+ Bucket: bucket,
+ Key: `${projectId}_${channelId}_${Date.now()}.${extension}`,
+ });
*/ | ||
import { ApiProperty } from '@nestjs/swagger'; | ||
import { IsString } from 'class-validator'; | ||
import { IsBoolean, IsString } from 'class-validator'; |
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.
Validation bug: optional field will fail without @IsOptional and TS optional
enablePresignedUrlDownload is documented as not required but will fail validation when omitted. Make it truly optional.
-import { IsBoolean, IsString } from 'class-validator';
+import { IsBoolean, IsOptional, IsString } from 'class-validator';
@@
- @ApiProperty({ required: false })
- @IsBoolean()
- enablePresignedUrlDownload: boolean;
+ @ApiProperty({ required: false })
+ @IsOptional()
+ @IsBoolean()
+ enablePresignedUrlDownload?: boolean;
Also applies to: 44-47
🤖 Prompt for AI Agents
In
apps/api/src/domains/admin/channel/channel/dtos/requests/image-config-request.dto.ts
around line 17 (and also apply the same change to the properties at lines
44-47), the boolean field enablePresignedUrlDownload is documented as optional
but lacks @IsOptional and a TS optional marker, so validation fails when
omitted; update the import line to include IsOptional, annotate the
enablePresignedUrlDownload property with @IsOptional(), and make the property
optional in TypeScript (use the ? suffix and boolean type) for both occurrences
so class-validator will allow the field to be omitted.
secretAccessKey: string; | ||
endpoint: string; | ||
region: string; | ||
bucket: string; |
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.
🛠️ Refactor suggestion
Add validation and Swagger metadata for the new bucket field (and consider marking secrets writeOnly).
To keep DTOs consistent and avoid leaking secrets in logs/docs, annotate fields. At minimum, add decorators for bucket; optionally add writeOnly for secretAccessKey across this DTO.
Apply locally within this file:
export class ImageUploadUrlTestDto {
accessKeyId: string;
secretAccessKey: string;
endpoint: string;
region: string;
- bucket: string;
+ // S3 bucket name for the test upload
+ bucket: string;
}
And add these imports and decorators (outside the shown range) for stronger typing and API docs:
import { ApiProperty } from '@nestjs/swagger';
import { IsString, IsNotEmpty } from 'class-validator';
export class ImageUploadUrlTestDto {
@ApiProperty({ description: 'S3 access key ID', writeOnly: true })
@IsString() @IsNotEmpty()
accessKeyId: string;
@ApiProperty({ description: 'S3 secret access key', writeOnly: true })
@IsString() @IsNotEmpty()
secretAccessKey: string;
@ApiProperty({ description: 'S3-compatible endpoint (https://...)' })
@IsString() @IsNotEmpty()
endpoint: string;
@ApiProperty({ description: 'AWS region (e.g., ap-northeast-2)' })
@IsString() @IsNotEmpty()
region: string;
@ApiProperty({ description: 'Bucket name' })
@IsString() @IsNotEmpty()
bucket: string;
}
Also ensure request logging (if any) masks secretAccessKey.
🤖 Prompt for AI Agents
In apps/api/src/domains/admin/feedback/dtos/image-upload-url-test.dto.ts around
line 22, the new bucket field lacks validation and Swagger metadata; update the
DTO to import ApiProperty from @nestjs/swagger and IsString, IsNotEmpty from
class-validator, then annotate bucket with @ApiProperty({ description: 'Bucket
name' }) and @IsString() @IsNotEmpty(); also add the suggested decorators for
accessKeyId, secretAccessKey (mark secretAccessKey writeOnly: true), endpoint
and region as shown in the review comment so all fields have proper validation
and documentation, and ensure any request logging masks secretAccessKey.
<> | ||
{Array.isArray(value) && (value as string[]).length > 0 ? | ||
<ImagePreviewButton urls={value}> | ||
<Tag | ||
aria-label="View images" | ||
variant="outline" | ||
size="small" | ||
className="cursor-pointer gap-1" | ||
onClick={(e) => e.stopPropagation()} | ||
> | ||
<Icon name="RiImageFill" size={12} /> | ||
Image | ||
</Tag> | ||
</ImagePreviewButton> | ||
: '-'} | ||
</> |
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.
Click suppressed by child; remove stopPropagation on Tag.
Tag’s onClick stops the event, likely preventing ImagePreviewButton from opening and creating nested-interactive semantics. Let the wrapper handle clicks.
- <ImagePreviewButton urls={value}>
+ <ImagePreviewButton urls={value}>
<Tag
aria-label="View images"
variant="outline"
size="small"
className="cursor-pointer gap-1"
- onClick={(e) => e.stopPropagation()}
>
<Icon name="RiImageFill" size={12} />
Image
</Tag>
</ImagePreviewButton>
📝 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.
<> | |
{Array.isArray(value) && (value as string[]).length > 0 ? | |
<ImagePreviewButton urls={value}> | |
<Tag | |
aria-label="View images" | |
variant="outline" | |
size="small" | |
className="cursor-pointer gap-1" | |
onClick={(e) => e.stopPropagation()} | |
> | |
<Icon name="RiImageFill" size={12} /> | |
Image | |
</Tag> | |
</ImagePreviewButton> | |
: '-'} | |
</> | |
<> | |
{Array.isArray(value) && (value as string[]).length > 0 ? | |
<ImagePreviewButton urls={value}> | |
<Tag | |
aria-label="View images" | |
variant="outline" | |
size="small" | |
className="cursor-pointer gap-1" | |
> | |
<Icon name="RiImageFill" size={12} /> | |
Image | |
</Tag> | |
</ImagePreviewButton> | |
: '-'} | |
</> |
🤖 Prompt for AI Agents
In apps/web/src/entities/feedback/ui/feedback-cell.tsx around lines 74 to 89,
the Tag child is calling onClick={(e) => e.stopPropagation()} which suppresses
the click event and prevents the parent ImagePreviewButton from receiving
clicks; remove the onClick prop (and any stopPropagation use) from the Tag so
the wrapper handles clicks normally and ensure Tag remains non-interfering (no
nested interactive handlers) so ImagePreviewButton can open as intended.
const imageKey = useMemo(() => { | ||
if (!channelData?.imageConfig?.enablePresignedUrlDownload) return ''; | ||
|
||
const s3Pattern = /(s3[.-][a-z0-9-]+\.amazonaws\.com|s3\.amazonaws\.com)/; | ||
if (!s3Pattern.test(url)) return ''; | ||
|
||
const parsedUrl = new URL(url); | ||
const key = decodeURIComponent(parsedUrl.pathname.replace(/^\/+/, '')); | ||
const host = parsedUrl.hostname; | ||
|
||
const parts = key.split('/', 2); | ||
if ( | ||
(host === 's3.amazonaws.com' || host.startsWith('s3.')) && | ||
parts.length >= 2 | ||
) { | ||
return parts.slice(1).join('/'); | ||
} | ||
|
||
return key; | ||
}, [channelData, url]); |
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.
🛠️ Refactor suggestion
Harden S3 host detection; avoid false positives and handle more patterns.
RegExp.test(url)
can match query strings on non-S3 hosts. Parse first and test hostname
; also guard new URL
with try/catch. This reduces misclassification and broken presign attempts.
Apply this diff:
- const imageKey = useMemo(() => {
- if (!channelData?.imageConfig?.enablePresignedUrlDownload) return '';
-
- const s3Pattern = /(s3[.-][a-z0-9-]+\.amazonaws\.com|s3\.amazonaws\.com)/;
- if (!s3Pattern.test(url)) return '';
-
- const parsedUrl = new URL(url);
- const key = decodeURIComponent(parsedUrl.pathname.replace(/^\/+/, ''));
- const host = parsedUrl.hostname;
-
- const parts = key.split('/', 2);
- if (
- (host === 's3.amazonaws.com' || host.startsWith('s3.')) &&
- parts.length >= 2
- ) {
- return parts.slice(1).join('/');
- }
-
- return key;
- }, [channelData, url]);
+ const imageKey = useMemo(() => {
+ if (!channelData?.imageConfig?.enablePresignedUrlDownload) return '';
+ let parsed: URL;
+ try {
+ parsed = new URL(url);
+ } catch {
+ return '';
+ }
+ const host = parsed.hostname.toLowerCase();
+ const keyPath = decodeURIComponent(parsed.pathname.replace(/^\/+/, ''));
+
+ // Accept common S3 host patterns: path-style (s3.*.amazonaws.com), global (s3.amazonaws.com),
+ // and virtual-hosted-style (<bucket>.s3.*.amazonaws.com)
+ const isAwsS3Host =
+ host === 's3.amazonaws.com' || host.startsWith('s3.') || host.includes('.s3.');
+ if (!isAwsS3Host) return '';
+
+ // Path-style: /<bucket>/<key> => strip first segment
+ if (host === 's3.amazonaws.com' || host.startsWith('s3.')) {
+ const [_, ...rest] = keyPath.split('/');
+ return rest.length ? rest.join('/') : '';
+ }
+ // Virtual-hosted-style: /<key>
+ return keyPath;
+ }, [channelData?.imageConfig?.enablePresignedUrlDownload, url]);
📝 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 imageKey = useMemo(() => { | |
if (!channelData?.imageConfig?.enablePresignedUrlDownload) return ''; | |
const s3Pattern = /(s3[.-][a-z0-9-]+\.amazonaws\.com|s3\.amazonaws\.com)/; | |
if (!s3Pattern.test(url)) return ''; | |
const parsedUrl = new URL(url); | |
const key = decodeURIComponent(parsedUrl.pathname.replace(/^\/+/, '')); | |
const host = parsedUrl.hostname; | |
const parts = key.split('/', 2); | |
if ( | |
(host === 's3.amazonaws.com' || host.startsWith('s3.')) && | |
parts.length >= 2 | |
) { | |
return parts.slice(1).join('/'); | |
} | |
return key; | |
}, [channelData, url]); | |
const imageKey = useMemo(() => { | |
if (!channelData?.imageConfig?.enablePresignedUrlDownload) return ''; | |
let parsed: URL; | |
try { | |
parsed = new URL(url); | |
} catch { | |
return ''; | |
} | |
const host = parsed.hostname.toLowerCase(); | |
const keyPath = decodeURIComponent(parsed.pathname.replace(/^\/+/, '')); | |
// Accept common S3 host patterns: path-style (s3.*.amazonaws.com), global (s3.amazonaws.com), | |
// and virtual-hosted-style (<bucket>.s3.*.amazonaws.com) | |
const isAwsS3Host = | |
host === 's3.amazonaws.com' || host.startsWith('s3.') || host.includes('.s3.'); | |
if (!isAwsS3Host) return ''; | |
// Path-style: /<bucket>/<key> => strip first segment | |
if (host === 's3.amazonaws.com' || host.startsWith('s3.')) { | |
const [_, ...rest] = keyPath.split('/'); | |
return rest.length ? rest.join('/') : ''; | |
} | |
// Virtual-hosted-style: /<key> | |
return keyPath; | |
}, [channelData?.imageConfig?.enablePresignedUrlDownload, url]); |
🤖 Prompt for AI Agents
In apps/web/src/shared/ui/feedback-image.tsx around lines 43 to 62, the current
s3 detection uses s3Pattern.test(url) which can falsely match query strings and
also calls new URL without guarding — wrap URL parsing in a try/catch and return
'' on parse failure, extract parsedUrl.hostname and test that against an s3
hostname pattern (instead of testing the full URL), then compute the key from
parsedUrl.pathname as before; keep the early guard for
channelData?.imageConfig?.enablePresignedUrlDownload and ensure the function
returns '' for non-S3 hosts to avoid broken presign attempts.
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…4.7.0 (#2010) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
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: 0
🧹 Nitpick comments (1)
apps/web/package.json (1)
90-93
: Switch to built-in jsdom and include .tsx in ESM extensions
In apps/web/jest.config.ts, set testEnvironment to 'jsdom' (or 'jest-environment-jsdom'), remove the jest-fixed-jsdom dependency, and update extensionsToTreatAsEsm to ['.ts', '.tsx'].
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (1)
apps/web/package.json
(2 hunks)
⏰ 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). (2)
- GitHub Check: integration-test
- GitHub Check: publish-api-docs
🔇 Additional comments (11)
apps/web/package.json (11)
31-32
: LGTM: React Query patch bumps.Safe patch-level updates; Devtools kept in lockstep with core.
45-45
: LGTM: dayjs patch bump.No breaking changes expected.
61-61
: LGTM: react-i18next patch.No expected API changes; keep an eye on typings.
69-71
: LGTM: zod + zustand patches.Safe bumps; no breaking diffs from prior 4.0.x/5.0.x.
73-73
: LGTM: @babel/core patch.No action needed.
78-78
: LGTM: @testing-library/jest-dom patch.Ensure setup file still imports "jest-dom/extend-expect" (auto in v6).
82-85
: LGTM: @types patches.Just type surface updates; watch for stricter DOM/React types in CI.
76-76
: Heads-up: Major @swc/core bump—test SWC transform pipeline
Ensure your Jest setup (apps/web/jest.config.ts using '@swc/jest') still passes with @swc/core v1.13.5 (including native bindings). If transforms break, pin or align your @swc/jest and core versions.
55-57
: No edge-runtime usage of pino detected—no action needed. Logger is imported and instantiated only in apps/web/src/server/logger.ts, which runs on the Node server, not in any Edge/Worker context.
47-49
: Confirm single i18next initialization
Noi18next.init
orinitReactI18next
calls detected in apps/web; ensure your SSR setup runs initialization only once and there are no duplicate i18next instances.
52-52
: Next.js patch bump—runtime matrix. No deprecated flags found innext.config.*
; local Node is v24.3.0. Next.js 15 requires Node ≥ 18.18.0 and is officially tested on Node 18/20/22—pin CI/runtime to one of those versions.
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
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: 0
♻️ Duplicate comments (1)
apps/api/package.json (1)
106-111
: Jest 30 + ts-jest 29 mismatch — align or drop ts-jestUsing jest ^30.1.3 with ts-jest ^29.4.1 will likely break. Options:
- Upgrade ts-jest to a Jest 30–compatible release (if available), or
- Remove ts-jest and use @swc-node/jest (already present), or
- Pin jest back to 29.x.
Recommend removing ts-jest to avoid split transforms. Patch:
- "ts-jest": "^29.4.1",
If keeping ts-jest, ensure all Jest configs target a single transformer. Run to verify usage:
#!/bin/bash rg -n -C2 -iP '(preset\s*:\s*["'\'']ts-jest["'\'']|ts-jest)' --glob '!**/node_modules/**' fd -HI "^jest(config(\.cjs|\.js|\.mjs|\.ts)|\.json)$" -t f
🧹 Nitpick comments (3)
apps/web/package.json (2)
52-52
: Next.js 15.4.7: verify Node/tooling alignmentGiven .nvmrc moved to 22.19.0 in this PR, add an engines gate and confirm build succeeds under Node 22.19.
"private": true, "type": "module", + "engines": { + "node": ">=22.19.0" + },Run locally/CI:
- pnpm -w -r run typecheck
- pnpm -w -r run build
90-92
: Jest 30.x stack update: confirm ESM + transformsWith "type": "module" and @swc/jest, confirm tests run under Node 22.
If not already set in repo-level config, ensure Jest config includes:
- testEnvironment: "jsdom" for web
- transform via @swc/jest for ts/tsx
- extensionsToTreatAsEsm for .ts/.tsx when needed
Example jest.config.ts (repo-level, not necessarily in this package):
import type { Config } from '@jest/types'; const config: Config = { testEnvironment: 'jsdom', transform: { '^.+\\.(t|j)sx?$': ['@swc/jest'] }, extensionsToTreatAsEsm: ['.ts', '.tsx'], moduleNameMapper: { '\\.(css|scss)$': 'identity-obj-proxy' }, }; export default config;apps/api/package.json (1)
97-99
: Drop @types/nodemailerNodemailer v7 ships its own types and no import or
/// <reference types="nodemailer">
usages remain, so it’s safe to remove this dependency.- "@types/nodemailer": "^7.0.1",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
apps/api/package.json
(4 hunks)apps/web/package.json
(2 hunks)
⏰ 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). (2)
- GitHub Check: integration-test
- GitHub Check: publish-api-docs
🔇 Additional comments (14)
apps/web/package.json (10)
31-32
: React Query patch bump looks safeNo API surface changes expected; should be no-op at runtime.
45-45
: dayjs patch bump is fineNo breaking changes anticipated.
47-48
: i18next + immer minor bumpsLooks routine. Ensure any strict type inits (i18next) still pass CI.
55-57
: nuqs + pino minor bumpsLow risk; logging and URL-state libs should behave identically.
61-61
: react-i18next minor bumpCheck peer compatibility with i18next 25.4.x; snapshots may need update.
69-70
: zod/zustand minor bumpsTypes could tighten slightly. Watch for inferred type regressions in stores/schemas.
73-73
: @babel/core patch bumpNo concerns.
76-76
: @swc/core 1.13.xEnsure the Docker/base image has compatible glibc/musl for SWC binaries.
78-78
: @testing-library/jest-dom 6.8API unchanged; update snapshots if matchers’ messages changed.
82-84
: @types updates (node/react/react-dom)Run typecheck; watch for narrowed DOM/Node types affecting Next config and server utils.
apps/api/package.json (4)
29-30
: Deps patch/minor bumps look safeAWS SDK v3, joi, mysql2, nodemailer, and typeorm bumps are patch/minor and low risk. Proceed.
Also applies to: 61-61, 64-64, 68-68, 79-79
89-92
: SWC stack bumps LGTM@swc-node/jest and @swc/core updates align with Jest 30 and Node 22.
101-101
: @typescript-eslint/parser bump OKNo action needed assuming eslint config stays compatible.
111-111
: ts-loader minor bump OKNo breaking changes expected; webpack rules unchanged here.
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Summary by CodeRabbit
New Features
UI
Documentation
Chores