ronantakizawa/codereview-bench huggingface dataset:
Repository: strapi/strapi
File: packages/core/admin/admin/src/components/SubNav.tsx (TypeScript)

Code before:
            aria-hidden
            fill="neutral500"
            style={{
              transform: `rotate(${isOpen ? '0deg' : '-90deg'})`,
              transition: 'transform 0.5s',
            }}
          />
          <Box paddingLeft={2}>
            <Typography tag="span" fontWeight="semiBold" textColor="neutral800">
              {label}
            </Typography>
          </Box>
        </SubSectionHeader>
      </Flex>
      <Flex
        ref={contentRef}
        tag="ul"
        id={listId}
        direction="column"
        gap="2px"
        alignItems={'stretch'}
        style={{
          maxHeight: isOpen ? `${contentHeight}px` : 0,
          overflow: 'hidden',
          transition: isOpen
            ? 'max-height 1s ease-in-out'
            : 'max-height 0.5s cubic-bezier(0, 1, 0, 1)',
        }}
      >
        {children.map((child, index) => {
          return <SubSectionLinkWrapper key={index}>{child}</SubSectionLinkWrapper>;
        })}
      </Flex>
    </Box>
  );
};

const PageWrapper = styled(Box)`
  ${MainSubNav} {
    background-color: transparent;
    border-right: none;
  }

  ${({ theme }) => theme.breakpoints.medium} {
    ${MainSubNav} {
      top: 0;
    }
  }
`;

const Content = ({ children }: { children: React.ReactNode }) => {

Review comment (suggestion):
I don't want to block the PR for that but I would use the same transition than the line after if possible `max-height 0.5s cubic-bezier(0, 1, 0, 1)`.

Code after:
            aria-hidden
            fill="neutral500"
            style={{
              transform: `rotate(${isOpen ? '0deg' : '-90deg'})`,
              transition: 'transform 0.5s',
            }}
          />
          <Box paddingLeft={2}>
            <Typography tag="span" fontWeight="semiBold" textColor="neutral800">
              {label}
            </Typography>
          </Box>
        </SubSectionHeader>
      </Flex>
      <Flex
        ref={contentRef}
        tag="ul"
        id={listId}
        direction="column"
        gap="2px"
        alignItems={'stretch'}
        style={{
          maxHeight: isOpen ? `${contentHeight}px` : 0,
          overflow: 'hidden',
          transition: 'max-height 0.5s cubic-bezier(0, 1, 0, 1)',
        }}
      >
        {children.map((child, index) => {
          return <SubSectionLinkWrapper key={index}>{child}</SubSectionLinkWrapper>;
        })}
      </Flex>
    </Box>
  );
};

const PageWrapper = styled(Box)`
  ${MainSubNav} {
    background-color: transparent;
    border-right: none;
  }

  ${({ theme }) => theme.breakpoints.medium} {
    ${MainSubNav} {
      top: 0;
    }
  }
`;

const Content = ({ children }: { children: React.ReactNode }) => {
  return <ScrollArea>{children}</ScrollArea>;
};
Repository: strapi/strapi
File: tests/api/utils/index.ts (TypeScript)

Code before:
  beforeEach(async () => {
    if (!isDataCaptured) {
      await captureInitialTestData();
      isDataCaptured = true;
    }
  });

  afterEach(async () => {
    if (isDataCaptured) {
      await resetTestDatabase();
    }
  });
}

/* -------------------------------------------------------------------------------------------------
 * Legacy transaction wrapper
 * -----------------------------------------------------------------------------------------------*/

export const wrapInTransaction = (test) => {
  console.warn('wrapInTransaction is deprecated. Use setupDatabaseReset() instead.');
  return test;
};

// Keep old interface for backward compatibility but log deprecation
export const testInTransaction = (...args: Parameters<jest.It>) => {
  console.warn('testInTransaction is deprecated. Use setupDatabaseReset() instead.');
  return it(...args);
};


