ronantakizawa/codereview-bench huggingface dataset:
Repository: medusajs/medusa
File: packages/modules/payment/src/services/payment-module.ts (TypeScript)

Code before:
        payment.provider_id,
        {
          data: payment.data!,
          context: {
            idempotency_key: capture?.id,
          },
        }
      )

      await this.paymentService_.update(
        {
          id: payment.id,
          data: paymentData.data,
          captured_at: options.isFullyCaptured ? new Date() : undefined,
        },
        sharedContext
      )
    } else if (options.isFullyCaptured && !payment.captured_at) {
      /**
       * In the case of auto capture, we need to update the payment to set the captured_at date
       * only if fully captured.
       */
      await this.paymentService_.update(
        {
          id: payment.id,
          captured_at: options.isFullyCaptured ? new Date() : undefined,
        },
        sharedContext
      )
    }

    return payment
  }

  @InjectManager()
  @EmitEvents()
  async refundPayment(
    data: CreateRefundDTO,
    @MedusaContext() sharedContext: Context = {}
  ): Promise<PaymentDTO> {
    const payment = await this.paymentService_.retrieve(
      data.payment_id,
      {
        select: [
          "id",
          "data",
          "provider_id",
          "payment_collection_id",
          "amount",
          "raw_amount",
        ],

Review comment (suggestion):
nit: we are already checking for a full capture in the else-if
```suggestion
          captured_at: new Date(),
```

Code after:
      const paymentData = await this.paymentProviderService_.capturePayment(
        payment.provider_id,
        {
          data: payment.data!,
          context: {
            idempotency_key: capture?.id,
          },
        }
      )

      await this.paymentService_.update(
        {
          id: payment.id,
          data: paymentData.data,
          captured_at: options.isFullyCaptured ? new Date() : undefined,
        },
        sharedContext
      )
    } else if (options.isFullyCaptured && !payment.captured_at) {
      /**
       * In the case of auto capture, we need to update the payment to set the captured_at date
       * only if fully captured.
       */
      await this.paymentService_.update(
        {
          id: payment.id,
          captured_at: new Date(),
        },
        sharedContext
      )
    }

    return payment
  }

  @InjectManager()
  @EmitEvents()
  async refundPayment(
    data: CreateRefundDTO,
    @MedusaContext() sharedContext: Context = {}
  ): Promise<PaymentDTO> {
    const payment = await this.paymentService_.retrieve(
      data.payment_id,
      {
        select: [
          "id",
          "data",
          "provider_id",
          "payment_collection_id",
          "amount",
          "raw_amount",
Repository: medusajs/medusa
File: packages/medusa/src/api/admin/price-lists/query-config.ts (TypeScript)

Code before:
export enum PriceListRelations {
  PRICES = "prices",
}

// Note: renamed to avoid referencing remoteQuery which i legacy
export const adminPriceListPriceQueryFields = [
  "id",
  "currency_code",
  "amount",
  "min_quantity",
  "max_quantity",
  "created_at",
  "deleted_at",
  "updated_at",
  "price_set.variant.id",
  "price_rules.value",
  "price_rules.attribute",
]

export const adminPriceListRemoteQueryFields = [
  "id",
  "type",
  "description",
  "title",
  "status",
  "starts_at",
  "ends_at",
  "created_at",
  "updated_at",
  "deleted_at",

Review comment (suggestion):
```suggestion
// Note: renamed to avoid referencing remoteQuery which is legacy
```

Code after:
export enum PriceListRelations {
  PRICES = "prices",
}

// Note: renamed to avoid referencing remoteQuery which is legacy
export const adminPriceListPriceQueryFields = [
  "id",
  "currency_code",
  "amount",
  "min_quantity",
  "max_quantity",
  "created_at",
  "deleted_at",
  "updated_at",
  "price_set.variant.id",
  "price_rules.value",
  "price_rules.attribute",
]

export const adminPriceListRemoteQueryFields = [
  "id",
  "type",
  "description",
  "title",
  "status",
  "starts_at",
  "ends_at",
  "created_at",
  "updated_at",
  "deleted_at",
Repository: medusajs/medusa
File: packages/core/core-flows/src/product/steps/dismiss-product-variants-inventory.ts (TypeScript)

Code before:
}

async function dismissVariantsInventory(
  variantIds: string[],
  query: Query,
  link: Link
): Promise<Record<string, string[]>> {
  const dismissedVariantInventoryItems: Record<string, string[]> = {}

  const { data: variantInventoryItems } = await query.graph({
    entity: "product_variant_inventory_item",
    fields: ["inventory_item_id", "variant_id"],
    filters: {
      variant_id: variantIds,
    },
  })

  const dismissLinks: LinkDefinition[] = []
  for (const variantId of variantIds) {
    if (!variantId) {
      continue
    }

    dismissedVariantInventoryItems[variantId] = variantInventoryItems
      .filter((item) => item.variant_id === variantId)
      .map((item: { inventory_item_id: string }) => {
        dismissLinks.push({
          [Modules.PRODUCT]: { variant_id: variantId },
          [Modules.INVENTORY]: {
            inventory_item_id: item.inventory_item_id,
          },
        })
        return item.inventory_item_id
      })
  }

  await link.dismiss(dismissLinks)

  return dismissedVariantInventoryItems
}

export const dismissProductVariantsInventoryStep = createStep(
  dismissProductVariantsInventoryStepId,
  async (data: DismissProductVariantsInventoryStepInput, { container }) => {
    const query = container.resolve(ContainerRegistrationKeys.QUERY)
    const link = container.resolve(ContainerRegistrationKeys.LINK)
    const variantIds = data.variantIds || []

    if (!variantIds.length) {
      return new StepResponse(void 0)
    }

Review comment (suggestion):
suggestion: `map` return a new array, which here we dont need as we just want to loop through. I would either suggest `forEach` or `for of` loop

Code after:
}

async function dismissVariantsInventory(
  variantIds: string[],
  query: Query,
  link: Link
): Promise<Record<string, string[]>> {
  const dismissedVariantInventoryItems: Record<string, string[]> = {}
  if (!variantIds.length) {
    return dismissedVariantInventoryItems
  }

  const { data: variantInventoryItems } = await query.graph({
    entity: "product_variant_inventory_item",
    fields: ["inventory_item_id", "variant_id"],
    filters: {
      variant_id: variantIds,
    },
  })

  const variantInventoryItemsMap = new Map<string, string[]>()
  for (const item of variantInventoryItems) {
    variantInventoryItemsMap.set(item.variant_id, [
      ...(variantInventoryItemsMap.get(item.variant_id) ?? []),
      item.inventory_item_id,
    ])
  }

  const dismissLinks: LinkDefinition[] = []
  for (const variantId of variantIds) {
    if (!variantId) {
      continue
    }

    dismissedVariantInventoryItems[variantId] =
      variantInventoryItemsMap.get(variantId) ?? []

    for (const inventoryItemId of variantInventoryItemsMap.get(variantId) ??
      []) {
      dismissLinks.push({
        [Modules.PRODUCT]: { variant_id: variantId },
        [Modules.INVENTORY]: { inventory_item_id: inventoryItemId },
      })
    }
  }

  await link.dismiss(dismissLinks)

  return dismissedVariantInventoryItems
}
Repository: medusajs/medusa
File: packages/core/core-flows/src/product/workflows/update-product-variants.ts (TypeScript)

Code before:

      return {
        selector: data.input.selector,
        update: {
          ...data.input.update,
          prices: undefined,
        },
      }
    })

    const updatedVariants = updateProductVariantsStep(updateWithoutPrices)

    const variantsToDismissInventory = transform(
      { input, updatedVariants },
      (data) => {
        const variantIds: string[] = []

        if ("product_variants" in data.input) {
          variantIds.push(
            ...data.input.product_variants
              .filter((v) => v.id && v.manage_inventory === false)
              .map((v) => v.id!)
          )
        } else if (
          data.input.update &&
          data.input.update.manage_inventory === false
        ) {
          variantIds.push(...data.updatedVariants.map((v) => v.id))
        }

        return variantIds
      }
    )

    dismissProductVariantsInventoryStep({
      variantIds: variantsToDismissInventory,
    })

    // We don't want to do any pricing updates if the prices didn't change
    const variantIds = transform({ input, updatedVariants }, (data) => {
      if ("product_variants" in data.input) {
        const variantsWithPriceUpdates = new Set(
          data.input.product_variants.filter((v) => !!v.prices).map((v) => v.id)
        )

        return data.updatedVariants
          .map((v) => v.id)
          .filter((id) => variantsWithPriceUpdates.has(id))
      }

      if (!data.input.update.prices) {

Review comment (suggestion):
```suggestion
          data.input.update?.manage_inventory === false
```

Code after:

      return {
        selector: data.input.selector,
        update: {
          ...data.input.update,
          prices: undefined,
        },
      }
    })

    const updatedVariants = updateProductVariantsStep(updateWithoutPrices)

    const variantsToDismissInventory = transform(
      { input, updatedVariants },
      (data) => {
        const variantIds: string[] = []

        if ("product_variants" in data.input) {
          for (const variant of data.input.product_variants) {
            if (variant.id && variant.manage_inventory === false) {
              variantIds.push(variant.id)
            }
          }
        } else if (
          data.input.update &&
          data.input.update?.manage_inventory === false
        ) {
          variantIds.push(...data.updatedVariants.map((v) => v.id))
        }

        return variantIds
      }
    )

    dismissProductVariantsInventoryStep({
      variantIds: variantsToDismissInventory,
    })

    // We don't want to do any pricing updates if the prices didn't change
    const variantIds = transform({ input, updatedVariants }, (data) => {
      if ("product_variants" in data.input) {
        const variantsWithPriceUpdates = new Set(
          data.input.product_variants.filter((v) => !!v.prices).map((v) => v.id)
        )

        return data.updatedVariants
          .map((v) => v.id)
          .filter((id) => variantsWithPriceUpdates.has(id))
      }

      if (!data.input.update.prices) {
Repository: medusajs/medusa
File: packages/modules/event-bus-local/src/services/event-bus-local.ts (TypeScript)

Code before:
  constructor(
    { logger }: MedusaContainer & InjectedDependencies,
    moduleOptions = {},
    moduleDeclaration: InternalModuleDeclaration
  ) {
    // @ts-ignore
    // eslint-disable-next-line prefer-rest-params
    super(...arguments)

    this.logger_ = logger ?? console
    this.eventEmitter_ = eventEmitter
    this.groupedEventsMap_ = new Map()
  }

  static logProcessingEvent(
    eventData: Message,
    options: Record<string, unknown> = {},
    totalSubscribers: number,
    logger: Logger
  ) {
    if (
      totalSubscribers &&
      !options?.internal &&
      !eventData.options?.internal
    ) {
      logger?.info(
        `Processing ${eventData.name} which has ${totalSubscribers} subscribers`
      )
    }
  }

  /**
   * Accept an event name and some options
   *
   * @param eventsData
   * @param options The options can include `internal` which will prevent the event from being logged
   */
  async emit<T = unknown>(
    eventsData: Message<T> | Message<T>[],
    options: Record<string, unknown> = {}
  ): Promise<void> {
    const normalizedEventsData = Array.isArray(eventsData)
      ? eventsData
      : [eventsData]

    for (const eventData of normalizedEventsData) {
      await this.groupOrEmitEvent({
        ...eventData,
        options,
      })
    }

Review comment (suggestion):
nit: `?` is not necessary here
```suggestion
      logger.info(
```

Code after:
  constructor(
    { logger }: MedusaContainer & InjectedDependencies,
    moduleOptions = {},
    moduleDeclaration: InternalModuleDeclaration
  ) {
    // @ts-ignore
    // eslint-disable-next-line prefer-rest-params
    super(...arguments)

    this.logger_ = logger ?? console
    this.eventEmitter_ = eventEmitter
    this.groupedEventsMap_ = new Map()
  }

  private logProcessingEvent(
    eventData: Message,
    options: Record<string, unknown> = {},
    totalSubscribers: number
  ) {
    if (
      totalSubscribers &&
      !options?.internal &&
      !eventData.options?.internal
    ) {
      this.logger_.info(
        `Processing ${eventData.name} which has ${totalSubscribers} subscribers`
      )
    }
  }

  /**
   * Accept an event name and some options
   *
   * @param eventsData
   * @param options The options can include `internal` which will prevent the event from being logged
   */
  async emit<T = unknown>(
    eventsData: Message<T> | Message<T>[],
    options: Record<string, unknown> = {}
  ): Promise<void> {
    const normalizedEventsData = Array.isArray(eventsData)
      ? eventsData
      : [eventsData]

    for (const eventData of normalizedEventsData) {
      await this.groupOrEmitEvent({
        ...eventData,
        options,
      })
    }
  }
Repository: medusajs/medusa
File: packages/medusa/src/api/store/locales/route.ts (TypeScript)

Code before:
import { MedusaRequest, MedusaResponse } from "@medusajs/framework/http"
import { ContainerRegistrationKeys } from "@medusajs/framework/utils"

export const GET = async (req: MedusaRequest, res: MedusaResponse) => {
  const query = req.scope.resolve(ContainerRegistrationKeys.QUERY)

  const {
    data: [store],
  } = await query.graph({
    entity: "store",
    fields: ["supported_locales.*", "supported_locales.locale.*"],
    pagination: {
      take: 1,
    },
  })

  res.json({
    locales: store?.supported_locales ?? [],
  })
}


Review comment (suggestion):
thought(non-blocking): should we map it to something a bit more simple?
```
{ code: "en-US", name: "English (US)" }
```

Don't think you need anything else from the locales. Makes it easier to consume in the storefront.

Code after:
import { MedusaRequest, MedusaResponse } from "@medusajs/framework/http"
import { ContainerRegistrationKeys } from "@medusajs/framework/utils"

export const GET = async (req: MedusaRequest, res: MedusaResponse) => {
  const query = req.scope.resolve(ContainerRegistrationKeys.QUERY)

  const {
    data: [store],
  } = await query.graph({
    entity: "store",
    fields: ["supported_locales.*", "supported_locales.locale.*"],
    pagination: {
      take: 1,
    },
  })

  const locales = store?.supported_locales.reduce((acc, locale) => {
    acc.push({
      code: locale.locale_code,
      name: locale.locale.name,
    })
    return acc
  }, [])

  res.json({
    locales,
  })
}
Repository: medusajs/medusa
File: packages/medusa/src/api/store/product-options/[id]/route.ts (TypeScript)

Code before:
import { HttpTypes } from "@medusajs/framework/types"
import {
  ContainerRegistrationKeys,
  MedusaError,
} from "@medusajs/framework/utils"
import {
  AuthenticatedMedusaRequest,
  MedusaResponse,
} from "@medusajs/framework/http"

export const GET = async (
  req: AuthenticatedMedusaRequest<HttpTypes.SelectParams>,
  res: MedusaResponse<HttpTypes.StoreProductOptionResponse>
) => {
  const query = req.scope.resolve(ContainerRegistrationKeys.QUERY)

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

  if (!data.length) {
    throw new MedusaError(
      MedusaError.Types.NOT_FOUND,
      `Product option with id: ${req.params.id} was not found`
    )
  }

  res.json({ product_option: data[0] })
}


Review comment (security):
q: shouldn't this be `req: AuthenticatedMedusaRequest<{}, HttpTypes.SelectParams>,`

Code after:
import { HttpTypes } from "@medusajs/framework/types"
import {
  ContainerRegistrationKeys,
  MedusaError,
} from "@medusajs/framework/utils"
import {
  AuthenticatedMedusaRequest,
  MedusaResponse,
} from "@medusajs/framework/http"

export const GET = async (
  req: AuthenticatedMedusaRequest<{}, HttpTypes.SelectParams>,
  res: MedusaResponse<HttpTypes.StoreProductOptionResponse>
) => {
  const query = req.scope.resolve(ContainerRegistrationKeys.QUERY)

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

  if (!data.length) {
    throw new MedusaError(
      MedusaError.Types.NOT_FOUND,
      `Product option with id: ${req.params.id} was not found`
    )
  }

  res.json({ product_option: data[0] })
}
Repository: medusajs/medusa
File: packages/core/utils/src/core-flows/events.ts (TypeScript)

Code before:
   * @eventPayload
   * ```ts
   * {
   *   id, // The ID of the user
   * }
   * ```
   */
  DELETED: "user.deleted",
}

/**
 * @category Auth
 * @customNamespace Auth
 */
export const AuthWorkflowEvents = {
  /**
   * Emitted when a reset password token is generated. You can listen to this event
   * to send a reset password email to the user or customer, for example.
   *
   * @eventPayload
   * ```ts
   * {
   *   entity_id, // The identifier of the user or customer. For example, an email address.
   *   actor_type, // The type of actor. For example, "customer", "user", or custom.
   *   token, // The generated token.
   *   metadata?, // Optional custom metadata passed from the request.
   * }
   * ```
   */
  PASSWORD_RESET: "auth.password_reset",
}

/**
 * @category Sales Channel
 * @customNamespace Sales Channel
 */
export const SalesChannelWorkflowEvents = {
  /**
   * Emitted when sales channels are created.
   *
   * @eventPayload
   * ```ts
   * {
   *   id, // The ID of the sales channel
   * }
   * ```
   */
  CREATED: "sales-channel.created",
  /**
   * Emitted when sales channels are updated.
   *

Review comment (suggestion):
```suggestion
   *   metadata, // Optional custom metadata passed from the request.
```

Code after:
   * @eventPayload
   * ```ts
   * {
   *   id, // The ID of the user
   * }
   * ```
   */
  DELETED: "user.deleted",
}

/**
 * @category Auth
 * @customNamespace Auth
 */
export const AuthWorkflowEvents = {
  /**
   * Emitted when a reset password token is generated. You can listen to this event
   * to send a reset password email to the user or customer, for example.
   *
   * @eventPayload
   * ```ts
   * {
   *   entity_id, // The identifier of the user or customer. For example, an email address.
   *   actor_type, // The type of actor. For example, "customer", "user", or custom.
   *   token, // The generated token.
   *   metadata, // Optional custom metadata passed from the request.
   * }
   * ```
   */
  PASSWORD_RESET: "auth.password_reset",
}

/**
 * @category Sales Channel
 * @customNamespace Sales Channel
 */
export const SalesChannelWorkflowEvents = {
  /**
   * Emitted when sales channels are created.
   *
   * @eventPayload
   * ```ts
   * {
   *   id, // The ID of the sales channel
   * }
   * ```
   */
  CREATED: "sales-channel.created",
  /**
   * Emitted when sales channels are updated.
   *
Repository: medusajs/medusa
File: packages/modules/product/src/repositories/product.ts (TypeScript)

Code before:
    }
  }

  async getOptionValueIdsByProductIds(
    productIds: string[],
    context: Context = {}
  ): Promise<Map<string, Set<string>>> {
    const allowedValueIdsByProduct = new Map<string, Set<string>>()

    if (!productIds.length) {
      return allowedValueIdsByProduct
    }

    const knex = this.getActiveManager<SqlEntityManager>(context)
      .getConnection()
      .getKnex()

    const rows = await knex("product_product_option_value as ppov")
      .select("ppo.product_id", "ppov.product_option_value_id")
      .innerJoin(
        "product_product_option as ppo",
        "ppo.id",
        "ppov.product_product_option_id"
      )
      .whereIn("ppo.product_id", productIds)
      .whereNull("ppo.deleted_at")
      .whereNull("ppov.deleted_at")

    rows.forEach((row) => {
      if (!allowedValueIdsByProduct.has(row.product_id)) {
        allowedValueIdsByProduct.set(row.product_id, new Set())
      }
      allowedValueIdsByProduct
        .get(row.product_id)!
        .add(row.product_option_value_id)
    })

    return allowedValueIdsByProduct
  }
}


Review comment (suggestion):
This `ppo.delete_at is null` could be added to the inner join directly.
I don't believe it will change the performace because it is very likely the query planner will do that, but it worth trying.

Code after:
    }
  }

  async getOptionValueIdsByProductIds(
    productIds: string[],
    context: Context = {}
  ): Promise<Map<string, Set<string>>> {
    const allowedValueIdsByProduct = new Map<string, Set<string>>()

    if (!productIds.length) {
      return allowedValueIdsByProduct
    }

    const knex = this.getActiveManager<SqlEntityManager>(context)
      .getConnection()
      .getKnex()

    const rows = await knex("product_product_option_value as ppov")
      .select("ppo.product_id", "ppov.product_option_value_id")
      .innerJoin("product_product_option as ppo", function () {
        this.on("ppo.id", "ppov.product_product_option_id").andOnNull(
          "ppo.deleted_at"
        )
      })
      .whereIn("ppo.product_id", productIds)
      .whereNull("ppov.deleted_at")

    rows.forEach((row) => {
      if (!allowedValueIdsByProduct.has(row.product_id)) {
        allowedValueIdsByProduct.set(row.product_id, new Set())
      }
      allowedValueIdsByProduct
        .get(row.product_id)!
        .add(row.product_option_value_id)
    })

    return allowedValueIdsByProduct
  }
}