Bug（バグ） #2425
プロフィール編集画面にて、プロフィール項目で重複した値が登録できる
100%
説明
Overview (現象)¶
プロフィール編集画面にて、プロフィール項目で重複した値が登録できる
Environment (再現環境)¶
OpenPNE 3.6RC1
Way to repro (再現手順)¶
1. 管理画面プロフィール項目登録ページ(/pc_backend.php/profile/edit)にアクセスする
2. 重複の可否を「表示する」、入力値タイプを「数値」に設定してプロフィール項目を作成する
3. メンバーAでSNSログイン後、プロフィール編集ページ(/member/edit/profile)にアクセスする
4. 手順2で作成したプロフィール項目に数値を入力する(入力した数値を控えておく)
5. メンバーBでSNSログイン後、プロフィール編集ページ(/member/edit/profile)にアクセスする
6. 手順2で作成したプロフィール項目に、手順4で入力した数値を入力する
7. 重複する値が登録できる。
Way to fix (修正内容)¶
プロフィール項目で重複した値が登録できないように修正お願いします。
関連するチケット
関係しているリビジョン
(fixes #2425) validate duplicated value
(refs #2425) fixed for coding standard
(fixes #2425) optimize DQL and rename method
(refs #2425) fixed i18n
(fixes #2425) add help to unique setting
(fixes #2425) fixed for coding standard
(fixes #2425) fixed to make check duplication in the case of 'text' or 'textarea'
(fixes #2425) fixed invalid input type
(fixes #2425) validate duplicated value
(refs #2425) fixed for coding standard
(fixes #2425) optimize DQL and rename method
(refs #2425) fixed i18n
(fixes #2425) add help to unique setting
(fixes #2425) fixed for coding standard
(fixes #2425) fixed to make check duplication in the case of 'text' or 'textarea'
(fixes #2425) fixed invalid input type
履歴
#1
Mutsumi Imamura が12年以上前に更新
- 対象バージョン を OpenPNE 3.7.0 にセット
- 360対象 を 3.6.0 にセット
#2
Shingo Yamada が12年以上前に更新
- 担当者 を Yuya Watanabe にセット
#3
Yuya Watanabe が12年以上前に更新
- ステータス を New（新規） から Accepted（着手） に変更
#4
wa ta が12年以上前に更新
- ステータス を Accepted（着手） から Pending Review（レビュー待ち） に変更
- 進捗率 を 0 から 50 に変更
更新履歴 bd5b7b04d6ff81746b63a6966a94985b93ad11c5 で適用されました。
#5
Kousuke Ebihara が12年以上前に更新
- ステータス を Pending Review（レビュー待ち） から Rejected（差し戻し） に変更
- グローバルな i18n/messages.ja.xml において、 Duplicate という一語に対して「すでに登録されています」という訳語を当てるのはどうかなと思います（別のカタログが使用されているのであればまだ理解できますが）。たとえば、「Your inputted value is already registered (もしくは動詞 exist を使う)」や「You've inputted duplicated value」などとするわけにはいかないでしょうか。ちなみに、 sfValidatorDoctrineUnique のデフォルトのエラーメッセージは「An object with the same "%column%" already exist.」です。
- MemberProfileTable::retrieveByProfileIdAndValueExceptMemberId() で発行されるクエリの NOT IN に指定される id は 1 件のみなので、普通に member_id <> ? と書くのがよいと思います。もっとも、この程度のクエリであれば MySQL 側で最適化するかもしれませんが……
- 重複確認をおこなう目的であれば、入力値に合致するプロフィールの値を持つ id をすべて検索する必要はないはずです。また、これらの結果をハイドレートする必要はないはずです。ですので、 MemberProfileTable::retrieveByProfileIdAndValueExceptMemberId() を以下のように変更してみてはどうでしょうか。
- fetchArray() ではなく fetchOne() を使用する (暗黙的に DQL に "LIMIT 1" が追加される)
- fetchOne() の第二引数に Doctrine_Core::HYDRATE_NONE を指定し、ハイドレートをおこなわないようにする
- このメソッドの返り値を bool にし、メソッド名を MemberProfileTable::checkDuplicatedProfileValue() などとする (bool を返すのであれば isExist... とか has... とかのほうが適当か？)
#6
wa ta が12年以上前に更新
- ステータス を Rejected（差し戻し） から Pending Review（レビュー待ち） に変更
更新履歴 74305381b7d2487e7ec770e93273c4195c956ff4 で適用されました。
#7
Yuya Watanabe が12年以上前に更新
以下の点を修正しました．
- 「すでに登録されています」の訳を「Your inputted value is already exist」とする
- メッセージ末尾に「。」および「.」を付与する
- 重複したプロフィールが存在するかどうかを得るメソッド名を変更して返り値をboolになるようにする
- 検索結果数を1つにしてハイドレートを行わないようにする
#8
Kousuke Ebihara が12年以上前に更新
- ステータス を Pending Review（レビュー待ち） から Pending Testing（テスト待ち） に変更
- 進捗率 を 50 から 70 に変更
OK です。
fetchOne() の第一引数に空配列ではなく null を指定していることについて、妥当かどうか熟考しましたが、
- この第一引数は内部で array 型へのキャストをしたうえで用いられる。 null を array 型にキャストした場合は空配列が得られるので事実上等価（ただしこの型キャストの前に第一引数を利用した場合の結果は異なる場合がありうる）
- fetchOne() など DQL 実行系のメソッドで、バインドする値の数が 1 つのみである場合、配列ではなくスカラー値を指定しているケースはかなり存在していると思われる。これはこの第一引数が array 型にキャストされることを想定した使われ方である
ことから、特に問題はないだろうと判断しました。
#9
Fumie Toyooka が12年以上前に更新
- ステータス を Pending Testing（テスト待ち） から Rejected（差し戻し） に変更
- 進捗率 を 70 から 50 に変更
重複登録できてしまいます。（プルダウンメニューでの重複禁止というと、プルダウンメニュー項目がメンバーの数だけ存在せねばならず、現実的ではないと思いますが。）テスト手順は以下のとおりです。
1. 管理画面プロフィール項目登録ページ(/pc_backend.php/profile/edit)にアクセスする 2. 重複の可否を「禁止」にする 3. フォームタイプを「単一選択(プルダウン)」に設定してプロフィール項目を作成する 4. プロフィール選択肢を複数作成する。 5. メンバーAでSNSログイン後、プロフィール編集ページ(/member/edit/profile)にアクセスする 6. 手順2,3で作成したプロフィール項目で手順4で作成した項目を選択する 7. 招待メールを新規のユーザーに送る。(frontend,backend問わない) 7. 新規ユーザーでメールにあるURLからプロフィール登録ページにアクセスする 8. 手順2で作成したプロフィール項目に、手順6で選択した値を選択し、送信ボタンを押す
間違えて記述してしまいましたので、この部分は無視してください（2011/10/07）
#10
Yuya Watanabe が12年以上前に更新
- ステータス を Rejected（差し戻し） から Accepted（着手） に変更
#11
Yuya Watanabe が12年以上前に更新
note-9について¶
Fumie Toyooka は書きました:
重複登録できてしまいます。（プルダウンメニューでの重複禁止というと、プルダウンメニュー項目がメンバーの数だけ存在せねばならず、現実的ではないと思いますが。）テスト手順は以下のとおりです。
[...]
数値文字列内に改行が入るためか、frontend の方で「整数ではありません。」と表示されます。
プロフィール項目の「フォームタイプ」を「単一選択（プルダウン）」とした場合に重複登録が可能であることを確認しました．
しかし，重複登録が成功するため「整数ではありません。」という表示はされず，確認できませんでした．
以降では『プロフィール項目の「フォームタイプ」を「単一選択（プルダウン）」とした場合に重複登録が可能である』という問題について議論を行います．
「フォームタイプ」を「単一選択（プルダウン）」とした場合に重複登録が可能な原因¶
MemberProfileTable::isDuplicatedProfileValue()ではmember_profileテーブルのvalueについてのみ重複しているかどうかを判定しています．
しかし，「フォームタイプ」を「単一選択（プルダウン）」とした場合にはmember_profileテーブルのprofile_option_idが更新され，valueの列にはデータは格納されないという状態になっていました．同様に，「フォームタイプ」が選択式および日付の場合に同様の現象が発生することを確認しました．
#12
Yuya Watanabe が12年以上前に更新
修正方針（追記）¶
選択式及び日付については，重複不可の設定を反映するとnote-9で言及されているようにプルダウンメニュー項目数とメンバ数によってはプロフィールが編集不可という状態になりえます．また，複数選択可能な状態での重複不可はどうするのかという点については議論の余地が存在すると思います．そのため，本チケットでは重複可否の設定を反映しない方針とします．
対応方針としては重複可否の設定項目には「フォームタイプがテキストの場合にのみ適用されます」という旨の文章を追加します．
また，この時点では「数値文字列内に改行が入るためか、frontend の方で「整数ではありません。」と表示されます。」という問題について確認できないため対応をしないことします．
#13
wa ta が12年以上前に更新
- ステータス を Accepted（着手） から Pending Review（レビュー待ち） に変更
更新履歴 096bf2065cc5a82e1123bc58cc6dbef7f47d3542 で適用されました。
#14
wa ta が12年以上前に更新
更新履歴 66dea25693b91703e003538d6d0d38e4807b67f3 で適用されました。
#15
Kousuke Ebihara が12年以上前に更新
以下のような問題があることを疑っています（まだ確認はしていません。追って調査する予定です）。
- 重複の可否を「許可」にした、フォームタイプがテキストのプロフィール項目を作成し、 2 名以上のメンバーで重複する入力値をプロフィールとして設定したあと、重複の可否を「禁止」かつフォームタイプを「単一選択（プルダウン）」等に設定した場合に、プロフィールの更新がおこなえなくなるのではないか
#16
Yuya Watanabe が12年以上前に更新
- ステータス を Pending Review（レビュー待ち） から Accepted（着手） に変更
問題¶
note-15で懸念された問題は発生しませんでしたが，以下の問題が発生する可能性があることがわかりました．
問題１ 重複不可，「単一選択（プルダウン）」の状態のとき，選択可能なprofile_option_idを「テキスト」等で入力されている場合にプルダウンメニューでそのIDに相当する項目を保存しようとすると「すでに登録されています」と表示され保存できない
問題２ 重複不可，「複数選択（チェックボックス）」の状態のとき，Internal Server Errorが発生する
それぞれの問題について以下に記述します．
問題１の原因¶
下記箇所において$value['value']が返す値がプルダウン等の単一選択の場合に，選択されたprofile_options_idが入っているため，その値をmember_profileテーブルのvalueの値を検索してしまうため，問題１が発生してしまう状態になっています．
lib/validator/opValidatorProfile.class.php 55行目
50 if ($this->profile->getIsUnique())
51 {
52 $profileId = $this->profile->getId();
53 $memberId = sfContext::getInstance()->getUser()->getMemberId();
54
55 $isDuplicate = Doctrine::getTable('MemberProfile')->isDuplicatedProfileValue($profileId, $value['value'], $memberId);
56 if ($isDuplicate)
57 {
58 throw new sfValidatorError($this, 'The inputted value is already exist.');
59 }
60 }
問題２の原因¶
問題１と同じ場所で$value['value']の値が配列で渡されるためDQLのエラーが発生してしまう状態になっています．具体的には以下のエラーメッセージが得られます．
SQLSTATE[HY093]: Invalid parameter number: number of bound variables does not match number of tokens
修正方針¶
lib/validator/opValidatorProfile.class.php 50行目で$this->profile->isUnique()がtrueの場合のみに重複判定をしてますが，これをnote-12の方針通り，「テキスト」および「テキスト（複数行）」のみ実行するように修正を行います．
#17
Yuya Watanabe が12年以上前に更新
実装案¶
diff --git a/lib/validator/opValidatorProfile.class.php b/lib/validator/opValidatorProfile.class.php
index 5609f03..91bcaba 100644
--- a/lib/validator/opValidatorProfile.class.php
+++ b/lib/validator/opValidatorProfile.class.php
@@ -47,7 +47,7 @@ class opValidatorProfile extends sfValidatorBase
$clean['public_flag'] = $validator->clean($value['public_flag']);
}
- if ($this->profile->getIsUnique())
+ if ($this->profile->getIsUnique() && in_array($this->profile->getFormType(), array('text', 'textarea')))
{
$profileId = $this->profile->getId();
$memberId = sfContext::getInstance()->getUser()->getMemberId();
#18
wa ta が12年以上前に更新
- ステータス を Accepted（着手） から Pending Review（レビュー待ち） に変更
更新履歴 3cddf475eaa9ca80a936c91b1b7364b3874651a4 で適用されました。
#19
Kousuke Ebihara が12年以上前に更新
- ステータス を Pending Review（レビュー待ち） から Pending Testing（テスト待ち） に変更
- 進捗率 を 50 から 70 に変更
#20
Mutsumi Imamura が12年以上前に更新
- ステータス を Pending Testing（テスト待ち） から Rejected（差し戻し） に変更
- 進捗率 を 70 から 50 に変更
- フォームタイプはテキスト
- 入力値タイプは文字列
でプロフィール項目を新規作成し、プロフィール編集でメンバーAとBそれぞれ同じ文字列を入力し編集ボタンを押すと編集が完了できてしまいます。
#21
wa ta が12年以上前に更新
- ステータス を Rejected（差し戻し） から Pending Review（レビュー待ち） に変更
更新履歴 854613196b5bf1e54b28d25b8c32b7a00e6eff39 で適用されました。
#22
Yuya Watanabe が12年以上前に更新
#23
Kousuke Ebihara が12年以上前に更新
- ステータス を Pending Review（レビュー待ち） から Pending Testing（テスト待ち） に変更
- 進捗率 を 50 から 70 に変更
#24
Yuma Sakata が12年以上前に更新
- ステータス を Pending Testing（テスト待ち） から Fixed（完了） に変更
- 進捗率 を 70 から 100 に変更
テストOKです。
#25
Yuya Watanabe が約12年前に更新
- 3.6 で発生するか を Unknown (未調査) にセット
- 3.4 で発生するか を Unknown (未調査) にセット
本チケットのコミットは以下のものです．
https://redmine.openpne.jp/projects/op3/repository/revisions/bd5b7b04d6ff81746b63a6966a94985b93ad11c5
https://redmine.openpne.jp/projects/op3/repository/revisions/42eb6859bea4d4ef3928ff274eb02b6b582cf430
https://redmine.openpne.jp/projects/op3/repository/revisions/74305381b7d2487e7ec770e93273c4195c956ff4
https://redmine.openpne.jp/projects/op3/repository/revisions/1c9e4cfcd90a73d1c4121e3e90215af2bd0bfaab
https://redmine.openpne.jp/projects/op3/repository/revisions/096bf2065cc5a82e1123bc58cc6dbef7f47d3542
https://redmine.openpne.jp/projects/op3/repository/revisions/66dea25693b91703e003538d6d0d38e4807b67f3
https://redmine.openpne.jp/projects/op3/repository/revisions/3cddf475eaa9ca80a936c91b1b7364b3874651a4
下記チケット群は release-3.8beta1 でコミットされたものです．
https://redmine.openpne.jp/projects/op3/repository/revisions/9bbd4c1cb046e4aabfa3fa6804c2a0d3779df91c
https://redmine.openpne.jp/projects/op3/repository/revisions/ac394cc28a660f1e659a5ce2690087b84a7d09d1
https://redmine.openpne.jp/projects/op3/repository/revisions/e2aa49e6863a6c13b7cb383b0beebdae855693d2
https://redmine.openpne.jp/projects/op3/repository/revisions/57d09825d55e402cc72d17ef1d4f70f3fc5208fd
https://redmine.openpne.jp/projects/op3/repository/revisions/9149cba3836f86cf44a3841f61ed216e1a3579ae
https://redmine.openpne.jp/projects/op3/repository/revisions/55f906f5876f2ae38dcbba2e2fe8b9c578eeef57
https://redmine.openpne.jp/projects/op3/repository/revisions/e3bcf4224699318ca79e1de3fde0dfbf1601209b
https://redmine.openpne.jp/projects/op3/repository/revisions/32f9d0fa1271f37e14614ad6b25bd3667f23b94a
