ronantakizawa/codereview-bench huggingface dataset:
Repository: twentyhq/twenty
File: packages/twenty-front/src/modules/action-menu/actions/record-actions/single-record/utils/exportBlockNoteEditorToPdf.ts (TypeScript)

Code before:
        });
      });
    }
    return registrationPromise;
  };
})();

export const exportBlockNoteEditorToPdf = async (
  parsedBody: PartialBlock[],
  filename: string,
) => {
  await registerInterFonts();

  const editor = BlockNoteEditor.create({
    initialContent: parsedBody,
  });

  const exporter = new PDFExporter(editor.schema, pdfDefaultSchemaMappings, {
    resolveFileUrl: async (url: string) => {
      const response = await fetch(url, {
        mode: 'cors',
        credentials: 'omit',
      });

      if (!response.ok) {
        throw new Error(`Failed to fetch asset at ${url}`);
      }

      return await response.blob();
    },
  });

  const pdfDocument = await exporter.toReactPDFDocument(editor.document);

  const blob = await pdf(pdfDocument).toBlob();
  saveAs(blob, `${filename}.pdf`);
};


Review comment (bug):
The error message should include the HTTP status code and status text to aid in debugging. Consider using:
```typescript
throw new Error(`Failed to fetch asset at ${url}: ${response.status} ${response.statusText}`);
```
```suggestion
        throw new Error(`Failed to fetch asset at ${url}: ${response.status} ${response.statusText}`);
```

Code after:
        });
      });
    }
    return registrationPromise;
  };
})();

export const exportBlockNoteEditorToPdf = async (
  parsedBody: PartialBlock[],
  filename: string,
) => {
  await registerInterFonts();

  const editor = BlockNoteEditor.create({
    initialContent: parsedBody,
  });

  const exporter = new PDFExporter(editor.schema, pdfDefaultSchemaMappings, {
    resolveFileUrl: async (url: string) => {
      try {
        const response = await fetch(url, {
          mode: 'cors',
          credentials: 'omit',
        });

        if (!response.ok) {
          throw new Error(
            `Failed to fetch asset at ${url}: ${response.status} ${response.statusText}`,
          );
        }

        return await response.blob();
      } catch (error) {
        if (
          error instanceof Error &&
          error.message.includes('Failed to fetch asset')
        ) {
          throw error;
        }
        throw new Error(
          `Failed to fetch asset at ${url}: ${error instanceof Error ? error.message : 'Unknown error'}`,
        );
      }
    },
  });

  const pdfDocument = await exporter.toReactPDFDocument(editor.document);

  const blob = await pdf(pdfDocument).toBlob();
  saveAs(blob, `${filename}.pdf`);
};
Repository: twentyhq/twenty
File: packages/twenty-front/src/modules/page-layout/widgets/graph/components/GraphWidgetLegend.tsx (TypeScript)

Code before:
  hidden: {
    y: 10,
    opacity: 0,
  },
  visible: {
    y: 0,
    opacity: 1,
  },
  exit: {
    y: 10,
    opacity: 0,
  },
};

export const GraphWidgetLegend = ({
  items,
  show = true,
}: GraphWidgetLegendProps) => {
  const containerRef = useRef<HTMLDivElement>(null);

  const [currentPage, setCurrentPage] = useState(0);

  const [containerWidth, setContainerWidth] = useState(0);

  const [animationDirection, setAnimationDirection] = useState<
    'forward' | 'backward'
  >('forward');

  const theme = useTheme();

  const shouldShowLegend = show && items.length > 1;

  const availableWidth = containerWidth - LEGEND_PAGINATION_CONTROLS_WIDTH;

  const itemsPerPage = Math.max(
    1,
    Math.floor(availableWidth / LEGEND_ITEM_ESTIMATED_WIDTH),
  );

  const needsPagination = items.length > itemsPerPage;
  const totalPages = Math.ceil(items.length / itemsPerPage);
  const safeCurrentPage = Math.min(currentPage, Math.max(0, totalPages - 1));

  const startIndex = safeCurrentPage * itemsPerPage;
  const visibleItems = needsPagination
    ? items.slice(startIndex, startIndex + itemsPerPage)
    : items;

  const handlePreviousPage = (e: React.MouseEvent<HTMLButtonElement>) => {
    e.stopPropagation();
    setAnimationDirection('backward');

Review comment (suggestion):
type LegendAnimationDirection = 'forward' | 'backward'

could be reused below

Code after:
    y: 0,
    opacity: 1,
  },
  exit: {
    y: 10,
    opacity: 0,
  },
};