Review comment (suggestion):
Thought this was nice so if you go to reach for it you know what to do next but happy to remove it otherwise

Code after:
      await captureInitialTestData();
      isDataCaptured = true;
    }
  });

  afterEach(async () => {
    if (isDataCaptured) {
      await resetTestDatabase();
    }
  });
}

/* -------------------------------------------------------------------------------------------------
 * testInTransaction
 * -----------------------------------------------------------------------------------------------*/

export const wrapInTransaction = (test) => {
  return async (...args) => {
    await strapi.db.transaction(async ({ trx, rollback }) => {
      await test(trx, ...args);
      await rollback();
    });
  };
};

/**
 * Resets the database by leveragin a transaction rollback
 *
 * NOTE:
 * Alternatively, use setupDatabaseReset() which is slower but avoids errors thrown by asnyc operations
 * executed after the test's transaction context has closed.
 */
export const testInTransaction = (...args: Parameters<jest.It>) => {
  if (args.length > 1) {
    return it(
      args[0], // name
      wrapInTransaction(args[1]), // fn
      args[2] // timeout
    );
  }
  return it(...args);
};

testInTransaction.skip = it.skip as jest.It['skip'];
testInTransaction.only = it.only as jest.It['only'];
testInTransaction.todo = it.todo as jest.It['todo'];
testInTransaction.each = it.each as jest.It['each'];
Repository: strapi/strapi
File: packages/plugins/i18n/admin/src/contentManagerHooks/editView.tsx (TypeScript)

Code before:
};

const Span = styled(Flex)`
  svg {
    width: 12px;
    height: 12px;

    fill: ${({ theme }) => theme.colors.neutral500};

    path {
      fill: ${({ theme }) => theme.colors.neutral500};
    }
  }
`;

/**
 * Compute effective localization for a specific field occurrence based on:
 * - Content type i18n setting
 * - Ancestor attribute (dynamic zone or component) i18n setting
 * - Field-level i18n setting for root fields
 */
const getEffectiveLocalization = (
  name: string,
  attribute: EditFieldLayout['attribute'],
  edit: EditLayout
): boolean => {
  const contentTypeLocalized =
    !!(edit.options as any)?.i18n && !!(edit.options as any).i18n.localized;

  if (!contentTypeLocalized) {
    return false;
  }

  const path = name.split('.');
  const topLevelAttrName = path[0];
  const topLevelField = findTopLevelField(edit, topLevelAttrName);
  if (!topLevelField) {
    return false;
  }

  // If the top-level attribute is a component or a dynamic zone, all nested fields inherit it.
  if (
    topLevelField.attribute.type === 'component' ||
    topLevelField.attribute.type === 'dynamiczone'
  ) {
    return getBooleanLocalized(getPluginOptions(topLevelField.attribute));
  }

  // Otherwise, we're dealing with a root-level field.
  return getBooleanLocalized(getPluginOptions(attribute));
};

Review comment (suggestion):
```suggestion
) => {
```
 
I'm not sure we need to explicitly type the return value here the boolean should be inferred

Code after:

const LabelAction = ({ title }: LabelActionProps) => {
  const { formatMessage } = useIntl();

  return (
    <Span tag="span" title={title}>
      <VisuallyHidden tag="span">{formatMessage(title)}</VisuallyHidden>
      <Tooltip label={formatMessage(title)}>
        <Earth aria-hidden focusable={false} />
      </Tooltip>
    </Span>
  );
};

const Span = styled(Flex)`
  svg {
    width: 12px;
    height: 12px;

    fill: ${({ theme }) => theme.colors.neutral500};

    path {
      fill: ${({ theme }) => theme.colors.neutral500};
    }
  }
`;

export { mutateEditViewHook };
Repository: strapi/strapi
File: packages/core/types/src/core/config/admin.ts (TypeScript)

Code before:
export interface ApiTokenProp {
  salt: string;
}

export interface AuthSessionsProp {
  options?: {
    algorithm?: string;
    [key: string]: any;
  };
  accessTokenLifespan?: number;
  maxRefreshTokenLifespan?: number;
  idleRefreshTokenLifespan?: number;
  maxSessionLifespan?: number;
  idleSessionLifespan?: number;
}

export interface AuthCookieProp {
  secure?: boolean;
  domain?: string;
  path?: string;
  sameSite?: 'strict' | 'lax' | 'none';
}

export interface AuthEventsProp {
  onConnectionSuccess?: (user: any, provider: string) => void | Promise<void>;
  onConnectionError?: (error: Error, provider: string) => void | Promise<void>;
}

export interface AuthProviderProp {
  [key: string]: any;
}

export interface AuthProp {

Review comment (suggestion):
Suggestion: Won't be better for all indexed access objects to have the type `unknown` instead of `any`? 👀 

Code after:
export interface ApiToken {
  salt: string;
}

export interface AuthSessions {
  options?: {
    algorithm?: string;
    [key: string]: unknown;
  };
  accessTokenLifespan?: number | undefined;
  maxRefreshTokenLifespan?: number;
  idleRefreshTokenLifespan?: number;
  maxSessionLifespan?: number;
  idleSessionLifespan?: number;
}

export interface AuthCookie {
  secure?: boolean;
  domain?: string;
  path?: string;
  sameSite?: 'strict' | 'lax' | 'none' | boolean | null;
}

export interface AuthEvents {
  onConnectionSuccess?: (user: unknown, provider: string) => void | Promise<void>;
  onConnectionError?: (error: Error, provider: string) => void | Promise<void>;
}

export interface AuthProvider {
  [key: string]: unknown;
}

export interface Auth {
Repository: strapi/strapi
File: packages/core/types/src/core/config/admin.ts (TypeScript)

Code before:
export interface ApiTokenProp {
  salt: string;
}

export interface AuthSessionsProp {
  options?: {
    algorithm?: string;
    [key: string]: any;
  };
  accessTokenLifespan?: number;
  maxRefreshTokenLifespan?: number;
  idleRefreshTokenLifespan?: number;
  maxSessionLifespan?: number;
  idleSessionLifespan?: number;
}

export interface AuthCookieProp {
  secure?: boolean;
  domain?: string;
  path?: string;
  sameSite?: 'strict' | 'lax' | 'none';
}

export interface AuthEventsProp {
  onConnectionSuccess?: (user: any, provider: string) => void | Promise<void>;
  onConnectionError?: (error: Error, provider: string) => void | Promise<void>;
}

export interface AuthProviderProp {
  [key: string]: any;
}

export interface AuthProp {
  secret: string;
  domain?: string;

Review comment (suggestion):
Suggestion: Can we add also `undefined` for the optional fields? This will make it compatible with the TypeScript `exactOptionalPropertyTypes` flag, when someone want's to explicitly pass `undefined`

```suggestion
  accessTokenLifespan?: number | undefined;
```

https://www.typescriptlang.org/tsconfig/#exactOptionalPropertyTypes

Context: https://github.com/strapi/strapi/issues/23480

Code after:
export interface ApiToken {
  salt: string;
}

export interface AuthSessions {
  options?: {
    algorithm?: string;
    [key: string]: unknown;
  };
  accessTokenLifespan?: number | undefined;
  maxRefreshTokenLifespan?: number;
  idleRefreshTokenLifespan?: number;
  maxSessionLifespan?: number;
  idleSessionLifespan?: number;
}

export interface AuthCookie {
  secure?: boolean;
  domain?: string;
  path?: string;
  sameSite?: 'strict' | 'lax' | 'none' | boolean | null;
}

export interface AuthEvents {
  onConnectionSuccess?: (user: unknown, provider: string) => void | Promise<void>;
  onConnectionError?: (error: Error, provider: string) => void | Promise<void>;
}

export interface AuthProvider {
  [key: string]: unknown;
}

export interface Auth {
  secret: string;
  domain?: string;
Repository: strapi/strapi
File: packages/core/upload/server/src/content-types/file.ts (TypeScript)

Code before:
export default {
  schema: {
    collectionName: 'files',
    info: {
      singularName: 'file',
      pluralName: 'files',
      displayName: 'File',
      description: '',
    },
    options: {},
    pluginOptions: {
      'content-manager': {
        visible: false,
      },
      'content-type-builder': {
        visible: false,
      },
    },
    attributes: {
      name: {
        type: 'string',
        configurable: false,
        required: true,
      },
      alternativeText: {
        type: 'string',
        configurable: false,
      },
      caption: {
        type: 'string',
        configurable: false,
      },
      width: {
        type: 'integer',
        configurable: false,
      },
      height: {
        type: 'integer',
        configurable: false,
      },
      formats: {
        type: 'json',
        configurable: false,
      },
      hash: {
        type: 'string',
        configurable: false,
        required: true,
      },
      ext: {
        type: 'string',

Review comment (suggestion):
This one I wouldn't change, having alt text longer than 255 is very bad practice so I wouldn't encourage it unless we need to

Code after:
export default {
  schema: {
    collectionName: 'files',
    info: {
      singularName: 'file',
      pluralName: 'files',
      displayName: 'File',
      description: '',
    },
    options: {},
    pluginOptions: {
      'content-manager': {
        visible: false,
      },
      'content-type-builder': {
        visible: false,
      },
    },
    attributes: {
      name: {
        type: 'text',
        configurable: false,
        required: true,
      },
      alternativeText: {
        type: 'text',
        configurable: false,
      },
      caption: {
        type: 'text',
        configurable: false,
      },
      width: {
        type: 'integer',
        configurable: false,
      },
      height: {
        type: 'integer',
        configurable: false,
      },
      formats: {
        type: 'json',
        configurable: false,
      },
      hash: {
        type: 'string',
        configurable: false,
        required: true,
      },
      ext: {
        type: 'string',
Repository: strapi/strapi
File: packages/core/admin/admin/src/components/SubNav.tsx (TypeScript)

Code before:
        large: 5,
      }}
    >
      <Typography variant="beta" tag="h2">
        {label}
      </Typography>
    </StyledHeader>
  );
};

const Sections = ({
  children,
  ...props
}: {
  children: React.ReactNode[];
  [key: string]: unknown;
}) => {
  return (
    <Box
      paddingTop={{ initial: 5, large: 4 }}
      paddingBottom={{ initial: 5, large: 4 }}
      paddingLeft={{ initial: 3, large: 0 }}
      paddingRight={{ initial: 3, large: 0 }}
      maxWidth={{ initial: '100%', medium: '23.2rem' }}
    >
      <Flex tag="ul" gap="6" direction="column" alignItems="stretch" {...props}>
        {children.map((child, index) => {
          return <li key={index}>{child}</li>;
        })}
      </Flex>
    </Box>
  );
};

/**
 * TODO:
 * This would be better in the content-type-builder package directly but currently
 * the SubNav API does not expose a way to wrap the link, instead it wraps the link and the list
 */
const GuidedTourTooltip = ({
  sectionId,
  children,
}: {
  sectionId?: string;
  children: React.ReactNode;
}) => {
  switch (sectionId) {
    case 'models':
      return (
        <tours.contentTypeBuilder.CollectionTypes>
          <tours.contentTypeBuilder.YourTurn>{children}</tours.contentTypeBuilder.YourTurn>

Review comment (suggestion):
I know it's the same but for consistency, we'd use a type number instead of a string for gap => `gap={6}`

Code after:
  [key: string]: unknown;
}) => {
  return (
    <Box
      paddingTop={{ initial: 5, large: 4 }}
      paddingBottom={{ initial: 5, large: 4 }}
      paddingLeft={{ initial: 3, large: 0 }}
      paddingRight={{ initial: 3, large: 0 }}
      maxWidth={{ initial: '100%', medium: '23.2rem' }}
    >
      <Flex tag="ul" gap={6} direction="column" alignItems="stretch" {...props}>
        {children.map((child, index) => {
          return <li key={index}>{child}</li>;
        })}
      </Flex>
    </Box>
  );
};

/**
 * TODO:
 * This would be better in the content-type-builder package directly but currently
 * the SubNav API does not expose a way to wrap the link, instead it wraps the link and the list
 */
const GuidedTourTooltip = ({
  sectionId,
  children,
}: {
  sectionId?: string;
  children: React.ReactNode;
}) => {
  switch (sectionId) {
    case 'models':
      return (
        <tours.contentTypeBuilder.CollectionTypes>
          <tours.contentTypeBuilder.YourTurn>{children}</tours.contentTypeBuilder.YourTurn>
        </tours.contentTypeBuilder.CollectionTypes>
      );
    case 'singleTypes':
      return (
        <tours.contentTypeBuilder.SingleTypes>{children}</tours.contentTypeBuilder.SingleTypes>
      );
    case 'components':
      return <tours.contentTypeBuilder.Components>{children}</tours.contentTypeBuilder.Components>;
    default:
      return children;
  }
};

const Section = ({
  label,
Repository: strapi/strapi
File: packages/core/content-manager/admin/src/pages/EditView/components/FormInputs/DynamicZone/ComponentCategory.tsx (TypeScript)

Code before:
                </Typography>
              </Flex>
            </ComponentBox>
          ))}
        </Grid>
      </ResponsiveAccordionContent>
    </Accordion.Item>
  );
};

const ResponsiveAccordionContent = styled(Accordion.Content)`
  container-type: inline-size;
`;

/**
 * TODO:
 * JSDOM cannot handle container queries.
 * This is a temporary workaround so that tests do not fail in the CI when jestdom throws an error
 * for failing to parse the stylesheet.
 */
const Grid =
  process.env.NODE_ENV !== 'test'
    ? styled(Box)`
        display: grid;
        grid-template-columns: repeat(auto-fill, 100%);
        grid-gap: 4px;

        ${({ theme }) => theme.breakpoints.medium} {
          grid-template-columns: repeat(auto-fill, 14rem);
        }
      `
    : styled(Box)`
        display: grid;
        grid-template-columns: repeat(auto-fill, 100%);
        grid-gap: 4px;
      `;

const ComponentBox = styled<FlexComponent<'button'>>(Flex)`
  color: ${({ theme }) => theme.colors.neutral600};
  cursor: pointer;

  @media (prefers-reduced-motion: no-preference) {
    transition: color 120ms ${(props) => props.theme.motion.easings.easeOutQuad};
  }

  &:focus,
  &:hover {
    border: 1px solid ${({ theme }) => theme.colors.primary200};
    background: ${({ theme }) => theme.colors.primary100};
    color: ${({ theme }) => theme.colors.primary600};
  }

Review comment (suggestion):
You could use the spacing from the theme here: `${({ theme }) => theme.spaces[3]}`

Code after:
                </Typography>
              </Flex>
            </ComponentBox>
          ))}
        </Grid>
      </ResponsiveAccordionContent>
    </Accordion.Item>
  );
};

const ResponsiveAccordionContent = styled(Accordion.Content)`
  container-type: inline-size;
`;

/**
 * TODO:
 * JSDOM cannot handle container queries.
 * This is a temporary workaround so that tests do not fail in the CI when jestdom throws an error
 * for failing to parse the stylesheet.
 */
const Grid =
  process.env.NODE_ENV !== 'test'
    ? styled(Box)`
        display: grid;
        grid-template-columns: repeat(auto-fill, 100%);
        grid-gap: 12px;

        ${({ theme }) => theme.breakpoints.medium} {
          grid-template-columns: repeat(auto-fill, 14rem);
          grid-gap: 4px;
        }
      `
    : styled(Box)`
        display: grid;
        grid-template-columns: repeat(auto-fill, 100%);
        grid-gap: 12px;

        ${({ theme }) => theme.breakpoints.medium} {
          grid-gap: 4px;
        }
      `;

const ComponentBox = styled<FlexComponent<'button'>>(Flex)`
  color: ${({ theme }) => theme.colors.neutral600};
  cursor: pointer;

  @media (prefers-reduced-motion: no-preference) {
    transition: color 120ms ${(props) => props.theme.motion.easings.easeOutQuad};
  }

  &:focus,
Repository: strapi/strapi
File: packages/core/admin/ee/server/src/audit-logs/services/lifecycles.ts (TypeScript)

Code before:
        });
      }

