操作
Bug（バグ） #2478
未完了
プロフィール項目で「単一選択」の状態で保存したあとに「複数選択」の状態で保存を行おうとしても保存されていないように見える
Yuya Watanabe さんが約13年前に追加. 4年以上前に更新.
開始日:
2011-10-07
期日:
進捗率:
50%
予定工数:
(合計: 0:00時間)
3.6 で発生するか:
Unknown (未調査)
3.8 で発生するか:
Unknown (未調査)
説明
概要¶
プロフィール項目で「単一選択」の状態で保存したあとに「複数選択」の状態で保存を行おうとしても保存されていないように見える．
DBを見ると保存されているが，プロフィール閲覧画面（/member/profile）で閲覧しても「単一選択」の時に保存された内容のみが表示される．
再現手順¶
- 管理画面でプロフィール一覧画面から「プロフィール項目登録」をクリックする
- あるいはプロフィール項目登録画面（pc_backend.php/profile/edit）のページを開く
- 「フォームタイプ」を「単一選択（プルダウン）」を選択して「追加」をクリックする
- プロフィール項目一覧画面（pc_backend.php/profile/list）の「プロフィール選択肢一覧」で２つ以上の選択肢を保存する
- SNSのプロフィール編集画面（member/edit/profile）で先程追加した項目で任意の項目を選択して「送信」をクリックする
- 選んだ項目が保存されていることを確認する
- 管理画面で先程作ったプロフィール項目を「複数選択（チェックボックス）」を選択して「変更」をクリックする
- SNSのプロフィール編集画面で先程と同じ項目で２つ以上の項目にチェックを入れて「送信」をクリックする
- プロフィール閲覧画面（member/profile）において「単一選択（プルダウン）」で選択した項目のみが表示される
- DB上で確認すると「単一選択（プルダウン）」で保存されたデータと「複数選択（チェックボックス）」で保存されたデータ両方が存在している
確認環境¶
OpenPNE 3.7.0-dev (master)
原因¶
「複数選択（チェックボックス）」のデータ保存方法はツリー構造となっており，ルートとなるレコードを元にしてリーフのレコードが「複数選択（チェックボックス）」で選択した項目が保持される．
このとき，ルートとなるレコードが無い場合は新規に作成されてデータがないレコードがルートとして作成され，そのツリーのリーフにデータが保持されたレコードが作成される．そしてプロフィール項目表示時にはそのリーフとなるデータをすべて取得し，表示が行われる．
しかし，「単一選択」ですでにルートとなるレコードが存在する場合（つまりprofile_option_idがNULLでないデータが存在する場合）にはそのレコードにリーフが存在したとしても，ルートのレコードのデータのみが取得されて表示されるようになる．
「テキスト」->「複数選択」の場合もルートとなるレコードは「テキスト」のデータ保存時に作成されたレコードであるが，profile_option_idがNULLであるために本問題が発生しない状態であると考えられる．
修正方針¶
修正方針は２通り考えられる．- 「複数選択」で保存される場合にルートとなるレコードの情報を削除する
- プロフィール閲覧画面で表示する際にフォームタイプが「複数選択（チェックボックス）」であるプロフィール項目はツリー構造のリーフの部分を表示する
Yuya Watanabe さんが約13年前に更新
メモ¶
記録に関して関係ありそうな部分のコードを記載．
lib/action/opMemberAction.class.php
209 public function executeEditProfile($request)
210 {
211 $this->memberForm = new MemberForm($this->getUser()->getMember());
212
213 $profiles = $this->getUser()->getMember()->getProfiles();
214 $this->profileForm = new MemberProfileForm($profiles);
215 $this->profileForm->setConfigWidgets();
216
217 if ($request->isMethod('post'))
218 {
219 $this->memberForm->bind($request->getParameter('member'));
220 $this->profileForm->bind($request->getParameter('profile'));
221 if ($this->memberForm->isValid() && $this->profileForm->isValid())
222 {
223 $this->memberForm->save();
224 $this->profileForm->save($this->getUser()->getMemberId());
225 $this->redirect('@member_profile_mine');
226 }
227 }
228
229 return sfView::SUCCESS;
230 }
lib/form/doctrine/MemberProfileForm.class.php
43 public function save($memberId)
44 {
45 $values = $this->getValues();
46
47 foreach ($values as $key => $value)
48 {
49 $profile = Doctrine::getTable('Profile')->retrieveByName($key);
50 if (!$profile)
51 {
52 continue;
53 }
54
55 $memberProfile = Doctrine::getTable('MemberProfile')->retrieveByMemberIdAndProfileId($memberId, $profile->getId());
56
57 if (is_null($value['value']))
58 {
59 if ($memberProfile)
60 {
61 if ($profile->isMultipleSelect())
62 {
63 $memberProfile->clearChildren();
64 }
65 $memberProfile->delete();
66 }
67 continue;
68 }
69 if (!$memberProfile)
70 {
71 $memberProfile = new MemberProfile();
72 $memberProfile->setMemberId($memberId);
73 $memberProfile->setProfileId($profile->getId());
74 }
75
76 $memberProfile->setPublicFlag($memberProfile->getProfile()->getDefaultPublicFlag());
77 if (isset($value['public_flag']))
78 {
79 $memberProfile->setPublicFlag($value['public_flag']);
80 }
81 $memberProfile->save();
82
83 if ($profile->isMultipleSelect())
84 {
85 $ids = array();
86 $_values = array();
87 if ('date' === $profile->getFormType())
88 {
89 $_values = array_map('intval', explode('-', $value['value']));
90 $options = $profile->getProfileOption();
91 foreach ($options as $option)
92 {
93 $ids[] = $option->getId();
94 }
95 $memberProfile->setValue($value['value']);
96 }
97 else
98 {
99 $ids = $value['value'];
100 }
101 Doctrine::getTable('MemberProfile')->createChild($memberProfile, $memberId, $profile->getId(), $ids, $_values);
102 }
103 else
104 {
105 $memberProfile->setValue($value['value']);
106 }
107
108 $memberProfile->save();
109 }
110
111 return true;
112 }
lib/model/doctrine/MemberProfileTable.class.php
88 public function retrieveByMemberIdAndProfileId($memberId, $profileId)
89 {
90 return $this->createQuery()
91 ->where('member_id = ?', $memberId)
92 ->andWhere('profile_id = ?', $profileId)
93 ->fetchOne();
94 }
Yuya Watanabe さんが約13年前に更新
- ステータス を New（新規） から Accepted（着手） に変更
- 担当者 を Yuya Watanabe にセット
- 対象バージョン を OpenPNE 3.7.0 にセット
Yuya Watanabe さんが約13年前に更新
メモ¶
データ取得に関して関係ありそうなコードを記載．
lib/action/opMemberAction.class.php 213行目
209 public function executeEditProfile($request)
210 {
211 $this->memberForm = new MemberForm($this->getUser()->getMember());
212
213 $profiles = $this->getUser()->getMember()->getProfiles();
214 $this->profileForm = new MemberProfileForm($profiles);
lib/model/doctrine/Member.class.php
13 public function getProfiles($viewableCheck = false, $myMemberId = null)
14 {
15 if ($viewableCheck)
16 {
17 return Doctrine::getTable('MemberProfile')->getViewableProfileListByMemberId($this->getId(), $myMemberId);
18 }
19
20 return Doctrine::getTable('MemberProfile')->getProfileListByMemberId($this->getId());
21 }
lib/model/doctrine/MemberProfileTable.class.php
12 {
13 public function getProfileListByMemberId($memberId)
14 {
15 $profiles = Doctrine::getTable('Profile')->createQuery()
16 ->select('id')
17 ->orderBy('sort_order')
18 ->execute(array(), Doctrine::HYDRATE_NONE);
19
20 $queryCacheHash = '';
21
22 $q = $this->createQuery()
23 ->where('member_id = ?')
24 ->andWhere('profile_id = ?')
25 ->andWhere('level = 0')
26 ->orderBy('id');
27
28 $memberProfiles = array();
29 foreach ($profiles as $profile)
30 {
31 if ($queryCacheHash)
32 {
33 $q->setCachedQueryCacheHash($queryCacheHash);
34 }
35
36 $memberProfile = $q->fetchOne(array($memberId, $profile[0]));
37 if ($memberProfile)
38 {
39 $memberProfiles[] = $memberProfile;
40 }
41
42 if (!$queryCacheHash)
43 {
44 $queryCacheHash = $q->calculateQueryCacheHash();
45 }
46 }
47
48 // NOTICE: this returns Array not Doctrine::Collection
49 return $memberProfiles;
50 }
apps/pc_frontend/modules/member/templates/_profileListBox.php
28 $profileValue = (string)$profile;
lib/model/doctrine/MemberProfile.class.php
13 public function __toString()
14 {
15 if ('date' !== $this->getFormType())
16 {
17 if ($this->getProfileOptionId())
18 {
19 $option = Doctrine::getTable('ProfileOption')->find($this->getProfileOptionId());
20 return (string)$option->getValue();
21 }
22
23 $children = $this->getChildrenValues(true);
24 if ($children)
25 {
26 return implode(', ', $children);
27 }
28 }
29
30 return (string)$this->getValue();
31 }
44 public function getValue()
45 {
46 if ($this->_get('value_datetime'))
47 {
48 if ($this->_get('value'))
49 {
50 if ('0000-00-00 00:00:00' === $this->_get('value_datetime'))
51 {
52 return null;
53 }
54
55 $obj = new DateTime($this->_get('value_datetime'));
56 return $obj->format('Y-m-d');
57 }
58
59 return null;
60 }
61
62 if ($this->getProfile()->isPreset())
63 {
64 if ('op_preset_birthday' === $this->getProfile()->getName())
65 {
66 return null;
67 }
68
69 return $this->_get('value');
70 }
71 elseif ('date' !== $this->getFormType() && $this->getProfileOptionId())
72 {
73 return $this->getProfileOptionId();
74 }
75
76 $children = $this->getChildrenValues();
77 if ($children)
78 {
79 if ('date' === $this->getFormType())
80 {
81 if (count($children) == 3 && $children[0] && $children[1] && $children[2])
82 {
83 $obj = new DateTime();
84 $obj->setDate($children[0], $children[1], $children[2]);
85 return $obj->format('Y-m-d');
86 }
87 return null;
88 }
89 return $children;
90 }
Yuya Watanabe さんが約13年前に更新
メモ¶
データベース内で木構造を表現する手法として「入れ子集合モデル」というものがあり，本チケットで扱われている複数項目のレコード記録方法としてこのモデルが使われているようである．
config/doctrine/schema.yml 145行目でNestedSetと定義されている
143 MemberProfile:
144 actAs:
145 NestedSet:
146 hasManyRoots: true
147 rootColumnName: tree_key
148 Timestampable:
149 columns:
150 id: { type: integer(4), primary: true, autoincrement: true, comment: "Serial number" }
151 member_id: { type: integer(4), notnull: true, comment: "Member id" }
152 profile_id: { type: integer(4), notnull: true, comment: "Profile id" }
153 profile_option_id: { type: integer(4), comment: "Profile option id" }
154 value: { type: string, default: "", notnull: true, comment: "Text content for this profile item" }
155 value_datetime: { type: timestamp, comment: "Profile datetime value" }
156 public_flag: { type: integer(1), comment: "Public flag" }
157 relations:
158 Member: { local: member_id, foreign: id, onDelete: cascade }
159 Profile: { local: profile_id, foreign: id, onDelete: cascade }
160 ProfileOption: { local: profile_option_id, foreign: id, onDelete: cascade }
161 indexes:
162 lft_INDEX:
163 fields: [lft]
164 options:
165 type: INNODB
166 collate: utf8_unicode_ci
167 charset: utf8
168 comment: "Saves informations of every member''s profile"
Yuya Watanabe さんが約13年前に更新
保存する際に既存の値を保持しておく必要はないと思うので，あるメンバがそのプロフィール項目ですでに保存した値がある場合はそのレコードを上書きするのではなく削除してあたらしく作りなおす方針を検討する．
このとき，$profile->isMultipleSelect()は管理画面で設定した値を用いているため，これから記録しようとするデータに対してこれを用いるのは問題ないが，DB内にあるデータを表していないので記録の前処理で行う削除時にisMultipleSelect()の値を用いるとデータに則さない処理を行なってしまう可能性があることに注意する必要がある．おそらく$memberProfile->getNode()->hasChildren()などで判定できると思われる．
Yuya Watanabe さんが約13年前に更新
note-6の修正案を適用した実装案について¶
実装案¶
diff --git a/lib/form/doctrine/MemberProfileForm.class.php b/lib/form/doctrine/MemberProfileForm.class.php
index 83bb56f..0ab1d03 100644
--- a/lib/form/doctrine/MemberProfileForm.class.php
+++ b/lib/form/doctrine/MemberProfileForm.class.php
@@ -54,25 +54,24 @@ class MemberProfileForm extends BaseForm
$memberProfile = Doctrine::getTable('MemberProfile')->retrieveByMemberIdAndProfileId($memberId, $profile->getId());
- if (is_null($value['value']))
+ if ($memberProfile)
{
- if ($memberProfile)
+ if ($memberProfile->getNode()->hasChildren())
{
- if ($profile->isMultipleSelect())
- {
- $memberProfile->clearChildren();
- }
- $memberProfile->delete();
+ $memberProfile->clearChildren();
}
- continue;
+ $memberProfile->delete();
}
- if (!$memberProfile)
+
+ if (is_null($value['value']))
{
- $memberProfile = new MemberProfile();
- $memberProfile->setMemberId($memberId);
- $memberProfile->setProfileId($profile->getId());
+ continue;
}
+ $memberProfile = new MemberProfile();
+ $memberProfile->setMemberId($memberId);
+ $memberProfile->setProfileId($profile->getId());
+
$memberProfile->setPublicFlag($memberProfile->getProfile()->getDefaultPublicFlag());
if (isset($value['public_flag']))
{
問題点¶
表面の動作的には問題ないが，プロフィールを更新するたびにDB側のmember_profileテーブルでプロフィール数+複数選択分のIDが消費されてゆく．
Yuya Watanabe さんが約13年前に更新
修正方針¶
テキスト，単一選択，複数選択それぞれのフォームタイプの場合の値が同時に保存されているようなので，フォームタイプに即した値を出力時に返すようにする．
実装案¶
diff --git a/lib/model/doctrine/MemberProfile.class.php b/lib/model/doctrine/MemberProfile.class.php
index a085613..a6368a4 100644
--- a/lib/model/doctrine/MemberProfile.class.php
+++ b/lib/model/doctrine/MemberProfile.class.php
@@ -14,16 +14,23 @@ class MemberProfile extends BaseMemberProfile implements opAccessControlRecordIn
{
if ('date' !== $this->getFormType())
{
- if ($this->getProfileOptionId())
+ if ($this->Profile->isMultipleSelect())
{
- $option = Doctrine::getTable('ProfileOption')->find($this->getProfileOptionId());
- return (string)$option->getValue();
+ $children = $this->getChildrenValues(true);
+ if ($children)
+ {
+ return implode(', ', $children);
+ }
+ else
+ {
+ return '';
+ }
}
-
- $children = $this->getChildrenValues(true);
- if ($children)
+ if ($this->Profile->isSingleSelect() && $this->getProfileOptionId())
{
- return implode(', ', $children);
+ $option = Doctrine::getTable('ProfileOption')->find($this->getProfileOptionId());
+
+ return (string)$option->getValue();
}
}
@@ -68,17 +75,27 @@ class MemberProfile extends BaseMemberProfile implements opAccessControlRecordIn
return $this->_get('value');
}
- elseif ('date' !== $this->getFormType() && $this->getProfileOptionId())
+ elseif ('date' !== $this->getFormType())
{
- return $this->getProfileOptionId();
+ if ($this->Profile->isMultipleSelect())
+ {
+ $children = $this->getChildrenValues();
+ if ($children)
+ {
+ return $children;
+ }
+ }
+ elseif ($this->Profile->isSingleSelect() && $this->getProfileOptionId())
+ {
+ return $this->getProfileOptionId();
+ }
}
-
- $children = $this->getChildrenValues();
- if ($children)
+ else
{
- if ('date' === $this->getFormType())
+ $children = $this->getChildrenValues();
+ if ($children)
{
- if (count($children) == 3 && $children[0] && $children[1] && $children[2])
+ if (3 =< count($children) && $children[0] && $children[1] && $children[2])
{
$obj = new DateTime();
$obj->setDate($children[0], $children[1], $children[2]);
@@ -86,7 +103,6 @@ class MemberProfile extends BaseMemberProfile implements opAccessControlRecordIn
}
return null;
}
- return $children;
}
return parent::rawGet('value');
問題点¶
この修正案の場合には日付がどのように記録されているかを考慮していなかった．
実際にDBを確認してみると，日付の値は年月日それぞれを子ノードの列valueに保存されていることを確認した．
しかし，年月日が保存されるかどうかは子ノードが存在しないまたは子ノードの数が３つ以上であるときのみ正しく記録されることを確認した．
つまり，複数選択によってすでに値がDBに記録されている場合，フォームタイプが日付の場合に正しく記録できない状態であると言える．
そのため，表示部分だけでなく同時に記録部分も修正する必要がある．
Yuya Watanabe さんが約13年前に更新
修正方針¶
DB内にはテキスト，単一選択，複数選択，日付の４つのフォームタイプの値を共存させつつ，取得時にはそのフォームタイプに従った値を返すように修正します．
現在の状態だと取得時にはDB内の値が存在するかどうかで返す値を決定しているので，複数のフォームタイプの値がDB内に記録されていると正しい値を返さないという問題を解決するためにnote-8のような修正を行います．
また，note-8の修正だけでは４つのフォームタイプの値を共存させるという条件を満たしていないため，正しく値を記録できるような実装を行います．
wa ta さんが約13年前に更新
- ステータス を Accepted（着手） から Pending Review（レビュー待ち） に変更
- 進捗率 を 0 から 50 に変更
更新履歴 f84facbf5ea0f7433881b5ccfd6735763ecf61bc で適用されました。
Minoru Takai さんが約13年前に更新
- ステータス を Pending Review（レビュー待ち） から Rejected（差し戻し） に変更
詳細まで追っていないので不具合が生じているというだけの指摘になりますが、次の問題が生じています。
- 管理画面からプロフィール項目に「日付」のものを追加
- メンバー側でプロフィール編集を開き、（特に値を変更せずに）保存する
- 次の Fatal Error が生じる
Fatal error: Call to a member function setValue() on a non-object in /path/to/OpenPNE/lib/form/doctrine/MemberProfileForm.class.php on line 99
lib/form/doctrine/MemberProfileForm.class.php 87- $children = $memberProfile->getNode()->getChildren(); 88- if ('date' === $profile->getFormType()) 89- { 90- foreach ($children as $child) 91- { 92- $child->setValue(null); 93- $child->save(); 94- } 95- $_values = array_map('intval', explode('-', $value['value'])); 96- $options = $profile->getProfileOption(); 97- foreach ($_values as $i => $value) 98- { 99- $children[$i]->setValue($value); 100- $children[$i]->save(); 101- } 102- } 103- else
Yuya Watanabe さんが約12年前に更新
- 対象バージョン を OpenPNE 3.8.x から OpenPNE 3.9.0-old に変更
- 3.6 で発生するか を Unknown (未調査) にセット
- 3.8 で発生するか を Unknown (未調査) にセット
Shinichi Urabe さんが7年以上前に更新
最初に登録したフォームタイプを編集で変更できない仕様としてもいいように思いますが、どうでしょうか。
→ フォームタイプが違うプロフィールに変更したい場合は、削除して追加する仕様とする
操作