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

Feature/translateedit #27

Merged
merged 2 commits into from
Jul 15, 2024
Merged

Feature/translateedit #27

merged 2 commits into from
Jul 15, 2024

Conversation

ttizze
Copy link
Owner

@ttizze ttizze commented Jul 15, 2024

Summary by CodeRabbit

## リリースノート

### New Feature
- 言語選択機能を`Header`コンポーネントに追加
- `Textarea`コンポーネントの追加
- 翻訳表示と投票機能を持つ`Translation`コンポーネントの追加
- 投票ボタンを持つ`VoteButtons`コンポーネントの追加
- クリック外検知用カスタムフック`useClickOutside`の追加

### Refactor
- ユーザーの言語設定をセッションに保存し、HTMLのlang属性と`Header`コンポーネントに反映
- `TranslatedList`コンポーネントのデータ取得ロジックの更新
- `TranslatedContent`コンポーネントに新しいコールバック関数と`userId`を追加

### Chore
- `URLTranslationForm`から`targetLanguage`選択機能を削除
- 読者ページの`loader`関数と`action`関数の更新

Copy link

github-actions bot commented Jul 15, 2024

Image description CodeRabbit

## ウォークスルー
この変更は、言語選択機能の追加、翻訳と投票機能の強化、および新しいカスタムフックの導入を含みます。これにより、ユーザーは多言語対応のインターフェースで翻訳を管理し、評価できるようになります。

## 変更点
| ファイル | 要約 |
| --- | --- |
| web/app/components/Header.tsx | `Header`コンポーネントに言語選択機能を追加 |
| web/app/root.tsx | ユーザーの言語設定をセッションに保存し、HTMLのlang属性とHeaderコンポーネントに反映 |
| web/app/components/ui/textarea.tsx | 新しい`Textarea`コンポーネントを追加 |
| web/app/routes/_index/components/URLTranslationForm.tsx | `targetLanguage`の選択機能を削除 |
| web/app/routes/_index/components/translatedList.tsx | `TranslatedList`コンポーネントを更新し、言語情報を表示 |
| web/app/routes/reader.$encodedUrl/components/TranslatedContent.tsx | `TranslatedContent`コンポーネントに新しいコールバック関数と`userId`を追加 |
| web/app/routes/reader.$encodedUrl/components/Translation.tsx | 翻訳の表示と投票機能を持つ`Translation`コンポーネントを追加 |
| web/app/routes/reader.$encodedUrl/components/VoteButtons.tsx | `VoteButtons`コンポーネントを追加 |
| web/app/routes/reader.$encodedUrl/hooks/useClickOutside.ts | `useClickOutside`カスタムフックを追加 |
| web/app/routes/reader.$encodedUrl/route.tsx | `loader`関数と`action`関数を追加し、投票と翻訳追加のハンドラを実装 |

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.

@ttizze ttizze merged commit 8a01789 into main Jul 15, 2024
2 checks passed
@ttizze ttizze deleted the feature/translateedit branch July 15, 2024 13:16
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 34098f5 and 62c88bf commits.
Files selected (21)
  • web/app/components/Header.tsx (1)
  • web/app/components/ui/textarea.tsx (1)
  • web/app/root.tsx (1)
  • web/app/routes/_index/components/URLTranslationForm.tsx (4)
  • web/app/routes/_index/components/translatedList.tsx (1)
  • web/app/routes/reader.$encodedUrl/components/TranslatedContent.tsx (1)
  • web/app/routes/reader.$encodedUrl/components/Translation.tsx (1)
  • web/app/routes/reader.$encodedUrl/components/VoteButtons.tsx (1)
  • web/app/routes/reader.$encodedUrl/hooks/useClickOutside.ts (1)
  • web/app/routes/reader.$encodedUrl/route.tsx (2)
  • web/app/routes/reader.$encodedUrl/types.ts (1)
  • web/app/routes/reader.$encodedUrl/utils.ts (1)
  • web/app/routes/reader.$encodedUrl/utils/actions.ts (1)
  • web/app/routes/reader.$encodedUrl/utils/create.ts (1)
  • web/package.json (1)
  • web/prisma/migrations/20240715072354_/migration.sql (1)
  • web/prisma/migrations/20240715073559_/migration.sql (1)
  • web/prisma/migrations/20240715074033_/migration.sql (1)
  • web/prisma/migrations/20240715083506_/migration.sql (1)
  • web/prisma/migrations/20240715093411_/migration.sql (1)
  • web/prisma/schema.prisma (1)
