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

♻️ Repository にあったバリデーションを usecase で行う #289

Merged
merged 1 commit into from
Sep 7, 2024

Conversation

anko9801
Copy link
Contributor

@anko9801 anko9801 commented Sep 7, 2024

User description

fix: #95


PR Type

enhancement


Description

  • リポジトリから日付のバリデーションロジックを削除し、ユースケースに移動しました。
  • src/features/request/usecase.tssrc/features/transaction/usecase.ts に日付のバリデーションを追加しました。
  • 日付は yyyy-MM-dd の形式である必要があります。

Changes walkthrough 📝

Relevant files
Enhancement
repository.ts
Remove date validation from request repository                     

src/features/request/repository.ts

  • Removed date validation logic from the repository.
+0/-8     
usecase.ts
Add date validation to request usecase                                     

src/features/request/usecase.ts

  • Added date validation logic to the usecase.
+10/-3   
repository.ts
Remove date validation from transaction repository             

src/features/transaction/repository.ts

  • Removed date validation logic from the repository.
+0/-8     
usecase.ts
Add date validation to transaction usecase                             

src/features/transaction/usecase.ts

  • Added date validation logic to the usecase.
+8/-0     

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

@anko9801 anko9801 requested a review from mehm8128 September 7, 2024 02:08
@anko9801 anko9801 self-assigned this Sep 7, 2024
@anko9801 anko9801 requested a review from reiroop September 7, 2024 02:08
Copy link

github-actions bot commented Sep 7, 2024

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Key issues to review

重複コード
日付のバリデーションロジックが src/features/request/usecase.tssrc/features/transaction/usecase.ts で重複しています。このロジックを共通のユーティリティ関数にリファクタリングすることを検討してください。

重複コード
日付のバリデーションロジックが src/features/request/usecase.tssrc/features/transaction/usecase.ts で重複しています。このロジックを共通のユーティリティ関数にリファクタリングすることを検討してください。

@reiroop
Copy link
Contributor

reiroop commented Sep 7, 2024

/improve

Copy link

github-actions bot commented Sep 7, 2024

PR Code Suggestions ✨

CategorySuggestion                                                                                                                                    Score
Maintainability
正規表現を定数として定義して、コードの重複を避ける。

正規表現のパターンを定数として定義し、コードの重複を避けましょう。これにより、将来的にパターンを変更する際に一箇所の修正で済みます。

src/features/request/usecase.ts [17]

-const rule = /^2[0-9]{3}-[0-9]{1,2}-[0-9]{1,2}$/
+const DATE_REGEX = /^2[0-9]{3}-[0-9]{1,2}-[0-9]{1,2}$/;
+const rule = DATE_REGEX;
 
Suggestion importance[1-10]: 7

Why: Defining the regex pattern as a constant improves maintainability by reducing code duplication and making future changes easier. However, the improvement is minor as it only affects a small part of the code.

7

Copy link
Contributor

@mehm8128 mehm8128 left a comment

Choose a reason for hiding this comment

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

よさそうです
そもそも入力欄をtype='date'とかにすればバリデーションを自前実装しなくてよさそうな気もしてきましたが、今フィルタリングUIがないので一旦これでOKです

@anko9801 anko9801 merged commit 6340d93 into main Sep 7, 2024
7 checks passed
@anko9801 anko9801 deleted the refactor/validation branch September 7, 2024 03:52
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.

バリデーションなどを必要に応じてusecaseに入れる
3 participants