-
Notifications
You must be signed in to change notification settings - Fork 9
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
Kumo raku step12 add end date #1459
Changes from 5 commits
957214b
68ea8e2
b3ec875
637ae5a
88e4d33
58fb78c
9993889
fc8678e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,3 +4,4 @@ ja: | |
title: 'タスク一覧' | ||
sort: | ||
latest: '新しい順' | ||
expiring: '終了期限が近い順' |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
# frozen_string_literal: true | ||
|
||
class AddColumnEndDateToTasks < ActiveRecord::Migration[6.0] | ||
def change | ||
add_column :tasks, :end_date, :datetime | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,5 +4,6 @@ | |
factory :task do | ||
title { 'Spec' } | ||
description { 'test' } | ||
end_date { Time.new(2022, 12, 7, 10, 30) } | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,7 +18,7 @@ | |
|
||
context 'when there are tasks,' do | ||
let!(:task1) { FactoryBot.create(:task) } | ||
let!(:task2) { FactoryBot.create(:task, title: 'Spec2', description: 'test2') } | ||
let!(:task2) { FactoryBot.create(:task, title: 'Spec2', description: 'test2', end_date: Time.new(2023, 11, 1, 10, 30)) } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. let!にする必要はないと思いました。 before do
FactoryBot.create(:task)
FactoryBot.create(:task, title: 'Spec2', description: 'test2', end_date: Time.new(2023, 11, 1, 10, 30))
end There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 参考議論なども併せて確認してください There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ありがとうございます。 |
||
|
||
before do | ||
# 一覧画面を開く | ||
|
@@ -30,8 +30,10 @@ | |
expect(page).to have_content 'タスク一覧' | ||
expect(page).to have_content 'Spec' | ||
expect(page).to have_content 'test' | ||
expect(page).to have_content '2022年12月07日(水) 10時30分00秒' | ||
expect(page).to have_content 'Spec2' | ||
expect(page).to have_content 'test2' | ||
expect(page).to have_content '2023年11月01日(水) 10時30分00秒' | ||
expect(Task.count).to eq 2 | ||
end | ||
|
||
|
@@ -43,6 +45,15 @@ | |
expect(page.text).to match(/Spec2.*Spec/) | ||
expect(Task.count).to eq 2 | ||
end | ||
|
||
it 'sorts correctly after push sort button: expiring' do | ||
click_link '終了期限が近い順' | ||
|
||
expect(page).to have_content 'タスク一覧' | ||
# 正規表現で並び順をチェック | ||
expect(page.text).to match(/Spec.*Spec2/) | ||
expect(Task.count).to eq 2 | ||
end | ||
end | ||
|
||
context 'when error,' do | ||
|
@@ -72,6 +83,7 @@ | |
# titleとdescriptionを入力 | ||
fill_in 'task_title', with: 'Spec test new task' | ||
fill_in 'task_description', with: 'Spec test new task description' | ||
fill_in 'task_end_date', with: Time.new(2023, 11, 1, 10, 30) | ||
|
||
# 登録 | ||
click_button 'タスクを登録する' | ||
|
@@ -81,6 +93,7 @@ | |
expect(page).to have_content 'タスク詳細' | ||
expect(page).to have_content 'Spec test new task' | ||
expect(page).to have_content 'Spec test new task description' | ||
expect(page).to have_content '2023年11月01日(水) 10時30分00秒' | ||
end | ||
end | ||
|
||
|
@@ -224,6 +237,7 @@ | |
# titleとdescriptionを入力する | ||
fill_in 'task_title', with: 'Spec first task' | ||
fill_in 'task_description', with: 'Spec first task description' | ||
fill_in 'task_end_date', with: Time.new(2023, 11, 1, 10, 30) | ||
|
||
# 更新実行 | ||
click_button 'タスクを更新する' | ||
|
@@ -233,6 +247,7 @@ | |
expect(page).to have_content 'タスク詳細' | ||
expect(page).to have_content 'Spec first task' | ||
expect(page).to have_content 'Spec first task description' | ||
expect(page).to have_content '2023年11月01日(水) 10時30分00秒' | ||
end | ||
end | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
こういう書き方にするとユーザが任意の操作でデータベースの並び順を帰れます。
あと間違ったパラメータを渡すとアプリケーションエラーが発生するので直しましょう、、
Search用で使用するメソッドではないと思います
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
すみません、ここは理解できなかった。もう詳しく説明お願いいただければありがたいです。
ユーザが間違ったパラメーターに編集するとエラーページに遷移するとは正しいと思ってますが、
一応リクエストパラメーターのバリデーションを追加します。パラメーターが正しいときだけ結果返すようにします。
検索機能を実装したときこのメソッドにwhere処理を追加して、検索ボタンを押すとこのメソッドを呼んで結果をとるつもりですが、何かご懸念点ございますか?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shinさんがアドバイスした実装方法も考えてみましたが、今後たとえ並び方法が多くなったらコントローラー側は
みたいになちゃいます。
latest.newest
みたいなソースを読むと意味わかりづらいので、同じ機能のscopeを全部繋ぐのは避けたいということを考えてます。mm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
こういう書き方にするとユーザが任意の操作でデータベースの並び順を「変えれます」でした。。すみません誤字です。
この辺をもっと説明すると
taskで並び順を変えたいところ以外のカラムを入力しても動きます例えば現在はcreated_atを並び順変えを入れているのにdescriptionなど他のカラムをパラメーターに渡して入力しても並び順を変えれます。
こちらは開発者が意図してない並び順変更ができてしまい、
今後indexがない状態のカラムで膨大なデータがある場合、
並び順変更だけでtimeoutエラーとかDBへの負荷も高まる可能性があります。
あと意図してないカラムを他の人に漏出してしまうのも良くないと思うので
意図したカラム以外の入力はできなくしたいですね
あとアプリケーションエラーの補足説明ですが、

urlで入力するカラム名を変え入力すると500系エラーが表示されます。
一般にはアプリケーションの動作が止まることはないと思います404に移動するか、
並び順が変わったりなど動作は担保されるはずです。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sortと並び順をパラメータに渡すことでlatest、newestなどはなくせるのでは?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
scopeに全部書くを避けたいイメージはどれほどの列の並び変えを考えてます?
最初のちょうさんが書いた状態からasc descの変更ができればいいと思いましたが、、