ronantakizawa/codereview-bench huggingface dataset:
Repository: storybookjs/storybook
File: code/addons/docs/src/blocks/controls/Object.tsx (TypeScript)

Code before:

import { cloneDeep } from 'es-toolkit/object';
import { type Theme, srOnlyStyles, styled, useTheme } from 'storybook/theming';

import { getControlId, getControlSetterButtonId } from './helpers';
import { JsonTree } from './react-editable-json-tree';
import type { ControlProps, ObjectConfig, ObjectValue } from './types';

const { window: globalWindow } = globalThis;

type JsonTreeProps = ComponentProps<typeof JsonTree>;

const Wrapper = styled.div(({ theme }) => ({
  position: 'relative',
  display: 'flex',
  isolation: 'isolate',

  // Enable container queries for child responsive styles
  '@supports (container-type: inline-size)': {
    containerType: 'inline-size',
  },

  '@media (max-width: 400px)': {
    flexDirection: 'column' as const,
  },

  '.rejt-tree': {
    marginLeft: '1rem',
    fontSize: '13px',
    listStyleType: 'none',
  },
  '.rejt-value-node:hover': {
    '& > button': {
      opacity: 1,
    },
  },
  '.rejt-add-form': {
    marginLeft: 10,
  },
  '.rejt-add-value-node': {
    display: 'inline-flex',
    alignItems: 'center',
  },
  '.rejt-name': {
    lineHeight: '22px',
  },
  '.rejt-not-collapsed-delimiter': {
    lineHeight: '22px',
  },
  '.rejt-value': {
    display: 'inline-block',

Review comment (suggestion):
There's a specific size where shenanigans like this happen because you set flex direction only on media width and not container width. I've removed the column direction entirely in my commit.

<img width="424" height="177" alt="Image" src="https://github.com/user-attachments/assets/2aa08e88-0117-42a2-9bc0-2940bba51b25" />



Code after:

import { cloneDeep } from 'es-toolkit/object';
import { styled, useTheme } from 'storybook/theming';

import { getControlId, getControlSetterButtonId } from './helpers';
import { JsonTree } from './react-editable-json-tree';
import type { ControlProps, ObjectConfig, ObjectValue } from './types';

const { window: globalWindow } = globalThis;

const Wrapper = styled.div(({ theme }) => ({
  position: 'relative',
  display: 'flex',
  isolation: 'isolate',
  gap: 8,

  '.rejt-tree': {
    flex: 1,
    marginLeft: '1rem',
    fontSize: '13px',
    listStyleType: 'none',
  },
  '.rejt-value-node:hover': {
    '& > button': {
      opacity: 1,
    },
  },
  '.rejt-add-form': {
    marginLeft: 10,
  },
  '.rejt-add-value-node': {
    display: 'inline-flex',
    alignItems: 'center',
  },
  '.rejt-name': {
    color: theme.color.secondary,
    lineHeight: '22px',
  },
  '.rejt-not-collapsed-list': {
    listStyle: 'none',
    margin: '0 0 0 1rem',
    padding: 0,
  },
  '.rejt-not-collapsed-delimiter': {
    lineHeight: '22px',
  },
  '.rejt-value': {
    display: 'inline-block',
    border: '1px solid transparent',
    borderRadius: 4,
    margin: '1px 0',
Repository: strapi/strapi
File: packages/core/admin/server/src/controllers/webhooks.ts (TypeScript)

Code before:
        strapi.webhookRunner.remove(webhook);
      }
    }

    ctx.send({ data: ids } satisfies DeleteWebhooks.Response);
  },

  async triggerWebhook(ctx: Context) {
    const { id } = ctx.params;

    const webhook = await strapi.webhookStore.findWebhook(id);

    const response = await strapi.webhookRunner
      .run(webhook as Webhook, 'trigger-test', {})
      .then((res: { statusCode: number; message: string }) => {
        // Mask the response to avoid leaking sensitive information
        if (+res.statusCode > 300) {
          strapi.log.error(`Failed to manually trigger webhook: \n${JSON.stringify(res, null, 2)}`);
          return {
            statusCode: 500,
            message: 'Webhook URL request failed',
          };
        }

        return res;
      });

    ctx.body = { data: response } satisfies TriggerWebhook.Response;
  },
};


Review comment (bug):
```suggestion
    let response = await strapi.webhookRunner.run(webhook as Webhook, 'trigger-test', {});

    if (Number(response.statusCode) > 300) {
      strapi.log.error(`Failed to manually trigger webhook: \n${JSON.stringify(response, null, 2)}`);
      response = {
        statusCode: 500,
        message: 'Webhook URL request failed',
      };
    }
```
(just to simplify it)

Are we sure we don't need to suppress any success codes?

Code after:

    if (!webhook) {
      return ctx.notFound('webhook.notFound');
    }

    await strapi.webhookStore.deleteWebhook(id);

    strapi.webhookRunner.remove(webhook);

    ctx.body = { data: webhook } satisfies DeleteWebhook.Response;
  },

  async deleteWebhooks(ctx: Context) {
    const { ids } = ctx.request.body as DeleteWebhooks.Request['body'];

    if (!Array.isArray(ids) || ids.length === 0) {
      return ctx.badRequest('ids must be an array of id');
    }

    for (const id of ids) {
      const webhook = await strapi.webhookStore.findWebhook(id);

      if (webhook) {
        await strapi.webhookStore.deleteWebhook(id);
        strapi.webhookRunner.remove(webhook);
      }
    }

    ctx.send({ data: ids } satisfies DeleteWebhooks.Response);
  },

  async triggerWebhook(ctx: Context) {
    const { id } = ctx.params;

    const webhook = await strapi.webhookStore.findWebhook(id);

    const response = await strapi.webhookRunner.run(webhook as Webhook, 'trigger-test', {});

    ctx.body = { data: response } satisfies TriggerWebhook.Response;
  },
};
Repository: strapi/strapi
File: packages/core/upload/admin/src/components/EditAssetDialog/PreviewBox/AssetPreview.tsx (TypeScript)

Code before:
/* eslint-disable jsx-a11y/media-has-caption */
import * as React from 'react';

import MuxPlayer from '@mux/mux-player-react';
import { Box, Flex, Typography } from '@strapi/design-system';
import { File } from '@strapi/icons';
import { useIntl } from 'react-intl';
import { styled, useTheme } from 'styled-components';

import { AssetType } from '../../../constants';
import { getFileIconComponent } from '../../../utils/icons';
import { typeFromMime } from '../../../utils';

const CardAsset = styled(Flex)`
  min-height: 26.4rem;
  border-radius: ${({ theme }) => theme.borderRadius} ${({ theme }) => theme.borderRadius} 0 0;
  background: linear-gradient(
    180deg,
    ${({ theme }) => theme.colors.neutral0} 0%,
    ${({ theme }) => theme.colors.neutral100} 121.48%
  );
`;

interface AssetPreviewProps {
  mime: string;
  name: string;
  url: string;
  onLoad?: () => void;
}

export const AssetPreview = React.forwardRef<
  HTMLImageElement | HTMLVideoElement | HTMLAudioElement,
  AssetPreviewProps
>(({ mime, url, name, ...props }, ref) => {
  const theme = useTheme();

Review comment (suggestion):
this import should occur before import of `../../../utils/icons`

Code after:
/* eslint-disable jsx-a11y/media-has-caption */
import * as React from 'react';

import MuxPlayer from '@mux/mux-player-react';
import { Box, Flex, Typography } from '@strapi/design-system';
import { useIntl } from 'react-intl';
import { styled, useTheme } from 'styled-components';

import { AssetType } from '../../../enums';
import { typeFromMime } from '../../../utils';
import { getFileIconComponent } from '../../../utils/icons';

const CardAsset = styled(Flex)`
  min-height: 26.4rem;
  border-radius: ${({ theme }) => theme.borderRadius} ${({ theme }) => theme.borderRadius} 0 0;
  background: linear-gradient(
    180deg,
    ${({ theme }) => theme.colors.neutral0} 0%,
    ${({ theme }) => theme.colors.neutral100} 121.48%
  );
`;

interface AssetPreviewProps {
  mime: string;
  name: string;
  url: string;
  onLoad?: () => void;
}

export const AssetPreview = React.forwardRef<
  HTMLImageElement | HTMLVideoElement | HTMLAudioElement,
  AssetPreviewProps
>(({ mime, url, name, ...props }, ref) => {
  const theme = useTheme();
Repository: strapi/strapi
File: packages/core/upload/admin/src/utils/tests/typeFromMime.test.ts (TypeScript)

Code before:

  it('gives a type of pdf when the mime contains "pdf"', () => {
    const type = typeFromMime('application/pdf');

    expect(type).toBe(DocType.Pdf);
  });

  it('gives a type of csv when the mime contains "csv"', () => {
    const type = typeFromMime('text/csv');

    expect(type).toBe(DocType.Csv);
  });

  it('gives a type of xls when the mime contains "excel"', () => {
    const type = typeFromMime('application/vnd.ms-excel');

    expect(type).toBe(DocType.Xls);
  });

  it('gives a type of zip when the mime contains "zip"', () => {
    const type = typeFromMime('application/zip');

    expect(type).toBe(DocType.Zip);
  });

  it('gives a type of document when the mime is neither video nor image', () => {
    const type = typeFromMime('application/pdf');

    expect(type).toBe(AssetType.Document);
  });
});


Review comment (bug):
this test is failing right now
`typeFromMime › gives a type of document when the mime is neither video nor image

    expect(received).toBe(expected) // Object.is equality

    Expected: "doc"
    Received: "pdf"`

Code after:

  it('gives a type of pdf when the mime contains "pdf"', () => {
    const type = typeFromMime('application/pdf');

    expect(type).toBe(DocType.Pdf);
  });

  it('gives a type of csv when the mime contains "csv"', () => {
    const type = typeFromMime('text/csv');

    expect(type).toBe(DocType.Csv);
  });

  it('gives a type of xls when the mime contains "excel"', () => {
    const type = typeFromMime('application/vnd.ms-excel');

    expect(type).toBe(DocType.Xls);
  });

  it('gives a type of zip when the mime contains "zip"', () => {
    const type = typeFromMime('application/zip');

    expect(type).toBe(DocType.Zip);
  });

  it('gives a type of document as the default when the doc type is not recognised and handled', () => {
    const type = typeFromMime('application/random-document-type');

    expect(type).toBe(AssetType.Document);
  });
});
Repository: strapi/strapi
File: packages/core/upload/admin/src/components/EditAssetDialog/PreviewBox/AssetPreview.tsx (TypeScript)

Code before:
import { getFileIconComponent } from '../../../utils/icons';

const CardAsset = styled(Flex)`
  min-height: 26.4rem;
  border-radius: ${({ theme }) => theme.borderRadius} ${({ theme }) => theme.borderRadius} 0 0;
  background: linear-gradient(
    180deg,
    ${({ theme }) => theme.colors.neutral0} 0%,
    ${({ theme }) => theme.colors.neutral100} 121.48%
  );
`;

interface AssetPreviewProps {
  mime: string;
  name: string;
  url: string;
  onLoad?: () => void;
}

export const AssetPreview = React.forwardRef<
  HTMLImageElement | HTMLVideoElement | HTMLAudioElement,
  AssetPreviewProps
>(({ mime, url, name, ...props }, ref) => {
  const theme = useTheme();

  const documentType = typeFromMime(mime);

  const { formatMessage } = useIntl();

  if (documentType === AssetType.Image) {
    return (
      <img ref={ref as React.ForwardedRef<HTMLImageElement>} src={url} alt={name} {...props} />
    );
  }

  if (documentType === AssetType.Video) {
    return <MuxPlayer src={url} accentColor={theme.colors.primary500} />;
  }

  if (documentType === AssetType.Audio) {
    return (
      <Box margin="5">
        <audio controls src={url} ref={ref as React.ForwardedRef<HTMLAudioElement>} {...props}>
          {name}
        </audio>
      </Box>
    );
  }

  // getFileIconComponent will handle all other file types, eg. PDF, CSV, XLS, ZIP
  // If the file type is not recognized, the default icon will be used

Review comment (suggestion):
Let's keep the naming consistent.

```suggestion
  const assetType = getAssetTypeFromMime(mime);
```

Code after:
import { getFileIconComponent } from '../../../utils/icons';

const CardAsset = styled(Flex)`
  min-height: 26.4rem;
  border-radius: ${({ theme }) => theme.borderRadius} ${({ theme }) => theme.borderRadius} 0 0;
  background: linear-gradient(
    180deg,
    ${({ theme }) => theme.colors.neutral0} 0%,
    ${({ theme }) => theme.colors.neutral100} 121.48%
  );
`;

interface AssetPreviewProps {
  mime: string;
  name: string;
  url: string;
  onLoad?: () => void;
}

export const AssetPreview = React.forwardRef<
  HTMLImageElement | HTMLVideoElement | HTMLAudioElement,
  AssetPreviewProps
>(({ mime, url, name, ...props }, ref) => {
  const theme = useTheme();

  const assetType = typeFromMime(mime);

  const { formatMessage } = useIntl();

  if (assetType === AssetType.Image) {
    return (
      <img ref={ref as React.ForwardedRef<HTMLImageElement>} src={url} alt={name} {...props} />
    );
  }

  if (assetType === AssetType.Video) {
    return <MuxPlayer src={url} accentColor={theme.colors.primary500} />;
  }

  if (assetType === AssetType.Audio) {
    return (
      <Box margin="5">
        <audio controls src={url} ref={ref as React.ForwardedRef<HTMLAudioElement>} {...props}>
          {name}
        </audio>
      </Box>
    );
  }

  // getFileIconComponent will handle all other file types, eg. PDF, CSV, XLS, ZIP
  // If the file type is not recognized, the default icon will be used
Repository: strapi/strapi
File: packages/core/upload/admin/src/utils/typeFromMime.ts (TypeScript)

Code before:
import { AssetType, DocType } from '../enums';

const MIME_TYPE_MAP: Record<string, string> = {
  image: AssetType.Image,
  video: AssetType.Video,
  audio: AssetType.Audio,
  pdf: DocType.Pdf,
  csv: DocType.Csv,
  // For XLS files the mime is application/vnd.ms-excel so we need to check for 'excel' not 'xls'
  excel: DocType.Xls,
  zip: DocType.Zip,
};

export const typeFromMime = (mime: string) => {
  const lowerCasedMime = mime.toLowerCase();

  for (const [key, value] of Object.entries(MIME_TYPE_MAP)) {
    if (lowerCasedMime.includes(key)) {
      return value;
    }
  }

  return AssetType.Document;
};


Review comment (suggestion):
you can probably avoid the for loop here 
```suggestion
  const mimeTypeKey = Object.keys(MIME_TYPE_MAP).find(m => lowerCasedMime.includes(m));
  
  if (mimeTypeKey === -1) return; 
  
  return MIME_TYPE_MAP[mimeTypeKey];
```

Code after:
import { AssetType, DocType } from '../enums';

const MIME_TYPE_MAP: Record<string, AssetType | DocType> = {
  image: AssetType.Image,
  video: AssetType.Video,
  audio: AssetType.Audio,
  pdf: DocType.Pdf,
  csv: DocType.Csv,
  // For XLS files the mime is application/vnd.ms-excel so we need to check for 'excel' not 'xls'
  excel: DocType.Xls,
  zip: DocType.Zip,
};

const MIME_TYPE_KEYS = Object.keys(MIME_TYPE_MAP) as Array<keyof typeof MIME_TYPE_MAP>;

export const typeFromMime = (mime: string): AssetType | DocType => {
  const mimeTypeKey = MIME_TYPE_KEYS.find((m) => mime.toLowerCase().includes(m));

  if (mimeTypeKey === undefined) return AssetType.Document;

  return MIME_TYPE_MAP[mimeTypeKey] ?? AssetType.Document;
};
Repository: strapi/strapi
File: packages/providers/upload-aws-s3/src/__tests__/upload-aws-s3.test.ts (TypeScript)

Code before:
jest.mock('@aws-sdk/lib-storage', () => ({
  Upload: jest.fn().mockImplementation(() => uploadMock),
}));

describe('AWS-S3 provider', () => {
  beforeEach(() => {
    jest.clearAllMocks();
  });

  describe('upload', () => {
    test('Should add url to file object', async () => {
      const providerInstance = awsProvider.init({
        s3Options: {
          params: {
            Bucket: 'test',
          },
        },
      });

      const file: File = {
        name: 'test',
        size: 100,
        sizeInBytes: 100 * 1024,
        url: '',
        path: 'tmp',
        hash: 'test',
        ext: '.json',
        mime: 'application/json',
        buffer: Buffer.from(''),
      };

      await providerInstance.upload(file);

      expect(uploadMock.done).toBeCalled();
      expect(file.url).toBeDefined();
      expect(file.url).toEqual('https://validurl.test/tmp/test.json');
    });

    test('Should add to the url the https protocol as it is missing', async () => {
      const providerInstance = awsProvider.init({
        s3Options: {
          params: {
            Bucket: 'test',
          },
        },
      });

      const file: File = {
        name: 'test',
        size: 100,
        sizeInBytes: 100 * 1024,

Review comment (suggestion):
This prop was missing in this object but was defined in the `File` type. I have taken the opportunity to add here to remove a type error in this file.

Code after:
  beforeEach(() => {
    jest.clearAllMocks();
  });

  describe('upload', () => {
    test('Should populate file.url with the baseUrl when provided, appending the file key to the url', async () => {
      uploadMock.done.mockImplementationOnce(() =>
        Promise.resolve({
          Location: 'https://strapi.io/path/to/fileNameHash.json',
          $metadata: {},
        })
      );

      const providerInstance = awsProvider.init({
        baseUrl: 'https://assets.strapi.io',
        s3Options: {
          params: {
            Bucket: 'bucket-name',
          },
        },
      });

      const file: File = {
        name: 'fileName',
        size: 100,
        sizeInBytes: 100 * 1024,
        url: '',
        path: 'path/to',
        hash: 'fileNameHash',
        ext: '.json',
        mime: 'application/json',
        buffer: Buffer.from(''),
      };

      await providerInstance.upload(file);

      expect(uploadMock.done).toBeCalled();
      expect(file.url).toBeDefined();
      expect(file.url).toEqual('https://assets.strapi.io/path/to/fileNameHash.json');
    });
    test('Should populate file.url with the returned Location when no baseUrl is provided', async () => {
      uploadMock.done.mockImplementationOnce(() =>
        Promise.resolve({
          Location: 'https://strapi.io/path/from/location/fileName.json',
          $metadata: {},
        })
      );

      const providerInstance = awsProvider.init({
        s3Options: {
          params: {
Repository: strapi/strapi
File: packages/core/strapi/src/cli/commands/transfer/command.ts (TypeScript)

Code before:
              { failMessage: 'Transfer process aborted' }
            )(thisCommand);
          }
        )
      )
      // If --to is used, validate the URL, token, and confirm restore
      .hook(
        'preAction',
        ifOptions(
          (opts) => opts.to,
          async (thisCommand) => {
            assertUrlHasProtocol(thisCommand.opts().to, ['https:', 'http:']);
            if (!thisCommand.opts().toToken) {
              const answers = await inquirer.prompt([
                {
                  type: 'password',
                  message: 'Please enter your transfer token for the remote Strapi destination',
                  name: 'toToken',
                },
              ]);
              if (!answers.toToken?.length) {
                exitWith(1, 'No token provided for remote destination, aborting transfer.');
              }
              thisCommand.opts().toToken = answers.toToken;
            }

            await getCommanderConfirmMessage(
              'The transfer will delete existing data from the remote Strapi! Are you sure you want to proceed?',
              { failMessage: 'Transfer process aborted' }
            )(thisCommand);
          }
        )
      )
      .action(action)
  );
};

export default command;


Review comment (suggestion):
Shouldn't this be

```typescript
if (opts.from) {
  return 'from';
}
if (opts.to) {
  return 'to';
}
```

Right now if I do `strapi transfer --from [URL] --from-token [TOKEN]` it still asks me about a URL and token, and the direction message on line 134 also says 'send data to', because the `direction` will be set to my `--from` URL instead of to the string `'from'`.

Code after:
            );
          }

          if (hasEnvToken) {
            console.info(
              '   → Environment STRAPI_TRANSFER_TOKEN value will be used as the transfer token'
            );
          }

          console.info(); // Empty line for better readability
        };

        const determineDirection = async () => {
          // If user has not provided a direction from CLI, log the documentation
          if (!opts.from && !opts.to) {
            logDocumentation();
          }

          logEnvironmentVariables();

          if (opts.from) {
            return opts.from;
          }
          if (opts.to) {
            return opts.to;
          }

          const { dir } = await inquirer.prompt([
            {
              type: 'list',
              name: 'dir',
              message: 'Choose transfer direction:',
              choices: [
                { name: 'Pull data from remote Strapi to local', value: 'from' },
                { name: 'Push local data to remote Strapi', value: 'to' },
              ],
            },
          ]);

          return dir;
        };

        const determineUrl = async (direction: 'from' | 'to') => {
          if (opts[direction]) {
            return new URL(opts[direction]);
          }

          if (process.env.STRAPI_TRANSFER_URL) {
            return new URL(process.env.STRAPI_TRANSFER_URL);
          }
Repository: strapi/strapi
File: packages/core/admin/admin/src/pages/Settings/pages/ApiTokens/EditView/EditViewPage.tsx (TypeScript)

Code before:
      setApiToken(data);

      if (data.type === 'read-only') {
        dispatch({
          type: 'ON_CHANGE_READ_ONLY',
        });
      }
      if (data.type === 'full-access') {
        dispatch({
          type: 'SELECT_ALL_ACTIONS',
        });
      }
      if (data.type === 'custom') {
        dispatch({
          type: 'UPDATE_PERMISSIONS',
          value: data?.permissions,
        });
      }
    }
  }, [data]);

  React.useEffect(() => {
    // Only set up timer when token is shown
    if (showToken) {
      hideTimerRef.current = setTimeout(() => {
        setShowToken(false);
      }, 30000); // 30 seconds

      // Cleanup on unmount or when showToken changes
      return () => {
        if (hideTimerRef.current) {
          clearTimeout(hideTimerRef.current);
          hideTimerRef.current = null;
        }
      };
    }
  }, [showToken]);

  const [createToken] = useCreateAPITokenMutation();
  const [updateToken] = useUpdateAPITokenMutation();

  interface FormValues extends Pick<Get.Response['data'], 'name' | 'description'> {
    lifespan: Get.Response['data']['lifespan'] | undefined;
    type: Get.Response['data']['type'] | undefined;
  }

  const handleSubmit = async (body: FormValues, formik: FormikHelpers<FormValues>) => {
    trackUsage(isCreating ? 'willCreateToken' : 'willEditToken', {
      tokenType: API_TOKEN_TYPE,
    });


Review comment (security):
So it auto hides after 30 seconds and that's the only way of hiding the token?

For me being able to manually close/hide it seems like a better experience? Or
maybe having both so we still have the added security of auto hiding ?

Code after:

      if (data.type === 'read-only') {
        dispatch({
          type: 'ON_CHANGE_READ_ONLY',
        });
      }
      if (data.type === 'full-access') {
        dispatch({
          type: 'SELECT_ALL_ACTIONS',
        });
      }
      if (data.type === 'custom') {
        dispatch({
          type: 'UPDATE_PERMISSIONS',
          value: data?.permissions,
        });
      }
    }
  }, [data]);

  React.useEffect(() => {
    // Only set up timer when token is shown
    if (showToken) {
      hideTimerRef.current = setTimeout(() => {
        setShowToken(false);
      }, 30000); // 30 seconds

      // Cleanup on unmount or when showToken changes
      return () => {
        if (hideTimerRef.current) {
          clearTimeout(hideTimerRef.current);
          hideTimerRef.current = null;
        }
      };
    }
  }, [showToken]);

  const [createToken] = useCreateAPITokenMutation();
  const [updateToken] = useUpdateAPITokenMutation();

  interface FormValues extends Pick<Get.Response['data'], 'name' | 'description'> {
    lifespan: Get.Response['data']['lifespan'] | undefined;
    type: Get.Response['data']['type'] | undefined;
  }

  const handleSubmit = async (body: FormValues, formik: FormikHelpers<FormValues>) => {
    trackUsage(isCreating ? 'willCreateToken' : 'willEditToken', {
      tokenType: API_TOKEN_TYPE,
    });

    try {