テストの実装を行っていたのですが、ISBNによる本登録に対してのエラー確認時にある問題点に気づきました。
まず、この問題について先に修正を行うことにしました。
詳しい問題点については、本文に記載します。
今回の目的
エラー処理を独自処理からバリデーション利用に変更する
なぜやるか
- エラー処理をバリデーション化して統一するため
- 処理を統一することでテスト時の処理も統一化させるため
やりたいこと
- 問題点を整理する
- バリデーションルールを追加する
- バリデーション設定を適用する
- 検索結果エラー時にバリデーションエラーとして表示させる
やったこと
- 問題点を確認する
- フォーム入力データのチェックをバリデーションで実施する
- 検索結果がない場合、結果をバリデーションエラーとして表示する
- ビューのメッセージ表示方法を修正する
実施内容
エラーメッセージをバリデーション化する
なにが問題か
ISBNコード登録ページは、どういう状況かを表示するためにメッセージを表示させるようにしていました。
$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(
);
$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つです。
問題点:処理毎にifで比較し、else時にメッセージ出力
改善策:バリデーションを利用して検証し、NG時はバリデーション機能でエラーを出力させる
長々とifで処理を書いていますが、結構短くできそうです。結果もバリデーションエラーで出力できるので直すべきと思いました。
問題点:本情報がない場合のメッセージを通常メッセージとして扱っている
改善策:バリデーション化してエラーとして出力する。
こちらはメッセージ出力でもよかったかもしれませんが、エラーという扱いで統一させるということで、変更をかけることにしました。
ビューファイルの状態も確認しておきます。
# ~/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(
);
$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){
$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]);
}
フォームからISBNコード入力し、初めにバリデーションチェックすることにしました。
ifで処理していた判定文は全て削除されています。
# ~/app/Bookdata.php
public static $isbnEntryRules = array(
'isbn' => 'required|digits:13|unique:bookdata,isbn'
);
ルールについても、入力必須・数値13桁・DB登録ユニークを設定し、かなりシンプルになりました。
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登録ページのエラーメッセージについてバリデーションを適用させる形に修正ができました。
この状態でテストをしていきます。