イベント名がマークダウン記号から始まるのを禁止するバリデーションを設定 #8199
Conversation
hagiya0121
force-pushed
the
bug/validate-event-name
branch
from
f43720f to
ce07556
Compare
|
@sugiwe |
|
@hagiya0121 |
|
@hagiya0121 |
hagiya0121
changed the title
There was a problem hiding this comment.
@hagiya0121
お疲れ様です！
コード確認させていただきました、変更点が明確で確認もしやすかったです🙏
正規表現のところで気になることがいくつかあったのでコメントさせていただきました、ご確認よろしくお願いいたします！
また、コードの中身でない部分ですが、PR文章中の「変更確認方法」内に記載のあったgit fetch origin pull/7916/head:bug/validate-event-nameの番号がissueの番号になっておりfetchできず、PRの番号であるgit fetch origin pull/8199/head:bug/validate-event-nameにしたらfetchできましたので、併せてご報告いたします🙏
|
|
||
| class MarkdownProhibitedValidator < ActiveModel::EachValidator | ||
| def validate_each(record, attribute, value) | ||
| return unless /\A\s*\u3000*[#-*+>`]\s+/.match?(value) |
There was a problem hiding this comment.
こちらの正規表現について、いくつか改善できそうなところがありそうでした👀
- タイトルがエラーにならないようです。-が文字の途中にあるため「範囲指定（この場合#から*まで ）」として解釈されているかもしれません。- 「
#から*まで」というのはASCIIコードの順番によると$ % & ' ( )が該当するようです。試しに& タイトルや( タイトルでイベントを作ろうとしたところエラーになったので、間違いなさそうです！ - このような場合、
-を冒頭に持ってくると良いみたいです。-#*にしてみたら、無事- タイトルがエラーになってくれました！
- 「
- 現状の正規表現ですと、「全角スペースが冒頭に1文字以上あった上で次に続くMarkdown用の記号が入力された場合」にもマッチしますが、全角スペースが冒頭にある場合はMarkdown表示されなくなると思います。「イベントタイトルとして冒頭に全角スペースがあるのはおかしい」という観点もありそうですが、今回のバリデーションには冒頭の全角スペースという条件は含めなくても良いのではと思いましたがいかがでしょうか？
- 現在の正規表現ですと、以下などのMarkdown記法がキャッチできないように思いました。どこまでカバーするのかというのはかなり悩ましそうですが、少なくとも元のissueに含まれていたコードブロック・h2以下のタイトルと、あと個人的には数値型リストについてもカバーしておくと良さそうに思いましたがいかがでしょうか？
- コードブロック（
```rubyや```など） - h2以下のタイトル（
## タイトルや### タイトルなど） - 数値型リスト（
1.や10.など） - 水平線（
---と***）
- コードブロック（
test/models/regular_event_test.rb
Outdated
| test 'title is invalid when it starts with markdown symbols' do | ||
| regular_event = regular_events(:regular_event1) | ||
| regular_event.title = '# 無効なタイトル' | ||
| assert regular_event.invalid? | ||
| regular_event.title = ' # 先頭に半角スペースを含む無効なタイトル' | ||
| assert regular_event.invalid? | ||
| regular_event.title = ' # 先頭に全角スペースを含む無効なタイトル' | ||
| assert regular_event.invalid? | ||
| end | ||
|
|
There was a problem hiding this comment.
バリデーションの正規表現の修正に合わせて、テストも修正をお願いできればと思います🙏
どこまで網羅して書くのかかなり悩ましそうですが、issue #7916 で書かれていた例など参考にすると良いかもしれないと思いました。
|
@sugiwe
|
|
@hagiya0121 こちらからはApproveとさせていただきます！ |
|
@sugiwe |
|
@komagata |
| @@ -0,0 +1,10 @@ | |||
| # frozen_string_literal: true | |||
|
|
|||
| class MarkdownProhibitedValidator < ActiveModel::EachValidator | |||
Issue
概要
変更確認方法
bug/validate-event-nameをローカルに取り込むgit fetch origin pull/8199/head:bug/validate-event-namegit checkout bug/validate-event-nameforeman start -f Procfile.devでローカルサーバーを立ち上げ# test(マークダウン記号から始まるタイトル)を入力してイベントを作成するタイトルには禁止されている記号が含まれていますというエラーメッセージが表示されることを確認Screenshot
バリデーション設定後