ronantakizawa/codereview-bench huggingface dataset:
Repository: hashicorp/vault
File: ui/app/components/mount-backend/type-form.js (JavaScript)

Code before:

/**
 *
 * @module MountBackendTypeForm
 * MountBackendTypeForm components are used to display type options for
 * mounting either an auth method or secret engine.
 *
 * @example
 * ```js
 * <MountBackend::TypeForm @setMountType={{this.setMountType}} @mountType="secret" />
 * ```
 * @param {CallableFunction} setMountType - function will receive the mount type string. Should update the model type value
 * @param {string} [mountType=auth] - mount type can be `auth` or `secret`
 */

export default class MountBackendTypeForm extends Component {
  @service version;

  get secretEngines() {
    return this.version.isEnterprise
      ? ALL_ENGINES.filter((engine) => engine.mountType !== 'auth')
      : ALL_ENGINES.filter((engine) => engine.mountType !== 'auth' && !engine.requiresEnterprise);
  }

  get authMethods() {
    return this.version.isEnterprise
      ? ALL_ENGINES.filter((engine) => engine.mountType !== 'secret')
      : ALL_ENGINES.filter((engine) => engine.mountType !== 'secret' && !engine.requiresEnterprise);
  }

  get mountTypes() {
    return this.args.mountType === 'secret' ? this.secretEngines : this.authMethods;
  }
}


Review comment (security):
```suggestion
// If an enterprise license is present, return all auth engines;
// otherwise, return only the secret engines supported in OSS.
    return this.version.isEnterprise
```

Code after:

/**
 *
 * @module MountBackendTypeForm
 * MountBackendTypeForm components are used to display type options for
 * mounting either an auth method or secret engine.
 *
 * @example
 * ```js
 * <MountBackend::TypeForm @setMountType={{this.setMountType}} @mountCategory="secret" />
 * ```
 * @param {CallableFunction} setMountType - function will receive the mount type string. Should update the model type value
 * @param {string} [mountCategory=auth] - mount category type can be `auth` or `secret`
 */

export default class MountBackendTypeForm extends Component {
  @service version;

  get secretEngines() {
    // If an enterprise license is present, return all secret engines;
    // otherwise, return only the secret engines supported in OSS.
    return filterEnginesByMountCategory({ mountCategory: 'secret', isEnterprise: this.version.isEnterprise });
  }

  get authMethods() {
    // If an enterprise license is present, return all auth methods;
    // otherwise, return only the auth methods supported in OSS.
    return filterEnginesByMountCategory({ mountCategory: 'auth', isEnterprise: this.version.isEnterprise });
  }

  get mountTypes() {
    return this.args.mountCategory === 'secret' ? this.secretEngines : this.authMethods;
  }
}
Repository: hashicorp/vault
File: ui/app/helpers/engines-display-data.ts (TypeScript)

Code before:
/**
 * Copyright (c) HashiCorp, Inc.
 * SPDX-License-Identifier: BUSL-1.1
 */

import { ALL_ENGINES } from 'vault/utils/all-engines-metadata';

export default function engineDisplayData(methodType: string) {
  const engine = ALL_ENGINES?.find((t) => t.type === methodType);
  return engine;
}


Review comment (security):
```suggestion
/**
 * Helper function to retrieve engine metadata for a given `methodType`.
 * It searches the `ALL_ENGINES` array for an engine with a matching type and returns its metadata object. 
 * The `ALL_ENGINES` array includes secret and auth engines, including those supported only in enterprise.
 * These details (such as mount type and enterprise licensing) are included in the returned engine object.
 *
 * Example usage:
 * const engineMetadata = engineDisplayData('kmip');
 * if (engineMetadata?.requiresEnterprise) {
 *   console.log(`This mount: ${engineMetadata.engineType} requires an enterprise license`);
 * }
 *
 * @param {string} methodType - The engine type (sometimes called backend) to look up (e.g., "aws", "azure"). 
 * @returns {Object|undefined} - The engine metadata, which includes information about its mount type (e.g., secret or auth)
 *   and whether it requires an enterprise license. Returns undefined if no match is found.
 */

```

Code after:
/**
 * Copyright (c) HashiCorp, Inc.
 * SPDX-License-Identifier: BUSL-1.1
 */

import { ALL_ENGINES } from 'vault/utils/all-engines-metadata';

/**
 * Helper function to retrieve engine metadata for a given `methodType`.
 * It searches the `ALL_ENGINES` array for an engine with a matching type and returns its metadata object.
 * The `ALL_ENGINES` array includes secret and auth engines, including those supported only in enterprise.
 * These details (such as mount type and enterprise licensing) are included in the returned engine object.
 *
 * Example usage:
 * const engineMetadata = engineDisplayData('kmip');
 * if (engineMetadata?.requiresEnterprise) {
 *   console.log(`This mount: ${engineMetadata.engineType} requires an enterprise license`);
 * }
 *
 * @param {string} methodType - The engine type (sometimes called backend) to look up (e.g., "aws", "azure").
 * @returns {Object|undefined} - The engine metadata, which includes information about its mount type (e.g., secret or auth)
 *   and whether it requires an enterprise license. Returns undefined if no match is found.
 */
export default function engineDisplayData(methodType: string) {
  const engine = ALL_ENGINES?.find((t) => t.type === methodType);
  return engine;
}
Repository: hashicorp/vault
File: ui/app/utils/all-engines-metadata.ts (TypeScript)

Code before:
/**
 * Copyright (c) HashiCorp, Inc.
 * SPDX-License-Identifier: BUSL-1.1
 */

/**
 * These are all the secret and auth methods, including enterprise.
 * If filtering for enterprise-only engines or a specific category is required, that logic should be done within the consumer of this utility.
 */

export interface EngineDisplayData {
  category?: string;
  displayName: string;
  glyph?: string;
  mountType: 'secret' | 'auth' | 'both';
  requiresEnterprise?: boolean;
  type: string;
  value?: string;
}

