ronantakizawa/codereview-bench huggingface dataset:
Repository: medusajs/medusa
File: packages/admin/admin-bundler/src/utils/config.ts (TypeScript)

Code before:
      __STOREFRONT_URL__: JSON.stringify(storefrontUrl),
    },
    server: {
      fs: {
        allow: [searchForWorkspaceRoot(process.cwd())],
      },
      hmr: hmrOptions,
    },
    plugins: [
      writeStaticFiles({
        plugins: options.plugins,
      }),
      injectTailwindCSS({
        entry: root,
        sources: options.sources,
        plugins: options.plugins,
      }),
      react(),
      medusa({
        sources: options.sources,
      }),
    ],
  }

  // Inject plugin environment variables with vite define
  const pluginEnv = Object.fromEntries(
    Object.entries(process.env).filter(([key]) =>
      key.startsWith("PLUGIN_")
    )
  )
  pluginEnv["BACKEND_URL"] = backendUrl
  baseConfig.define!["process.env"] = JSON.stringify(pluginEnv)

  if (options.vite) {
    const customConfig = options.vite(baseConfig)
    return mergeConfig(baseConfig, customConfig)
  }

  return baseConfig
}

function getHmrConfig(hmrPort: number): HmrOptions | boolean {
  const options: HmrOptions = {
    port: hmrPort,
  }

  if (process.env.HMR_PROTOCOL) {
    options.protocol = process.env.HMR_PROTOCOL
  }
  if (process.env.HMR_HOST) {
    options.host = process.env.HMR_HOST

Review comment (suggestion):
Will that strip `PLUGIN_`? Like you suggested, it would probably be better that for PLUGIN_TEST, the dev inside the plugin can use process.env.TEST

Code after:
      __STOREFRONT_URL__: JSON.stringify(storefrontUrl),
    },
    server: {
      fs: {
        allow: [searchForWorkspaceRoot(process.cwd())],
      },
      hmr: hmrOptions,
    },
    plugins: [
      writeStaticFiles({
        plugins: options.plugins,
      }),
      injectTailwindCSS({
        entry: root,
        sources: options.sources,
        plugins: options.plugins,
      }),
      react(),
      medusa({
        sources: options.sources,
      }),
    ],
  }

  // Inject plugin environment variables with vite define
  const pluginEnv: Record<string, string | undefined> = { BACKEND_URL: backendUrl }
  for (const [key, value] of Object.entries(process.env)) {
    if (key.startsWith("PLUGIN_")) {
      pluginEnv[key.replace(/^PLUGIN_/, "")] = value
    }
  }
  baseConfig.define!["process.env"] = JSON.stringify(pluginEnv)

  if (options.vite) {
    const customConfig = options.vite(baseConfig)
    return mergeConfig(baseConfig, customConfig)
  }

  return baseConfig
}

function getHmrConfig(hmrPort: number): HmrOptions | boolean {
  const options: HmrOptions = {
    port: hmrPort,
  }

  if (process.env.HMR_PROTOCOL) {
    options.protocol = process.env.HMR_PROTOCOL
  }
  if (process.env.HMR_HOST) {
    options.host = process.env.HMR_HOST
Repository: medusajs/medusa
File: packages/core/core-flows/src/cart/workflows/complete-cart.ts (TypeScript)

Code before:
                }
              })) ??
            []

          return transactions
        }
      )

      addOrderTransactionStep(orderTransactions)

      /**
       * @ignore
       */
      createHook("orderCreated", {
        order_id: createdOrder.id,
        cart_id: cart.id,
      })

      return createdOrder
    })

    releaseLockStep({
      key: input.id,
    })

    const result = transform({ order, orderId }, async ({ order, orderId }) => {
      return { id: order?.id ?? orderId } as CompleteCartWorkflowOutput
    })

    return new WorkflowResponse(result, {
      hooks: [validate],
    })
  }
)


Review comment (suggestion):
```suggestion
    const result = transform({ order, orderId }, ({ order, orderId }) => {
```

