ronantakizawa/codereview-bench huggingface dataset:
Repository: toeverything/AFFiNE
File: packages/backend/server/src/core/notification/job.ts (TypeScript)

Code before:
  @OnJob('notification.sendInvitation')
  async sendInvitation({
    inviterId,
    inviteId,
  }: Jobs['notification.sendInvitation']) {
    const invite = await this.models.workspaceUser.getById(inviteId);
    if (!invite) {
      return;
    }
    await this.service.createInvitation({
      userId: invite.userId,
      body: {
        workspaceId: invite.workspaceId,
        createdByUserId: inviterId,
        inviteId,
      },
    });
  }

  @OnJob('notification.sendInvitationAccepted')
  async sendInvitationAccepted({
    inviterId,
    inviteId,
  }: Jobs['notification.sendInvitationAccepted']) {
    const invite = await this.models.workspace.getMemberInvitation(inviteId);
    if (!invite || invite.status !== WorkspaceMemberStatus.Accepted) {
      return;
    }
    await this.service.createInvitationAccepted({
      userId: inviterId,
      body: {
        workspaceId: invite.workspaceId,
        createdByUserId: invite.userId,
        inviteId,
      },
    });
  }
}


Review comment (suggestion):
`invite.status !== WorkspaceMemberStatus.Accepted` should biz logic on service

Code after:
  @OnJob('notification.sendInvitation')
  async sendInvitation({
    inviterId,
    inviteId,
  }: Jobs['notification.sendInvitation']) {
    const invite = await this.models.workspaceUser.getById(inviteId);
    if (!invite) {
      return;
    }
    await this.service.createInvitation({
      userId: invite.userId,
      body: {
        workspaceId: invite.workspaceId,
        createdByUserId: inviterId,
        inviteId,
      },
    });
  }

  @OnJob('notification.sendInvitationAccepted')
  async sendInvitationAccepted({
    inviterId,
    inviteId,
  }: Jobs['notification.sendInvitationAccepted']) {
    const invite = await this.models.workspaceUser.getById(inviteId);
    if (!invite) {
      return;
    }
    await this.service.createInvitationAccepted({
      userId: inviterId,
      body: {
        workspaceId: invite.workspaceId,
        createdByUserId: invite.userId,
        inviteId,
      },
    });
  }
}
Repository: toeverything/AFFiNE
File: packages/backend/server/src/core/user/resolver.ts (TypeScript)