export const ALL_ENGINES: EngineDisplayData[] = [
  {
    category: 'cloud',
    displayName: 'AliCloud',
    glyph: 'alibaba-color',
    mountType: 'both',
    type: 'alicloud',
  },
  {
    category: 'generic',
    displayName: 'AppRole',
    glyph: 'cpu',
    mountType: 'auth',
    type: 'approle',
    value: 'approle',
  },
  {
    category: 'cloud',
    displayName: 'AWS',
    glyph: 'aws-color',

Review comment (security):
```suggestion
  mountType: ['auth'] | ['secret'] | ['auth', 'secret'];
```

Code after:
/**
 * Copyright (c) HashiCorp, Inc.
 * SPDX-License-Identifier: BUSL-1.1
 */

/**
 * These are all the secret and auth methods, including enterprise.
 */

export interface EngineDisplayData {
  category?: string;
  displayName: string;
  glyph?: string;
  mountCategory: string[];
  requiresEnterprise?: boolean;
  type: string;
  value?: string;
}

/**
 * @param mountCategory - Given mount category to filter by, e.g., 'auth' or 'secret'.
 * @param isEnterprise - Optional boolean to indicate if enterprise engines should be included in the results.
 * @returns Filtered array of engines that match the given mount category
 */
export function filterEnginesByMountCategory({
  mountCategory,
  isEnterprise = false,
}: {
  mountCategory: 'auth' | 'secret';
  isEnterprise: boolean;
}) {
  return isEnterprise
    ? ALL_ENGINES.filter((engine) => engine.mountCategory.includes(mountCategory))
    : ALL_ENGINES.filter(
        (engine) => engine.mountCategory.includes(mountCategory) && !engine.requiresEnterprise
      );
}

export const ALL_ENGINES: EngineDisplayData[] = [
  {
Repository: hashicorp/vault
File: ui/app/utils/all-engines-metadata.ts (TypeScript)

Code before:
/**
 * Copyright (c) HashiCorp, Inc.
 * SPDX-License-Identifier: BUSL-1.1
 */

/**
 * These are all the secret and auth methods, including enterprise.
 * If filtering for enterprise-only engines or a specific category is required, that logic should be done within the consumer of this utility.
 */

export interface EngineDisplayData {
  category?: string;
  displayName: string;
  glyph?: string;
  mountType: 'secret' | 'auth' | 'both';
  requiresEnterprise?: boolean;
  type: string;
  value?: string;
}

export const ALL_ENGINES: EngineDisplayData[] = [
  {
    category: 'cloud',
    displayName: 'AliCloud',
    glyph: 'alibaba-color',
    mountType: 'both',
    type: 'alicloud',
  },
  {
    category: 'generic',
    displayName: 'AppRole',
    glyph: 'cpu',
    mountType: 'auth',
    type: 'approle',
    value: 'approle',
  },
  {
    category: 'cloud',
    displayName: 'AWS',
    glyph: 'aws-color',
    mountType: 'both',
    type: 'aws',
  },
  {
    category: 'cloud',
    displayName: 'Azure',
    glyph: 'azure-color',
    mountType: 'both',
    type: 'azure',
  },
  {

Review comment (security):
Update these according.
```suggestion
    mountType: ['auth', 'secret'],
```

Code after:
/**
 * Copyright (c) HashiCorp, Inc.
 * SPDX-License-Identifier: BUSL-1.1
 */

/**
 * These are all the secret and auth methods, including enterprise.
 */

export interface EngineDisplayData {
  category?: string;
  displayName: string;
  glyph?: string;
  mountCategory: string[];
  requiresEnterprise?: boolean;
  type: string;
  value?: string;
}

/**
 * @param mountCategory - Given mount category to filter by, e.g., 'auth' or 'secret'.
 * @param isEnterprise - Optional boolean to indicate if enterprise engines should be included in the results.
 * @returns Filtered array of engines that match the given mount category
 */
export function filterEnginesByMountCategory({
  mountCategory,
  isEnterprise = false,
}: {
  mountCategory: 'auth' | 'secret';
  isEnterprise: boolean;
}) {
  return isEnterprise
    ? ALL_ENGINES.filter((engine) => engine.mountCategory.includes(mountCategory))
    : ALL_ENGINES.filter(
        (engine) => engine.mountCategory.includes(mountCategory) && !engine.requiresEnterprise
      );
}

export const ALL_ENGINES: EngineDisplayData[] = [
  {
    category: 'cloud',
    displayName: 'AliCloud',
    glyph: 'alibaba-color',
    mountCategory: ['auth', 'secret'],
    type: 'alicloud',
  },
  {
    category: 'generic',
    displayName: 'AppRole',
    glyph: 'cpu',
    mountCategory: ['auth'],
Repository: hashicorp/vault
File: ui/app/utils/all-engines-metadata.ts (TypeScript)

Code before:
/**
 * Copyright (c) HashiCorp, Inc.
 * SPDX-License-Identifier: BUSL-1.1
 */

/**
 * These are all the secret and auth methods, including enterprise.
 * If filtering for enterprise-only engines or a specific category is required, that logic should be done within the consumer of this utility.
 */

export interface EngineDisplayData {
  category?: string;
  displayName: string;
  glyph?: string;
  mountType: 'secret' | 'auth' | 'both';
  requiresEnterprise?: boolean;
  type: string;
  value?: string;
}

export const ALL_ENGINES: EngineDisplayData[] = [
  {
    category: 'cloud',
    displayName: 'AliCloud',
    glyph: 'alibaba-color',
    mountType: 'both',
    type: 'alicloud',
  },
  {
    category: 'generic',
    displayName: 'AppRole',
    glyph: 'cpu',
    mountType: 'auth',
    type: 'approle',
    value: 'approle',
  },
  {
    category: 'cloud',
    displayName: 'AWS',
    glyph: 'aws-color',

Review comment (suggestion):
I love the cleanup in this PR! One thought - the word "type" is used _a lot_. I agree with the update to this being an array, but wonder if we could rename it to `engineCategory`/`engineGroups`/`mountGroups` or something. `mountType` could easily be confused for `methodType` 

( I think my vote is `mountGroups`? - but no strong feelings)

Code after:
/**
 * Copyright (c) HashiCorp, Inc.
 * SPDX-License-Identifier: BUSL-1.1
 */

/**
 * These are all the secret and auth methods, including enterprise.
 */

export interface EngineDisplayData {
  category?: string;
  displayName: string;
  glyph?: string;
  mountCategory: string[];
  requiresEnterprise?: boolean;
  type: string;
  value?: string;
}

/**
 * @param mountCategory - Given mount category to filter by, e.g., 'auth' or 'secret'.
 * @param isEnterprise - Optional boolean to indicate if enterprise engines should be included in the results.
 * @returns Filtered array of engines that match the given mount category
 */
export function filterEnginesByMountCategory({
  mountCategory,
  isEnterprise = false,
}: {
  mountCategory: 'auth' | 'secret';
  isEnterprise: boolean;
}) {
  return isEnterprise
    ? ALL_ENGINES.filter((engine) => engine.mountCategory.includes(mountCategory))
    : ALL_ENGINES.filter(
        (engine) => engine.mountCategory.includes(mountCategory) && !engine.requiresEnterprise
      );
}

export const ALL_ENGINES: EngineDisplayData[] = [
  {
Repository: hashicorp/vault
File: ui/app/utils/all-engines-metadata.ts (TypeScript)

Code before:
/**
 * Copyright (c) HashiCorp, Inc.
 * SPDX-License-Identifier: BUSL-1.1
 */

/**
 * These are all the secret and auth methods, including enterprise.
 */

export interface EngineDisplayData {
  category?: string;
  displayName: string;
  glyph?: string;
  mountGroup: string[]; // ['auth'] | ['secret'] | ['auth', 'secret']; // - changed to string[] because of ts error with filterEnginesByMountType - couldn't check mount of type 'never'
  requiresEnterprise?: boolean;
  type: string;
  value?: string;
}

/**
 * @param mountType - Given mount type to filter by, e.g., 'auth' or 'secret'.
 * @param useEnterprise - Optional boolean to indicate if enterprise engines should be included in the results.
 * @returns Filtered array of engines that match the given mount type
 */
export function filterEnginesByMountType(mountType: string, useEnterprise?: boolean) {
  return useEnterprise
    ? ALL_ENGINES.filter((engine) => engine.mountGroup.includes(mountType))
    : ALL_ENGINES.filter((engine) => engine.mountGroup.includes(mountType) && !engine.requiresEnterprise);
}

export const ALL_ENGINES: EngineDisplayData[] = [
  {
    category: 'cloud',
    displayName: 'AliCloud',
    glyph: 'alibaba-color',
    mountGroup: ['auth', 'secret'],
    type: 'alicloud',
  },
  {
    category: 'generic',
    displayName: 'AppRole',
    glyph: 'cpu',
    mountGroup: ['auth'],
    type: 'approle',
    value: 'approle',
  },
  {
    category: 'cloud',
    displayName: 'AWS',
    glyph: 'aws-color',

Review comment (suggestion):
I went with what Claire said about making a filter function since its used in multiple places,
open to changing param names and such

Code after:
/**
 * Copyright (c) HashiCorp, Inc.
 * SPDX-License-Identifier: BUSL-1.1
 */

/**
 * These are all the secret and auth methods, including enterprise.
 */

export interface EngineDisplayData {
  category?: string;
  displayName: string;
  glyph?: string;
  mountCategory: string[];
  requiresEnterprise?: boolean;
  type: string;
  value?: string;
}

/**
 * @param mountCategory - Given mount category to filter by, e.g., 'auth' or 'secret'.
 * @param isEnterprise - Optional boolean to indicate if enterprise engines should be included in the results.
 * @returns Filtered array of engines that match the given mount category
 */
export function filterEnginesByMountCategory({
  mountCategory,
  isEnterprise = false,
}: {
  mountCategory: 'auth' | 'secret';
  isEnterprise: boolean;
}) {
  return isEnterprise
    ? ALL_ENGINES.filter((engine) => engine.mountCategory.includes(mountCategory))
    : ALL_ENGINES.filter(
        (engine) => engine.mountCategory.includes(mountCategory) && !engine.requiresEnterprise
      );
}

export const ALL_ENGINES: EngineDisplayData[] = [
  {
    category: 'cloud',
    displayName: 'AliCloud',
    glyph: 'alibaba-color',
    mountCategory: ['auth', 'secret'],
    type: 'alicloud',
  },
  {
    category: 'generic',
    displayName: 'AppRole',
    glyph: 'cpu',
Repository: hashicorp/vault
File: ui/app/utils/all-engines-metadata.ts (TypeScript)

Code before:
/**
 * Copyright (c) HashiCorp, Inc.
 * SPDX-License-Identifier: BUSL-1.1
 */

/**
 * These are all the secret and auth methods, including enterprise.
 */

export interface EngineDisplayData {
  category?: string;
  displayName: string;
  glyph?: string;
  mountGroup: string[]; // ['auth'] | ['secret'] | ['auth', 'secret']; // - changed to string[] because of ts error with filterEnginesByMountType - couldn't check mount of type 'never'
  requiresEnterprise?: boolean;
  type: string;
  value?: string;
}

/**
 * @param mountType - Given mount type to filter by, e.g., 'auth' or 'secret'.
 * @param useEnterprise - Optional boolean to indicate if enterprise engines should be included in the results.
 * @returns Filtered array of engines that match the given mount type
 */
export function filterEnginesByMountType(mountType: string, useEnterprise?: boolean) {
  return useEnterprise
    ? ALL_ENGINES.filter((engine) => engine.mountGroup.includes(mountType))
    : ALL_ENGINES.filter((engine) => engine.mountGroup.includes(mountType) && !engine.requiresEnterprise);
}

export const ALL_ENGINES: EngineDisplayData[] = [
  {
    category: 'cloud',
    displayName: 'AliCloud',
    glyph: 'alibaba-color',
    mountGroup: ['auth', 'secret'],
    type: 'alicloud',
  },
  {

Review comment (suggestion):

![image](https://github.com/user-attachments/assets/6004a770-7058-49c0-ac68-68869aea9205)

Also with adding that filter function here, I kept getting a typescript error when trying to check if a string is included  (like we do in js files) - so I swapped it out for a generic string array. 

Open to thoughts if we wanna keep it this way or try to get the other way to work 

Code after:
/**
 * Copyright (c) HashiCorp, Inc.
 * SPDX-License-Identifier: BUSL-1.1
 */

/**
 * These are all the secret and auth methods, including enterprise.
 */

export interface EngineDisplayData {
  category?: string;
  displayName: string;
  glyph?: string;
  mountCategory: string[];
  requiresEnterprise?: boolean;
  type: string;
  value?: string;
}

/**
 * @param mountCategory - Given mount category to filter by, e.g., 'auth' or 'secret'.
 * @param isEnterprise - Optional boolean to indicate if enterprise engines should be included in the results.
 * @returns Filtered array of engines that match the given mount category
 */
export function filterEnginesByMountCategory({
  mountCategory,
  isEnterprise = false,
}: {
  mountCategory: 'auth' | 'secret';
  isEnterprise: boolean;
}) {
  return isEnterprise
    ? ALL_ENGINES.filter((engine) => engine.mountCategory.includes(mountCategory))
    : ALL_ENGINES.filter(
        (engine) => engine.mountCategory.includes(mountCategory) && !engine.requiresEnterprise
      );
}

export const ALL_ENGINES: EngineDisplayData[] = [
Repository: hashicorp/vault
File: ui/app/utils/all-engines-metadata.ts (TypeScript)

Code before:
/**
 * Copyright (c) HashiCorp, Inc.
 * SPDX-License-Identifier: BUSL-1.1
 */

/**
 * These are all the secret and auth methods, including enterprise.
 */

export interface EngineDisplayData {
  category?: string;
  displayName: string;
  glyph?: string;
  mountGroup: string[]; // ['auth'] | ['secret'] | ['auth', 'secret']; // - changed to string[] because of ts error with filterEnginesByMountType - couldn't check mount of type 'never'
  requiresEnterprise?: boolean;
  type: string;
  value?: string;
}

/**
 * @param mountType - Given mount type to filter by, e.g., 'auth' or 'secret'.
 * @param useEnterprise - Optional boolean to indicate if enterprise engines should be included in the results.
 * @returns Filtered array of engines that match the given mount type
 */
export function filterEnginesByMountType(mountType: string, useEnterprise?: boolean) {
  return useEnterprise
    ? ALL_ENGINES.filter((engine) => engine.mountGroup.includes(mountType))
    : ALL_ENGINES.filter((engine) => engine.mountGroup.includes(mountType) && !engine.requiresEnterprise);
}

export const ALL_ENGINES: EngineDisplayData[] = [
  {
    category: 'cloud',
    displayName: 'AliCloud',
    glyph: 'alibaba-color',
    mountGroup: ['auth', 'secret'],
    type: 'alicloud',
  },
  {
    category: 'generic',
    displayName: 'AppRole',
    glyph: 'cpu',
    mountGroup: ['auth'],
    type: 'approle',
    value: 'approle',
  },

Review comment (security):
```suggestion
 * @param mountGroup - Given mount group to filter by, e.g., 'auth' or 'secret'.
```

Code after:
/**
 * Copyright (c) HashiCorp, Inc.
 * SPDX-License-Identifier: BUSL-1.1
 */

/**
 * These are all the secret and auth methods, including enterprise.
 */

export interface EngineDisplayData {
  category?: string;
  displayName: string;
  glyph?: string;
  mountCategory: string[];
  requiresEnterprise?: boolean;
  type: string;
  value?: string;
}

/**
 * @param mountCategory - Given mount category to filter by, e.g., 'auth' or 'secret'.
 * @param isEnterprise - Optional boolean to indicate if enterprise engines should be included in the results.
 * @returns Filtered array of engines that match the given mount category
 */
export function filterEnginesByMountCategory({
  mountCategory,
  isEnterprise = false,
}: {
  mountCategory: 'auth' | 'secret';
  isEnterprise: boolean;
}) {
  return isEnterprise
    ? ALL_ENGINES.filter((engine) => engine.mountCategory.includes(mountCategory))
    : ALL_ENGINES.filter(
        (engine) => engine.mountCategory.includes(mountCategory) && !engine.requiresEnterprise
      );
}

export const ALL_ENGINES: EngineDisplayData[] = [
  {
    category: 'cloud',
    displayName: 'AliCloud',
    glyph: 'alibaba-color',
    mountCategory: ['auth', 'secret'],
    type: 'alicloud',
  },