Files skipped from review due to trivial changes (3)
  • web/package.json
  • web/app/routes/reader.$encodedUrl/components/Translation.tsx (diff too large)
  • web/app/routes/reader.$encodedUrl/route.tsx (diff too large)
Review comments generated (43)
  • Review: 13
  • LGTM: 30

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.

@@ -0,0 +1,24 @@
import * as React from "react";

import { cn } from "~/utils";

Choose a reason for hiding this comment

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

Image description CodeRabbit

cn関数のインポート元が不明確です。~/utilsから正しくインポートされているか確認してください。

- import { cn } from "~/utils";
+ import { cn } from "../path/to/utils"; // 正しいパスに変更してください

select: {
number: true,
translateTexts: {
where: { language: "ja" },

Choose a reason for hiding this comment

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

Image description CodeRabbit

language: "ja"がハードコードされています。動的に言語を指定できるようにするため、関数の引数として受け取るべきです。

- 						where: { language: "ja" },
+ 						where: { language },

select: {
number: true,
translateTexts: {
where: { language: "ja" },

Choose a reason for hiding this comment

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

Image description CodeRabbit

関数の引数にlanguageを追加し、fetchLatestPageVersionWithTranslationsが特定の言語で翻訳を取得できるようにします。

export async function fetchLatestPageVersionWithTranslations(
	url: string,
	userId: number | null,
+ 	language: string,
): Promise<LoaderData | null> {

Comment on lines +3 to +20
export const useClickOutside = (callback: () => void) => {
const ref = useRef<HTMLDivElement>(null);

useEffect(() => {
const handleClickOutside = (event: MouseEvent) => {
if (ref.current && !ref.current.contains(event.target as Node)) {
callback();
}
};

document.addEventListener("mousedown", handleClickOutside);
return () => {
document.removeEventListener("mousedown", handleClickOutside);
};
}, [callback]);

return ref;
};

Choose a reason for hiding this comment

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

Image description CodeRabbit

このカスタムフックは、クリックイベントを監視して指定されたコールバックを実行するためのものですが、useRefの型が固定されているため、他の要素にも対応できるように汎用性を持たせると良いでしょう。また、mousedownイベントだけでなく、touchstartイベントも考慮するとモバイルデバイスでも動作します。

-export const useClickOutside = (callback: () => void) => {
-	const ref = useRef<HTMLDivElement>(null);
+export const useClickOutside = <T extends HTMLElement>(callback: () => void) => {
+	const ref = useRef<T>(null);

	useEffect(() => {
		const handleClickOutside = (event: MouseEvent | TouchEvent) => {
			if (ref.current && !ref.current.contains(event.target as Node)) {
				callback();
			}
		};

		document.addEventListener("mousedown", handleClickOutside);
+		document.addEventListener("touchstart", handleClickOutside);
		return () => {
			document.removeEventListener("mousedown", handleClickOutside);
+			document.removeEventListener("touchstart", handleClickOutside);
		};
	}, [callback]);

	return ref;
};

Comment on lines +12 to +16
const userVoteStatus = translation.userVote
? translation.userVote.isUpvote
? "upvoted"
: "downvoted"
: "not_voted";

Choose a reason for hiding this comment

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

Image description CodeRabbit

userVoteStatusの計算は冗長です。三項演算子を使って簡潔に書くことができます。

-	const userVoteStatus = translation.userVote
-		? translation.userVote.isUpvote
-			? "upvoted"
-			: "downvoted"
-		: "not_voted";
+	const userVoteStatus = translation.userVote ? (translation.userVote.isUpvote ? "upvoted" : "downvoted") : "not_voted";

/*
Warnings:

- You are about to drop the column `point` on the `translate_texts` table. All the data in the column will be lost.

Choose a reason for hiding this comment

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

Image description CodeRabbit

この警告は重要です。point列を削除すると、すべてのデータが失われます。これが意図された変更であることを確認してください。また、必要に応じてバックアップを取るか、データ移行戦略を検討してください。

-  - You are about to drop the column `point` on the `translate_texts` table. All the data in the column will be lost.
+  - You are about to drop the column `point` on the `translate_texts` table. All the data in the column will be lost. Ensure this is intentional and consider backing up or migrating data if necessary.

/*
Warnings:

- Added the required column `updated_at` to the `votes` table without a default value. This is not possible if the table is not empty.

Choose a reason for hiding this comment

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

Image description CodeRabbit

updated_atカラムを追加する際にデフォルト値が設定されていないため、既存のレコードがある場合にエラーが発生します。デフォルト値を設定するか、既存のレコードを更新するマイグレーションを追加してください。

- ALTER TABLE "votes" ADD COLUMN     "updated_at" TIMESTAMP(3) NOT NULL;
+ ALTER TABLE "votes" ADD COLUMN     "updated_at" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP;

Comment on lines +8 to +41
await prisma.$transaction(async (prisma) => {
const existingVote = await prisma.vote.findUnique({
where: {
translateTextId_userId: { translateTextId, userId },
},
});

if (existingVote) {
if (existingVote.isUpvote === isUpvote) {
await prisma.vote.delete({ where: { id: existingVote.id } });
await prisma.translateText.update({
where: { id: translateTextId },
data: { point: { increment: isUpvote ? -1 : 1 } },
});
} else {
await prisma.vote.update({
where: { id: existingVote.id },
data: { isUpvote },
});
await prisma.translateText.update({
where: { id: translateTextId },
data: { point: { increment: isUpvote ? 2 : -2 } },
});
}
} else {
await prisma.vote.create({
data: { userId, translateTextId, isUpvote },
});
await prisma.translateText.update({
where: { id: translateTextId },
data: { point: { increment: isUpvote ? 1 : -1 } },
});
}
});

Choose a reason for hiding this comment

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

Image description CodeRabbit

prisma.$transactionの使用方法に問題があります。現在の実装では、トランザクション内で非同期関数を直接呼び出していますが、これは推奨されていません。代わりに、トランザクションブロック内でprismaオブジェクトを使用する必要があります。

- await prisma.$transaction(async (prisma) => {
+ await prisma.$transaction(async (tx) => {
    const existingVote = await tx.vote.findUnique({
        where: {
            translateTextId_userId: { translateTextId, userId },
        },
    });

    if (existingVote) {
        if (existingVote.isUpvote === isUpvote) {
            await tx.vote.delete({ where: { id: existingVote.id } });
            await tx.translateText.update({
                where: { id: translateTextId },
                data: { point: { increment: isUpvote ? -1 : 1 } },
            });
        } else {
            await tx.vote.update({
                where: { id: existingVote.id },
                data: { isUpvote },
            });
            await tx.translateText.update({
                where: { id: translateTextId },
                data: { point: { increment: isUpvote ? 2 : -2 } },
            });
        }
    } else {
        await tx.vote.create({
            data: { userId, translateTextId, isUpvote },
        });
        await tx.translateText.update({
            where: { id: translateTextId },
            data: { point: { increment: isUpvote ? 1 : -1 } },
        });
    }
});

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

if (sourceText) {
await prisma.translateText.create({
data: {
language: "ja",
text,
sourceTextId,
pageId: sourceText.page.id,
userId,
},
});
}

Choose a reason for hiding this comment

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

Image description CodeRabbit

sourceTextが見つからなかった場合のエラーハンドリングが不足しています。sourceTextnullの場合に適切なレスポンスを返すように修正してください。

if (sourceText) {
    await prisma.translateText.create({
        data: {
            language: "ja",
            text,
            sourceTextId,
            pageId: sourceText.page.id,
            userId,
        },
    });
+ } else {
+     return json({ success: false, message: "Source text not found" }, { status: 404 });
}

Comment on lines +1 to +2
-- AlterTable
ALTER TABLE "translate_texts" ADD COLUMN "point" INTEGER NOT NULL DEFAULT 0;

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, so it was mapped to the patch with the greatest overlap. Original lines [1-27]

useLoaderDataのインポートが不足しています。

+import { useLoaderData } from 'remix';

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.

1 participant