Code after:
                }
              })) ??
            []

          return transactions
        }
      )

      addOrderTransactionStep(orderTransactions)

      /**
       * @ignore
       */
      createHook("orderCreated", {
        order_id: createdOrder.id,
        cart_id: cart.id,
      })

      return createdOrder
    })

    releaseLockStep({
      key: input.id,
    })

    const result = transform({ order, orderId }, ({ order, orderId }) => {
      return { id: order?.id ?? orderId } as CompleteCartWorkflowOutput
    })

    return new WorkflowResponse(result, {
      hooks: [validate],
    })
  }
)
Repository: medusajs/medusa
File: packages/core/types/src/caching/index.ts (TypeScript)

Code before:
  }: {
    key?: string
    tags?: string[]
    options?: {
      autoInvalidate?: boolean
    }
    providers?: string[]
  }): Promise<void>

  computeKey(input: object): Promise<string>

  computeTags(input: object, options?: Record<string, any>): Promise<string[]>
}

export interface ICachingProviderService {
  get({ key, tags }: { key?: string; tags?: string[] }): Promise<any>
  set({
    key,
    data,
    ttl,
    tags,
    options,
  }: {
    key: string
    data: object
    ttl?: number
    tags?: string[]
    options?: { autoInvalidate?: boolean }
  }): Promise<void>
  clear({
    key,
    tags,
    options,
  }: {
    key?: string
    tags?: string[]
    options?: { autoInvalidate?: boolean }
  }): Promise<void>
}

export interface EntityReference {
  type: string
  id: string | number
  field?: string
}

