Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pageversionとsourceTextsの関係を修正 pageversionが違ってもnumberと内容が同じなら同一と判定 ジョブのバージョンシステムの導入 #143

Merged
merged 2 commits into from
Aug 3, 2024

Conversation

ttizze
Copy link
Owner

@ttizze ttizze commented Aug 3, 2024

Closes #136

Summary by CodeRabbit

## リリースノート

### New Feature
- 新しい関数 `getOrCreateSourceTextIdAndPageVersionSourceText` を追加し、テキストのハッシュ生成とデータベース操作を統合。

### Refactor
- 変数名 `sourceTextInfoWithTranslations` を `sourceTextWithTranslations` に変更。
- 関数 `getOrCreateSourceTextId` を削除し、新しい関数に置き換え。
- クエリロジックを `pageVersionSourceTexts` にリネームし、構造を変更。
- キュー管理にバージョン管理を導入。

### Chore
- 不要な定数 `AI_MODEL` を削除。

Copy link

github-actions bot commented Aug 3, 2024

This PR is linked to issue #136

Copy link

github-actions bot commented Aug 3, 2024

Image description CodeRabbit

- **ウォークスルー**: この変更は、テキストのハッシュ生成とデータベース操作を行う新しい関数の追加、および関連する関数や変数名のリファクタリングに焦点を当てています。また、キューのバージョン管理と新しいテーブルの作成も含まれています。

- **変更点**:
  | ファイル | 要約 |
  | --- | --- |
  | web/app/feature/translate/functions/mutations.server.ts | 新しい関数 `getOrCreateSourceTextIdAndPageVersionSourceText` が追加されました。 |
  | web/app/feature/translate/libs/translationUtils.ts | 関数呼び出しが `getOrCreateSourceTextIdAndPageVersionSourceText` に置き換えられました。 |
  | web/app/libs/sourceTextService.ts | `getOrCreateSourceTextId` 関数が削除されました。 |
  | web/app/routes/reader.$/components/ContentWithTranslations.tsx, web/app/routes/reader.$/route.tsx | 変数名 `sourceTextInfoWithTranslations``sourceTextWithTranslations` に変更されました。 |
  | web/app/routes/reader.$/functions/mutations.server.ts | `handleAddTranslationAction` 関数内で `pageId` パラメータが削除されました。 |
  | web/app/routes/reader.$/functions/queries.server.ts | クエリロジックが変更され、新しいデータ構造に基づいてマッピングが行われました。 |
  | web/app/routes/reader.$/types.ts | インターフェース名が `SourceTextWithTranslations` にリネームされました。 |
  | web/app/feature/translate/translate-user-queue.ts | `getTranslateUserQueue` 関数のシグネチャが変更されました。 |
  | web/app/routes/translate/constants.ts | `AI_MODEL` 定数が削除されました。 |
  | web/app/utils/queue.server.ts | キューのバージョン管理が導入されました。 |
  | web/prisma/migrations/20240803024214_/migration.sql | 新しいテーブル `page_version_source_texts` が作成されました。 |

Uplevel your code reviews with CodeRabbit Pro

CodeRabbit Pro

If you like this project, please support us by purchasing the Pro version. The Pro version has advanced context, superior noise reduction and several proprietary improvements compared to the open source version. Moreover, CodeRabbit Pro is free for open source projects.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image description CodeRabbit

Commits Files that changed from the base of the PR and between 89c2583 and ab9a376 commits.
Files selected (16)
  • web/app/feature/translate/functions/mutations.server.ts (1)
  • web/app/feature/translate/libs/translationUtils.ts (3)
  • web/app/feature/translate/translate-user-queue.ts (1)
  • web/app/libs/sourceTextService.ts (1)
  • web/app/routes/reader.$/components/ContentWithTranslations.tsx (3)
  • web/app/routes/reader.$/functions/mutations.server.ts (2)
  • web/app/routes/reader.$/functions/queries.server.ts (3)
  • web/app/routes/reader.$/route.tsx (1)
  • web/app/routes/reader.$/types.ts (2)
  • web/app/routes/translate/constants.ts (1)
  • web/app/utils/queue.server.ts (2)
  • web/prisma/migrations/20240803024214_/migration.sql (1)
  • web/prisma/migrations/20240803025830_/migration.sql (1)
  • web/prisma/migrations/20240803031103_/migration.sql (1)
  • web/prisma/migrations/20240803031817_/migration.sql (1)
  • web/prisma/schema.prisma (3)
Files skipped from review due to trivial changes (1)
  • web/app/routes/reader.$/types.ts
Review comments generated (48)
  • Review: 12
  • LGTM: 36

Tips

Chat with Image description CodeRabbit Bot (@coderabbitai)

  • Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
  • Invite the bot into a review comment chain by tagging @coderabbitai in a reply.

Code suggestions

  • The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
  • You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.

Pausing incremental reviews

  • Add @coderabbitai: ignore anywhere in the PR description to pause further reviews from the bot.

Comment on lines 53 to 55
const sourceText = await prisma.sourceText.findUnique({
where: { id: sourceTextId },
include: { page: true },
});
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image description CodeRabbit

include: { page: true } が削除されていますが、これにより sourceText.page.id にアクセスできなくなります。もし pageId が不要であれば問題ありませんが、必要であれば再度追加する必要があります。

+		include: { page: true },

Comment on lines 1 to 2
export const REDIS_URL = process.env.REDIS_URL || "redis://localhost:6379";
export const MAX_CHUNK_SIZE = 20000;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image description CodeRabbit

Note: This review was outside of the patch, but no patch was found that overlapped with it. Original lines [3-19]

エラー処理が不足しています。トランザクション内でエラーが発生した場合に備えて、try-catchブロックを追加することをお勧めします。

+ try {
    const result = await prisma.$transaction(async (prisma) => {
      const sourceText = await prisma.sourceText.upsert({
        where: { hash: textHash },
        update: {},
        create: { text, hash: textHash },
      });

      await prisma.pageVersionSourceText.upsert({
        where: { sourceTextId_pageVersionId: { sourceTextId: sourceText.id, pageVersionId } },
        update: {},
        create: { sourceTextId: sourceText.id, pageVersionId },
      });

      return sourceText.id;
    });

    return result;
+ } catch (error) {
+   console.error("Error in getOrCreateSourceTextIdAndPageVersionSourceText:", error);
+   throw new Error("Failed to get or create source text and page version source text");
+ }

@ttizze ttizze changed the title Ttizze/feat-issue-136 pageversionとsourceTextsの関係を修正 pageversionが違ってもnumberと内容が同じなら同一と判定 ジョブのバージョンシステムの導入 Aug 3, 2024
@ttizze ttizze merged commit e28dc49 into main Aug 3, 2024
6 checks passed
@ttizze ttizze deleted the ttizze/feat-issue-136 branch August 3, 2024 06:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

別URLからの同一テクスト翻訳をどう突合するか
1 participant