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

スコア計算の修正とトーストをダイアログに集約した #80

Merged
merged 3 commits into from
Nov 16, 2024

Conversation

hikahana
Copy link
Collaborator

@hikahana hikahana commented Nov 16, 2024

User description

概要

  • このプルリクエストの目的や背景を簡単に説明してください。
    スコア計算にsimilarityPointとspeedPointを追加した。最低保証みたいな感じでsimilarityとanswerTimeが低かったらぽ員を増やすようにした。

撮影した後ダイアログとトーストを表示していたけどダイアログに集約させました。

変更内容

  • どのような変更を行ったのか具体的に記述してください。

動作確認

  • どのような手順で動作確認を行ったのか記述してください。
    image

関連するIssue

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

備考

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

PR Type

enhancement, bug_fix


Description

  • PointDialog now consolidates messages previously shown as toast notifications, improving user experience by displaying them in a dialog.
  • Introduced a new function experiencePointData to retrieve user experience points, which are now factored into the score calculation.
  • Enhanced score calculation logic to provide a minimum score guarantee based on similarity and speed points.
  • Improved message formatting in PointDialog to support line breaks, enhancing readability.

Changes walkthrough 📝

Relevant files
Enhancement
page.tsx
Integrate toast notifications into PointDialog                     

app/src/app/camera/page.tsx

  • Removed toast notifications and integrated messages into PointDialog.
  • Added a new state variable message to store dialog messages.
  • Updated message formatting to include line breaks.
  • +7/-6     
    PointDialog.tsx
    Enhance PointDialog to support HTML formatted messages     

    app/src/components/view/PointDialog.tsx

  • Modified PointDialog to accept custom sub-messages with HTML
    formatting.
  • Updated message display to handle line breaks.
  • +8/-2     
    scoreRegister.ts
    Enhance score calculation with experience points                 

    app/src/functions/scoreRegister.ts

  • Added experiencePointData function to fetch user experience points.
  • Modified score calculation to include experience points for low
    similarity or speed.
  • +32/-1   

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    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

    Logic Complexity
    スコア計算のロジックが複雑になっており、特に新しいポイント計算の条件が多く含まれています。これにより、意図しないスコアが計算される可能性があるため、詳細なテストが必要です。

    State Management
    message ステートが新たに追加されましたが、適切に初期化されているか、または他の部分での使用方法が明確であるか確認する必要があります。

    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Security
    Sanitize user-generated content to prevent XSS vulnerabilities

    Ensure that message is sanitized to prevent XSS vulnerabilities when displaying
    user-generated content.

    app/src/app/camera/page.tsx [234-236]

     setMessage(
    -    `キャプション: ${data.text} \n類似度: ${percentSimilarity}% スコア: ${data.score} \nランキングから順位を確認しましょう!`,
    +    sanitizeHTML(`キャプション: ${data.text} \n類似度: ${percentSimilarity}% スコア: ${data.score} \nランキングから順位を確認しましょう!`),
     );
    Suggestion importance[1-10]: 8

    Why: The suggestion addresses a critical security concern by ensuring that user-generated content is sanitized, which is essential to prevent XSS vulnerabilities.

    8
    Possible bug
    Implement error handling for fetching experience point data to improve robustness

    Handle potential errors when fetching experience point data to avoid unhandled
    promise rejections.

    app/src/functions/scoreRegister.ts [54]

    -const resExp = await experiencePointData(scoreData.userId);
    +let resExp;
    +try {
    +    resExp = await experiencePointData(scoreData.userId);
    +} catch (error) {
    +    // Handle error appropriately
    +}
    Suggestion importance[1-10]: 7

    Why: This suggestion improves the robustness of the code by adding error handling for asynchronous operations, which is important to avoid unhandled promise rejections.

    7
    Possible issue
    Validate the response structure to prevent runtime errors when accessing properties

    Validate the response from experiencePointData to ensure it contains the expected
    structure before accessing properties.

    app/src/functions/scoreRegister.ts [70]

    -const similarityPoint = expDataValue.similarityPoint ?? 0;
    +const similarityPoint = expDataValue?.similarityPoint ?? 0;
    Suggestion importance[1-10]: 6

    Why: The suggestion enhances the code's safety by ensuring that the response structure is validated before accessing its properties, thus preventing potential runtime errors.

    6
    Maintainability
    Use a more descriptive variable name to enhance code readability

    Consider using a more descriptive variable name than point to clarify its purpose in
    the context of scoring.

    app/src/functions/scoreRegister.ts [73-79]

    -const point = Math.min(
    +const calculatedScore = Math.min(
         scoreData.similarity * 70 +
         (scoreData.similarity < 0.5 ? similarityPoint * 0.1 : 0) +
         (1 - normalizedTime) * 30 +
         (normalizedTime > 0.5 ? speedPoint * 0.2 : 0),
         100,
     );
    Suggestion importance[1-10]: 5

    Why: While the suggestion improves code readability by using a more descriptive variable name, it does not address any critical issues, hence the moderate score.

    5

    Copy link
    Collaborator

    @Kubosaka Kubosaka left a comment

    Choose a reason for hiding this comment

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

    動作おけです

    @@ -232,7 +231,9 @@ const CameraApp = () => {

    const percentSimilarity = Math.floor(data.similarity * 100);

    const message = `${data.text} 類似度 ${percentSimilarity}% スコア: ${data.score} ランキングから順位を確認しましょう!`;
    setMessage(
    `キャプション: ${data.text} \n類似度: ${percentSimilarity}% スコア: ${data.score} \nランキングから順位を確認しましょう!`,
    Copy link
    Collaborator

    Choose a reason for hiding this comment

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

    キャプションって表示変えますか?
    あと正解の単語(日本語)追加できたら良いかと思います

    @nose221834
    Copy link
    Collaborator

    image

    @nose221834 nose221834 merged commit a93f6b4 into main Nov 16, 2024
    1 check passed
    @nose221834 nose221834 deleted the fix/hikahana/score-keisan branch November 16, 2024 18:53
    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.

    3 participants