      // Handle license being disabled
      // @ts-expect-error- update event hub to receive callback argument
      state.eeDisableUnsubscribe = strapi.eventHub.on('ee.disable', () => {
        // Turn off service when the license gets disabled
        // Only ee.enable and ee.update listeners remain active to recreate the service
        this.destroy();
      });

      // Check current state of license
      if (!strapi.ee.features.isEnabled('audit-logs')) {
        return this;
      }

      // Start saving events
      state.eventHubUnsubscribe = strapi.eventHub.subscribe(handleEvent);

      // Manage audit logs auto deletion
      const retentionDays = getRetentionDays(strapi);

      strapi.cron.add({
        deleteExpiredAuditLogs: {
          task: async () => {
            try {
              const expirationDate = new Date(Date.now() - retentionDays * 24 * 60 * 60 * 1000);
              await auditLogsService.deleteExpiredEvents(expirationDate);
            } catch (error) {
              strapi.log.error('Error deleting expired audit logs', error);
            }
          },
          options: '0 0 * * *',
        },
      });

      return this;
    },

    unsubscribe() {
      if (state.eeDisableUnsubscribe) {
        state.eeDisableUnsubscribe();
      }

      if (state.eventHubUnsubscribe) {
        state.eventHubUnsubscribe();
      }

      strapi.cron.remove('deleteExpiredAuditLogs');

      return this;

Review comment (suggestion):
Same here, we don't need to log this, we can remove the try/catch, or keep it to rethrow a more specific error

Code after:
        });
      }

      // Handle license being disabled
      // @ts-expect-error- update event hub to receive callback argument
      state.eeDisableUnsubscribe = strapi.eventHub.on('ee.disable', () => {
        // Turn off service when the license gets disabled
        // Only ee.enable and ee.update listeners remain active to recreate the service
        this.destroy();
      });

      // Check current state of license
      if (!strapi.ee.features.isEnabled('audit-logs')) {
        return this;
      }

      // Start saving events
      state.eventHubUnsubscribe = strapi.eventHub.subscribe(handleEvent);

      // Manage audit logs auto deletion
      const retentionDays = getRetentionDays(strapi);

      strapi.cron.add({
        deleteExpiredAuditLogs: {
          task: async () => {
            const expirationDate = new Date(Date.now() - retentionDays * 24 * 60 * 60 * 1000);
            await auditLogsService.deleteExpiredEvents(expirationDate);
          },
          options: '0 0 * * *',
        },
      });

      return this;
    },

    unsubscribe() {
      if (state.eeDisableUnsubscribe) {
        state.eeDisableUnsubscribe();
      }

      if (state.eventHubUnsubscribe) {
        state.eventHubUnsubscribe();
      }

      strapi.cron.remove('deleteExpiredAuditLogs');

      return this;
    },

    destroy() {
      return this.unsubscribe();