Code before:

    // TODO(@forehalo): need to limit a user can only get another user witch is in the same workspace
    const user = await this.models.user.getUserByEmail(email);

    // return empty response when user not exists
    if (!user) return null;

    if (currentUser) {
      return sessionUser(user);
    }

    // only return limited info when not logged in
    return {
      email: user.email,
      hasPassword: !!user.password,
    };
  }

  @Throttle('strict')
  @Query(() => [PublicUserType], {
    name: 'usersByIds',
    description: 'Find users by ids',
  })
  @Public()
  async findUsersByIds(
    @Args('ids', { type: () => [String] }) ids: string[]
  ): Promise<PublicUserType[]> {
    return (await this.models.user.getPublicUsers(ids)).map(user =>
      omit(user, 'email')
    );
  }

  @Mutation(() => UserType, {
    name: 'uploadAvatar',
    description: 'Upload user avatar',
  })
  async uploadAvatar(
    @CurrentUser() user: CurrentUser,
    @Args({ name: 'avatar', type: () => GraphQLUpload })
    avatar: FileUpload
  ) {
    if (!avatar.mimetype.startsWith('image/')) {
      throw new Error('Invalid file type');
    }

    if (!user) {
      throw new UserNotFound();
    }

    const avatarUrl = await this.storage.put(
      `${user.id}-avatar-${Date.now()}`,

Review comment (performance):
look weird. maybe called by unlimited ids, to bypass throttler. we could provide a simple by id query. I've checked github apis a lot today, it also only provide user fetching by login, no batching options

Code after:

    // TODO(@forehalo): need to limit a user can only get another user witch is in the same workspace
    const user = await this.models.user.getUserByEmail(email);

    // return empty response when user not exists
    if (!user) return null;

    if (currentUser) {
      return sessionUser(user);
    }

    // only return limited info when not logged in
    return {
      email: user.email,
      hasPassword: !!user.password,
    };
  }

  @Throttle('strict')
  @Query(() => PublicUserType, {
    name: 'publicUserById',
    description: 'Get public user by id',
    nullable: true,
  })
  @Public()
  async getPublicUserById(
    @Args('id', { type: () => String }) id: string
  ): Promise<PublicUserType | null> {
    return await this.models.user.getPublicUser(id);
  }

  @Mutation(() => UserType, {
    name: 'uploadAvatar',
    description: 'Upload user avatar',
  })
  async uploadAvatar(
    @CurrentUser() user: CurrentUser,
    @Args({ name: 'avatar', type: () => GraphQLUpload })
    avatar: FileUpload
  ) {
    if (!avatar.mimetype.startsWith('image/')) {
      throw new Error('Invalid file type');
    }

    if (!user) {
      throw new UserNotFound();
    }

    const avatarUrl = await this.storage.put(
      `${user.id}-avatar-${Date.now()}`,
      avatar.createReadStream(),
Repository: toeverything/AFFiNE
File: packages/backend/server/src/__tests__/user/user.e2e.ts (TypeScript)

Code before:
  t.is(user1!.name, u1.name);
  t.truthy(user1!.avatarUrl);
  let user2 = await getPublicUserById(app, u2.id);
  t.truthy(user2);
  t.is(user2!.id, u2.id);
  t.is(user2!.name, u2.name);
  t.is(user2!.avatarUrl, null);
  let user3 = await getPublicUserById(app, randomUUID());
  t.is(user3, null);

  // anonymous user can access
  await app.logout();
  user1 = await getPublicUserById(app, u1.id);
  t.truthy(user1);
  t.is(user1!.id, u1.id);
  t.is(user1!.name, u1.name);
  t.truthy(user1!.avatarUrl);
  user2 = await getPublicUserById(app, u2.id);
  t.truthy(user2);
  t.is(user2!.id, u2.id);
  t.is(user2!.name, u2.name);
  t.is(user2!.avatarUrl, null);
  user3 = await getPublicUserById(app, randomUUID());
  t.is(user3, null);
});


Review comment (suggestion):
assert no private data leaking, `deepEqual({ id, name, avatar })`

Code after:
  t.is(user1!.name, u1.name);
  t.truthy(user1!.avatarUrl);
  let user2 = await getPublicUserById(app, u2.id);
  t.deepEqual(user2, {
    id: u2.id,
    name: u2.name,
    avatarUrl: null,
  });
  let user3 = await getPublicUserById(app, randomUUID());
  t.is(user3, null);

  // anonymous user can access
  await app.logout();
  user1 = await getPublicUserById(app, u1.id);
  t.truthy(user1);
  t.is(user1!.id, u1.id);
  t.is(user1!.name, u1.name);
  t.truthy(user1!.avatarUrl);
  user2 = await getPublicUserById(app, u2.id);
  t.deepEqual(user2, {
    id: u2.id,
    name: u2.name,
    avatarUrl: null,
  });
  user3 = await getPublicUserById(app, randomUUID());
  t.is(user3, null);
});
Repository: toeverything/AFFiNE
File: packages/backend/server/src/models/doc.ts (TypeScript)