export const GraphWidgetLegend = ({
  items,
  show = true,
}: GraphWidgetLegendProps) => {
  const containerRef = useRef<HTMLDivElement>(null);

  const [currentPage, setCurrentPage] = useState(0);

  const [containerWidth, setContainerWidth] = useState(0);

  const [animationDirection, setAnimationDirection] =
    useState<AnimationDirection>('forward');

  const theme = useTheme();

  const shouldShowLegend = show && items.length > 1;

  const availableWidth = containerWidth - LEGEND_PAGINATION_CONTROLS_WIDTH;

  const itemsPerPage = Math.max(
    1,
    Math.floor(availableWidth / LEGEND_ITEM_ESTIMATED_WIDTH),
  );

  const needsPagination = items.length > itemsPerPage;
  const totalPages = Math.ceil(items.length / itemsPerPage);
  const safeCurrentPage = Math.min(currentPage, Math.max(0, totalPages - 1));

  const startIndex = safeCurrentPage * itemsPerPage;
  const visibleItems = needsPagination
    ? items.slice(startIndex, startIndex + itemsPerPage)
    : items;

  const handlePreviousPage = (e: React.MouseEvent<HTMLButtonElement>) => {
    e.stopPropagation();
    setAnimationDirection('backward');
    setCurrentPage((prevPage) => Math.max(0, prevPage - 1));
  };

  const handleNextPage = (e: React.MouseEvent<HTMLButtonElement>) => {
    e.stopPropagation();
    setAnimationDirection('forward');
Repository: twentyhq/twenty
File: packages/twenty-server/src/engine/api/graphql/workspace-schema-builder/graphql-type-generators/input-types/order-by-input/object-metadata-order-by-base.generator.ts (TypeScript)

Code before:
    };
  }

  private generateAtomicFieldOrderByInputType({
    fieldMetadata,
    typeOptions,
    logger,
    isForGroupBy,
  }: {
    fieldMetadata: FlatFieldMetadata;
    typeOptions: TypeOptions;
    logger: Logger;
    isForGroupBy?: boolean;
  }) {
    if (
      isForGroupBy === true &&
      (fieldMetadata.type === FieldMetadataType.DATE ||
        fieldMetadata.type === FieldMetadataType.DATE_TIME)
    ) {
      const orderByDateGranularityInputType =
        this.gqlTypesStorage.getGqlTypeByKey(
          ORDER_BY_DATE_GRANULARITY_INPUT_KEY,
        );

      if (!isDefined(orderByDateGranularityInputType)) {
        throw new Error('isForGroupByInputType not found');
      }

      return {
        [fieldMetadata.name]: {
          type: orderByDateGranularityInputType,
          description: fieldMetadata.description,
        },
      };
    }

    const orderByType = this.typeMapperService.mapToOrderByType(
      fieldMetadata.type,
    );

    if (!isDefined(orderByType)) {
      const message = `Could not find a GraphQL input type for ${fieldMetadata.type} field metadata`;

      logger.error(message, {
        fieldMetadata,
        typeOptions,
      });
      throw new Error(message);
    }

    return {

Review comment (suggestion):
is it expected? seems we are talking about orderByDateGranularityInputType (which is based on ORDER_BY_DATE_GRANULARITY_INPUT_KEY) but the log refers to isForGroupByInputType
@ijreilly 

Code after:
    };
  }

  private generateAtomicFieldOrderByInputType({
    fieldMetadata,
    typeOptions,
    logger,
    isForGroupBy,
  }: {
    fieldMetadata: FlatFieldMetadata;
    typeOptions: TypeOptions;
    logger: Logger;
    isForGroupBy?: boolean;
  }) {
    if (
      isForGroupBy === true &&
      (fieldMetadata.type === FieldMetadataType.DATE ||
        fieldMetadata.type === FieldMetadataType.DATE_TIME)
    ) {
      const orderByDateGranularityInputType =
        this.gqlTypesStorage.getGqlTypeByKey(
          ORDER_BY_DATE_GRANULARITY_INPUT_KEY,
        );

      if (!isDefined(orderByDateGranularityInputType)) {
        throw new Error('OrderByDateGranularityInputType not found');
      }

      return {
        [fieldMetadata.name]: {
          type: orderByDateGranularityInputType,
          description: fieldMetadata.description,
        },
      };
    }

    const orderByType = this.typeMapperService.mapToOrderByType(
      fieldMetadata.type,
    );

    if (!isDefined(orderByType)) {
      const message = `Could not find a GraphQL input type for ${fieldMetadata.type} field metadata`;

      logger.error(message, {
        fieldMetadata,
        typeOptions,
      });
      throw new Error(message);
    }

    return {
Repository: twentyhq/twenty
File: packages/twenty-shared/src/types/FieldMetadataOptions.ts (TypeScript)

Code before:
import { type FieldMetadataType } from '@/types/FieldMetadataType';
import { type IsExactly } from '@/types/IsExactly';

export type TagColor =
  | 'green'
  | 'turquoise'
  | 'sky'
  | 'blue'
  | 'purple'
  | 'pink'
  | 'red'
  | 'orange'
  | 'yellow'
  | 'gray';

export class FieldMetadataDefaultOption {
  id?: string;
  position!: number;
  label!: string;
  value!: string;
}

export class FieldMetadataComplexOption extends FieldMetadataDefaultOption {
  color!: TagColor;
}

type FieldMetadataOptionsMapping = {
  [FieldMetadataType.RATING]: FieldMetadataDefaultOption[];
  [FieldMetadataType.SELECT]: FieldMetadataComplexOption[];
  [FieldMetadataType.MULTI_SELECT]: FieldMetadataComplexOption[];
};

export type FieldMetadataOptions<
  T extends FieldMetadataType = FieldMetadataType,
> =
  IsExactly<T, FieldMetadataType> extends true
    ? null | (FieldMetadataDefaultOption[] | FieldMetadataComplexOption[]) // Could be improved to be | unknown
    : T extends keyof FieldMetadataOptionsMapping
      ? FieldMetadataOptionsMapping[T]
      : never | null;


Review comment (suggestion):
Removing the definite assignment assertion operator (`!`) changes these properties from required to optional. This is a breaking change that could cause type-safety issues where code expects `position`, `label`, and `value` to always be defined.

If these properties should be optional, consider making this explicit by adding `?` after the property name. If they should remain required, the `!` operator should be kept, or alternatively, provide default values in the class constructor.
```suggestion
  position!: number;
  label!: string;
  value!: string;
```

Code after:
import { type FieldMetadataType } from '@/types/FieldMetadataType';
import { type IsExactly } from '@/types/IsExactly';

export type TagColor =
  | 'green'
  | 'turquoise'
  | 'sky'
  | 'blue'
  | 'purple'
  | 'pink'
  | 'red'
  | 'orange'
  | 'yellow'
  | 'gray';

export class FieldMetadataDefaultOption {
  id?: string;
  position: number;
  label: string;
  value: string;
}

export class FieldMetadataComplexOption extends FieldMetadataDefaultOption {
  color: TagColor;
}

type FieldMetadataOptionsMapping = {
  [FieldMetadataType.RATING]: FieldMetadataDefaultOption[];
  [FieldMetadataType.SELECT]: FieldMetadataComplexOption[];
  [FieldMetadataType.MULTI_SELECT]: FieldMetadataComplexOption[];
};

export type FieldMetadataOptions<
  T extends FieldMetadataType = FieldMetadataType,
> =
  IsExactly<T, FieldMetadataType> extends true
    ? null | (FieldMetadataDefaultOption[] | FieldMetadataComplexOption[]) // Could be improved to be | unknown
    : T extends keyof FieldMetadataOptionsMapping
      ? FieldMetadataOptionsMapping[T]
      : never | null;
Repository: twentyhq/twenty
File: packages/twenty-shared/src/types/FieldMetadataOptions.ts (TypeScript)

Code before:
import { type FieldMetadataType } from '@/types/FieldMetadataType';
import { type IsExactly } from '@/types/IsExactly';

export type TagColor =
  | 'green'
  | 'turquoise'
  | 'sky'
  | 'blue'
  | 'purple'
  | 'pink'
  | 'red'
  | 'orange'
  | 'yellow'
  | 'gray';

export class FieldMetadataDefaultOption {
  id?: string;
  position!: number;
  label!: string;
  value!: string;
}

export class FieldMetadataComplexOption extends FieldMetadataDefaultOption {
  color!: TagColor;
}

type FieldMetadataOptionsMapping = {
  [FieldMetadataType.RATING]: FieldMetadataDefaultOption[];
  [FieldMetadataType.SELECT]: FieldMetadataComplexOption[];
  [FieldMetadataType.MULTI_SELECT]: FieldMetadataComplexOption[];
};

export type FieldMetadataOptions<
  T extends FieldMetadataType = FieldMetadataType,
> =
  IsExactly<T, FieldMetadataType> extends true
    ? null | (FieldMetadataDefaultOption[] | FieldMetadataComplexOption[]) // Could be improved to be | unknown
    : T extends keyof FieldMetadataOptionsMapping
      ? FieldMetadataOptionsMapping[T]
      : never | null;


Review comment (suggestion):
Removing the definite assignment assertion operator (`!`) changes the `color` property from required to optional, which is a breaking change for code expecting this property to always be defined in `FieldMetadataComplexOption`.

If this property should be optional, make it explicit with `?`. Otherwise, restore the `!` operator or provide a default value.
```suggestion
  color!: TagColor;
```

Code after:
import { type FieldMetadataType } from '@/types/FieldMetadataType';
import { type IsExactly } from '@/types/IsExactly';

export type TagColor =
  | 'green'
  | 'turquoise'
  | 'sky'
  | 'blue'
  | 'purple'
  | 'pink'
  | 'red'
  | 'orange'
  | 'yellow'
  | 'gray';

export class FieldMetadataDefaultOption {
  id?: string;
  position: number;
  label: string;
  value: string;
}

export class FieldMetadataComplexOption extends FieldMetadataDefaultOption {
  color: TagColor;
}

type FieldMetadataOptionsMapping = {
  [FieldMetadataType.RATING]: FieldMetadataDefaultOption[];
  [FieldMetadataType.SELECT]: FieldMetadataComplexOption[];
  [FieldMetadataType.MULTI_SELECT]: FieldMetadataComplexOption[];
};

export type FieldMetadataOptions<
  T extends FieldMetadataType = FieldMetadataType,
> =
  IsExactly<T, FieldMetadataType> extends true
    ? null | (FieldMetadataDefaultOption[] | FieldMetadataComplexOption[]) // Could be improved to be | unknown
    : T extends keyof FieldMetadataOptionsMapping
      ? FieldMetadataOptionsMapping[T]
      : never | null;
Repository: twentyhq/twenty
File: packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/display/components/ChipFieldDisplay.tsx (TypeScript)

Code before:
import { RecordChip } from '@/object-record/components/RecordChip';
import { useChipFieldDisplay } from '@/object-record/record-field/ui/meta-types/hooks/useChipFieldDisplay';
import { isDefined } from 'twenty-shared/utils';
import { ChipSize } from 'twenty-ui/components';
import { useIsMobile } from 'twenty-ui/utilities';

export const ChipFieldDisplay = () => {
  const {
    recordValue,
    objectNameSingular,
    labelIdentifierLink,
    disableChipClick,
    maxWidth,
    triggerEvent,
    onRecordChipClick,
  } = useChipFieldDisplay();

  const isMobile = useIsMobile();

  // TODO: reimplement scrolled horizontally here.
  const isLabelIdentifierCompact = isMobile;

  if (!isDefined(recordValue)) {
    return null;
  }

  return (
    <RecordChip
      maxWidth={maxWidth}
      objectNameSingular={objectNameSingular}
      record={recordValue}
      size={ChipSize.Small}
      to={labelIdentifierLink}
      isLabelHidden={isLabelIdentifierCompact}
      forceDisableClick={disableChipClick}
      triggerEvent={triggerEvent}
      onClick={onRecordChipClick}
    />
  );
};


Review comment (performance):
@lucasbordeau @bosiraphael this is a recoil state access in a field display which has performance impact.

In the current setup we should add this to a context (FieldContext likely here)
+ this is adding a dependency on ContextStoreContext which is not necessary I think
+ breaking Settings page! (we need more stories I guess)

Code after:
import { RecordChip } from '@/object-record/components/RecordChip';
import { useChipFieldDisplay } from '@/object-record/record-field/ui/meta-types/hooks/useChipFieldDisplay';
import { shouldCompactRecordIndexLabelIdentifierComponentState } from '@/object-record/record-index/states/shouldCompactRecordIndexLabelIdentifierComponentState';
import { useRecoilComponentValue } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentValue';
import { isDefined } from 'twenty-shared/utils';
import { ChipSize } from 'twenty-ui/components';

export const ChipFieldDisplay = () => {
  const {
    recordValue,
    objectNameSingular,
    labelIdentifierLink,
    disableChipClick,
    maxWidth,
    triggerEvent,
    onRecordChipClick,
  } = useChipFieldDisplay();

  const shouldCompactRecordIndexLabelIdentifier = useRecoilComponentValue(
    shouldCompactRecordIndexLabelIdentifierComponentState,
  );

  const isLabelIdentifierCompact = shouldCompactRecordIndexLabelIdentifier;

  if (!isDefined(recordValue)) {
    return null;
  }

  return (
    <RecordChip
      maxWidth={maxWidth}
      objectNameSingular={objectNameSingular}
      record={recordValue}
      size={ChipSize.Small}
      to={labelIdentifierLink}
      isLabelHidden={isLabelIdentifierCompact}
      forceDisableClick={disableChipClick}
      triggerEvent={triggerEvent}
      onClick={onRecordChipClick}
    />
  );
};
Repository: twentyhq/twenty
File: packages/twenty-front/src/modules/ui/field/display/components/LinksDisplay.tsx (TypeScript)

Code before:
}: LinksDisplayProps) => {
  const links = useMemo(() => {
    if (!isDefined(value)) {
      return [];
    }

    return getFieldLinkDefinedLinks(value).map(({ url, label }) => {
      let absoluteUrl = '';
      let hostname = '';
      try {
        absoluteUrl = getAbsoluteUrlOrThrow(url);
        hostname = getUrlHostnameOrThrow(absoluteUrl);
      } catch {
        absoluteUrl = '';
        hostname = '';
      }
      return {
        url: absoluteUrl,
        label: label || hostname,
        type: checkUrlType(absoluteUrl),
      };
    });
  }, [value]);

  const handleClick = (url: string, event: React.MouseEvent<HTMLElement>) => {
    if (clickAction === FieldClickAction.COPY && isDefined(onLinkClick)) {
      onLinkClick(url, event);
    }
  };

  return (
    <ExpandableList>
      {links.map(({ url, label, type }, index) =>
        type === LinkType.LinkedIn || type === LinkType.Twitter ? (
          <SocialLink
            key={index}
            href={url}
            type={type}
            label={label}
            onClick={(event) => handleClick(url, event)}
          />
        ) : (
          <RoundedLink
            key={index}
            href={url}
            label={label}
            onClick={(event) => handleClick(url, event)}
          />
        ),
      )}
    </ExpandableList>

Review comment (suggestion):
The condition `&& isDefined(onLinkClick)` is unnecessary because when `clickAction === FieldClickAction.COPY`, the parent component (`LinksFieldDisplay`) always provides the `onLinkClick` callback. This check can be simplified to just `if (clickAction === FieldClickAction.COPY)` for cleaner code.
```suggestion
    if (clickAction === FieldClickAction.COPY) {
```

Code after:
      try {
        absoluteUrl = getAbsoluteUrlOrThrow(url);
        hostname = getUrlHostnameOrThrow(absoluteUrl);
      } catch {
        absoluteUrl = '';
        hostname = '';
      }
      return {
        url: absoluteUrl,
        label: label || hostname,
        type: checkUrlType(absoluteUrl),
      };
    });
  }, [value]);

  return (
    <ExpandableList>
      {links.map(({ url, label, type }, index) =>
        type === LinkType.LinkedIn || type === LinkType.Twitter ? (
          <SocialLink
            key={index}
            href={url}
            type={type}
            label={label}
            onClick={(event) => onLinkClick?.(url, event)}
          />
        ) : (
          <RoundedLink
            key={index}
            href={url}
            label={label}
            onClick={(event) => onLinkClick?.(url, event)}
          />
        ),
      )}
    </ExpandableList>
  );
};
Repository: twentyhq/twenty
File: packages/twenty-front/src/modules/ui/field/display/components/EmailsDisplay.tsx (TypeScript)

