Project

General

Profile

Backport(バックポート) #4043

Bug(バグ) #4000: MemberConfigFormのIsUnique制約に対するエラーメッセージが適切に出力されていない

MemberConfigFormのIsUnique制約に対するエラーメッセージが適切に出力されていない

Added by isao sano over 2 years ago. Updated about 1 year ago.

Status:
Fixed(完了)
Priority:
Normal(通常)
Target version:
Start date:
2016-11-15
Due date:
% Done:

100%


Description

現象

「PC メールアドレス設定」(/member/config?category=pcAddress) で、変更後のメールアドレスが他のメンバーとメードアドレスが重複する場合に「Invalid pc_address.」というエラーメッセージが表示される。

原因

当初 #3736 ではこの不具合を英日の翻訳漏れとして扱っていたが、実際には #1069 で追加したエラーメッセージが「Invalid %name%.」であったのに対して翻訳は「Invalid %name%.」ではなく「Invalid mobile_address.」に対して行われていたことに起因する問題であった。

source:lib/form/doctrine/MemberConfigForm.class.php@43190aa1#L232:

    throw new sfValidatorError($validator, 'Invalid %name%.', array('name' => $name));

source:i18n/messages.ja.xml@43190aa1#L85

      <trans-unit id="">
        <source>Invalid mobile_address.</source>
        <target>メールアドレスが無効です。</target>
      </trans-unit>

このことで mobile_address 以外の IsUnique 制約を使用する全ての設定項目に対しては未翻訳のエラーメッセージが出力される状態となっていた。

また、この問題は「Invalid %name%.」に対する翻訳を追加するだけでは解決しない(「Invalid pc_address.」と表示されるまま変わらない)。
そもそも sfValidatorError::__construct($validator, $code, $arguments) の第 2 引数はエラーメッセージそのものを渡すのではなくバリデーター内部で識別するエラーコードを渡すものであり、今まで Invalid pc_address. のようなエラーメッセージが表示されていたのはエラーコードに対応するメッセージが存在しなかったためである。「%name%」のようなトークンを含むテキストに対して opI18n クラスによるローカライゼーションが正しく機能するためには、例えば以下のように適切な用法に書き換える必要がある。

  public function setMemberConfigWidget($name)
  {
....
    if (!empty($config['IsUnique']))
    {
      $uniqueValidator = new sfValidatorCallback(array(
        'callback'    => array($this, 'isUnique'),
        'arguments'   => array('name' => $name),
        'empty_value' => $this->validatorSchema[$name]->getOption('empty_value'),
      )); 

      $uniqueValidator->addMessage('duplicate', 'Invalid %name%.');
....
  }

  public function isUnique($validator, $value, $arguments = array())
  {
....
    throw new sfValidatorError($validator, 'duplicate', array('name' => $name));
  }

ここまで修正してようやく「Invalid %name%」に対して翻訳文が出力される状態になる。

これらの経緯をまとめると、下記の問題点が積み重なった結果として冒頭の「現象」が現れたといえる。

  • 独自のエラーメッセージを追加するために sfValidatorBase::addMessage() を使用せず sfValidatorErorr::__construct() の引数 $code に直接エラーメッセージを指定していた
  • 上記が原因でローカライゼーションが正しく機能せず「Invalid %name%.」に対する翻訳が行えなかったため、やむなく「Invalid mobile_address.」に対して訳文を追加していた(と思われる)
  • mobile_address 以外の IsUnique 制約を使用する member_config の設定項目については、訳文が存在しないため原文のままエラーメッセージが出力される状態となった

修正内容

MemberConfigForm 内でエラーメッセージを記述している箇所を上記の sfValidatorBase::addMessage() を使用した用法に書き換えた上で、下記の翻訳文を追加する。

      <trans-unit id="">
        <source>Invalid %name%.</source>
        <target>不正な %name% です。</target>
      </trans-unit>

また、この翻訳文の改善について #3999 にて扱っており、修正の際にはコンフリクトする可能性がある点を留意する必要がある。


Related issues

Related to OpenPNE 3 - Backport(バックポート) #2571: メールアドレス設定など、確認欄がある場合のエラー表示が適切ではない Fixed(完了) 2011-10-14

History

#1 Updated by isao sano over 2 years ago

  • Related to Bug(バグ) #4000: MemberConfigFormのIsUnique制約に対するエラーメッセージが適切に出力されていない added

#2 Updated by isao sano over 2 years ago

https://redmine.openpne.jp/issues/2571
このチケットでの修正とコンフリクトします。 #2571 は master では取り込まれている修正です。

また、本チケット #4043 を手動でコンフリクト解決し、後から #2571 を取り込んだ場合にもコンフリクトします。

以上の理由から #2571 の修正を取り込んだ後、本チケットのプルリクエストを作成します。

#3 Updated by isao sano over 2 years ago

#4 Updated by isao sano over 2 years ago

  • Target version changed from OpenPNE 3.6.x to OpenPNE 3.6.25

#5 Updated by kaoru n over 2 years ago

  • Target version changed from OpenPNE 3.6.25 to OpenPNE 3.6.x

#6 Updated by Youichi Kimura over 2 years ago

  • Status changed from New(新規) to Pending Review(レビュー待ち)
  • Assignee changed from isao sano to Youichi Kimura
  • % Done changed from 0 to 50

下記 Pull Request にて修正しました (#4043, #4045 の修正を含んでいます)
https://github.com/openpne/OpenPNE3/pull/441

#8 Updated by kaoru n over 1 year ago

  • Parent task set to #4000

#9 Updated by Rimpei Ogawa about 1 year ago

  • Status changed from Pending Review(レビュー待ち) to Pending Testing(テスト待ち)
  • % Done changed from 50 to 70

#10 Updated by isao sano about 1 year ago

  • Status changed from Pending Testing(テスト待ち) to Pending Merge(マージ待ち)
  • % Done changed from 70 to 80

試験完了しました。問題ありません。

#11 Updated by isao sano about 1 year ago

  • Target version changed from OpenPNE 3.6.x to OpenPNE 3.6.31

#12 Updated by kaoru n about 1 year ago

  • Status changed from Pending Merge(マージ待ち) to Fixed(完了)
  • % Done changed from 80 to 100

マージしました

Also available in: Atom PDF