Enhancement（機能追加・改善） #2275
opValidatorDate の date_format_range_error が直書きされているので option で変更できるようにする
100%
Description
背景¶
#754 では opValidatorDate で配列でない入力値に対しての下限・上限のバリデートがされていない問題が修正されていますが、そこではバリデートに引っかかった場合に出力するエラーメッセージに用いる日付のフォーマットが直に書かれています。
opValidatorDate クラスでは、許容範囲外の日付が入力された場合のエラーメッセージのフォーマットに 'Y-m-d' を用いるという仕様であっても良いとは思いますが、そうであっても不用意に拡張性を低下させないようにしたほうが好ましい思います。
- (1) 例外時のフォーマットは $this->getOption('date_format_range_error') を用いるようにする
- (2) opValidatorDate クラスでこのデフォルト値を 'Y-m-d' とすべきだと考えるのであれば、 $this->getOption('date_format_range_error') のデフォルト値をこのクラス内で 'Y-m-d' にセットする
#754 の修正内容を含めて考察しましたが、(2) の必要性は特に見当たらないので (1) のみ対応します（デフォルト値は sfValidatorDate と同様になります。関連 #1630 ）。
概要¶
メンバー側のプロフィール入力ページでは、日付のプロフィール項目は opValidatorDate を用いている。
管理画面でのプロフィール項目設定で指定された最小値や最大値を超える日付を入力した場合などにはエラーメッセージが表示されるが、そこで併せて表示される日付のフォーマットは opValidatorDate を使う限り 'Y-m-d' に固定されてしまっている。
修正方針¶
opValidatorDate のインスタンスを生成する時点で設定できる $option には、この範囲エラー時の日付フォーマットを指定するための date_format_range_error オプションが用意されている。これを opValidatorDate 内で用いるようにし、日付のフォーマットは opValidatorDate のインスタンスを生成する時点で指定できるようにする。
修正内容¶
lib/util/opFormItemGenerator.class.php
@@ -233,6 +233,7 @@ class opFormItemGenerator
if ($field['FormType'] === 'date')
{
+ $option['date_format_range_error'] = 'Y-m-d';
$obj = new opValidatorDate($option);
return $obj;
}
lib/validator/opValidatorDate.class.php
@@ -56,7 +56,7 @@ class opValidatorDate extends sfValidatorDate
$max = new DateTime($this->getOption('max'));
if ($max && $clean->format('U') > $max->format('U'))
{
- throw new sfValidatorError($this, 'max', array('max' => $max->format('Y-m-d')));
+ throw new sfValidatorError($this, 'max', array('max' => $max->format($this->getOption('date_format_range_error'))));
}
}
@@ -65,7 +65,7 @@ class opValidatorDate extends sfValidatorDate
$min = new DateTime($this->getOption('min'));
if ($min && $clean->format('U') < $min->format('U'))
{
- throw new sfValidatorError($this, 'min', array('min' => $min->format('Y-m-d')));
+ throw new sfValidatorError($this, 'min', array('min' => $min->format($this->getOption('date_format_range_error'))));
}
}
補足¶
date_format_range_error については http://www.symfony-project.org/forms/1_4/ja/B-Validators にも記述されている。
Related issues
Associated revisions
(fixes #2275) use option 'date_format_range_error' in opValidatorDate
fix coding style issue (fixes #2275)
(fixes #2275) use option 'date_format_range_error' in opValidatorDate
fix coding style issue (fixes #2275)
History
#1
Updated by Minoru Takai about 13 years ago
Updated by - Status changed from Accepted（着手） to Pending Review（レビュー待ち）
- % Done changed from 0 to 50
更新履歴 b6045142184085971062775555cf48d3db34de56 で適用されました。
#2
Updated by Shouta Kashiwagi over 12 years ago
Updated by - Target version changed from OpenPNE 3.7.0 to 252
#3
Updated by Youichi Kimura over 12 years ago
Updated by - Priority changed from Normal（通常） to Urgent（急いで）
#4
Updated by Shouta Kashiwagi over 12 years ago
- Target version changed from 252 to OpenPNE 3.8beta1
#5
Updated by Yuya Watanabe over 12 years ago
Updated by - Status changed from Pending Review（レビュー待ち） to Rejected（差し戻し）
- Assignee changed from Minoru Takai to Youichi Kimura
修正内容自体は問題ないですが，lib/util/opFormItemGenerator.class.php でのコーディング規約違反があるので修正をお願いします．
#6
Updated by Minoru Takai over 12 years ago
Yuya Watanabe は書きました:
修正内容自体は問題ないですが，lib/util/opFormItemGenerator.class.php でのコーディング規約違反があるので修正をお願いします．
具体的にどのような点で規約違反があるのかを指摘してください。修正すべき箇所が不明です。
#7
Updated by Yuya Watanabe over 12 years ago
変数と値を比較する際に最初に値を置いていない．
lib/util/opFormItemGenerator.class.php 74 行目
72 if (in_array($field['FormType'], self::$choicesType))
73 {
74 if ($field['FormType'] === 'select')
75 {
76 if (!$field['IsRequired'])
77 {
...
167 }
168
169 if ($field['FormType'] === 'checkbox')
170 {
171 $option['choices'] = $choices;
...
174 return $obj;
175 }
176 if ($field['FormType'] === 'select' || $field['FormType'] === 'radio')
177 {
178 $option = array('choices' => $choices);
...
182 }
183
184 if ($field['ValueType'] === 'integer')
185 {
186 if (isset($field['ValueMin']) && is_numeric($field['ValueMin']))
...
198 }
199 }
200 elseif ($field['FormType'] === 'date')
201 {
202 if (isset($field['ValueMin']) && false !== strtotime($field['ValueMin']))
...
232 }
233
234 if ($field['FormType'] === 'date')
235 {
236 $option['date_format_range_error'] = 'Y-m-d';
...
return 文の直前には可読性向上のために空行を入れるべき．
172 $option['multiple'] = true; 173 $obj = new sfValidatorChoice($option); 174 return $obj; 175 } 176 if ($field['FormType'] === 'select' || $field['FormType'] === 'radio') ... 179 $option['required'] = $field['IsRequired']; 180 $obj = new sfValidatorChoice($option); 181 return $obj; 182 } 183 184 if ($field['ValueType'] === 'integer') ... 236 $option['date_format_range_error'] = 'Y-m-d'; 237 $obj = new opValidatorDate($option); 238 return $obj; 239 } 240 ...
#8
Updated by Yuya Watanabe over 12 years ago
コーディング規約には記述されていませんが，個人的に気になるものとして switch 文の default ラベル部分に break が書かれていないというものがあります．これについては修正不要と思います．一般的にはどうなんでしょうね．
150 $obj = new sfWidgetFormChoice(array('choices' => $list));
151 break;
152 default:
153 $obj = new sfWidgetFormInput($params);
154 }
...
270 break;
271 default:
272 $obj = new opValidatorString($option);
273 }
274
...
334 // text and something else
335 default:
336 $obj = new sfWidgetFormInput($params);
337 }
338
#9
Updated by Youichi Kimura over 12 years ago
- Status changed from Rejected（差し戻し） to Pending Review（レビュー待ち）
更新履歴 2bf278a5802ae85a329d9649be5c8a1a84e91d07 で適用されました。
#10
Updated by Youichi Kimura over 12 years ago
当チケットでの変更範囲外ですが 2bf278a5802ae85a329d9649be5c8a1a84e91d07 にてnote-7,8の指摘箇所を修正しました。
#11
Updated by Minoru Takai over 12 years ago
更新履歴 8eccb9ab0313aa42cad51b453c4dbbb31f8c791f で適用されました。
#12
Updated by Youichi Kimura over 12 years ago
更新履歴 681ae12ea344193544a6cbd476e9b4fbbce19e1a で適用されました。
#13
Updated by Yuya Watanabe over 12 years ago
- Status changed from Pending Review（レビュー待ち） to Pending Testing（テスト待ち）
- % Done changed from 50 to 70
#14
Updated by Yuma Sakata over 12 years ago
Updated by - Status changed from Pending Testing（テスト待ち） to Fixed（完了）
- % Done changed from 70 to 100
テストOKです。