Project

General

Profile

Bug(バグ) #1449

opFormItemGenerator::generateValidator() の 数値用バリデータ生成時 ValueMax, ValueMin が 0の場合は 範囲指定されない

Added by Shogo Kawahara about 9 years ago. Updated almost 2 years ago.

Status:
Fixed(完了)
Priority:
Normal(通常)
Target version:
Start date:
2010-07-28
Due date:
% Done:

100%

3.6 で発生するか:
Yes
3.8 で発生するか:
Unknown (未調査)

Description

Overview

opFormItemGenerator::generateValidator() の 数値用バリデータ生成時 ValueMax, ValueMin が 0の場合は 範囲指定されない


再現手順

  • 範囲が 0 〜 1 である数値プロフィール項目を作成。
  • プロフィール編集で -1 を入力
  • 正常にプロフィールが登録できてしまう。

対象

OpenPNE3.0.x 〜 OpenPNE3.7.x-dev

Causes

184     if ($field['ValueType'] === 'integer' || $field['FormType'] === 'date')
185     {   
186         if (!empty($field['ValueMin']))
187         {
188           $option['min'] = $field['ValueMin'];
189         }
190         if (!empty($field['ValueMax']))
191         {
192           $option['max'] = $field['ValueMax'];
193         }
194     } 

ValueMinおよびValueMax を empty() を利用して判定しているためです。

Way to fix


Related issues

Related to OpenPNE 3 - Bug(バグ) #837: プロフィール項目設定の文字数制限でmax=0,min=0と設定できてしまう Fixed(完了) 2010-03-11
Related to OpenPNE 3 - Backport(バックポート) #1468: opFormItemGenerator::generateValidator() の 数値用バリデータ生成時 ValueMax, ValueMin が 0の場合は 範囲指定されない Fixed(完了) 2010-07-28
Related to OpenPNE 3 - Backport(バックポート) #1469: opFormItemGenerator::generateValidator() の 数値用バリデータ生成時 ValueMax, ValueMin が 0の場合は 範囲指定されない Fixed(完了) 2010-07-28
Related to OpenPNE 3 - Backport(バックポート) #1470: opFormItemGenerator::generateValidator() の 数値用バリデータ生成時 ValueMax, ValueMin が 0の場合は 範囲指定されない Won't fix(対応せず) 2010-07-28
Related to OpenPNE 3 - Bug(バグ) #1478: OpenPNE2コンバーターでc_profileのval_maxおよびval_minの仕様の違いを吸収できていない Fixed(完了) 2010-08-04
Related to OpenPNE 3 - Bug(バグ) #940: プロフィール項目の日付やテキストの最小値を最大値より大きくして設定できてしまう Pending Fixing(修正待ち) 2010-04-05

Associated revisions

Revision b7c64b57 (diff)
Added by Shogo Kawahara about 9 years ago

fixed opFormItemGenerator doesn't set min or max option to sfValidateInteger when ValueMin or ValueMax is "0" (fixes #1449)

Revision 08f072dc (diff)
Added by Shogo Kawahara about 9 years ago

