appirits-fukada / laravel_todo

laravelを使ったToDoサイト
0 stars 0 forks source link

タスク機能 #13

Open appirits-fukada opened 2 years ago

appirits-fukada commented 2 years ago
  1. save/updateメソッドは成功/失敗で、true/false が返ってきます。以下のようにしましょう。また、save, updateとで挙動が違うので意図と合うように使いましょう。
    if($task->save()) {
    成功処理
    }
    else {
    失敗処理
    }

    ※指摘の箇所(saveのところ)

    public function store(StoreTask $request)
    {
        $task = new Task;
        $task->title = $request->title;
        $task->content = $request->content;
        $task->user_id = Auth::id();
        $task->save();
        //TODO flashメッセージは別ファイルで設定できないか?
        return redirect()->route('tasks.show', ['task' => $task->id])->with('success', '新規タスクを作成しました。');
    }
  2. showメソッドで存在しないIDのパスを指定すると、500エラーになる。404のほうが適切です。findした後にnullチェックをして、存在しなければ404ステータスを返しましょう。
  3. editメソッドで存在しないIDのパスを指定すると、500エラーになる。こちらも同様に、404のほうが適切です。findした後にnullチェックをして存在しなければ404ステータスを返しましょう。
  4. StoreTaskRequestでvalidateしているならば、DBの方も制約つけましょう。 また、validationのメッセージが日本語ではありません。 ※modelの方ではなく、validationをリクエストにした意図ってありますか?どちらが良くてどちらかがダメってわけではなく、単純に意図してRequestにしたのかなって気になりました。
  5. 記法の指摘。StoreTask の後、Space開けましょう。
    public function update(StoreTask$request, $id)
    {
  6. Viewの指摘。入力のvalidationをつけているのならば、Inputなどにもmax制限つける、上限を表示するが良いです。 また、inputにtype="string" ってありましたっけ?多分 type="text" が正しいです。textareaにもtype="text"ないかもです。
  7. タスク一覧ページ、ページネーションをつけましょう。大量のデータを登録した時に無限にページが続いてしまいますし、引っ張るデータ量も多くなります。
  8. すでに気付いていてメモもありましたが、エラーメッセージはパーシャルのほうがいいですね。
  9. Seederに書いている、DB::table('tasks')->insert の箇所ってアプリケーソン側のvalidationに引っかかりますか?
appirits-fukada commented 2 years ago
  1. (自己起票)タスク詳細画面のページヘッダーがタスクの一覧になっている。
  2. (自己起票)使っていないなlayoutファイルが残っている。
appirits-fukada commented 2 years ago
  1. Validationエラーになった時に入力内容が消える 1:新規作成ページで、タスク名・詳細に色々入力して送信する。 2:タスク名がValiadtionに引っかかり、新規作成ページにリダイレクト 3:リダイレクトした新規作成ページでは1で入力した内容が消えている。 原因:textareaタグのvalue属性に{{ old('content') }} を割り当てていますが、textareaタグにvalueはありません。