export interface ICachingStrategy {
  /**
   * This method is called when the application starts. It can be useful to set up some auto
   * invalidation logic that reacts to something.
   *

Review comment (suggestion):
Q: when doing that you wont benefit from the auto invalidation, but if you use a conventional tag such as in this example `Product:prod_123` or `Product:list:*` then you can also have the default strategy auto invalidation kicking in, dont know if we want to mention that.

Basically, specifying tags manually without following convention kind of disable auto invalidation as well

Code after:
    providers?: string[]
  }): Promise<T | null>

  /**
   * This method stores data in the cache.
   *
   * @param param0 - The options for storing the item.
   * @returns A promise that resolves when the item has been stored.
   * 
   * @example
   * To store with key:
   * 
   * ```ts
   * await cacheModuleService.set({
   *   key: "products",
   *   data: { id: "prod_123", title: "Product 123" },
   * })
   * ```
   * 
   * To store with tags:
   * 
   * ```ts
   * await cacheModuleService.set({
   *   key: "products-list",
   *   data: [{ id: "prod_123", title: "Product 123" }],
   *   tags: ["products"]
   * })
   * ```
   * 
   * To disable auto-invalidation for the item:
   * 
   * ```ts
   * await cacheModuleService.set({
   *   key: "products-list",
   *   data: [{ id: "prod_123", title: "Product 123" }],
   *   options: { autoInvalidate: false }
   * })
   * ```
   * 
   * The item is now only invalidated when calling the `clear` method directly with the same key or tags.
   * 
   * To store with specific providers:
   * 
   * ```ts
   * await cacheModuleService.set({
   *   key: "products",
   *   data: { id: "prod_123", title: "Product 123" },
   *   providers: [
   *     "caching-redis",
   *     { id: "caching-memcached", ttl: 120 } // custom TTL for this provider
   *   ]
Repository: medusajs/medusa
File: packages/modules/product/src/services/product-module-service.ts (TypeScript)

Code before:
  }

  @InjectManager()
  // @ts-ignore
  async retrieveProduct(
    productId: string,
    config?: FindConfig<ProductTypes.ProductDTO>,
    @MedusaContext() sharedContext?: Context
  ): Promise<ProductTypes.ProductDTO> {
    const shouldLoadVariantImages =
      config?.relations?.includes("variants.images")

    // Ensure we load necessary relations
    const relations = [...(config?.relations || [])]
    if (shouldLoadVariantImages) {
      if (!relations.includes("variants")) {
        relations.push("variants")
      }
      if (!relations.includes("images")) {
        relations.push("images")
      }
    }

    const product = await this.productService_.retrieve(
      productId,
      this.getProductFindConfig_({ ...config, relations }),
      sharedContext
    )

    if (shouldLoadVariantImages && product.variants && product.images) {
      await this.buildVariantImagesFromProduct(
        product.variants,
        product.images,
        sharedContext
      )
    }

    return this.baseRepository_.serialize<ProductTypes.ProductDTO>(product)
  }

  @InjectManager()
  // @ts-ignore
  async listProducts(
    filters?: ProductTypes.FilterableProductProps,
    config?: FindConfig<ProductTypes.ProductDTO>,
    sharedContext?: Context
  ): Promise<ProductTypes.ProductDTO[]> {
    const shouldLoadVariantImages =
      config?.relations?.includes("variants.images")

    // Ensure we load necessary relations

Review comment (suggestion):
```suggestion
      this.getProductFindConfig_({ ...config, relations: Array.from(relationsSet) }),
```

Code after:
  }

  @InjectManager()
  // @ts-ignore
  async retrieveProduct(
    productId: string,
    config?: FindConfig<ProductTypes.ProductDTO>,
    @MedusaContext() sharedContext?: Context
  ): Promise<ProductTypes.ProductDTO> {
    const relationsSet = new Set(config?.relations ?? [])
    const shouldLoadVariantImages = relationsSet.has("variants.images")
    if (shouldLoadVariantImages) {
      relationsSet.add("variants")
      relationsSet.add("images")
    }

    const product = await this.productService_.retrieve(
      productId,
      this.getProductFindConfig_({
        ...config,
        relations: Array.from(relationsSet),
      }),
      sharedContext
    )

    if (shouldLoadVariantImages && product.variants && product.images) {
      await this.buildVariantImagesFromProduct(
        product.variants,
        product.images,
        sharedContext
      )
    }

    return this.baseRepository_.serialize<ProductTypes.ProductDTO>(product)
  }

  @InjectManager()
  // @ts-ignore
  async listProducts(
    filters?: ProductTypes.FilterableProductProps,
    config?: FindConfig<ProductTypes.ProductDTO>,
    sharedContext?: Context
  ): Promise<ProductTypes.ProductDTO[]> {
    const relationsSet = new Set(config?.relations ?? [])
    const shouldLoadVariantImages = relationsSet.has("variants.images")
    if (shouldLoadVariantImages) {
      relationsSet.add("variants")
      relationsSet.add("images")
    }

    const products = await this.productService_.list(
Repository: medusajs/medusa
File: packages/medusa/src/api/admin/users/[id]/route.ts (TypeScript)

Code before:
    ],
  }

  await workflow.run({ input })

  const user = await refetchUser(
    req.params.id,
    req.scope,
    req.queryConfig.fields
  )

  res.status(200).json({ user })
}

// delete user
export const DELETE = async (
  req: AuthenticatedMedusaRequest,
  res: MedusaResponse<HttpTypes.AdminUserDeleteResponse>
) => {
  const { id } = req.params
  const { actor_id } = req.auth_context

  if (actor_id === id) {
    throw new MedusaError(
      MedusaError.Types.NOT_ALLOWED,
      "You can't delete yourself"
    )
  }

  const workflow = removeUserAccountWorkflow(req.scope)

  await workflow.run({
    input: { userId: id },
  })

  res.status(200).json({
    id,
    object: "user",
    deleted: true,
  })
}

export const AUTHENTICATE = false


Review comment (suggestion):
```suggestion
      "A user cannot delete itself"
```

Code after:
    ],
  }

  await workflow.run({ input })

  const user = await refetchUser(
    req.params.id,
    req.scope,
    req.queryConfig.fields
  )

  res.status(200).json({ user })
}

// delete user
export const DELETE = async (
  req: AuthenticatedMedusaRequest,
  res: MedusaResponse<HttpTypes.AdminUserDeleteResponse>
) => {
  const { id } = req.params
  const { actor_id } = req.auth_context

  if (actor_id === id) {
    throw new MedusaError(
      MedusaError.Types.NOT_ALLOWED,
      "A user cannot delete itself"
    )
  }

  const workflow = removeUserAccountWorkflow(req.scope)

  await workflow.run({
    input: { userId: id },
  })

  res.status(200).json({
    id,
    object: "user",
    deleted: true,
  })
}

export const AUTHENTICATE = false
Repository: medusajs/medusa
File: packages/admin/dashboard/src/components/common/file-upload/file-upload.tsx (TypeScript)

Code before:

  const handleDragLeave = (event: DragEvent) => {
    event.preventDefault()
    event.stopPropagation()

    if (
      !dropZoneRef.current ||
      dropZoneRef.current.contains(event.relatedTarget as Node)
    ) {
      return
    }

    setIsDragOver(false)
  }

  const handleUploaded = (files: FileList | null) => {
    if (!files) {
      return
    }

    const fileList = Array.from(files)
    const validFiles: FileType[] = []
    const rejectedFiles: RejectedFile[] = []

    fileList.forEach((file) => {
      if (file.size > maxFileSize && maxFileSize !== 0) {
        rejectedFiles.push({ file, reason: "size" })
        return
      }

      const id = Math.random().toString(36).substring(7)
      const previewUrl = URL.createObjectURL(file)
      validFiles.push({
        id: id,
        url: previewUrl,
        file,
      })
    })

    if (validFiles.length > 0 || rejectedFiles.length > 0) {
      onUploaded(validFiles, rejectedFiles)
    }
  }

  const handleDrop = (event: DragEvent) => {
    event.preventDefault()
    event.stopPropagation()

    setIsDragOver(false)

    handleUploaded(event.dataTransfer?.files)

Review comment (suggestion):
```suggestion
    const normalizeMaxFileSize = Math.min(maxFileSize, Infinity)
    fileList.forEach((file) => {
      if (file.size > maxFileSize) {
```

Code after:

  const handleDragLeave = (event: DragEvent) => {
    event.preventDefault()
    event.stopPropagation()

    if (
      !dropZoneRef.current ||
      dropZoneRef.current.contains(event.relatedTarget as Node)
    ) {
      return
    }

    setIsDragOver(false)
  }

  const handleUploaded = (files: FileList | null) => {
    if (!files) {
      return
    }

    const fileList = Array.from(files)
    const validFiles: FileType[] = []
    const rejectedFiles: RejectedFile[] = []
    const normalizedMaxFileSize = Math.min(maxFileSize, Infinity)

    fileList.forEach((file) => {
      if (file.size > normalizedMaxFileSize) {
        rejectedFiles.push({ file, reason: "size" })
        return
      }

      const id = Math.random().toString(36).substring(7)
      const previewUrl = URL.createObjectURL(file)
      validFiles.push({
        id: id,
        url: previewUrl,
        file,
      })
    })

    onUploaded(validFiles, rejectedFiles)
  }

  const handleDrop = (event: DragEvent) => {
    event.preventDefault()
    event.stopPropagation()

    setIsDragOver(false)

    handleUploaded(event.dataTransfer?.files)
  }
Repository: medusajs/medusa
File: packages/admin/dashboard/src/routes/products/common/components/upload-media-form-item/upload-media-form-item.tsx (TypeScript)

Code before:
}) => {
  const { t } = useTranslation()

  const hasInvalidFiles = useCallback(
    (fileList: FileType[] = [], rejectedFiles: RejectedFile[] = []) => {
      const invalidFile = fileList.find(
        (f) => !SUPPORTED_FORMATS.includes(f?.file?.type)
      )

      let hasInvalidFile = false;

      if (invalidFile) {
        form.setError("media", {
          type: "invalid_file",
          message: t("products.media.invalidFileType", {
            name: invalidFile.file.name,
            types: SUPPORTED_FORMATS_FILE_EXTENSIONS.join(", "),
          }),
        })

        hasInvalidFile = true;
      }

      const fileSizeRejections = rejectedFiles.filter((f) => f?.reason === "size")

      if (fileSizeRejections.length > 0) {
        const fileNames = fileSizeRejections.map((f) => f.file.name).join(", ")
        form.setError("media", {
          type: "file_too_large",
          message: t("products.media.fileTooLarge", {
            name: fileNames,
            size: "1MB",
          }),
        })

        hasInvalidFile = true;
      }

      return hasInvalidFile;
    },
    [form, t]
  )

  const onUploaded = useCallback(
    (files: FileType[] = [], rejectedFiles: RejectedFile[] = []) => {
      form.clearErrors("media")
      if (hasInvalidFiles(files, rejectedFiles)) {
        return
      }

      files?.forEach((f) => append({ ...f, isThumbnail: false }))

Review comment (suggestion):
```suggestion
      if (fileSizeRejections.length) {
```

Code after:
}) => {
  const { t } = useTranslation()

  const hasInvalidFiles = useCallback(
    (fileList: FileType[] = [], rejectedFiles: RejectedFile[] = []) => {
      const invalidFile = fileList.find(
        (f) => !SUPPORTED_FORMATS.includes(f?.file?.type)
      )

      let hasInvalidFile = false;

      if (invalidFile) {
        form.setError("media", {
          type: "invalid_file",
          message: t("products.media.invalidFileType", {
            name: invalidFile.file.name,
            types: SUPPORTED_FORMATS_FILE_EXTENSIONS.join(", "),
          }),
        })

        hasInvalidFile = true;
      }

      const fileSizeRejections = rejectedFiles.filter((f) => f?.reason === "size")

      if (fileSizeRejections.length) {
        const fileNames = fileSizeRejections.map((f) => f.file.name).join(", ")
        form.setError("media", {
          type: "file_too_large",
          message: t("products.media.fileTooLarge", {
            name: fileNames,
            size: "1MB",
          }),
        })

        hasInvalidFile = true;
      }

      return hasInvalidFile;
    },
    [form, t]
  )

  const onUploaded = useCallback(
    (files: FileType[] = [], rejectedFiles: RejectedFile[] = []) => {
      form.clearErrors("media")
      if (hasInvalidFiles(files, rejectedFiles)) {
        return
      }

      files?.forEach((f) => append({ ...f, isThumbnail: false }))
Repository: medusajs/medusa
File: packages/admin/dashboard/src/routes/orders/order-detail/components/order-activity-section/order-timeline.tsx (TypeScript)

Code before:
        itemsMap={firstItem.itemsMap}
      >
        {firstItem.children}
      </OrderActivityItem>
    </div>
  )
}

type Activity = {
  title: string
  timestamp: string | Date
  children?: ReactNode
  itemsToSend?: (
    | AdminClaim["additional_items"]
    | AdminExchange["additional_items"]
  )[]
  itemsToReturn?: AdminReturn["items"]
  itemsMap?: Map<string, AdminOrderLineItem>
}

const useActivityItems = (order: AdminOrder): Activity[] => {
  const { t } = useTranslation()

  let firstOrderVersion = order
  if (firstOrderVersion.version !== 1) {
    const { order: initialOrder = {} } = useOrder(firstOrderVersion.id, {
      version: 1,
      fields: "created_at,total,currency_code",
    })
    firstOrderVersion = initialOrder
  }

  const { order_changes: orderChanges = [] } = useOrderChanges(order.id, {
    change_type: [
      "edit",
      "claim",
      "exchange",
      "return",
      "transfer",
      "update_order",
    ],
  })

  const rmaChanges = orderChanges.filter(
    (oc) => !NON_RMA_CHANGE_TYPES.includes(oc.change_type)
  )

  const missingLineItemIds = getMissingLineItemIds(order, rmaChanges)
  const { order_items: removedLineItems = [] } = useOrderLineItems(
    order.id,


Review comment (suggestion):
we shouldn't render a hook conditionally, you can use `enabled` flag:

```
const { order: initialOrder =  order} = useOrder(
    firstOrderVersion.id, 
    { version: 1, fileds: "..." }, 
    {enabled: firstOrderVersion.version !== 1}
)
```

Code after:
        itemsMap={firstItem.itemsMap}
      >
        {firstItem.children}
      </OrderActivityItem>
    </div>
  )
}

type Activity = {
  title: string
  timestamp: string | Date
  children?: ReactNode
  itemsToSend?: (
    | AdminClaim["additional_items"]
    | AdminExchange["additional_items"]
  )[]
  itemsToReturn?: AdminReturn["items"]
  itemsMap?: Map<string, AdminOrderLineItem>
}

const useActivityItems = (order: AdminOrder): Activity[] => {
  const { t } = useTranslation()

  const { order: initialOrder = order } = useOrder(order.id, {
    version: 1,
    fields: "created_at,total,currency_code",
  }, { enabled: order.version !== 1})

  const { order_changes: orderChanges = [] } = useOrderChanges(order.id, {
    change_type: [
      "edit",
      "claim",
      "exchange",
      "return",
      "transfer",
      "update_order",
    ],
  })

  const rmaChanges = orderChanges.filter(
    (oc) => !NON_RMA_CHANGE_TYPES.includes(oc.change_type)
  )

  const missingLineItemIds = getMissingLineItemIds(order, rmaChanges)
  const { order_items: removedLineItems = [] } = useOrderLineItems(
    order.id,

    {
      fields: "+quantity",
      item_id: missingLineItemIds,
    },
Repository: medusajs/medusa
File: packages/medusa/src/api/admin/draft-orders/[id]/route.ts (TypeScript)

Code before:
  })

  res.status(200).json({ draft_order: result as HttpTypes.AdminDraftOrder })
}

export const POST = async (
  req: AuthenticatedMedusaRequest<
    HttpTypes.AdminUpdateDraftOrder,
    HttpTypes.AdminDraftOrderParams
  >,
  res: MedusaResponse<HttpTypes.AdminDraftOrderResponse>
) => {
  const query = req.scope.resolve(ContainerRegistrationKeys.QUERY)

  let userId = req.auth_context.actor_id
  const authType = req.auth_context.actor_type as AuthType

  const shouldResolveUser = authType === 'api-key'
  if (shouldResolveUser) {
    const {data: [apiKey]} = await query.graph({
        entity: 'api_key',
        fields: ['created_by'],
        filters: { id: userId },
    })
    userId = apiKey.created_by
  }

  await updateDraftOrderWorkflow(req.scope).run({
    input: {
      ...req.validatedBody,
      user_id: userId,
      id: req.params.id,
    },
  })

  const result = await query.graph({
    entity: "order",
    filters: { id: req.params.id },
    fields: req.queryConfig.fields,
  })

  res
    .status(200)
    .json({ draft_order: result.data[0] as HttpTypes.AdminDraftOrder })
}

/**
 * @since 2.8.4
 */
export const DELETE = async (
  req: AuthenticatedMedusaRequest,

Review comment (suggestion):
thought: This seems like something that should live in a middleware, as this is not really specific to draft orders ... think we should consider doing that instead

Also, this code does not follow our linting conventions

Code after:

  res.status(200).json({ draft_order: result as HttpTypes.AdminDraftOrder })
}

export const POST = async (
  req: AuthenticatedMedusaRequest<
    HttpTypes.AdminUpdateDraftOrder,
    HttpTypes.AdminDraftOrderParams
  >,
  res: MedusaResponse<HttpTypes.AdminDraftOrderResponse>
) => {
  const query = req.scope.resolve(ContainerRegistrationKeys.QUERY)

  await updateDraftOrderWorkflow(req.scope).run({
    input: {
      ...req.validatedBody,
      user_id: req.secret_key_context?.created_by ?? req.auth_context.actor_id,
      id: req.params.id,
    },
  })

  const result = await query.graph({
    entity: "order",
    filters: { id: req.params.id },
    fields: req.queryConfig.fields,
  })

  res
    .status(200)
    .json({ draft_order: result.data[0] as HttpTypes.AdminDraftOrder })
}

/**
 * @since 2.8.4
 */
export const DELETE = async (
  req: AuthenticatedMedusaRequest,
  res: MedusaResponse
) => {
  const { id } = req.params

  await deleteDraftOrdersWorkflow(req.scope).run({
    input: {
      order_ids: [id],
    },
  })

  res.status(200).json({
    id,
    object: "draft-order",
    deleted: true,