ronantakizawa/codereview-bench huggingface dataset:
Repository: spotify/backstage
File: plugins/techdocs-node/src/stages/publish/awsS3.test.ts (TypeScript)

Code before:
      'also with spaces.js': 'found it too',
    },
  };

  beforeEach(async () => {
    process.env = { ...env };
    process.env.AWS_REGION = 'us-west-2';

    jest.resetAllMocks();
    getCredProviderMock.mockImplementation((_?: AwsCredentialProviderOptions) =>
      getMockCredentialProvider(),
    );

    // Create a fresh mockdirectory for each test to avoid windows file locking
    mockDir = createMockDirectory();
    // Calculate directory path with the new mockDir instance
    directory = getEntityRootDir(entity);

    // Set up the test files
    mockDir.setContent({
      [directory]: files,
    });

    s3Mock = mockClient(S3Client as any);

    s3Mock.on(HeadObjectCommand).callsFake((input: any) => {
      if (!fs.pathExistsSync(mockDir.resolve(input.Key))) {
        throw new Error('File does not exist');
      }
      return {};
    });

    s3Mock.on(GetObjectCommand).callsFake((input: any) => {
      if (fs.pathExistsSync(mockDir.resolve(input.Key))) {
        return {
          Body: Readable.from(fs.readFileSync(mockDir.resolve(input.Key))),
        };
      }

      throw new Error(`The file ${input.Key} does not exist!`);
    });

    s3Mock.on(HeadBucketCommand).callsFake((input: any) => {
      if (input.Bucket === 'errorBucket') {
        throw new Error('Bucket does not exist');
      }
      return {};
    });

    s3Mock.on(ListObjectsV2Command).callsFake((input: any) => {
      if (

Review comment (suggestion):
same below

```suggestion
    s3Mock.on(HeadObjectCommand).callsFake((input) => {
```

Code after:
      'unsafe.svg': '<svg></svg>',
    },
    'some folder': {
      'also with spaces.js': 'found it too',
    },
  };

  beforeEach(async () => {
    process.env = { ...env };
    process.env.AWS_REGION = 'us-west-2';

    jest.resetAllMocks();
    getCredProviderMock.mockImplementation((_?: AwsCredentialProviderOptions) =>
      getMockCredentialProvider(),
    );

    // Create a fresh mockdirectory for each test to avoid windows file locking
    mockDir = createMockDirectory();
    // Calculate directory path with the new mockDir instance
    directory = getEntityRootDir(entity);

    // Set up the test files
    mockDir.setContent({
      [directory]: files,
    });

    s3Mock = mockClient(S3Client);

    s3Mock.on(HeadObjectCommand).callsFake(input => {
      if (!fs.pathExistsSync(mockDir.resolve(input.Key))) {
        throw new Error('File does not exist');
      }
      return {};
    });

    s3Mock.on(GetObjectCommand).callsFake(input => {
      if (fs.pathExistsSync(mockDir.resolve(input.Key))) {
        return {
          Body: Readable.from(fs.readFileSync(mockDir.resolve(input.Key))),
        };
      }

      throw new Error(`The file ${input.Key} does not exist!`);
    });

    s3Mock.on(HeadBucketCommand).callsFake(input => {
      if (input.Bucket === 'errorBucket') {
        throw new Error('Bucket does not exist');
      }
      return {};
    });
Repository: spotify/backstage
File: plugins/techdocs-node/src/stages/publish/awsS3.test.ts (TypeScript)

Code before:
  PutObjectCommand,
  S3Client,
  S3ServiceException,
  UploadPartCommand,
} from '@aws-sdk/client-s3';
import { Entity, DEFAULT_NAMESPACE } from '@backstage/catalog-model';
import { ConfigReader } from '@backstage/config';
import {
  AwsCredentialProvider,
  AwsCredentialProviderOptions,
  DefaultAwsCredentialsManager,
} from '@backstage/integration-aws-node';
import { mockClient } from 'aws-sdk-client-mock';
import express from 'express';
import request from 'supertest';
import path from 'path';
import fs from 'fs-extra';
import { AwsS3Publish } from './awsS3';
import { Readable } from 'stream';
import {
  createMockDirectory,
  mockServices,
} from '@backstage/backend-test-utils';

const env = process.env;
let s3Mock: any;

// Create a new MockDirectory for each test to avoid Windows file locking issues
let mockDir: ReturnType<typeof createMockDirectory>;

function getMockCredentialProvider(): Promise<AwsCredentialProvider> {
  return Promise.resolve({
    sdkCredentialProvider: async () => {
      return Promise.resolve({
        accessKeyId: 'MY_ACCESS_KEY_ID',
        secretAccessKey: 'MY_SECRET_ACCESS_KEY',
      });
    },
  });
}
const getCredProviderMock = jest.spyOn(
  DefaultAwsCredentialsManager.prototype,
  'getCredentialProvider',
);

const getEntityRootDir = (entity: Entity) => {
  const {
    kind,
    metadata: { namespace, name },
  } = entity;


Review comment (suggestion):
Any reason why we had to move back to `any`? We'd like to keep the typing here as strict as possible if possible

Code after:
  PutObjectCommand,
  S3Client,
  S3ServiceException,
  UploadPartCommand,
} from '@aws-sdk/client-s3';
import { Entity, DEFAULT_NAMESPACE } from '@backstage/catalog-model';
import { ConfigReader } from '@backstage/config';
import {
  AwsCredentialProvider,
  AwsCredentialProviderOptions,
  DefaultAwsCredentialsManager,
} from '@backstage/integration-aws-node';
import { mockClient, AwsClientStub } from 'aws-sdk-client-mock';
import express from 'express';
import request from 'supertest';
import path from 'path';
import fs from 'fs-extra';
import { AwsS3Publish } from './awsS3';
import { Readable } from 'stream';
import {
  createMockDirectory,
  mockServices,
} from '@backstage/backend-test-utils';

jest.setTimeout(30_000);

const env = process.env;
let s3Mock: AwsClientStub<S3Client>;

// Create a new MockDirectory for each test to avoid Windows file locking issues
let mockDir: ReturnType<typeof createMockDirectory>;

function getMockCredentialProvider(): Promise<AwsCredentialProvider> {
  return Promise.resolve({
    sdkCredentialProvider: async () => {
      return Promise.resolve({
        accessKeyId: 'MY_ACCESS_KEY_ID',
        secretAccessKey: 'MY_SECRET_ACCESS_KEY',
      });
    },
  });
}
const getCredProviderMock = jest.spyOn(
  DefaultAwsCredentialsManager.prototype,
  'getCredentialProvider',
);

const getEntityRootDir = (entity: Entity) => {
  const {
    kind,
    metadata: { namespace, name },
Repository: spotify/backstage
File: plugins/notifications/src/components/NotificationsPage/NotificationsPage.tsx (TypeScript)

Code before:
    }
  }, [lastSignal, throttledSetRefresh]);

  const onUpdate = () => {
    throttledSetRefresh(true);
  };

  if (error) {
    return <ResponseErrorPanel error={error} />;
  }

  const notifications = value?.[0]?.notifications;
  const totalCount = value?.[0]?.totalCount;
  const isUnread = !!value?.[1]?.unread;
  const allTopics = value?.[2]?.topics;

  let tableTitle = `All notifications (${totalCount})`;
  if (saved) {
    tableTitle = `Saved notifications (${totalCount})`;
  } else if (unreadOnly === true) {
    tableTitle = `Unread notifications (${totalCount})`;
  } else if (unreadOnly === false) {
    tableTitle = `Read notifications (${totalCount})`;
  }

  return (
    <PageWithHeader
      title={title}
      themeId={themeId}
      tooltip={tooltip}
      subtitle={subtitle}
      type={type}
      typeLink={typeLink}
    >
      <Content>
        <ConfirmProvider>
          <Grid container>
            <Grid item xs={2}>
              <NotificationsFilters
                unreadOnly={unreadOnly}
                onUnreadOnlyChanged={setUnreadOnly}
                createdAfter={createdAfter}
                onCreatedAfterChanged={setCreatedAfter}
                onSortingChanged={setSorting}
                sorting={sorting}
                saved={saved}
                onSavedChanged={setSaved}
                severity={severity}
                onSeverityChanged={setSeverity}
                topic={topic}
                onTopicChanged={setTopic}

Review comment (suggestion):
```suggestion
  if (totalCount !== undefined) {
```
Maybe? 0 would also show no count

Code after:
    }
  }, [lastSignal, throttledSetRefresh]);

  const onUpdate = () => {
    throttledSetRefresh(true);
  };

  if (error) {
    return <ResponseErrorPanel error={error} />;
  }

  const notifications = value?.[0]?.notifications;
  const totalCount = value?.[0]?.totalCount;
  const isUnread = !!value?.[1]?.unread;
  const allTopics = value?.[2]?.topics;

  let tableTitle = `All notifications `;
  if (saved) {
    tableTitle = `Saved notifications`;
  } else if (unreadOnly === true) {
    tableTitle = `Unread notifications`;
  } else if (unreadOnly === false) {
    tableTitle = `Read notifications`;
  }

  if (totalCount) {
    tableTitle += ` (${totalCount})`;
  }

  return (
    <PageWithHeader
      title={title}
      themeId={themeId}
      tooltip={tooltip}
      subtitle={subtitle}
      type={type}
      typeLink={typeLink}
    >
      <Content>
        <ConfirmProvider>
          <Grid container>
            <Grid item xs={2}>
              <NotificationsFilters
                unreadOnly={unreadOnly}
                onUnreadOnlyChanged={setUnreadOnly}
                createdAfter={createdAfter}
                onCreatedAfterChanged={setCreatedAfter}
                onSortingChanged={setSorting}
                sorting={sorting}
                saved={saved}
                onSavedChanged={setSaved}
Repository: spotify/backstage
File: plugins/notifications/src/components/NotificationsPage/NotificationsPage.tsx (TypeScript)

Code before:
  };

  if (error) {
    return <ResponseErrorPanel error={error} />;
  }

  const notifications = value?.[0]?.notifications;
  const totalCount = value?.[0]?.totalCount;
  const isUnread = !!value?.[1]?.unread;
  const allTopics = value?.[2]?.topics;

  let tableTitle: string = t('notificationsPage.tableTitle.all', {
    count: totalCount ?? 0,
  } as any);
  if (saved) {
    tableTitle = t('notificationsPage.tableTitle.saved', {
      count: totalCount ?? 0,
    } as any);
  } else if (unreadOnly === true) {
    tableTitle = t('notificationsPage.tableTitle.unread', {
      count: totalCount ?? 0,
    } as any);
  } else if (unreadOnly === false) {
    tableTitle = t('notificationsPage.tableTitle.read', {
      count: totalCount ?? 0,
    } as any);
  }

  return (
    <PageWithHeader
      title={title}
      themeId={themeId}
      tooltip={tooltip}
      subtitle={subtitle}
      type={type}
      typeLink={typeLink}
    >
      <Content>
        <ConfirmProvider>
          <Grid container>
            <Grid item xs={2}>
              <NotificationsFilters
                unreadOnly={unreadOnly}
                onUnreadOnlyChanged={setUnreadOnly}
                createdAfter={createdAfter}
                onCreatedAfterChanged={setCreatedAfter}
                onSortingChanged={setSorting}
                sorting={sorting}
                saved={saved}
                onSavedChanged={setSaved}
                severity={severity}

Review comment (suggestion):
```suggestion
    tableTitle = t('notificationsPage.tableTitle.read', {
      count: totalCount ?? 0,
    } as string);
```

or 

```suggestion
    tableTitle = t('notificationsPage.tableTitle.read', {
      count: totalCount ?? 0,
    })!;
```

is probably better here? Let's remove the `as any` either way.

Code after:
  }, [refresh, setRefresh, retry, loading]);

  useEffect(() => {
    if (lastSignal && lastSignal.action) {
      throttledSetRefresh(true);
    }
  }, [lastSignal, throttledSetRefresh]);

  const onUpdate = () => {
    throttledSetRefresh(true);
  };

  if (error) {
    return <ResponseErrorPanel error={error} />;
  }

  const notifications = value?.[0]?.notifications;
  const totalCount = value?.[0]?.totalCount;
  const isUnread = !!value?.[1]?.unread;
  const allTopics = value?.[2]?.topics;

  let tableTitle: string = t(TableTitleKeys.all, {
    count: totalCount ?? 0,
  });
  if (saved) {
    tableTitle = t(TableTitleKeys.saved, {
      count: totalCount ?? 0,
    });
  } else if (unreadOnly === true) {
    tableTitle = t(TableTitleKeys.unread, {
      count: totalCount ?? 0,
    });
  } else if (unreadOnly === false) {
    tableTitle = t(TableTitleKeys.read, {
      count: totalCount ?? 0,
    });
  }

  return (
    <PageWithHeader
      title={title}
      themeId={themeId}
      tooltip={tooltip}
      subtitle={subtitle}
      type={type}
      typeLink={typeLink}
    >
      <Content>
        <ConfirmProvider>
          <Grid container>
            <Grid item xs={2}>
Repository: spotify/backstage
File: plugins/notifications/src/translation.ts (TypeScript)

Code before:
/*
 * Copyright 2024 The Backstage Authors
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *     http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

import { createTranslationRef } from '@backstage/core-plugin-api/alpha';

/** @alpha */
export const notificationsTranslationRef = createTranslationRef({
  id: 'plugin.notifications',
  messages: {
    notificationsPage: {
      title: 'Notifications',
      tableTitle: {
        all: 'All notifications ({{count}})',
        saved: 'Saved notifications ({{count}})',

Review comment (security):
```suggestion
 * Copyright 2025 The Backstage Authors
```

:smile: 

Code after:
/*
 * Copyright 2025 The Backstage Authors
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *     http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

import { createTranslationRef } from '@backstage/frontend-plugin-api';

/** @alpha */
export const notificationsTranslationRef = createTranslationRef({
  id: 'plugin.notifications',
  messages: {
    notificationsPage: {
      title: 'Notifications',
      tableTitle: {
        all_one: 'All notifications ({{count}})',
        all_other: 'All notifications ({{count}})',
Repository: spotify/backstage
File: plugins/catalog/src/alpha/components/EntityLayout/EntityLayout.tsx (TypeScript)

Code before:

  return (
    <Page themeId={entity?.spec?.type?.toString() ?? 'home'}>
      {!loading && header}

      {loading && <Progress />}

      {entity && <EntityTabs routes={routes} />}

      {error && (
        <Content>
          <Alert severity="error">{error.toString()}</Alert>
        </Content>
      )}

      {!loading && !error && !entity && (
        <Content>
          {NotFoundComponent ? (
            NotFoundComponent
          ) : (
            <WarningPanel title={t('entityLabels.warningPanelTitle')}>
              {t('entityLayout.notFoundMessage', { kind })}{' '}
              <Link to="https://backstage.io/docs/features/software-catalog/references">
                {t('entityLayout.notFoundLinkText')}
              </Link>
              .
            </WarningPanel>
          )}
        </Content>
      )}
    </Page>
  );
};

EntityLayout.Route = Route;


Review comment (suggestion):
Hi, can you pass the `Link` as prop to the `entityLayout.notFoundMessage` string and include the "final sentence dot" as well?

See https://backstage.io/docs/plugins/internationalization/ "use within a component" example

Code after:
  return (
    <Page themeId={entity?.spec?.type?.toString() ?? 'home'}>
      {header ?? (
        <EntityHeader
          parentEntityRelations={parentEntityRelations}
          UNSTABLE_contextMenuOptions={UNSTABLE_contextMenuOptions}
          UNSTABLE_extraContextMenuItems={UNSTABLE_extraContextMenuItems}
          contextMenuItems={contextMenuItems}
        />
      )}

      {loading && <Progress />}

      {entity && (
        <EntityTabs
          routes={routes}
          groupDefinitions={groupDefinitions}
          showIcons={showNavItemIcons}
        />
      )}

      {error && (
        <Content>
          <Alert severity="error">{error.toString()}</Alert>
        </Content>
      )}

      {!loading && !error && !entity && (
        <Content>
          {NotFoundComponent ? (
            NotFoundComponent
          ) : (
            <WarningPanel title={t('entityLabels.warningPanelTitle')}>
              {t('entityPage.notFoundMessage', {
                kind,
                link: (
                  <Link to="https://backstage.io/docs/features/software-catalog/references">
                    {t('entityPage.notFoundLinkText')}
                  </Link>
                ),
              })}
            </WarningPanel>
          )}
        </Content>
      )}
    </Page>
  );
};

EntityLayout.Route = Route;
Repository: spotify/backstage
File: plugins/scaffolder-backend-module-bitbucket-cloud/src/actions/helpers.ts (TypeScript)

Code before:
  throw new Error(
    `Authorization has not been provided for Bitbucket Cloud. Please add either provide a username and token or  username and appPassword to the Integrations config`,
  );
};

export const getAuthorizationHeader = (config: {
  username?: string;
  appPassword?: string;
  token?: string;
}) => {
  // TODO: appPassword can be removed once fully
  // deprecated by BitBucket on 9th June 2026.
  if (config.username && (config.token ?? config.appPassword)) {
    const buffer = Buffer.from(
      `${config.username}:${config.token ?? config.appPassword}`,
      'utf8',
    );
    return `Basic ${buffer.toString('base64')}`;
  }

  if (config.token) {
    return `Bearer ${config.token}`;
  }

  throw new Error(
    `Authorization has not been provided for Bitbucket Cloud. Please add either provide a username and token or  username and appPassword to the Integrations config`,
  );
};


Review comment (security):
```suggestion
    `Authorization has not been provided for Bitbucket Cloud. Please add either provide a username and token or username and appPassword to the Integrations config`,
```

Code after:
  throw new Error(
    `Authorization has not been provided for Bitbucket Cloud. Please add either provide a username and token or username and appPassword to the Integrations config`,
  );
};

export const getAuthorizationHeader = (config: {
  username?: string;
  appPassword?: string;
  token?: string;
}) => {
  // TODO: appPassword can be removed once fully
  // deprecated by BitBucket on 9th June 2026.
  if (config.username && (config.token ?? config.appPassword)) {
    const buffer = Buffer.from(
      `${config.username}:${config.token ?? config.appPassword}`,
      'utf8',
    );
    return `Basic ${buffer.toString('base64')}`;
  }

  if (config.token) {
    return `Bearer ${config.token}`;
  }

  throw new Error(
    `Authorization has not been provided for Bitbucket Cloud. Please add either provide a username and token or username and appPassword to the Integrations config`,
  );
};
Repository: spotify/backstage
File: plugins/catalog-backend-module-gitlab/src/providers/GitlabDiscoveryEntityProvider.ts (TypeScript)

Code before:
      entities: locations.map(location => ({
        locationKey: this.getProviderName(),
        entity: locationSpecToLocationEntity({ location }),
      })),
    });

    logger.info(`Processed ${locations.length} locations`);
  }

  /**
   * Determine the location on GitLab to be ingested.
   * Uses GitLab's search API to find projects matching provided configuration.
   *
   * @returns A list of location to be ingested
   */
  private async searchEntities() {
    const locations: LocationSpec[] = [];
    let foundProjects = 0;

    this.logger.info(`Using gitlab search API to lookup projects`);

    const foundFiles = paginated(
      options => this.gitLabClient.listFiles(options),
      {
        group: this.config.group,
        search: `file:${this.config.catalogFile}`,
        page: 1,
        per_page: 50,
      },
    );

    for await (const foundFile of foundFiles) {
      const project = await this.gitLabClient.getProjectById(
        foundFile.project_id,
      );
      foundProjects++;

      if (
        project &&
        this.isProjectCompliant(project) &&
        this.isGroupCompliant(project.path_with_namespace)
      ) {
        locations.push(
          this.createLocationSpecFromParams(
            project.web_url,
            foundFile.ref,
            foundFile.path,
          ),
        );
      }
    }

Review comment (suggestion):
hey, I believe you should use `filename:` instead of `file:` for the [group-level search API](https://docs.gitlab.com/api/search/#scope-blobs-1)

```suggestion
        search: `filename:${this.config.catalogFile}`,
```

Code after:
      entities: locations.map(location => ({
        locationKey: this.getProviderName(),
        entity: locationSpecToLocationEntity({ location }),
      })),
    });

    logger.info(`Processed ${locations.length} locations`);
  }

  /**
   * Determine the location on GitLab to be ingested.
   * Uses GitLab's search API to find projects matching provided configuration.
   *
   * @returns A list of location to be ingested
   */
  private async searchEntities() {
    const locations: LocationSpec[] = [];
    let foundProjects = 0;

    this.logger.info(`Using gitlab search API to lookup projects`);

    const foundFiles = paginated(
      options => this.gitLabClient.listFiles(options),
      {
        group: this.config.group,
        search: `filename:${this.config.catalogFile}`,
        page: 1,
        per_page: 50,
      },
    );

    for await (const foundFile of foundFiles) {
      const project = await this.gitLabClient.getProjectById(
        foundFile.project_id,
      );
      foundProjects++;

      if (
        project &&
        this.isProjectCompliant(project) &&
        this.isGroupCompliant(project.path_with_namespace)
      ) {
        locations.push(
          this.createLocationSpecFromParams(
            project.web_url,
            foundFile.ref,
            foundFile.path,
          ),
        );
      }
    }