Code before:
  justify-content: flex-start;

  max-width: 100%;

  overflow: hidden;

  width: 100%;
`;

export const EmailsDisplay = ({
  value,
  isFocused,
  onEmailClick,
  clickAction,
}: EmailsDisplayProps) => {
  const emails = useMemo(
    () =>
      [
        value?.primaryEmail ? value.primaryEmail : null,
        ...(value?.additionalEmails ?? []),
      ].filter(isDefined),
    [value?.primaryEmail, value?.additionalEmails],
  );

  const handleClick = (email: string, event: React.MouseEvent<HTMLElement>) => {
    if (clickAction === FieldClickAction.COPY && isDefined(onEmailClick)) {
      onEmailClick(email, event);
    }
  };

  return isFocused ? (
    <ExpandableList isChipCountDisplayed>
      {emails.map((email, index) => (
        <RoundedLink
          key={index}
          label={email}
          href={`mailto:${email}`}
          onClick={(event) => handleClick(email, event)}
        />
      ))}
    </ExpandableList>
  ) : (
    <StyledContainer>
      {emails.map((email, index) => (
        <RoundedLink
          key={index}
          label={email}
          href={`mailto:${email}`}
          onClick={(event) => handleClick(email, event)}
        />
      ))}

Review comment (suggestion):
The condition `&& isDefined(onEmailClick)` is unnecessary because when `clickAction === FieldClickAction.COPY`, the parent component (`EmailsFieldDisplay`) always provides the `onEmailClick` callback. This check can be simplified to just `if (clickAction === FieldClickAction.COPY)` for cleaner code.
```suggestion
    if (clickAction === FieldClickAction.COPY) {
```

Code after:

  width: 100%;
`;

export const EmailsDisplay = ({
  value,
  isFocused,
  onEmailClick,
}: EmailsDisplayProps) => {
  const emails = useMemo(
    () =>
      [
        value?.primaryEmail ? value.primaryEmail : null,
        ...(value?.additionalEmails ?? []),
      ].filter(isDefined),
    [value?.primaryEmail, value?.additionalEmails],
  );

  return isFocused ? (
    <ExpandableList isChipCountDisplayed>
      {emails.map((email, index) => (
        <RoundedLink
          key={index}
          label={email}
          href={`mailto:${email}`}
          onClick={(event) => onEmailClick?.(email, event)}
        />
      ))}
    </ExpandableList>
  ) : (
    <StyledContainer>
      {emails.map((email, index) => (
        <RoundedLink
          key={index}
          label={email}
          href={`mailto:${email}`}
          onClick={(event) => onEmailClick?.(email, event)}
        />
      ))}
    </StyledContainer>
  );
};