-
Notifications
You must be signed in to change notification settings - Fork 0
Feat/#281 select vertions #310
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
base: dev
Are you sure you want to change the base?
Conversation
Dokploy Preview Deployment
|
WalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant UI as SelectVersion Component
participant Hook as UseGetProductVersions Hook
participant API as getProductVersions API
participant Server as Core API Server
UI->>Hook: Call UseGetProductVersions({ productId })
Hook->>API: getProductVersions({ productId })
API->>Server: GET /core/shop/product-version/flat with product_id
Server-->>API: Respond with product version data
API-->>Hook: Return validated and transformed data
Hook-->>UI: Provide data and loading state
UI->>UI: Render Select with options from data
Possibly related issues
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
Scope: all 2 workspace projects This error happened while installing a direct dependency of /tmp/eslint @repo/eslint-config is not in the npm registry, or you have no permission to fetch it. No authorization header was set for the request. Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 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
CodeRabbit Configuration File (
|
Feat/#281 select vertions
🚨 Report Summary
For more details view the full report in OpenZeppelin Code Inspector |
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
🔭 Outside diff range comments (1)
packages/apis/turbo/generators/config.ts (1)
5-17: 🛠️ Refactor suggestionUnused function should be removed
The
loadSwaggerPathsfunction is now completely unused after changing the prompt type from list to input.Since the prompt no longer uses Swagger paths for selection, this entire function should be removed:
-// خواندن فایل Swagger JSON -function loadSwaggerPaths(filePath: string): string[] { - try { - const swaggerData = JSON.parse( - fs.readFileSync(path.resolve(__dirname, filePath), "utf-8") - ); - const paths = Object.keys(swaggerData.paths || {}); - return paths; - } catch (error) { - console.error("Error reading Swagger JSON:", error); - return []; - } -}
🧹 Nitpick comments (11)
packages/apis/src/services/core/shop/product-version/flat/get/get-product-versions.ts (3)
12-13: Consider using URL utilities instead of path.join for API endpointsUsing
path.joinfor URL construction can cause issues as it's designed for file system paths, not URLs. It may not handle URL slashes correctly.-const getProductVersionsURL = () => path.join("/shop/product-version/flat/"); +const getProductVersionsURL = () => "/shop/product-version/flat";
27-28: Remove unnecessary empty line); - - const dataParsed = schema.response.parse(response.data);
14-32: Add error handling and refine the implementationThe current implementation lacks explicit error handling, and there's no typing for the parsed payload.
export const getProductVersions = async ( props: GetProductVersionsRequest, ): Promise<ApiResponse<GetProductVersionsResponseTransformed>> => { const payloadParsed = schema.request.parse(props); + // Use type annotation to ensure correct typing after schema validation + const params: GetProductVersionsRequestTransofrmed = payloadParsed; const URL = getProductVersionsURL(); - const response = await requestHandler( - () => coreApi.get<GetProductVersionsResponse>(URL, { params: payloadParsed }), - schema.response._def.schema, - { - isMock: true, - }, - ); + try { + const response = await requestHandler( + () => coreApi.get<GetProductVersionsResponse>(URL, { params }), + schema.response._def.schema, + { + // Consider making this configurable based on environment + isMock: process.env.NEXT_PUBLIC_MOCK === "true", + }, + ); + + const dataParsed = schema.response.parse(response.data); + return { ...response, data: dataParsed }; + } catch (error) { + // Handle and transform errors appropriately + console.error("Failed to fetch product versions:", error); + throw error; + } - - - const dataParsed = schema.response.parse(response.data); - - return { ...response, data: dataParsed }; };packages/apis/src/services/core/shop/product-version/flat/get/get-product-versions.types.ts (2)
4-9: Fix comment label and type name typoThe comment label "Response" is incorrect as it appears above request-related types. Also, there's a typo in the type name "Transofrmed".
-// Response +// Request export type GetProductVersionsRequest = z.input<typeof getProductVersionsSchema.request>; -export type GetProductVersionsRequestTransofrmed = z.infer< +export type GetProductVersionsRequestTransformed = z.infer< typeof getProductVersionsSchema.request >;
11-16: Fix comment label and type name consistencyThe comment label "Request" is incorrect as it appears above response-related types.
-// Request +// Response export type GetProductVersionsResponse = z.input<typeof getProductVersionsSchema.response>; export type GetProductVersionsResponseTransformed = z.infer< typeof getProductVersionsSchema.response >;packages/apis/src/services/core/shop/product-version/flat/get/get-product-versions.schema.ts (3)
14-19: Fix schema name typo and remove unnecessary transformThere's a typo in the schema name ("Transofrmed" instead of "Transformed"), and the identity transform function doesn't do anything useful.
-export const getProductVersionsResponseSchemaTransofrmed = apiResponseSchema.extend({ +export const getProductVersionsResponseSchemaTransformed = apiResponseSchema.extend({ data: z.array(z.object({ version: z.string(), id: z.number(), })), -}).transform((data) => data); +});
21-24: Update schema export to use the corrected schema nameThe schema export references the misspelled schema name.
export const getProductVersionsSchema = { - response: getProductVersionsResponseSchemaTransofrmed, + response: getProductVersionsResponseSchemaTransformed, request: getProductVersionsRequestSchemaTransformed, };
15-18: Consider adding more specific validation for version propertiesThe current schema lacks specific validation for the version string format and id number range.
data: z.array(z.object({ - version: z.string(), - id: z.number(), + version: z.string().min(1, "Version cannot be empty"), + id: z.number().int().positive("ID must be a positive integer"), })),packages/apis/src/services/core/shop/product-version/flat/get/use-get-product-versions.ts (1)
24-28: Enable fetching only when parameters are validThe current implementation doesn't check if parameters are valid before fetching. Consider adding an
enabledoption to prevent unnecessary API calls.const query = useQuery<ApiResponse<GetProductVersionsResponseTransformed>, ApiError>({ queryKey: getProductVersionsQueryKey(), queryFn: () => getProductVersions(params), + enabled: !!params.productId, ...resProps, });apps/core/app/dashboard/products/create/page.tsx (2)
34-37: UI components wrapped in unnecessary div elementsAll three action components are individually wrapped in div elements without apparent styling or layout purpose. This creates unnecessary DOM nesting and could be simplified.
<div className="flex items-center gap-3 shrink-0"> - <div><SelectVersion productId="1" /></div> - <div><Button variant="secondary" size={"lg"}>Save as Draft</Button></div> - <div><Button type="submit" size={"lg"}>Submit to Review</Button></div> + <SelectVersion productId="1" /> + <Button variant="secondary" size="lg">Save as Draft</Button> + <Button type="submit" size="lg">Submit to Review</Button> </div>If there's a specific layout reason for these div wrappers, consider adding className attributes to make the purpose clear.
35-36: Inconsistent prop syntax for button sizeThe size prop uses curly braces with a string value
size={"lg"}rather than the more conventional JSX attribute formatsize="lg". While functionally equivalent, this is inconsistent with typical React coding conventions.- <Button variant="secondary" size={"lg"}>Save as Draft</Button> - <Button type="submit" size={"lg"}>Submit to Review</Button> + <Button variant="secondary" size="lg">Save as Draft</Button> + <Button type="submit" size="lg">Submit to Review</Button>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
apps/core/app/dashboard/products/create/_components/select-version.tsx(1 hunks)apps/core/app/dashboard/products/create/page.tsx(2 hunks)package.json(1 hunks)packages/apis/src/services/core/shop/product-version/flat/get/get-product-versions.schema.ts(1 hunks)packages/apis/src/services/core/shop/product-version/flat/get/get-product-versions.ts(1 hunks)packages/apis/src/services/core/shop/product-version/flat/get/get-product-versions.types.ts(1 hunks)packages/apis/src/services/core/shop/product-version/flat/get/use-get-product-versions.ts(1 hunks)packages/apis/turbo/generators/config.ts(1 hunks)packages/ui/src/components/molecules/label-wrapper.tsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
packages/apis/src/services/core/shop/product-version/flat/get/get-product-versions.schema.ts (1)
packages/apis/src/schema/api-response-schema.ts (1)
apiResponseSchema(3-27)
packages/apis/src/services/core/shop/product-version/flat/get/get-product-versions.ts (4)
packages/apis/src/services/core/shop/product-version/flat/get/get-product-versions.types.ts (3)
GetProductVersionsRequest(5-5)GetProductVersionsResponseTransformed(14-16)GetProductVersionsResponse(12-12)packages/apis/src/types/api.types.ts (1)
ApiResponse(10-10)packages/apis/src/utils/request-handler.ts (1)
requestHandler(17-35)packages/apis/src/instance/core-api.ts (1)
coreApi(9-14)
packages/apis/src/services/core/shop/product-version/flat/get/get-product-versions.types.ts (1)
packages/apis/src/services/core/shop/product-version/flat/get/get-product-versions.schema.ts (1)
getProductVersionsSchema(21-24)
packages/apis/src/services/core/shop/product-version/flat/get/use-get-product-versions.ts (3)
packages/apis/src/types/api.types.ts (4)
UseQueryProps(14-21)ApiResponse(10-10)WithParams(12-12)ApiError(30-30)packages/apis/src/services/core/shop/product-version/flat/get/get-product-versions.types.ts (2)
GetProductVersionsResponseTransformed(14-16)GetProductVersionsRequest(5-5)packages/apis/src/services/core/shop/product-version/flat/get/get-product-versions.ts (1)
getProductVersions(14-32)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: merge-pr
🔇 Additional comments (2)
packages/apis/src/services/core/shop/product-version/flat/get/use-get-product-versions.ts (1)
1-31: Overall implementation is well-structuredThe hook is well-implemented with proper TypeScript typing and follows the pattern of other React Query hooks. The use of type definitions from imported modules ensures type safety throughout the component chain.
packages/ui/src/components/molecules/label-wrapper.tsx (1)
40-40:Details
❓ Verification inconclusive
Vertical spacing adjustment: bottom margin removed from label
The bottom margin (
mb-2) has been removed from the Label component's className. While this reduces the space between the label and its associated input, ensure this change doesn't negatively affect the spacing in other components using LabelWrapper.
🏁 Script executed:
#!/bin/bash # Find all instances of LabelWrapper usage to assess the impact of this spacing change rg -A 3 "LabelWraper" --glob "*.tsx" --glob "*.jsx"Length of output: 4626
Verify LabelWrapper spacing across usages
Removing the
mb-2utility fromLabelWraperwill tighten the gap between label and its associated input everywhere it’s used. We’ve found instances in the following components—please review each to confirm the vertical rhythm remains as intended or adjust spacing accordingly:• packages/ui/src/components/molecules/select.tsx
• packages/ui/src/components/molecules/passwordInput.tsx
• packages/ui/src/components/molecules/input.tsx
• packages/ui/src/components/molecules/multi-select.tsxIf any layout looks too cramped, consider re-introducing a bottom margin (e.g. via a wrapper or child element) at those specific call sites rather than globally.
| type: "input", | ||
| name: "path", | ||
| message: "Select the API path from Swagger JSON:", | ||
| choices: swaggerPaths.length > 0 ? swaggerPaths : ["No paths available"], | ||
| when: () => swaggerPaths.length > 0, | ||
| message: "Enter the API path:" | ||
| }, |
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
Improved flexibility but created dead code
Changing from a list selection to free text input for API paths provides more flexibility for developers but may lead to inconsistency in path formats.
The swaggerPaths variable is now unused but the function to load it is still being called on line 20. This creates dead code that should be removed:
- const swaggerPaths = loadSwaggerPaths("./swagger.json");Since the Swagger paths are no longer used for providing choices in the prompt, this function call is unnecessary.
📝 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.
| type: "input", | |
| name: "path", | |
| message: "Select the API path from Swagger JSON:", | |
| choices: swaggerPaths.length > 0 ? swaggerPaths : ["No paths available"], | |
| when: () => swaggerPaths.length > 0, | |
| message: "Enter the API path:" | |
| }, | |
| function configGenerator() { | |
| - const swaggerPaths = loadSwaggerPaths("./swagger.json"); | |
| return { | |
| prompts: [ | |
| { | |
| type: "input", | |
| name: "path", | |
| message: "Enter the API path:" | |
| }, | |
| ] | |
| }; | |
| } |
| const SelectVersion = ({ productId }: { productId: string }) => { | ||
| const { data, isLoading } = UseGetProductVersions({ | ||
| params: { | ||
| productId, | ||
| }, | ||
| }); | ||
|
|
||
| return ( | ||
| <Select | ||
| id="version-select" | ||
| placeholder="Select a version" | ||
| size="lg" | ||
| options={data?.data?.data?.map((item) => ({ | ||
| label: item.version, | ||
| value: item.id.toString() | ||
| })) || []} | ||
| /> | ||
| ); | ||
| }; |
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
Enhance the SelectVersion component with proper state handling and functionality
While the basic structure is good, the component has several issues:
- You're destructuring
isLoadingbut not using it to show loading states - There's no error handling for API request failures
- The Select component lacks onChange handler and value state
Here's an improved implementation:
-const SelectVersion = ({ productId }: { productId: string }) => {
+const SelectVersion = ({
+ productId,
+ onChange,
+ value
+}: {
+ productId: string;
+ onChange?: (value: string) => void;
+ value?: string;
+}) => {
- const { data, isLoading } = UseGetProductVersions({
+ const { data, isLoading, error } = UseGetProductVersions({
params: {
productId,
},
});
+ const options = data?.data?.data?.map((item) => ({
+ label: item.version,
+ value: item.id.toString()
+ })) || [];
return (
<Select
id="version-select"
placeholder="Select a version"
size="lg"
+ isLoading={isLoading}
+ onChange={onChange}
+ value={value}
- options={data?.data?.data?.map((item) => ({
- label: item.version,
- value: item.id.toString()
- })) || []}
+ options={options}
+ isDisabled={isLoading || options.length === 0}
+ status={error ? "error" : undefined}
+ helperText={error ? "Failed to load versions" : options.length === 0 && !isLoading ? "No versions available" : undefined}
/>
);
};📝 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 SelectVersion = ({ productId }: { productId: string }) => { | |
| const { data, isLoading } = UseGetProductVersions({ | |
| params: { | |
| productId, | |
| }, | |
| }); | |
| return ( | |
| <Select | |
| id="version-select" | |
| placeholder="Select a version" | |
| size="lg" | |
| options={data?.data?.data?.map((item) => ({ | |
| label: item.version, | |
| value: item.id.toString() | |
| })) || []} | |
| /> | |
| ); | |
| }; | |
| const SelectVersion = ({ | |
| productId, | |
| onChange, | |
| value | |
| }: { | |
| productId: string; | |
| onChange?: (value: string) => void; | |
| value?: string; | |
| }) => { | |
| const { data, isLoading, error } = UseGetProductVersions({ | |
| params: { | |
| productId, | |
| }, | |
| }); | |
| const options = | |
| data?.data?.data?.map((item) => ({ | |
| label: item.version, | |
| value: item.id.toString(), | |
| })) || []; | |
| return ( | |
| <Select | |
| id="version-select" | |
| placeholder="Select a version" | |
| size="lg" | |
| isLoading={isLoading} | |
| onChange={onChange} | |
| value={value} | |
| options={options} | |
| isDisabled={isLoading || options.length === 0} | |
| status={error ? "error" : undefined} | |
| helperText={ | |
| error | |
| ? "Failed to load versions" | |
| : options.length === 0 && !isLoading | |
| ? "No versions available" | |
| : undefined | |
| } | |
| /> | |
| ); | |
| }; |
| const response = await requestHandler( | ||
| () => coreApi.get<GetProductVersionsResponse>(URL, { params: payloadParsed }), | ||
| schema.response._def.schema, | ||
| { | ||
| isMock: true, | ||
| }, | ||
| ); |
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
Hardcoded mock flag may prevent real API calls
The isMock: true setting will always return mock data instead of making real API calls. This might be intentional during development but could cause issues in production.
🏁 Script executed:
#!/bin/bash
# Check if there are any environment variables configured for controlling mock mode
grep -r "NEXT_PUBLIC_MOCK" --include="*.{ts,js,tsx,jsx,json}" .
# Also check if other API functions use this pattern
grep -r "isMock:" --include="*.{ts,js,tsx,jsx}" --after-context=1 packages/apis/src/services/Length of output: 157
I want to inspect how isMock is handled inside requestHandler to confirm whether it truly forces mock data in all environments. Let’s locate its definition and usage:
🏁 Script executed:
#!/bin/bash
# Find where requestHandler is imported
grep -R "import .*requestHandler" -n packages/apis/src
# Locate requestHandler’s implementation
grep -R "export function requestHandler" -n packages/apis/src || grep -R "const requestHandler" -n packages/apis/srcLength of output: 2170
To confirm how the isMock flag is handled inside requestHandler, let’s inspect its implementation:
🏁 Script executed:
#!/bin/bash
# Show how many lines we have and preview the implementation of requestHandler
wc -l packages/apis/src/utils/request-handler.ts
sed -n '1,200p' packages/apis/src/utils/request-handler.tsLength of output: 1148
Remove hardcoded mock flag in get-product-versions
The isMock: true option here will always short-circuit into mock mode—skipping real API calls even in production. Instead, derive mock behavior from an environment variable or NODE_ENV.
• File: packages/apis/src/services/core/shop/product-version/flat/get/get-product-versions.ts (lines 20–26)
• Change this:
const response = await requestHandler(
() => coreApi.get<GetProductVersionsResponse>(URL, { params: payloadParsed }),
schema.response._def.schema,
{ isMock: true },
);to something like:
const response = await requestHandler(
() => coreApi.get<GetProductVersionsResponse>(URL, { params: payloadParsed }),
schema.response._def.schema,
{ isMock: process.env.NEXT_PUBLIC_MOCK === 'true' },
);or guard by environment:
{ isMock: process.env.NODE_ENV === 'development' }|
|
||
| export const getProductVersionsQueryKey = () => ["getProductVersions"]; | ||
|
|
||
| export const UseGetProductVersions = (props: UseGetProductVersionsProps) => { |
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
Hook naming should follow React conventions
React hook names should use camelCase and start with 'use', not uppercase 'Use'. This convention helps tools like ESLint identify hooks properly.
-export const UseGetProductVersions = (props: UseGetProductVersionsProps) => {
+export const useGetProductVersions = (props: UseGetProductVersionsProps) => {📝 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 UseGetProductVersions = (props: UseGetProductVersionsProps) => { | |
| export const useGetProductVersions = (props: UseGetProductVersionsProps) => { |
| WithParams<GetProductVersionsRequest> | ||
| >; | ||
|
|
||
| export const getProductVersionsQueryKey = () => ["getProductVersions"]; |
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
Query key should include dependency parameters
The query key doesn't include any parameters that the API depends on, such as the product ID. This could lead to cache collisions if multiple components fetch versions for different products.
-export const getProductVersionsQueryKey = () => ["getProductVersions"];
+export const getProductVersionsQueryKey = (params: GetProductVersionsRequest) => ["getProductVersions", params];Then update the queryKey usage:
- queryKey: getProductVersionsQueryKey(),
+ queryKey: getProductVersionsQueryKey(params),📝 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 getProductVersionsQueryKey = () => ["getProductVersions"]; | |
| -export const getProductVersionsQueryKey = () => ["getProductVersions"]; | |
| +export const getProductVersionsQueryKey = (params: GetProductVersionsRequest) => ["getProductVersions", params]; | |
| - queryKey: getProductVersionsQueryKey(), | |
| + queryKey: getProductVersionsQueryKey(params), |
| "turbo": "2.0.6" | ||
| }, | ||
| "packageManager": "yarn@1.22.22", | ||
| "packageManager": "bun@1.1.19", |
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
Package manager changed from Yarn to Bun
The package manager has been switched from Yarn 1.22.22 to Bun 1.1.19, which is a significant change that could affect development workflows and CI/CD pipelines.
🏁 Script executed:
#!/bin/bash
# Check if there are any remaining Yarn-specific files or configurations
find . -name "yarn.lock" -o -name ".yarnrc" -o -name ".yarn" -type d
# Check if lockfile has been properly updated to Bun format
test -f "bun.lockb" && echo "Bun lockfile exists" || echo "Bun lockfile missing"
# Check CI configuration for Yarn references
find .github -type f -name "*.yml" -o -name "*.yaml" | xargs grep -l "yarn" || echo "No Yarn references in GitHub workflows"Length of output: 291
Address remaining Yarn artifacts and update CI workflows
There are residual Yarn configurations that need to be cleaned up now that you’ve switched to Bun:
- yarn.lock still exists at the repo root (and indirectly in node_modules).
- The GitHub Actions workflow (
.github/workflows/build-and-merge.yml) still invokes Yarn. - Although
bun.lockbis present, CI and docs may still reference Yarn install commands.
Please remove or replace all Yarn-specific files and commands:
- Delete
yarn.lock(and any stray.yarnrcor.yarn/directories). - Update your CI workflows to use
bun install(orbunCLI) instead ofyarn. - Review project documentation and scripts (e.g., README, any Makefiles) for Yarn references and switch them to Bun.
| <div>select version</div> | ||
| <Button variant="secondary">Save as Draft</Button> | ||
| <Button type="submit">Submit to Review</Button> | ||
| <div><SelectVersion productId="1" /></div> |
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.
Hardcoded productId in SelectVersion component
The SelectVersion component is initialized with a fixed productId="1", which may not be appropriate for a product creation flow where the ID is typically generated after creation. Consider using a more dynamic approach or clarifying why a fixed ID is used here.
- <div><SelectVersion productId="1" /></div>
+ <div><SelectVersion productId={productData?.id} /></div>If this is intentional (e.g., for selecting versions of an existing product template), please add a code comment explaining this design decision.
📝 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.
| <div><SelectVersion productId="1" /></div> | |
| <div><SelectVersion productId={productData?.id} /></div> |
|
The preview deployment failed. 🔴 Last updated at: 2025-05-27 20:51:11 CET |
Description
Related Issue
Closes #
Changes
Checks
Summary by CodeRabbit