ronantakizawa/codereview-bench huggingface dataset:
Repository: strapi/strapi
File: packages/core/admin/shared/utils/session-auth.ts (TypeScript)

Code before:
import crypto from 'crypto';
import type { Modules } from '@strapi/types';

export const REFRESH_COOKIE_NAME = 'strapi_admin_refresh';

const DEFAULT_ACCESS_TOKEN_LIFESPAN = 10 * 60;
const DEFAULT_MAX_REFRESH_TOKEN_LIFESPAN = 30 * 24 * 60 * 60;
const DEFAULT_IDLE_REFRESH_TOKEN_LIFESPAN = 14 * 24 * 60 * 60;
const DEFAULT_MAX_SESSION_LIFESPAN = 1 * 24 * 60 * 60;
const DEFAULT_IDLE_SESSION_LIFESPAN = 2 * 60 * 60;

export const getRefreshCookieOptions = () => {
  const isProduction = strapi.config.get('environment') === 'production';
  const domain: string | undefined =
    strapi.config.get('admin.auth.cookie.domain') || strapi.config.get('admin.auth.domain');
  const path: string = strapi.config.get('admin.auth.cookie.path', '/admin');

  const sameSite: boolean | 'lax' | 'strict' | 'none' =
    strapi.config.get('admin.auth.cookie.sameSite') ?? 'lax';

  return {
    httpOnly: true,
    secure: isProduction,
    overwrite: true,
    domain,
    path,
    sameSite,
    maxAge: undefined,
  };
};


Review comment (suggestion):
This may have revealed an existing issue; DEFAULT_ACCESS_TOKEN_LIFESPAN is not used.

But we may also still want to export these, in case someone needs access to them from outside. It might be useful to know the defaults.

Code after:
import crypto from 'crypto';
import type { Modules } from '@strapi/types';

export const REFRESH_COOKIE_NAME = 'strapi_admin_refresh';

export const DEFAULT_MAX_REFRESH_TOKEN_LIFESPAN = 30 * 24 * 60 * 60;
export const DEFAULT_IDLE_REFRESH_TOKEN_LIFESPAN = 14 * 24 * 60 * 60;
export const DEFAULT_MAX_SESSION_LIFESPAN = 1 * 24 * 60 * 60;
export const DEFAULT_IDLE_SESSION_LIFESPAN = 2 * 60 * 60;

export const getRefreshCookieOptions = () => {
  const isProduction = strapi.config.get('environment') === 'production';
  const domain: string | undefined =
    strapi.config.get('admin.auth.cookie.domain') || strapi.config.get('admin.auth.domain');
  const path: string = strapi.config.get('admin.auth.cookie.path', '/admin');

  const sameSite: boolean | 'lax' | 'strict' | 'none' =
    strapi.config.get('admin.auth.cookie.sameSite') ?? 'lax';

  return {
    httpOnly: true,
    secure: isProduction,
    overwrite: true,
    domain,
    path,
    sameSite,
    maxAge: undefined,
  };
};

const getLifespansForType = (
Repository: strapi/strapi
File: packages/core/admin/admin/src/components/GuidedTour/Context.tsx (TypeScript)

Code before:
      type: 'go_to_step';
      payload: {
        tourName: ValidTourName;
        step: number;
      };
    }
  | {
      type: 'skip_tour';
      payload: ValidTourName;
    }
  | {
      type: 'skip_all_tours';
    }
  | {
      type: 'reset_all_tours';
    }
  | {
      type: 'set_completed_actions';
      payload: CompletedActions;
    }
  | {
      type: 'remove_completed_action';
      payload: ValueOf<CompletedActions>;
    }
  | {
      type: 'set_tour_type';
      payload: {
        tourName: ValidTourName;
        tourType: string;
      };
    };

type TourState = Record<
  ValidTourName,
  { currentStep: number; isCompleted: boolean; tourType?: string }
>;
type State = {
  tours: TourState;
  enabled: boolean;
  completedActions: CompletedActions;
};

const [GuidedTourProviderImpl, useGuidedTour] = createContext<{
  state: State;
  dispatch: React.Dispatch<Action>;
}>('GuidedTour');

const getInitialTourState = (tours: Tours) => {
  return Object.keys(tours).reduce((acc, tourName) => {
    acc[tourName as ValidTourName] = {
      currentStep: 0,

Review comment (suggestion):
I needed to add this in order to be able to check if the user has enabled the AI feature after starting the tour to reset it.

Code after:
      type: 'go_to_step';
      payload: {
        tourName: ValidTourName;
        step: number;
      };
    }
  | {
      type: 'skip_tour';
      payload: ValidTourName;
    }
  | {
      type: 'skip_all_tours';
    }
  | {
      type: 'reset_all_tours';
    }
  | {
      type: 'set_completed_actions';
      payload: CompletedActions;
    }
  | {
      type: 'remove_completed_action';
      payload: ValueOf<CompletedActions>;
    }
  | {
      type: 'set_tour_type';
      payload: {
        tourName: ValidTourName;
        tourType: 'ContentTypeBuilderAI' | 'ContentTypeBuilderNoAI';
      };
    };

type TourState = Record<
  ValidTourName,
  { currentStep: number; isCompleted: boolean; tourType?: string }
>;
type State = {
  tours: TourState;
  enabled: boolean;
  completedActions: CompletedActions;
};

const [GuidedTourProviderImpl, useGuidedTour] = createContext<{
  state: State;
  dispatch: React.Dispatch<Action>;
}>('GuidedTour');

const getInitialTourState = (tours: Tours) => {
  return Object.keys(tours).reduce((acc, tourName) => {
    acc[tourName as ValidTourName] = {
      currentStep: 0,
Repository: strapi/strapi
File: packages/core/upload/admin/src/pages/SettingsPage/reducer.ts (TypeScript)

Code before:
import { produce } from 'immer';
import set from 'lodash/set';

export type InitialState = {
  initialData: {
    responsiveDimensions?: boolean;
    sizeOptimization?: boolean;
    autoOrientation?: boolean;
    videoPreview?: boolean;
    aiMetadata?: boolean;
  } | null;
  modifiedData: {
    responsiveDimensions?: boolean;
    sizeOptimization?: boolean;
    autoOrientation?: boolean;
    videoPreview?: boolean;
    aiMetadata?: boolean;
  } | null;
};

interface ActionGetDataSucceeded {
  type: 'GET_DATA_SUCCEEDED';
  data: InitialState['initialData'];
}

interface ActionOnChange {
  type: 'ON_CHANGE';
  keys: keyof NonNullable<InitialState['initialData']>;
  value: boolean;
}

export type Action = ActionGetDataSucceeded | ActionOnChange;

const initialState: InitialState = {
  initialData: {
    responsiveDimensions: true,
    sizeOptimization: true,
    autoOrientation: false,
    videoPreview: false,
    aiMetadata: true,
  },
  modifiedData: {
    responsiveDimensions: true,

Review comment (refactor):
Not related to your change, but I think it would be nice to extract a type for the object and reuse it for both initialData and modifiedData

Code after:
import { produce } from 'immer';
import set from 'lodash/set';

import { SettingsData } from '../../../../shared/contracts/settings';

const initialData: SettingsData = {
  responsiveDimensions: true,
  sizeOptimization: true,
  autoOrientation: false,
  videoPreview: false,
  aiMetadata: true,
};

export type InitialState = {
  initialData: SettingsData | null;
  modifiedData: SettingsData | null;
};

interface ActionGetDataSucceeded {
  type: 'GET_DATA_SUCCEEDED';
  data: InitialState['initialData'];
}

interface ActionOnChange {
  type: 'ON_CHANGE';
  keys: keyof NonNullable<InitialState['initialData']>;
  value: boolean;
}

export type Action = ActionGetDataSucceeded | ActionOnChange;

const initialState: InitialState = {
  initialData,
  modifiedData: { ...initialData },
};

const reducer = (state: InitialState, action: Action) =>
  produce(state, (drafState) => {
    switch (action.type) {
      case 'GET_DATA_SUCCEEDED': {
        drafState.initialData = action.data;
        drafState.modifiedData = action.data;
        break;
Repository: strapi/strapi
File: packages/plugins/i18n/server/src/services/settings.ts (TypeScript)

Code before:
import type { Settings } from '../controllers/validation/settings';

const settingsService = () => {
  async function getSettings() {
    const res = await strapi.store!({ type: 'plugin', name: 'i18n', key: 'settings' }).get({});

    return res as Settings | null;
  }

  function setSettings(value: Settings) {
    return strapi.store!({ type: 'plugin', name: 'i18n', key: 'settings' }).set({ value });
  }

  return {
    getSettings,
    setSettings,
  };
};

export default settingsService;

export type SettingsService = typeof settingsService;


Review comment (suggestion):
I recommend you use the createSettingsService syntax (like in ai-localizations for example) instead, this way you have access to the strapi without relying on the global

Code after:
import type { Core } from '@strapi/types';
import type { Settings } from '../validation/settings';

const createSettingsService = ({ strapi }: { strapi: Core.Strapi }) => {
  const settings = strapi.store!({ type: 'plugin', name: 'i18n', key: 'settings' });

  async function getSettings() {
    const res = (await settings.get({})) as Settings | null;

    return res;
  }

  function setSettings(value: Settings) {
    return settings.set({ value });
  }

  return {
    getSettings,
    setSettings,
  };
};

export { createSettingsService };
export type SettingsService = ReturnType<typeof createSettingsService>;
Repository: strapi/strapi
File: packages/plugins/i18n/server/src/services/settings.ts (TypeScript)

Code before:
import type { Settings } from '../controllers/validation/settings';

const settingsService = () => {
  async function getSettings() {
    const res = await strapi.store!({ type: 'plugin', name: 'i18n', key: 'settings' }).get({});

    return res as Settings | null;
  }

  function setSettings(value: Settings) {
    return strapi.store!({ type: 'plugin', name: 'i18n', key: 'settings' }).set({ value });
  }

  return {
    getSettings,
    setSettings,
  };
};

export default settingsService;

export type SettingsService = typeof settingsService;


Review comment (suggestion):
could it make sense to parse using the schema here? this way we get the type without casting

Code after:
import type { Core } from '@strapi/types';
import type { Settings } from '../validation/settings';

const createSettingsService = ({ strapi }: { strapi: Core.Strapi }) => {
  const settings = strapi.store!({ type: 'plugin', name: 'i18n', key: 'settings' });

  async function getSettings() {
    const res = (await settings.get({})) as Settings | null;

    return res;
  }

  function setSettings(value: Settings) {
    return settings.set({ value });
  }

  return {
    getSettings,
    setSettings,
  };
};

export { createSettingsService };
export type SettingsService = ReturnType<typeof createSettingsService>;
Repository: strapi/strapi
File: packages/plugins/i18n/server/src/services/settings.ts (TypeScript)

Code before:
import type { Settings } from '../controllers/validation/settings';

const settingsService = () => {
  async function getSettings() {
    const res = await strapi.store!({ type: 'plugin', name: 'i18n', key: 'settings' }).get({});

    return res as Settings | null;
  }

  function setSettings(value: Settings) {
    return strapi.store!({ type: 'plugin', name: 'i18n', key: 'settings' }).set({ value });
  }

  return {
    getSettings,
    setSettings,
  };
};

export default settingsService;

export type SettingsService = typeof settingsService;


Review comment (refactor):
i recommend you to extract a mini-orm outside the methods:

```
const settingsStore = strapi.store!({ type: 'plugin', name: 'i18n', key: 'settings' })
``` 

then you can use it in both getSettings and setSettings

Code after:
import type { Core } from '@strapi/types';
import type { Settings } from '../validation/settings';

const createSettingsService = ({ strapi }: { strapi: Core.Strapi }) => {
  const settings = strapi.store!({ type: 'plugin', name: 'i18n', key: 'settings' });

  async function getSettings() {
    const res = (await settings.get({})) as Settings | null;

    return res;
  }

  function setSettings(value: Settings) {
    return settings.set({ value });
  }

  return {
    getSettings,
    setSettings,
  };
};

export { createSettingsService };
export type SettingsService = ReturnType<typeof createSettingsService>;
Repository: strapi/strapi
File: packages/plugins/i18n/server/src/services/settings.ts (TypeScript)

Code before:
import type { Core } from '@strapi/types';
import type { Settings } from '../validation/settings';
import validateSettings from '../validation/settings';

const createSettingsService = ({ strapi }: { strapi: Core.Strapi }) => {
  const settings = strapi.store!({ type: 'plugin', name: 'i18n', key: 'settings' });

  async function getSettings() {
    const res = await settings.get({});

    return validateSettings(res);
  }

  function setSettings(value: Settings) {
    return settings.set({ value });
  }

  return {
    getSettings,
    setSettings,
  };
};

export { createSettingsService };
export type SettingsService = ReturnType<typeof createSettingsService>;


Review comment (suggestion):
I still get the error I shared earlier when I add a strapi license but don't have the setting in my db yet.

I think the issue is that the getSettings should also allow returning null, but with the validateSettings will throw an error

If it did handle null we probably wouldn't need to skip the query on the frontend when !isAiEnabled (the settings endpoint could host non-ai things in the future so it would be best not to skip it)

Code after:
import type { Core } from '@strapi/types';
import type { Settings } from '../validation/settings';

const createSettingsService = ({ strapi }: { strapi: Core.Strapi }) => {
  const settings = strapi.store!({ type: 'plugin', name: 'i18n', key: 'settings' });

  async function getSettings() {
    const res = (await settings.get({})) as Settings | null;

    return res;
  }

  function setSettings(value: Settings) {
    return settings.set({ value });
  }

  return {
    getSettings,
    setSettings,
  };
};

export { createSettingsService };
export type SettingsService = ReturnType<typeof createSettingsService>;
Repository: strapi/strapi
File: packages/plugins/i18n/admin/src/components/CMHeaderActions.tsx (TypeScript)

Code before:
): 'draft' | 'published' | 'modified' => {
  const docStatus = document?.status;
  const statuses = meta?.availableStatus ?? [];

  /**
   * Creating an entry
   */
  if (!docStatus) {
    return 'draft';
  }

  /**
   * We're viewing a draft, but the document could have a published version
   */
  if (docStatus === 'draft' && statuses.find((doc) => doc.publishedAt !== null)) {
    return 'published';
  }

  return docStatus;
};

/* -------------------------------------------------------------------------------------------------
 * AISettingsStatusAction
 * -----------------------------------------------------------------------------------------------*/

const AITanslationStatusIcon = styled(Status)<{ isAISettingEnabled: boolean }>`
  display: flex;
  gap: ${({ theme }) => theme.spaces[1]};
  justify-content: center;
  align-items: center;
  height: 100%;

  // Disabled state
  background-color: ${({ isAISettingEnabled, theme }) =>
    !isAISettingEnabled && theme.colors.neutral150};

  svg {
    fill: ${({ isAISettingEnabled, theme }) => !isAISettingEnabled && theme.colors.neutral300};
  }
`;

const AITranslationStatusAction = () => {
  const { formatMessage } = useIntl();
  const isAIAvailable = useAIAvailability();
  const { data: settings } = useGetSettingsQuery();
  const isAISettingEnabled = settings?.data?.aiLocalizations;
  const { hasI18n } = useI18n();

  // Do not display this action when i18n is not available
  if (!hasI18n) {
    return null;

Review comment (suggestion):
please use the $ prefix on the prop, it's to ensure the prop doesn't end up on the dom element as an attribute

Code after:
): 'draft' | 'published' | 'modified' => {
  const docStatus = document?.status;
  const statuses = meta?.availableStatus ?? [];

  /**
   * Creating an entry
   */
  if (!docStatus) {
    return 'draft';
  }

  /**
   * We're viewing a draft, but the document could have a published version
   */
  if (docStatus === 'draft' && statuses.find((doc) => doc.publishedAt !== null)) {
    return 'published';
  }

  return docStatus;
};

/* -------------------------------------------------------------------------------------------------
 * AISettingsStatusAction
 * -----------------------------------------------------------------------------------------------*/

const AITranslationStatusIcon = styled(Status)<{ $isAISettingEnabled: boolean }>`
  display: flex;
  gap: ${({ theme }) => theme.spaces[1]};
  justify-content: center;
  align-items: center;
  height: 100%;

  // Disabled state
  ${({ $isAISettingEnabled, theme }) =>
    !$isAISettingEnabled &&
    `
    background-color: ${theme.colors.neutral150};
  `}

  svg {
    ${({ $isAISettingEnabled, theme }) =>
      !$isAISettingEnabled &&
      `
        fill: ${theme.colors.neutral300};
      `}
  }
`;

const AITranslationStatusAction = () => {
  const { formatMessage } = useIntl();
  const isAIAvailable = useAIAvailability();
Repository: strapi/strapi
File: packages/plugins/i18n/server/src/bootstrap.ts (TypeScript)

Code before:
import type { Schema } from '@strapi/types';
import { isEqual } from 'lodash';
import { getService } from './utils';

const registerModelsHooks = () => {
  strapi.db.lifecycles.subscribe({
    models: ['plugin::i18n.locale'],

    async afterCreate() {
      await getService('permissions').actions.syncSuperAdminPermissionsWithLocales();
    },

    async afterDelete() {
      await getService('permissions').actions.syncSuperAdminPermissionsWithLocales();
    },
  });

  strapi.documents.use(async (context, next) => {
    const schema: Schema.ContentType = context.contentType;

    if (!['create', 'update', 'discardDraft', 'publish'].includes(context.action)) {
      return next();
    }

    if (!getService('content-types').isLocalizedContentType(schema)) {
      return next();
    }

Review comment (suggestion):
Could you use the export from `lodash/fp` I think we try to favor the functional programming lodash. Heads up the function signature could be different. 

Code after:
import type { Schema } from '@strapi/types';
import { isEqual } from 'lodash/fp';
import { getService } from './utils';

const registerModelsHooks = () => {
  strapi.db.lifecycles.subscribe({
    models: ['plugin::i18n.locale'],

    async afterCreate() {
      await getService('permissions').actions.syncSuperAdminPermissionsWithLocales();
    },

    async afterDelete() {
      await getService('permissions').actions.syncSuperAdminPermissionsWithLocales();
    },
  });

  strapi.documents.use(async (context, next) => {
    const schema: Schema.ContentType = context.contentType;

    if (!['create', 'update', 'discardDraft', 'publish'].includes(context.action)) {
      return next();
    }

    if (!getService('content-types').isLocalizedContentType(schema)) {
      return next();
    }
Repository: strapi/strapi
File: packages/plugins/i18n/admin/src/components/CMHeaderActions.tsx (TypeScript)

Code before:
 * -----------------------------------------------------------------------------------------------*/

const AITranslationStatusIcon = styled(Status)<{ $isAISettingEnabled: boolean }>`
  display: flex;
  gap: ${({ theme }) => theme.spaces[1]};
  justify-content: center;
  align-items: center;
  height: 100%;

  // Disabled state
  ${({ $isAISettingEnabled, theme }) =>
    !$isAISettingEnabled &&
    `
    background-color: ${theme.colors.neutral150};
  `}

  svg {
    ${({ $isAISettingEnabled, theme }) =>
      !$isAISettingEnabled &&
      `
        fill: ${theme.colors.neutral300};
      `}
  }
`;

const AITranslationStatusAction = ({ documentId, model, collectionType }: HeaderActionProps) => {
  const { formatMessage } = useIntl();
  const isAIAvailable = useAIAvailability();
  const { data: settings } = useGetSettingsQuery();
  const isAISettingEnabled = settings?.data?.aiLocalizations;
  const { hasI18n } = useI18n();

  // Poll for AI localizations jobs when AI is enabled and we have a documentId
  const { status } = useAILocalizationJobsPolling({
    documentId,
    model,
    collectionType,
  });
  const statusVariant = (() => {
    if (status === 'failed') {
      return 'warning';
    }

    if (isAISettingEnabled) {
      return 'alternative';
    }

    return 'neutral';
  })();

  // Do not display this action when i18n is not available

Review comment (suggestion):
Instead of creating this Spinning animation here, couldn't the Loader from the DS be used? It's not quite the same design, but the small version should fix inside the Box ok
(That is the `Loader from '@strapi/design-system'` which already has the animation instead of `Loader from '@strapi/icons'`)

Code after:
 * -----------------------------------------------------------------------------------------------*/

const AITranslationStatusIcon = styled(Status)<{ $isAISettingEnabled: boolean }>`
  display: flex;
  gap: ${({ theme }) => theme.spaces[1]};
  justify-content: center;
  align-items: center;
  height: 100%;

  // Disabled state
  ${({ $isAISettingEnabled, theme }) =>
    !$isAISettingEnabled &&
    `
    background-color: ${theme.colors.neutral150};
  `}

  svg {
    ${({ $isAISettingEnabled, theme }) =>
      !$isAISettingEnabled &&
      `
        fill: ${theme.colors.neutral300};
      `}
  }
`;

const SpinningLoader = styled(Loader)`
  @keyframes spin {
    from {
      transform: rotate(0deg);
    }
    to {
      transform: rotate(360deg);
    }
  }

  animation: spin 2s linear infinite;
`;

const AITranslationStatusAction = ({ documentId, model, collectionType }: HeaderActionProps) => {
  const { formatMessage } = useIntl();
  const isAIAvailable = useAIAvailability();
  const { data: settings } = useGetSettingsQuery();
  const isAISettingEnabled = settings?.data?.aiLocalizations;
  const { hasI18n } = useI18n();

  // Poll for AI localizations jobs when AI is enabled and we have a documentId
  const { status } = useAILocalizationJobsPolling({
    documentId,
    model,
    collectionType,
  });