Code before:
      spaceId: row.workspaceId,
      docId: row.id,
      blob: row.blob,
      timestamp: row.updatedAt.getTime(),
      editorId: row.updatedBy || undefined,
    };
  }

  async getMeta(workspaceId: string, docId: string) {
    return await this.db.snapshot.findUnique({
      where: {
        workspaceId_id: {
          workspaceId,
          id: docId,
        },
      },
      select: {
        createdAt: true,
        updatedAt: true,
        createdByUser: this.userSelectFields,
        updatedByUser: this.userSelectFields,
      },
    });
  }

  async updateContent(workspaceId: string, docId: string, content: DocContent) {
    return await this.db.workspaceDoc.upsert({
      where: {
        workspaceId_docId: {
          workspaceId,
          docId,
        },
      },
      update: {
        ...content,
      },
      create: {
        ...content,
        workspaceId,
        docId,
      },
    });
  }

  async getContent(
    workspaceId: string,
    docId: string
  ): Promise<DocContent | null> {
    return await this.db.workspaceDoc.findUnique({
      where: {
        workspaceId_docId: { workspaceId, docId },

Review comment (refactor):
use docMeta https://app.graphite.dev/github/pr/toeverything/AFFiNE/10592/refactor(server)-merge-PageModel-into-DocModel

Code after:
  }

  async setDefaultRole(workspaceId: string, docId: string, role: DocRole) {
    return await this.upsertMeta(workspaceId, docId, {
      defaultRole: role,
    });
  }

  async findMetas(ids: { workspaceId: string; docId: string }[]) {
    const rows = await this.db.workspaceDoc.findMany({
      where: {
        workspaceId: { in: ids.map(id => id.workspaceId) },
        docId: { in: ids.map(id => id.docId) },
      },
    });
    const resultMap = new Map(
      rows.map(row => [`${row.workspaceId}-${row.docId}`, row])
    );
    return ids.map(
      id => resultMap.get(`${id.workspaceId}-${id.docId}`) ?? null
    );
  }

  /**
   * Find the workspace public doc metas.
   */
  async findPublics(workspaceId: string) {
    return await this.db.workspaceDoc.findMany({
      where: {
        workspaceId,
        public: true,
      },
    });
  }

  /**
   * Get the workspace public docs count.
   */
  async getPublicsCount(workspaceId: string) {
    return await this.db.workspaceDoc.count({
      where: {
        workspaceId,
        public: true,
      },
    });
  }

  /**
   * Check if the workspace has any public docs.
   */
  async hasPublic(workspaceId: string) {
Repository: toeverything/AFFiNE
File: packages/backend/server/src/base/helpers/index.ts (TypeScript)

Code before:
import './config';

import { Global, Module } from '@nestjs/common';

import { CryptoHelper } from './crypto';
import { URLHelper } from './url';
import { WorkspaceHelper } from './workspace';

@Global()
@Module({
  providers: [URLHelper, CryptoHelper, WorkspaceHelper],
  exports: [URLHelper, CryptoHelper, WorkspaceHelper],
})
export class HelpersModule {}

export { CryptoHelper, URLHelper, WorkspaceHelper };


Review comment (suggestion):
base modules should not get invoived with business logic. put it in `WorkspaceBlobStorage`

Code after:
import './config';

import { Global, Module } from '@nestjs/common';

import { CryptoHelper } from './crypto';
import { URLHelper } from './url';

@Global()
@Module({
  providers: [URLHelper, CryptoHelper],
  exports: [URLHelper, CryptoHelper],
})
export class HelpersModule {}

export { CryptoHelper, URLHelper };
Repository: toeverything/AFFiNE
File: packages/frontend/core/src/components/affine/auth/oauth.tsx (TypeScript)

Code before:
function OAuthProvider({
  provider,
  redirectUrl,
  scheme,
  popupWindow,
}: {
  provider: OAuthProviderType;
  redirectUrl?: string;
  scheme?: string;
  popupWindow: (url: string) => void;
}) {
  const serverService = useService(ServerService);
  const auth = useService(AuthService);
  const { icon } = OAuthProviderMap[provider];

  const onClick = useCallback(() => {
    if (
      scheme &&
      (BUILD_CONFIG.isElectron || BUILD_CONFIG.isIOS || BUILD_CONFIG.isAndroid)
    ) {
      auth
        .oauthPreflight(provider, scheme)
        .then(oauthUrl => {
          popupWindow(oauthUrl);
        })
        .catch(e => {
          throw e;
        });
      return;
    }

    const params = new URLSearchParams();

    params.set('provider', provider);

    if (redirectUrl) {
      params.set('redirect_uri', redirectUrl);
    }

    if (scheme) {
      params.set('client', scheme);
    }

    // TODO: Android app scheme not implemented
    // if (BUILD_CONFIG.isAndroid) {}

    const oauthUrl =
      serverService.server.baseUrl + `/oauth/login?${params.toString()}`;

    track.$.$.auth.signIn({ method: 'oauth', provider });


Review comment (suggestion):
you can turn this into `useAsyncCallback` to let error boundary handle error

Code after:
    />
  ));
}

function OAuthProvider({
  provider,
  redirectUrl,
  scheme,
  popupWindow,
}: {
  provider: OAuthProviderType;
  redirectUrl?: string;
  scheme?: string;
  popupWindow: (url: string) => void;
}) {
  const serverService = useService(ServerService);
  const auth = useService(AuthService);
  const { icon } = OAuthProviderMap[provider];
  const t = useI18n();

  const onClick = useAsyncCallback(async () => {
    if (scheme && BUILD_CONFIG.isNative) {
      let oauthUrl = '';
      try {
        oauthUrl = await auth.oauthPreflight(provider, scheme);
      } catch (e) {
        console.error(e);
        const err = e as UserFriendlyError;
        notify.error({
          title: t[`error.${err.name}`](err.data),
        });
        return;
      }
      popupWindow(oauthUrl);
      return;
    }

    const params = new URLSearchParams();

    params.set('provider', provider);

    if (redirectUrl) {
      params.set('redirect_uri', redirectUrl);
    }

    if (scheme) {
      params.set('client', scheme);
    }

    // TODO: Android app scheme not implemented
    // if (BUILD_CONFIG.isAndroid) {}
Repository: toeverything/AFFiNE
File: packages/backend/server/src/models/copilot-job.ts (TypeScript)

Code before:
@Injectable()
export class CopilotJobModel extends BaseModel {
  async create(job: CreateCopilotJobInput) {
    const row = await this.db.aiJobs.create({
      data: {
        workspaceId: job.workspaceId,
        blobId: job.blobId,
        createdBy: job.createdBy,
        type: job.type,
        status: AiJobStatus.pending,
        payload: {},
      },
      select: {
        id: true,
        status: true,
      },
    });
    return row;
  }

  async has(workspaceId: string, blobId: string, type?: CopilotJobType) {
    const row = await this.db.aiJobs.findFirst({
      where: {
        workspaceId,
        blobId,
        type,
      },
    });
    return !!row;
  }

  async update(jobId: string, data: UpdateCopilotJobInput) {
    const ret = await this.db.aiJobs.updateMany({
      where: {
        id: jobId,
      },
      data: {
        status: data.status || undefined,
        payload: data.payload || undefined,
      },
    });
    return ret.count > 0;
  }

  @Transactional()
  async claim(jobId: string, userId: string) {
    const job = await this.get(jobId);

    if (
      job &&
      job.createdBy === userId &&

Review comment (suggestion):
workspaceId + blobId is the unique key, the type parameter should be redundant

Code after:
@Injectable()
export class CopilotJobModel extends BaseModel {
  async create(job: CreateCopilotJobInput) {
    const row = await this.db.aiJobs.create({
      data: {
        workspaceId: job.workspaceId,
        blobId: job.blobId,
        createdBy: job.createdBy,
        type: job.type,
        status: AiJobStatus.pending,
        payload: {},
      },
      select: {
        id: true,
        status: true,
      },
    });
    return row;
  }

  async has(workspaceId: string, blobId: string) {
    const row = await this.db.aiJobs.findFirst({
      where: {
        workspaceId,
        blobId,
      },
    });
    return !!row;
  }

  async update(jobId: string, data: UpdateCopilotJobInput) {
    const ret = await this.db.aiJobs.updateMany({
      where: {
        id: jobId,
      },
      data: {
        status: data.status || undefined,
        payload: data.payload || undefined,
      },
    });
    return ret.count > 0;
  }

  @Transactional()
  async claim(jobId: string, userId: string) {
    const job = await this.get(jobId);

    if (
      job &&
      job.createdBy === userId &&
      job.status === AiJobStatus.finished
Repository: toeverything/AFFiNE
File: packages/backend/server/src/models/copilot-job.ts (TypeScript)

Code before:
  ) {
    const row = await this.db.aiJobs.findFirst({
      where: {
        id: jobId,
        workspaceId,
        type,
        OR: [
          {
            createdBy: userId,
            status: { in: [AiJobStatus.finished, AiJobStatus.claimed] },
          },
          { createdBy: { not: userId }, status: AiJobStatus.claimed },
        ],
      },
    });

    if (!row) {
      return null;
    }

    return {
      id: row.id,
      workspaceId: row.workspaceId,
      blobId: row.blobId,
      createdBy: row.createdBy || undefined,
      type: row.type as CopilotJobType,
      status: row.status,
      payload: row.payload,
    };
  }

  async get(jobId: string): Promise<CopilotJob | null> {
    const row = await this.db.aiJobs.findFirst({
      where: {
        id: jobId,
      },
    });

    if (!row) {
      return null;
    }

    return {
      id: row.id,
      workspaceId: row.workspaceId,
      blobId: row.blobId,
      createdBy: row.createdBy || undefined,
      type: row.type as CopilotJobType,
      status: row.status,
      payload: row.payload,
    };

Review comment (suggestion):
If you don't use database enum types, then all places that read aijob will need to perform type conversion

Code after:
    const row = await this.db.aiJobs.findFirst({
      where: {
        id: jobId,
        workspaceId,
        type,
        OR: [
          {
            createdBy: userId,
            status: { in: [AiJobStatus.finished, AiJobStatus.claimed] },
          },
          { createdBy: { not: userId }, status: AiJobStatus.claimed },
        ],
      },
    });

    if (!row) {
      return null;
    }

    return {
      id: row.id,
      workspaceId: row.workspaceId,
      blobId: row.blobId,
      createdBy: row.createdBy || undefined,
      type: row.type,
      status: row.status,
      payload: row.payload,
    };
  }

  async get(jobId: string): Promise<CopilotJob | null> {
    const row = await this.db.aiJobs.findFirst({
      where: {
        id: jobId,
      },
    });

    if (!row) {
      return null;
    }

    return {
      id: row.id,
      workspaceId: row.workspaceId,
      blobId: row.blobId,
      createdBy: row.createdBy || undefined,
      type: row.type,
      status: row.status,
      payload: row.payload,
    };
  }
Repository: toeverything/AFFiNE
File: packages/frontend/core/src/modules/cloud/services/auth.ts (TypeScript)

Code before:
  async signOut() {
    await this.store.signOut();
    this.store.setCachedAuthSession(null);
    this.session.revalidate();
  }

  checkUserByEmail(email: string) {
    return this.store.checkUserByEmail(email);
  }

  captchaHeaders(token: string, challenge?: string) {
    const headers: Record<string, string> = {
      'x-captcha-token': token,
    };

    if (challenge) {
      headers['x-captcha-challenge'] = challenge;
    }

    return headers;
  }

  private setClientNonce() {
    if (
      BUILD_CONFIG.isElectron ||
      BUILD_CONFIG.isIOS ||
      BUILD_CONFIG.isAndroid
    ) {
      // send random client nonce on desktop app or mobile app
      this.store.setClientNonce(nanoid());
    }
  }
}


Review comment (suggestion):
@forehalo Here we need to discuss whether to change the clientNonce each time, because if it's different each time, then when the user triggers the magic link login multiple times, only the last magic link received in the email will be valid, and opening the previous ones will be invalid.

Code after:
  async signOut() {
    await this.store.signOut();
    this.store.setCachedAuthSession(null);
    this.session.revalidate();
  }

  checkUserByEmail(email: string) {
    return this.store.checkUserByEmail(email);
  }

  captchaHeaders(token: string, challenge?: string) {
    const headers: Record<string, string> = {
      'x-captcha-token': token,
    };

    if (challenge) {
      headers['x-captcha-challenge'] = challenge;
    }

    return headers;
  }

  private setClientNonce() {
    if (BUILD_CONFIG.isNative) {
      // send random client nonce on native app
      this.store.setClientNonce(nanoid());
    }
  }
}
Repository: toeverything/AFFiNE
File: packages/frontend/core/src/blocksuite/ai/chat-panel/components/utils.ts (TypeScript)

Code before:
    return 'Failed to add to context';
  }
  return name;
}

export function getChipIcon(
  state: ChipState,
  icon: TemplateResult<1>
): TemplateResult<1> {
  const isLoading = state === 'processing';
  const isFailed = state === 'failed';
  if (isFailed) {
    return WarningIcon();
  }
  if (isLoading) {
    return LoadingIcon;
  }
  return icon;
}

export function isDocChip(chip: ChatChip): chip is DocChip {
  return 'docId' in chip;
}

export function isFileChip(chip: ChatChip): chip is FileChip {
  return 'file' in chip;
}

export function isDocContext(
  context: CopilotContextDoc | CopilotContextFile
): context is CopilotContextDoc {
  return !('blobId' in context);
}

export function isFileContext(
  context: CopilotContextDoc | CopilotContextFile
): context is CopilotContextFile {
  return 'blobId' in context;
}

export function getChipKey(chip: ChatChip) {
  if (isDocChip(chip)) {
    return chip.docId;
  }
  if (isFileChip(chip)) {
    return chip.fileId;
  }
  return null;
}

export function estimateTokenCount(text: string): number {

Review comment (suggestion):
```suggestion
  return 'file' in chip && chip.file instanceof File;
```

Code after:
    return 'Failed to add to context';
  }
  return name;
}

export function getChipIcon(
  state: ChipState,
  icon: TemplateResult<1>
): TemplateResult<1> {
  const isLoading = state === 'processing';
  const isFailed = state === 'failed';
  if (isFailed) {
    return WarningIcon();
  }
  if (isLoading) {
    return LoadingIcon;
  }
  return icon;
}

export function isDocChip(chip: ChatChip): chip is DocChip {
  return 'docId' in chip;
}

export function isFileChip(chip: ChatChip): chip is FileChip {
  return 'file' in chip && chip.file instanceof File;
}

export function isDocContext(
  context: CopilotContextDoc | CopilotContextFile
): context is CopilotContextDoc {
  return !('blobId' in context);
}

export function isFileContext(
  context: CopilotContextDoc | CopilotContextFile
): context is CopilotContextFile {
  return 'blobId' in context;
}

export function getChipKey(chip: ChatChip) {
  if (isDocChip(chip)) {
    return chip.docId;
  }
  if (isFileChip(chip)) {
    return chip.fileId;
  }
  return null;
}

export function estimateTokenCount(text: string): number {
Repository: toeverything/AFFiNE
File: packages/backend/server/src/mails/components/doc.tsx (TypeScript)

Code before:
import { Link } from '@react-email/components';

import { Bold } from './template';

export interface DocProps {
  title: string;
  url: string;
}

export const Doc = (props: DocProps) => {
  return (
    <Link href={props.url}>
      <Bold>{props.title || 'Untitled Doc'}</Bold>
    </Link>
  );
};


Review comment (suggestion):
should use title `Untitled` instead of `Untitled Doc` as frontend display 

![image.png](https://graphite-user-uploaded-assets-prod.s3.amazonaws.com/hTwOityLamd4hitrae7M/9c955148-7326-49ef-8e67-7535a120887f.png)



Code after:
import { Link } from '@react-email/components';

import { Bold } from './template';

export interface DocProps {
  title: string;
  url: string;
}

export const Doc = (props: DocProps) => {
  return (
    <Link href={props.url}>
      <Bold>{props.title || 'Untitled'}</Bold>
    </Link>
  );
};