Skip to content

Conversation

@hikahana
Copy link
Collaborator

@hikahana hikahana commented Dec 30, 2024

対応Issue

  • resolve #0

概要

adminのみの変更です。
numbers schemaのnumberにunique制約を付与しました。
フロント側でエラーハンドリングを追加
import周りのeslintを追加。邪魔だから消しました。

実装詳細

submitの際にonErrorでunique制約エラーの際、アラートで入力済みと返すように修正。
それ以外のエラーはエラーが発生しましたとだけ返しています。

画面スクリーンショット等

image

テスト項目

  • 同じ数値を入力しようとしてトーストメッセージが返ってくるか返ってくるか。
  • その際、サーバーサイドエラーでアプリが落ちないか。
  • コードレビューお願いします。

備考

@hikahana hikahana marked this pull request as ready for review December 30, 2024 20:40
@github-actions github-actions bot added frontend backend bug Something isn't working labels Dec 30, 2024
@TkymHrt TkymHrt requested a review from Copilot January 15, 2025 12:18

This comment was marked as outdated.

@hikahana hikahana requested a review from Copilot August 4, 2025 08:13
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a unique constraint to the number field in the numbers schema and implements error handling for duplicate number submissions in the admin interface.

  • Adds database unique constraint on numbers.number field to prevent duplicate entries
  • Implements client-side error handling with user-friendly alerts for duplicate number submissions
  • Updates ESLint configuration with improved import ordering and TypeScript rules

Reviewed Changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
api/migrations/default/1720521696814_auto/up.sql Adds unique constraint to numbers.number field
view-admin/src/pages/index.tsx Implements error handling for duplicate submissions and improves import organization
view-admin/package.json Updates ESLint dependencies and configuration
view-admin/.eslintrc.json Adds comprehensive ESLint rules for import ordering and TypeScript
.vscode/settings.json Configures VS Code for automatic linting and formatting

)
) {
// TODO: ちゃんとしたコンポーネントで実装する
alert(`${submitNumber} は既に入力済みです。`);
Copy link

Copilot AI Aug 4, 2025

Choose a reason for hiding this comment

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

Using the native alert() function is not recommended for user notifications in modern web applications. Consider using a proper toast notification or modal component for better user experience and accessibility.

Copilot uses AI. Check for mistakes.
// TODO: ちゃんとしたコンポーネントで実装する
alert(`${submitNumber} は既に入力済みです。`);
} else {
alert("エラーが発生しました。");
Copy link

Copilot AI Aug 4, 2025

Choose a reason for hiding this comment

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

Using the native alert() function is not recommended for user notifications in modern web applications. Consider using a proper toast notification or modal component for better user experience and accessibility.

Copilot uses AI. Check for mistakes.
onError: (err) => {
if (
err.graphQLErrors.some(
(e) => e.extensions?.code === "constraint-violation",
Copy link

Copilot AI Aug 4, 2025

Choose a reason for hiding this comment

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

The error code 'constraint-violation' is a magic string that should be extracted to a constant for better maintainability and to avoid typos.

Copilot uses AI. Check for mistakes.
@github-actions github-actions bot added the infra label Aug 4, 2025
@YosukeIida
Copy link
Collaborator

YosukeIida commented Aug 22, 2025

@hikahana

  • package.jsonに react-toastify を追加してください

パッと見たところ,package.json更新して,動作できたら良さそうだと思います.

Copy link
Collaborator

@YosukeIida YosukeIida left a comment

Choose a reason for hiding this comment

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

LGTM

@hikahana hikahana merged commit 9ed1d6c into develop Aug 31, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend bug Something isn't working frontend infra

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants