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

ステータス調整のAPI繋ぎ込み&ダイアログの修正 #69

Merged
merged 8 commits into from
Nov 16, 2024

Conversation

hikahana
Copy link
Collaborator

@hikahana hikahana commented Nov 15, 2024

概要

  • このプルリクエストの目的や背景を簡単に説明してください。

変更内容

  • どのような変更を行ったのか具体的に記述してください。
    experiencePointのget,putAPIを修正した。userIdで叩くようにした。
    userでgetを行い、statusChangeDialogでputを叩く。
    atomを修正して、PointDialogが出る問題を解決できたはず。。。
    image

動作確認

  • どのような手順で動作確認を行ったのか記述してください。
    API繋ぎ込みはexperiencePointにtotalPointを増やしてください。
    プレイスタイル設定でsliderを動かし保存するをクリックするとputが走り、リロードが走ります。
    その後、値が変更されていたら成功です。

ダイアログはとりあえず不用意にでないようになったはずです。
ダメだったら教えてください。

関連するIssue

  • 関連するIssue番号を記載してください。例: #123

備考

  • その他、レビュワーに伝えたいことがあれば記述してください。

Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Error Handling
The error handling in the PUT method does not provide sufficient feedback for invalid input types. Consider adding more specific error messages for better debugging.

Input Validation
The previous implementation used search parameters for ID extraction, which can lead to issues if the ID is not provided correctly. Ensure the new method is robust against such cases.

State Management
The state management for remainingPoint could be optimized. Consider using a derived state or memoization to avoid unnecessary calculations on every render.

Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Score
Enhancement
エラーハンドリングを強化し、ユーザーにフィードバックを提供します。

fetchのエラーハンドリングを強化し、ユーザーに適切なフィードバックを提供してください。

app/src/app/user/page.tsx [6]

 if (!response.ok) {
-  throw new Error("データの取得に失敗しました");
+  console.error("データの取得に失敗しました", response.statusText);
+  return; // ユーザーにフィードバックを提供
Suggestion importance[1-10]: 7

Why: The suggestion to enhance error handling by logging the error and providing user feedback is a valuable improvement that increases the robustness of the application. It addresses potential user experience issues effectively.

7
Possible issue
リクエストボディのフィールドを確認し、エラーを防ぎます。

PUTメソッドのリクエストボディに必要なフィールドが含まれているか確認してください。

app/src/app/api/experiencePoint/status/[id]/route.ts [5]

-const { speedPoint, similarityPoint, totalPoint } = await req.json();
+const body = await req.json(); // 変数名を変更し、必要なフィールドを確認
+const { speedPoint, similarityPoint, totalPoint } = body;
Suggestion importance[1-10]: 6

Why: The suggestion to change the variable name and ensure the presence of required fields in the request body is relevant and improves the robustness of the code. However, it does not explicitly handle cases where the required fields might be missing.

6
ポイント不足時にユーザーに警告を表示します。

remainingPointが負の値になった場合の処理を追加してください。

app/src/components/view/user/StatusChangeDialog.tsx [43]

 if (remainingPoint < 0) {
   setIsDisabled(true);
+  alert("ポイントが不足しています。"); // ユーザーに警告
Suggestion importance[1-10]: 5

Why: The suggestion to alert the user when points are insufficient is a good addition for user experience. However, it may not be necessary to use an alert, and a more user-friendly approach could be considered.

5
Possible bug
idの取得時にデフォルト値を設定して、エラーを防ぎます。

idを取得する際に、pathnameが正しい形式であることを確認してください。

app/src/app/api/experiencePoint/me/[id]/route.ts [7]

-const id = pathname.split("/").pop() || "";
+const id = pathname.split("/").pop() || "0"; // デフォルト値を設定
Suggestion importance[1-10]: 3

Why: The suggestion to set a default value of "0" for id is a minor improvement that can prevent potential errors if the pathname does not contain a valid ID. However, it does not address the root cause of the issue.

3

@Kubosaka
Copy link
Collaborator

コンフリクト直してください!

@nose221834 nose221834 self-requested a review November 16, 2024 06:47
@nose221834
Copy link
Collaborator

nose221834 commented Nov 16, 2024

ステータス調整のモーダルで保存おしたら、信じられないくらい重たくなった

@hikahana
Copy link
Collaborator Author

window.location.reload()にしているからかも。
useRouter使おうとしたけど値更新されなくてつかってた。

@hikahana hikahana merged commit e665854 into main Nov 16, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants