Fippiyのプログラム学習内容アウトプットBlog

日々の学習内容をアウトプットして振り返りを実施する。

Laravel開発、独自エラー処理していたコードをバリデーション利用に修正する

テストの実装を行っていたのですが、ISBNによる本登録に対してのエラー確認時にある問題点に気づきました。

まず、この問題について先に修正を行うことにしました。 

詳しい問題点については、本文に記載します。

今回の目的

エラー処理を独自処理からバリデーション利用に変更する

なぜやるか

  • エラー処理をバリデーション化して統一するため
  • 処理を統一することでテスト時の処理も統一化させるため

やりたいこと

  • 問題点を整理する
  • バリデーションルールを追加する
  • バリデーション設定を適用する
  • 検索結果エラー時にバリデーションエラーとして表示させる

やったこと

  • 問題点を確認する
  • フォーム入力データのチェックをバリデーションで実施する
  • 検索結果がない場合、結果をバリデーションエラーとして表示する
  • ビューのメッセージ表示方法を修正する

実施内容

エラーメッセージをバリデーション化する

なにが問題か

ISBNコード登録ページは、どういう状況かを表示するためにメッセージを表示させるようにしていました。

f:id:Fippiy:20190429120411p:plain

登録フォーム

 

$msgという変数を作成しておき、表示内容によって変化させる…といった仕様でしたが、バリデーションによる入力内容チェックをしていませんでした。

処理内容は後述として、見た目は問題なかったのですが、テストを実装するにあたりエラー確認方法を考えていると、やはりバリデーションによるチェックができるのであれば、しっかり対応させるべきと考え、修正を行うことにしました。

後半の開発でバリデーションの詳しい使用方法も分かってきたので、その時の実装を再度見直しながら実施していきます。

 

エラー以外の状況表示に$msgは継続して使用することとします。

メッセージを直接ビューに書き込むことも考えましたが、ifで処理分岐するよりもアクションの結果にメッセージを付与するようが見やすいと考えたためです。

具体的に内容を確認してきます。

 

現状のコードから問題点を確認する

まず今までのコードを確認します。 

コントローラ処理

# ~/app/Http/Controllers/BookController.php[変更前]

public function getIsbn(){
$msg = 'ISBNコードを入力して下さい。';
return view('book.isbn',['msg'=>$msg]);
}
public function postIsbn(Request $request){
unset($request['_token']);
$value = $request['isbn'];
〜〜〜〜〜〜〜〜〜〜 ポイント1 〜〜〜〜〜〜〜〜〜〜
// ISBNコード桁数を確認
if (strlen($value) != 13) {
// レコード数不一致時はエラーを返して終了
$msg = 'ISBNコードは13桁で入力してください';
return view('book.isbn',['msg'=>$msg]);
}

// ISBNコードが既に登録されているか確認
$isbn = \App\Bookdata::where('isbn', $value)->first();
if ($isbn != null){
$msg = '作成済みのデータがあります';
return view('book.isbn',['msg'=>$msg]);
}
〜〜〜〜〜〜〜〜〜〜 ポイント1 〜〜〜〜〜〜〜〜〜〜
// ISBNコードから本情報を取得
$response = file_get_contents(
$isbn_url.$value
);
$result = json_decode($response, true);
 
〜〜〜〜〜〜〜〜〜〜 ポイント2 〜〜〜〜〜〜〜〜〜〜
// ISBNレコード結果を確認
if($result[0]==null){
// 情報がない場合はエラーを返して終了
$msg = '該当するISBNコードは見つかりませんでした。';
return view('book.isbn',['msg'=>$msg]);
}
〜〜〜〜〜〜〜〜〜〜 ポイント2 〜〜〜〜〜〜〜〜〜〜
 
〜〜〜〜〜〜〜〜〜〜 中略〜〜〜〜〜〜〜〜〜〜
// 保存
$savedata->save();
// 保存完了メッセージ
$msg = 'データを新規作成しました';
// ビューに出力
return view('book.isbn',['msg'=>$msg,'book'=>$savedata]);
}

ポイントとなる箇所は2つです。

 

  • ポイント1

問題点:処理毎にifで比較し、else時にメッセージ出力

改善策:バリデーションを利用して検証し、NG時はバリデーション機能でエラーを出力させる

長々とifで処理を書いていますが、結構短くできそうです。結果もバリデーションエラーで出力できるので直すべきと思いました。

 

  • ポイント2

問題点:本情報がない場合のメッセージを通常メッセージとして扱っている

改善策:バリデーション化してエラーとして出力する。

こちらはメッセージ出力でもよかったかもしれませんが、エラーという扱いで統一させるということで、変更をかけることにしました。

 

ビューファイルの状態も確認しておきます。

# ~/resources/views/book/isbn.blade.php[変更前]

@section('content')
<div class="index-content">
<div class="books-list">
<div class="books-list__title bookpage-color">
ISBNコード登録
</div>
@if (isset($msg))
<div class="books-list__msg">
<span>{{$msg}}</span>
</div>
@endif
〜略〜

ビューファイルの冒頭部分です。

バリデーションによるエラー表示ではなく、すべて$msgで対応させていましたが、バリデーションエラーに対応させます。

エラー表示に関してはバリデーションエラーとして出力させるようにし、$errorとして出力させる記述を追加することにしました。 

 

処理をバリデーション化する 

上記の内容を元に、コードの見直しを行いました。  

# ~/app/Http/Controllers/BookController.php[変更後]

public function getIsbn(){
$msg = 'ISBNコードを入力して下さい。';
return view('book.isbn',['msg'=>$msg]);
}
public function postIsbn(Request $request){
〜〜〜〜〜〜〜〜〜〜 ポイント1 〜〜〜〜〜〜〜〜〜〜
// バリデーションチェック
$this->validate($request, Bookdata::$isbnEntryRules);
〜〜〜〜〜〜〜〜〜〜 ポイント1 〜〜〜〜〜〜〜〜〜〜

unset($request['_token']);
$value = $request['isbn'];

// ISBNコードから本情報を取得
$response = file_get_contents(
$isbn_url.$value
);
$result = json_decode($response, true);

〜〜〜〜〜〜〜〜〜〜 ポイント2 〜〜〜〜〜〜〜〜〜〜
// ISBNレコード結果を確認
// result[0]の情報有り無しで判定
if($result[0] == null)
{
$isbndata = false;
} else {
$isbndata = true;
}

// isbnレコード結果チェック
// false時は検索結果なしで未登録とし、バリデーションエラーを返す
$validator = Validator::make(['isbn' => $isbndata],
 ['isbn' => 'accepted'], ['該当するISBNコードは見つかりませんでした。']);
if ($validator->fails()) {
return redirect('book/isbn')
->withErrors($validator)
->withInput();
}
〜〜〜〜〜〜〜〜〜〜 ポイント2 〜〜〜〜〜〜〜〜〜〜
 

// ISBNレコードがあれば追加処理
$savedata = new Bookdata;

// summaryData取得
$getdata = $result[0]["summary"];

// 要素毎にレコードに追加
foreach($getdata as $key => $value){
if(strlen($value) == 0){
$savedata->$key = null;
} else {
$savedata->$key = $value;
}
}

// detail取得
$detail_datacheck = empty($result[0]["onix"]["DescriptiveDetail"]
 ["Contributor"][0]["BiographicalNote"]);
if(strlen($detail_datacheck) == true){
$savedata->detail = null;
} else {
$savedata->detail = $result[0]["onix"]["DescriptiveDetail"]
 ["Contributor"][0]["BiographicalNote"];
}
// 保存
$savedata->save();
// 保存完了メッセージ
$msg = 'データを新規作成しました。続けてISBNコードを登録できます。';
// ビューに出力
return view('book.isbn',['msg'=>$msg,'book'=>$savedata]);
}
  • ポイント1

フォームからISBNコード入力し、初めにバリデーションチェックすることにしました。

ifで処理していた判定文は全て削除されています。

# ~/app/Bookdata.php

public static $isbnEntryRules = array(
'isbn' => 'required|digits:13|unique:bookdata,isbn'
);

ルールについても、入力必須・数値13桁・DB登録ユニークを設定し、かなりシンプルになりました。

 

  • ポイント2

ISBN検索結果については、まずデータがあるかどうかを判定し、判定に対してバリデーションで判断させる形としました。

これでエラー関係のメッセージはすべてバリデーションエラーとして扱える様になりました。

 

後は、ビューファイルにエラー表示を適用します。

# ~/resources/views/book/isbn.blade.php[変更後]

@section('content')
<div class="index-content">
<div class="books-list">
<div class="books-list__title bookpage-color">
ISBNコード登録
</div>
<div class="books-list__msg">
<p class="auth-contents__message--message">{{ $msg }}</p>
@foreach ($errors->all() as $error)
<p class="auth-contents__message--error">{{ $error }}</p>
@endforeach
</div>

エラー時に$msgをあわせて$errorを表示させるようにしました。

 

以上でISBN登録ページのエラーメッセージについてバリデーションを適用させる形に修正ができました。

この状態でテストをしていきます。