fixed MemberProfileForm for readability of code (fixes #1449)

Revision 0bc23d2c (diff)
Added by Shogo Kawahara about 9 years ago

fixed opFormItemGenerator::generateValidator() to check option values (fixes #1449)

History

#1 Updated by Shogo Kawahara about 9 years ago

  • Assignee set to Shogo Kawahara

#2 Updated by Shogo Kawahara about 9 years ago

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

更新履歴 b7c64b5705ffb2c44d5e3c99617d4270ede9f3a5 で適用されました。

#3 Updated by Minoru Takai about 9 years ago

本件には直接関係しませんが、気になったのでコメントしておきます。

MemberProfileForm.class.php の 56〜74 行目ですが、現在次のように書かれています。

      if (!$memberProfile)
      {
        if (null === $value['value'])
        {
          continue;
        }
        $memberProfile = new MemberProfile();
        $memberProfile->setMemberId($memberId);
        $memberProfile->setProfileId($profile->getId());
      }
      elseif (null === $value['value'])
      {
        if ($profile->isMultipleSelect())
        {
          $memberProfile->clearChildren();
        }
        $memberProfile->delete();
        continue;
      }

この部分は次のように記述した方が読みやすいかもしれません。

      if (null === $value['value'])
      {
        if ($memberProfile)
        {
          if ($profile->isMultipleSelect())
          {
            $memberProfile->clearChildren();
          }
          $memberProfile->delete();
        }
        continue;
      }
      if (!$memberProfile)
      {
        $memberProfile = new MemberProfile();
        $memberProfile->setMemberId($memberId);
        $memberProfile->setProfileId($profile->getId());
      }

#4 Updated by Shogo Kawahara about 9 years ago

  • Status changed from Pending Review(レビュー待ち) to Accepted(着手)

takai 差戻し扱いとします

#5 Updated by Minoru Takai about 9 years ago

0を含む制限値の範囲指定ができるように修正されていますが、 #837 で対応したような、仮に (min, max) = (8, 4) のような大小関係が不正な場合に制限をしないとった対応がされていません。

また、2系のバリデータは (0, 0) が (制限なし) を意味していましたが、3系のバリデータは範囲制限を行ないます。そこで、(0, 0) を例外的に制限なしと見なすような仕様にするかを検討する必要があります。この問題の抜本的な解決方法はコンバータを改良することです(2系からのコンバート時に(0, 0)のものを(NULL, NULL)として保存するようにする)。

(0, 0) 問題について #837 ではこの範囲制限は文字列長を意味し、空文字を入力した場合は「0文字長のテキストを入力した」のではなく「入力を省略した」と見なされる仕様を理由に、(0, 0) を (NULL, NULL) と見なすような例外処理を加えていますが、数値範囲の場合は (0, 0) は即ち整数 0 のみを受け付ける制約として意味があるので (NULL, NULL) と見なしてよいか検討する必要があります。

上記3段落の内容をまとめると、

  • (1) (min, max) = (8, 4) の範囲指定がされたままコンバートされると、不正な制約のせいで入力できなくなるので、(NULL, NULL) と見なすように修正しよう
  • (2) 範囲指定がされていないままコンバートされると、(0, 0)という制約が付いてしまうので、コンバータの仕様を改善しよう
  • (3) コンバータの仕様を直さないのであれば、コンバート後の (0, 0) が (NULL, NULL) を意味するようなバリデータの仕様を変更してよいか検討しよう(バリデータの仕様を変更する場合、管理画面から(min, max)を指定する際に(0, 0)が指定できるが、それが範囲指定なしを意味することになるので管理画面にも注意書きが必要になる)

ということになります。

#6 Updated by Shogo Kawahara about 9 years ago

to takai

  • (1) (min, max) = (8, 4) の範囲指定がされたままコンバートされると、不正な制約のせいで入力できなくなるので、(NULL, NULL) と見なすように修正しよう
  • (2) 範囲指定がされていないままコンバートされると、(0, 0)という制約が付いてしまうので、コンバータの仕様を改善しよう
  • (3) コンバータの仕様を直さないのであれば、コンバート後の (0, 0) が (NULL, NULL) を意味するようなバリデータの仕様を変更してよいか検討しよう(バリデータの仕様を変更する場合、管理画面から(min, max)を指定する際に(0, 0)が指定できるが、それが範囲指定なしを意味することになるので管理画面にも注意書きが必要になる)
  • (1)の件は修正します。

コンバーターの仕様を見直す方向に持って行こうと考えていますが、本件とはまたトピックが異なるため別チケットとします。

#8 Updated by Shogo Kawahara about 9 years ago

  • Status changed from Accepted(着手) to Pending Review(レビュー待ち)

更新履歴 08f072dc491468f3f8c5dae03c6e6b5094c0de5f で適用されました。

#9 Updated by Shogo Kawahara about 9 years ago

  • Status changed from Pending Review(レビュー待ち) to Rejected(差し戻し)

#10 Updated by Shogo Kawahara about 9 years ago

  • Status changed from Rejected(差し戻し) to Pending Review(レビュー待ち)

更新履歴 0bc23d2c278dd23948d97ca6186522bfac7ba7bb で適用されました。

#11 Updated by Rimpei Ogawa almost 9 years ago

  • 3.6 で発生するか set to Yes

#12 Updated by Kousuke Ebihara almost 9 years ago

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

問題ありません。

このチケットの確認中に now などの文字列が実際の日付に変換された上で DB に保存されてしまっているというバグを発見しました。これは実際には文字列として保存しておき、その文字列をバリデーションのために都度日付に直すのが正しい挙動です。このバグは #9304e1f2665 の変更により混入したものと思われます。

このチケットでは日付の比較にも関わる変更がなされているので微妙なところですが、別のバグチケットを切って対処することにしましょう。

#13 Updated by Kousuke Ebihara almost 9 years ago

Kousuke Ebihara は書きました:

問題ありません。

このチケットの確認中に now などの文字列が実際の日付に変換された上で DB に保存されてしまっているというバグを発見しました。これは実際には文字列として保存しておき、その文字列をバリデーションのために都度日付に直すのが正しい挙動です。このバグは #9304e1f2665 の変更により混入したものと思われます。

このチケットでは日付の比較にも関わる変更がなされているので微妙なところですが、別のバグチケットを切って対処することにしましょう。

この件についてのチケットを作成しました。 http://redmine.openpne.jp/issues/1595

#14 Updated by Yuma Sakata almost 8 years ago

  • Status changed from Pending Testing(テスト待ち) to Fixed(完了)
  • % Done changed from 70 to 100

テストOKです。

#15 Updated by Chiharu Nakajima almost 2 years ago

3.6対応済み( #1468 )
3.8発生せず

Also available in: Atom PDF