786-feat: Add merch page #882
Conversation
YulikK
requested review from
andron13,
dzmitry-varabei,
SpaNb4,
Quiddlee,
ansivgit and
natanchik
as code owners
📝 Walkthrough""" WalkthroughA new "merch" page is introduced, accessible at Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MerchPage
participant MerchCatalog
participant MerchStore
participant Api
participant MerchApi
User->>MerchPage: Visit /merch
MerchPage->>MerchCatalog: Render
MerchCatalog->>MerchStore: loadMerchCatalog()
MerchStore->>Api: api.merch.queryMerchCatalog()
Api->>MerchApi: queryMerchCatalog()
MerchApi->>Api: GET merch/filelist.json
Api-->>MerchApi: MerchResponse
MerchApi-->>Api: MerchResponse
Api-->>MerchStore: MerchResponse
MerchStore->>MerchStore: transformMerchCatalog()
MerchStore-->>MerchCatalog: MerchProduct[]
MerchCatalog->>MerchList: Render with products
MerchList->>MerchCard: Render each product
User->>MerchCard: Click download
MerchCard->>downloadArchive: Download file(s)/ZIP
Assessment against linked issues
Suggested labels
Suggested reviewers
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (12)
dev-data/hero-page.data.ts (1)
30-36: Consider revising the subTitle array.The merch section has been added with appropriate content, but contains a subTitle array with a single empty string. Consider either removing the empty string or adding actual content, similar to other sections.
- subTitle: [''], + subTitle: [],src/widgets/merch-catalog/ui/merch-list/merch-list.module.scss (1)
5-9: Grid layout is well-definedThe 3-column grid with equal fractions and consistent gap creates a clean, responsive layout for merchandise items.
Consider adding media queries to adjust the number of columns for smaller screens (e.g., mobile devices) to maintain readability and usability.
.list { display: grid; grid-template-columns: repeat(3, 1fr); gap: 24px; + + @media (max-width: 768px) { + grid-template-columns: repeat(2, 1fr); + } + + @media (max-width: 480px) { + grid-template-columns: 1fr; + } }src/app/merch/page.tsx (1)
11-13: Function name doesn't match its purpose.The function name
CommunityRoutedoesn't reflect that this is a merch page. Consider renaming it toMerchRoutefor better clarity and consistency.-export default function CommunityRoute() { +export default function MerchRoute() { return <Merch />; }src/entities/merch/api/merch-api.ts (2)
7-9: Add explicit return type for better type safety.The
queryMerchCatalogmethod would benefit from an explicit return type definition.- public queryMerchCatalog() { + public queryMerchCatalog(): Promise<{ isSuccess: boolean; result: MerchResponse }> { return this.services.rest.get<MerchResponse>(`merch/filelist.json`); }
8-8: Consider moving hardcoded URL to constants.The endpoint URL
merch/filelist.jsonis hardcoded. For better maintainability, consider extracting it to a constants file.src/entities/merch/model/store.ts (1)
1-2: Missing import for MerchProduct type.The return type suggestion requires importing the MerchProduct type.
import { transformMerchCatalog } from '../helpers/transform-merch-catalog'; +import { MerchProduct } from '../types'; import { api } from '@/shared/api/api';src/entities/merch/ui/merch-card/merch-card.module.scss (3)
24-46: Consider adding z-index to overlayThe overlay uses absolute positioning but doesn't specify a z-index, which could cause layering issues.
&::after { pointer-events: none; content: ''; position: absolute; top: 0; left: 0; width: 100%; height: 100%; + z-index: 1; background: linear-gradient( to bottom, transparent 0%, hsla(from $color-black h s l / $opacity-10) 100% ); }
64-87: Add z-index to download buttonEnsure the download button appears above the gradient overlay by adding z-index.
cursor: pointer; position: absolute; right: 10px; bottom: 10px; + z-index: 2; padding: 8px 10px;
89-94: Remove unnecessary background-colorThe background-color is redundant for an image element that will display its own content.
.download-img { width: 20px; height: 20px; border-radius: 5px; - background-color: $color-yellow; }src/entities/merch/ui/merch-card/merch-card.tsx (1)
36-36: Consider handling multiple preview images.The component accepts an array of preview images but only uses the first one.
- <img className={cx('preview')} src={preview[0]} alt={title} /> + {preview.length > 0 ? ( + <img className={cx('preview')} src={preview[0]} alt={title} /> + ) : ( + <div className={cx('preview-placeholder')}>No image available</div> + )}src/entities/merch/types.ts (2)
15-15: Consider adding type discriminator for union type.
ApiMerchItemAdaptcombines three different types without a discriminator property, which could lead to runtime type checking issues.- export type ApiMerchItemAdapt = ApiMerchItem | ApiMerchCategory | ApiMerchData; + export type ApiMerchItemAdapt = + | (ApiMerchItem & { type: 'item' }) + | (ApiMerchCategory & { type: 'category' }) + | (ApiMerchData & { type: 'data' });
1-28: Add JSDoc comments to improve type documentation.The types lack documentation explaining their purpose and relationships.
+/** + * Represents a single merchandise item in the API response + */ export type ApiMerchItem = { name: string; preview: string[]; download: string[]; }; +/** + * Represents a category of merchandise items in the API response + */ type ApiMerchCategory = { [key: string]: ApiMerchItem; };
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonsrc/shared/assets/svg/download.svgis excluded by!**/*.svg
📒 Files selected for processing (22)
dev-data/hero-page.data.ts(2 hunks)package.json(1 hunks)src/app/merch/page.tsx(1 hunks)src/core/api/app-api.ts(2 hunks)src/core/styles/_constants.scss(1 hunks)src/entities/merch/api/merch-api.ts(1 hunks)src/entities/merch/helpers/download.ts(1 hunks)src/entities/merch/helpers/transform-merch-catalog.ts(1 hunks)src/entities/merch/index.ts(1 hunks)src/entities/merch/model/store.ts(1 hunks)src/entities/merch/types.ts(1 hunks)src/entities/merch/ui/merch-card/merch-card.module.scss(1 hunks)src/entities/merch/ui/merch-card/merch-card.tsx(1 hunks)src/shared/constants.ts(2 hunks)src/views/merch/merch.tsx(1 hunks)src/widgets/breadcrumbs/constants.ts(1 hunks)src/widgets/merch-catalog/index.ts(1 hunks)src/widgets/merch-catalog/ui/merch-catalog.module.scss(1 hunks)src/widgets/merch-catalog/ui/merch-catalog.tsx(1 hunks)src/widgets/merch-catalog/ui/merch-list/merch-list.module.scss(1 hunks)src/widgets/merch-catalog/ui/merch-list/merch-list.tsx(1 hunks)src/widgets/merch/ui/merch.tsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (6)
src/widgets/merch/ui/merch.tsx (3)
src/shared/ui/link-custom/link-custom.tsx (1)
LinkCustom(45-92)src/shared/constants.ts (1)
LINKS(79-87)dev-data/merch.data.ts (1)
merchData(1-7)
src/core/api/app-api.ts (3)
src/entities/merch/api/merch-api.ts (1)
MerchApi(4-10)src/shared/api/api-base-class.ts (1)
ApiBaseClass(14-127)src/entities/trainer/api/trainer-api.ts (1)
TrainerApi(6-19)
src/views/merch/merch.tsx (2)
src/shared/constants.ts (1)
PAGE_NAMES(25-31)src/widgets/merch-catalog/ui/merch-catalog.tsx (1)
MerchCatalog(10-20)
src/entities/merch/api/merch-api.ts (1)
src/entities/merch/types.ts (1)
MerchResponse(16-18)
src/widgets/merch-catalog/ui/merch-list/merch-list.tsx (2)
src/entities/merch/ui/merch-card/merch-card.tsx (2)
cx(13-13)MerchCard(15-46)src/entities/merch/types.ts (1)
MerchProduct(20-27)
src/entities/merch/helpers/transform-merch-catalog.ts (1)
src/entities/merch/types.ts (4)
MerchResponse(16-18)MerchProduct(20-27)ApiMerchItemAdapt(15-15)ApiMerchItem(1-5)
🔇 Additional comments (24)
src/widgets/merch-catalog/ui/merch-catalog.module.scss (1)
1-4: Simple, effective styling for the container component.The class provides basic flex layout which is appropriate for a container element. The styling is minimal but sufficient for this purpose.
dev-data/hero-page.data.ts (1)
2-2: Image import looks good.The welcome image import is properly added and will be used in the merch section.
package.json (1)
34-34: JSZip dependency added correctly.The jszip library is properly added with an appropriate version constraint for ZIP archive functionality in the merch feature.
src/core/styles/_constants.scss (1)
85-85: Opacity constant added in proper order.The new opacity value is correctly positioned in numerical sequence with the existing opacity constants.
src/widgets/breadcrumbs/constants.ts (1)
16-16: Addition of merch breadcrumb entry looks good.The new breadcrumb entry for the merch page is properly added and aligns with the PR objectives of creating a dedicated merchandise page.
src/widgets/merch/ui/merch.tsx (1)
24-26: Consider marking the merch link as external.The link points to an external domain (https://sloths.rs.school/) but is no longer marked as external. This could affect user expectations as they might not realize they're leaving the current site.
-<LinkCustom href={LINKS.MERCH} variant="primary"> +<LinkCustom href={LINKS.MERCH} variant="primary" external> {merchData.linkTitle} </LinkCustom>src/widgets/merch-catalog/index.ts (1)
1-1: Export of MerchCatalog component looks good.Clean barrel file export that provides a convenient import path for the MerchCatalog component.
src/shared/constants.ts (2)
30-30: Addition of MERCH page name is appropriate.The new constant follows the established pattern and supports the new merch page functionality.
105-105: Addition of MERCH route is correct.The route constant is properly added and will enable proper routing to the new merch page.
src/entities/merch/index.ts (1)
1-3: Well-structured barrel exportsThe exports are cleanly organized, following the barrel pattern to provide a centralized access point for merch-related entities.
src/core/api/app-api.ts (3)
1-1: Import added correctlyThe MerchApi import is properly added using the project's path alias pattern.
11-12: Property declaration follows established patternThe merch API property is correctly declared as public readonly, matching the pattern used for other API clients.
17-18: API initialization is consistentThe MerchApi instance is properly initialized with services dependency injection, consistent with other API clients.
src/views/merch/merch.tsx (2)
1-5: Imports are well-organizedThe imports follow the project's organization pattern, separating constants, widgets, and other dependencies.
6-14: Component structure is clean and follows async patternThe async component is properly structured with clear hierarchy. The page composition with HeroPage, Breadcrumbs, and MerchCatalog follows the application's consistent layout pattern.
src/widgets/merch-catalog/ui/merch-list/merch-list.module.scss (1)
1-3: Wrapper flex styling is appropriateThe flex property allows the wrapper to grow and shrink as needed, which is good for responsive layouts.
src/app/merch/page.tsx (1)
5-9: Clean metadata implementation.Good use of Next.js metadata API to set the page title appropriately.
src/widgets/merch-catalog/ui/merch-list/merch-list.tsx (3)
1-8: Clean imports and proper style setupThe imports are well-organized, and the CSS modules setup with classnames/bind follows best practices.
10-12: Well-typed props interfaceGood TypeScript practice with explicit typing for the component props.
13-21: Clean component implementationThe component follows React best practices with proper key usage and spreading props to child components.
src/entities/merch/ui/merch-card/merch-card.module.scss (1)
1-22: Well-structured card component stylesGood use of flexbox, transitions, and hover effects for the card container.
src/entities/merch/helpers/transform-merch-catalog.ts (2)
7-23: Well-implemented recursive category processingThe recursive approach for flattening nested categories is clean and efficient.
However, consider adding validation for empty preview or download arrays:
products.push({ id: index, name: key, title: value.name, - preview: value.preview.map((path) => `${baseUrl}/${path}`), - download: value.download.map((path) => `${baseUrl}/${path}`), + preview: value.preview.length ? value.preview.map((path) => `${baseUrl}/${path}`) : [], + download: value.download.length ? value.download.map((path) => `${baseUrl}/${path}`) : [], tags: parentTags, });
32-36: Robust type guard implementationThe type guard properly checks for all required properties of a merchandise item.
src/entities/merch/types.ts (1)
4-5:❌ Incorrect review comment
Clarify download type usage.
The
downloadproperty is defined as string[] but used as a single value in the MerchCard component.
🏁 Script executed:
#!/bin/bash # Check how the download property is used across the codebase rg -A 2 "downloadArchive.+download" --glob "*.{ts,tsx}"Length of output: 624
I’d like to inspect the
downloadArchivehelper signature to confirm what it expects. Let’s locate and view its definition:
🏁 Script executed:
#!/bin/bash # Find and display the downloadArchive function definition rg -n "downloadArchive" -C3 --glob "*.{ts,tsx}"Length of output: 1713
No changes needed for download type usage
The
downloadprop is correctly typed asstring[]inMerchProductand is passed directly intodownloadArchive(files: string[], filename: string), which already handles single-file arrays internally. You can ignore the original clarification request.Likely an incorrect or invalid review comment.
| export const MerchCatalog = async () => { | ||
| const products = await merchStore.loadMerchCatalog(); | ||
|
|
||
| return ( | ||
| <section className={cx('container')}> | ||
| <div className={cx('content', 'merch-catalog')}> | ||
| {products && <MerchList products={products} />} | ||
| </div> | ||
| </section> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add loading and error states for better UX.
The component lacks loading indicators and error handling that would improve user experience.
export const MerchCatalog = async () => {
+ // In a client component you'd typically use useState for these states
const products = await merchStore.loadMerchCatalog();
return (
<section className={cx('container')}>
<div className={cx('content', 'merch-catalog')}>
- {products && <MerchList products={products} />}
+ {!products && products === undefined ? (
+ <div className={cx('loading')}>Loading merchandise...</div>
+ ) : products && products.length > 0 ? (
+ <MerchList products={products} />
+ ) : (
+ <div className={cx('empty-state')}>No merchandise available at this time.</div>
+ )}
</div>
</section>
);
};Committable suggestion skipped: line range outside the PR's diff.
| public loadMerchCatalog = async () => { | ||
| try { | ||
| const res = await api.merch.queryMerchCatalog(); | ||
|
|
||
| if (res.isSuccess) { | ||
| return transformMerchCatalog(res.result); | ||
| } | ||
| } catch (e) { | ||
| console.error(e); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling and add explicit return type.
The current implementation lacks proper error handling and type annotations. Also consider adding caching for repeated calls.
- public loadMerchCatalog = async () => {
+ private cachedProducts = null;
+
+ public loadMerchCatalog = async (): Promise<Array<MerchProduct> | null> => {
+ if (this.cachedProducts) {
+ return this.cachedProducts;
+ }
+
try {
const res = await api.merch.queryMerchCatalog();
if (res.isSuccess) {
- return transformMerchCatalog(res.result);
+ this.cachedProducts = transformMerchCatalog(res.result);
+ return this.cachedProducts;
}
+
+ return null;
} catch (e) {
console.error(e);
+ return null;
}
};📝 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.
| public loadMerchCatalog = async () => { | |
| try { | |
| const res = await api.merch.queryMerchCatalog(); | |
| if (res.isSuccess) { | |
| return transformMerchCatalog(res.result); | |
| } | |
| } catch (e) { | |
| console.error(e); | |
| } | |
| }; | |
| private cachedProducts = null; | |
| public loadMerchCatalog = async (): Promise<Array<MerchProduct> | null> => { | |
| if (this.cachedProducts) { | |
| return this.cachedProducts; | |
| } | |
| try { | |
| const res = await api.merch.queryMerchCatalog(); | |
| if (res.isSuccess) { | |
| this.cachedProducts = transformMerchCatalog(res.result); | |
| return this.cachedProducts; | |
| } | |
| return null; | |
| } catch (e) { | |
| console.error(e); | |
| return null; | |
| } | |
| }; |
| async function createArchive(files: string[]): Promise<Blob> { | ||
| const zip = new JSZip(); | ||
|
|
||
| await Promise.all( | ||
| files.map(async (url) => { | ||
| const response = await fetch(url); | ||
| const blob = await response.blob(); | ||
| const filename = url.split('/').pop() || ''; | ||
|
|
||
| zip.file(filename, blob); | ||
| }), | ||
| ); | ||
|
|
||
| return zip.generateAsync({ type: 'blob' }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for fetch operations
The fetch operations could fail if URLs are invalid or network issues occur. Consider adding error handling.
await Promise.all(
files.map(async (url) => {
- const response = await fetch(url);
- const blob = await response.blob();
+ try {
+ const response = await fetch(url);
+ if (!response.ok) {
+ throw new Error(`Failed to fetch ${url}: ${response.status} ${response.statusText}`);
+ }
+ const blob = await response.blob();
+ const filename = url.split('/').pop() || '';
+ zip.file(filename, blob);
+ } catch (error) {
+ console.error(`Error processing file ${url}:`, error);
+ // Optionally: throw error or add a placeholder file indicating the error
+ }
- const filename = url.split('/').pop() || '';
-
- zip.file(filename, blob);
}),
);📝 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.
| async function createArchive(files: string[]): Promise<Blob> { | |
| const zip = new JSZip(); | |
| await Promise.all( | |
| files.map(async (url) => { | |
| const response = await fetch(url); | |
| const blob = await response.blob(); | |
| const filename = url.split('/').pop() || ''; | |
| zip.file(filename, blob); | |
| }), | |
| ); | |
| return zip.generateAsync({ type: 'blob' }); | |
| } | |
| async function createArchive(files: string[]): Promise<Blob> { | |
| const zip = new JSZip(); | |
| await Promise.all( | |
| files.map(async (url) => { | |
| try { | |
| const response = await fetch(url); | |
| if (!response.ok) { | |
| throw new Error(`Failed to fetch ${url}: ${response.status} ${response.statusText}`); | |
| } | |
| const blob = await response.blob(); | |
| const filename = url.split('/').pop() || ''; | |
| zip.file(filename, blob); | |
| } catch (error) { | |
| console.error(`Error processing file ${url}:`, error); | |
| // Optionally: throw error or add a placeholder file indicating the error | |
| } | |
| }), | |
| ); | |
| return zip.generateAsync({ type: 'blob' }); | |
| } |
| function downloadFile(url: string, filename: string) { | ||
| const link = document.createElement('a'); | ||
|
|
||
| link.href = url; | ||
| link.download = filename; | ||
| link.download = url.split('/').pop() || ''; | ||
| document.body.appendChild(link); | ||
| link.click(); | ||
| document.body.removeChild(link); | ||
| } |
There was a problem hiding this comment.
Fix duplicate download attribute assignment
There's a duplicate assignment to the download attribute which overwrites the provided filename.
link.href = url;
link.download = filename;
- link.download = url.split('/').pop() || '';Alternatively, if you want to use the filename from the URL as a fallback:
link.href = url;
- link.download = filename;
- link.download = url.split('/').pop() || '';
+ link.download = filename || url.split('/').pop() || '';📝 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.
| function downloadFile(url: string, filename: string) { | |
| const link = document.createElement('a'); | |
| link.href = url; | |
| link.download = filename; | |
| link.download = url.split('/').pop() || ''; | |
| document.body.appendChild(link); | |
| link.click(); | |
| document.body.removeChild(link); | |
| } | |
| function downloadFile(url: string, filename: string) { | |
| const link = document.createElement('a'); | |
| link.href = url; | |
| link.download = filename || url.split('/').pop() || ''; | |
| document.body.appendChild(link); | |
| link.click(); | |
| document.body.removeChild(link); | |
| } |
| export const transformMerchCatalog = (data: MerchResponse): MerchProduct[] => { | ||
| const products: MerchProduct[] = []; | ||
| const baseUrl = process.env.API_BASE_URL; | ||
| let index = 0; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add fallback for API base URL
If process.env.API_BASE_URL is undefined, URLs will be invalid. Add a fallback value.
export const transformMerchCatalog = (data: MerchResponse): MerchProduct[] => {
const products: MerchProduct[] = [];
- const baseUrl = process.env.API_BASE_URL;
+ const baseUrl = process.env.API_BASE_URL || '';
+ if (!baseUrl) {
+ console.warn('API_BASE_URL is not defined. URLs will be relative.');
+ }
let index = 0;📝 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.
| export const transformMerchCatalog = (data: MerchResponse): MerchProduct[] => { | |
| const products: MerchProduct[] = []; | |
| const baseUrl = process.env.API_BASE_URL; | |
| let index = 0; | |
| export const transformMerchCatalog = (data: MerchResponse): MerchProduct[] => { | |
| const products: MerchProduct[] = []; | |
| const baseUrl = process.env.API_BASE_URL || ''; | |
| if (!baseUrl) { | |
| console.warn('API_BASE_URL is not defined. URLs will be relative.'); | |
| } | |
| let index = 0; |
| <button onClick={handleDownload} className={cx('download')} disabled={isLoading}> | ||
| <Image src={downloadImg} alt="download link" className={cx('download-img')} /> | ||
| </button> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve accessibility for the download button.
The button lacks accessible text and loading state indication.
<button
onClick={handleDownload}
className={cx('download')}
disabled={isLoading}
+ aria-label="Download merch files"
>
<Image src={downloadImg} alt="download link" className={cx('download-img')} />
+ {isLoading && <span className="sr-only">Downloading...</span>}
</button>| } catch (error) { | ||
| console.error(error); | ||
| } finally { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add user feedback for download errors.
The error is logged to console but there's no feedback to the user when the download fails.
try {
setIsLoading(true);
await downloadArchive(download, `${title}.zip`);
} catch (error) {
console.error(error);
+ // Consider adding user-visible error notification here
+ // Example: toast.error('Failed to download. Please try again.');
} finally {
setIsLoading(false);
}📝 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.
| } catch (error) { | |
| console.error(error); | |
| } finally { | |
| } catch (error) { | |
| console.error(error); | |
| // Consider adding user-visible error notification here | |
| // Example: toast.error('Failed to download. Please try again.'); | |
| } finally { |
|
|
||
| try { | ||
| setIsLoading(true); | ||
| await downloadArchive(download, `${title}.zip`); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify download array handling.
The code assumes download contains at least one URL, but doesn't handle empty arrays.
🏁 Script executed:
#!/bin/bash
# Check if there's validation for empty download arrays
rg -A 3 -B 3 "download.+\.length" --glob "*.{ts,tsx,js,jsx}"Length of output: 62
🏁 Script executed:
#!/bin/bash
# Locate the definition and usages of downloadArchive to inspect empty-array handling
rg "downloadArchive" -n --glob "*.{ts,tsx,js,jsx}"Length of output: 381
🏁 Script executed:
#!/bin/bash
# Show the downloadArchive function implementation to check for empty-array handling
rg -n -A 20 -B 5 "export async function downloadArchive" src/entities/merch/helpers/download.tsLength of output: 456
Add empty-array guard before downloading
The downloadArchive helper only handles one or many files—passing an empty download array will fall through to createArchive and produce an empty (or malformed) ZIP. Please add a check for zero-length before invoking:
• In src/entities/merch/ui/merch-card/merch-card.tsx:25, wrap the call:
if (download.length === 0) {
// e.g. show “No files to download” feedback
return;
}
await downloadArchive(download, `${title}.zip`);• Optionally, in src/entities/merch/helpers/download.ts (at the top of downloadArchive), add:
if (files.length === 0) {
throw new Error('No files to download');
}
What type of PR is this? (select all that apply)
Description
Add merch page
Add merch card view
Add api for fetch and adapt merch catalog
Related Tickets & Documents
Screenshots, Recordings
Added/updated tests?
[optional] Are there any post deployment tasks we need to perform?
[optional] What gif best describes this PR or how it makes you feel?
Summary by CodeRabbit
New Features
Style
Chores