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

ステップ10: タスク一覧を作成日時の順番で並び替えましょう #928

Merged
merged 2 commits into from
Mar 4, 2021

Conversation

UYO123
Copy link

@UYO123 UYO123 commented Mar 4, 2021

参考: rails研修 step10

環境構築方法
環境構築でエラーが発生した際もお申し付けください。

実施内容

  • 作成日時の降順で並び替え
  • 並び替えがうまく行っていることをsystem specで記載
    • systemspecできるよう view にid設定 created_at表示

確認事項

  • 作成日時が降順で表示

画面
スクリーンショット 2021-03-04 11 21 01

spec実行
スクリーンショット 2021-03-04 11 15 01

rubocop実行
スクリーンショット 2021-03-04 11 15 09

@UYO123 UYO123 requested a review from kotaro0522 March 4, 2021 02:23
Comment on lines 5 to 7
Rails/DynamicFindBy:
Whitelist:
- find_by_id
Copy link

@kotaro0522 kotaro0522 Mar 4, 2021

Choose a reason for hiding this comment

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

h ttps://github.com/rubocop/rubocop-jp/issues/ 54
Excludeパラメータで指定すると良さそうです。

@@ -5,7 +5,7 @@ class TasksController < ApplicationController

# GET /tasks or /tasks.json
def index
@tasks = Task.all
@tasks = Task.all.order(created_at: 'DESC')

Choose a reason for hiding this comment

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

ただの好みなんですが、自分は order(created_at: :desc)order('created_at DESC') って書きたいかなと思いました。
(ラクマ のソースコードはそうなってることが多いです。)

@UYO123 UYO123 requested a review from kotaro0522 March 4, 2021 03:21
Copy link

@kotaro0522 kotaro0522 left a comment

Choose a reason for hiding this comment

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

LGTM

@UYO123 UYO123 merged commit 4111d6e into UYO123 Mar 4, 2021
@UYO123 UYO123 deleted the UYO123-step10 branch March 4, 2021 03:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants