Bug（バグ） #1872
When a value of MemberConfig's "age_public_flag" is not exists, the label of "Settings" is inconsistent with behavior (MemberConfigの "age_public_flag" の値が存在しないとき、「設定変更」の表示と実際の挙動が異なる)
100%
Description
Overview¶
When a value of MemberConfig's "age_public_flag" is not exists, the caption of "Settings" is inconsistent with behavior.
MemberConfigの "age_public_flag" の値が空のとき、
「設定変更」での表示は「全員に公開」と表示されているのにも関わらず、
実際は「非公開」として取り扱われる。
非常に紛らわしい状況になっている。
Causes¶
lib/config/config/member_config.yml の age_public_flag
の Default が "1" になっている。
Way to fix¶
Default を 3 (private) としておくべき。
(表示の方を現状の挙動に合わせる。
これは、今までリリースしてきた各安定版バージョンが
存在するからである。)Environment¶
OpenPNE3.4.x 〜
Related issues
Associated revisions
fixed the configuration of member_config, the label of settings is inconsistent with behavior when a value of MemberConfig's "age_public_flag" isn't exists (fixes #1872)
Member::getAge() now never handles null as age_public_flag value (fixes #1872)
fixed the configuration of member_config, the label of settings is inconsistent with behavior when a value of MemberConfig's "age_public_flag" isn't exists (fixes #1872)
Member::getAge() now never handles null as age_public_flag value (fixes #1872)
fixed the configuration of member_config, the label of settings is inconsistent with behavior when a value of MemberConfig's "age_public_flag" isn't exists (fixes #1872)
(cherry picked from commit a5bafb22ac71a02ef407bb0356ba14e93c7c2cff)
Member::getAge() now never handles null as age_public_flag value (fixes #1872)
(cherry picked from commit 9bcca0767963f8b32f43d82d2118404bd8795858)
fixed comparing ProfileTable::PUBLIC_FLAG_* to a value to follow conding standard in the Member::getAge() (fixes #1872)
History
#1
Updated by Shogo Kawahara over 13 years ago
Updated by - Subject changed from MemberConfigの "age_public_flag" の値が空のとき、「設定変更」の表示と実際の挙動が異なる to MemberConfigの "age_public_flag" の値が存在しないとき、「設定変更」の表示と実際の挙動が異なる
#2
Updated by Shogo Kawahara over 13 years ago
- Subject changed from MemberConfigの "age_public_flag" の値が存在しないとき、「設定変更」の表示と実際の挙動が異なる to When a value of MemberConfig's "age_public_flag" is not exists, the caption of "Settings" is inconsistent with behavior (MemberConfigの "age_public_flag" の値が存在しないとき、「設定変更」の表示と実際の挙動が異なる)
#3
Updated by Shogo Kawahara over 13 years ago
- Subject changed from When a value of MemberConfig's "age_public_flag" is not exists, the caption of "Settings" is inconsistent with behavior (MemberConfigの "age_public_flag" の値が存在しないとき、「設定変更」の表示と実際の挙動が異なる) to When a value of MemberConfig's "age_public_flag" is not exists, the label of "Settings" is inconsistent with behavior (MemberConfigの "age_public_flag" の値が存在しないとき、「設定変更」の表示と実際の挙動が異なる)
#4
Updated by Shogo Kawahara over 13 years ago
- Status changed from New（新規） to Accepted（着手）
- Assignee set to Shogo Kawahara
#5
Updated by Shogo Kawahara over 13 years ago
- Status changed from Accepted（着手） to Pending Review（レビュー待ち）
- % Done changed from 0 to 50
更新履歴 23d59234e0000215fe3e252d7db1418200e8f77a で適用されました。
#6
Updated by Masato Nagasawa over 13 years ago
Updated by - Status changed from Pending Review（レビュー待ち） to Pending Testing（テスト待ち）
- % Done changed from 50 to 70
#7
Updated by Kousuke Ebihara over 13 years ago
Updated by - Status changed from Pending Testing（テスト待ち） to Rejected（差し戻し）
- % Done changed from 70 to 50
再度レビューしました。差し戻します。
age_public_flag が空の時に年齢が非公開になるのは Member::getAge() が「公開範囲が ProfileTable::PUBLIC_FLAG_SNS や ProfileTable::PUBLIC_FLAG_FRIEND や ProfileTable::PUBLIC_FLAG_WEB であれば公開可能かどうかの検証をし、検証に失敗するか公開範囲がそれ以外の値の場合はすべて null を返す」という実装になっているからであり、この挙動は非常に偶然性の強いものです。この挙動がトラブルを引き起こす可能性としては、たとえば、今後、「『公開範囲が ProfileTable::PUBLIC_FLAG_PRIVATE であった場合に限定した挙動』が追加された場合に age_public_flag が空だと意図どおりに動作しなくなる」といったことが考えられます。このような変更を加えるにあたり、 Member::getAge() の実装を見て「現状『公開しない』として扱われる age_public_flag の値は ProfileTable::PUBLIC_FLAG_PRIVATE の他に null があるので、 null の場合の考慮もしなければならない」と判断するのは難しいことでしょう。もちろん、「公開範囲が ProfileTable::PUBLIC_FLAG_PRIVATE であった場合」という条件を加えることは危険でありそのような修正は避けたほうが無難ですが、今後混入する可能性が否定出来ない以上、この偶然に頼り切るのは危険です。
したがって、 Member::getAge() 側で「age_public_flag が空の場合の公開範囲が明確に ProfileTable::PUBLIC_FLAG_PRIVATE として扱われるようにする」ことも必要なのではないでしょうか。
具体的な対応方法のひとつとしては、 Member::getConfig() 側で、 DB の設定値が取得できなかった場合に設定ファイルを確認し、そちらで指定されているデフォルト値を返すようにするということが考えられます。ただし影響範囲が膨大となるため、それが現実的に厳しそうであれば、 Member::getAge() における Member::getConfig() のコール時に、第二引数の値として ProfileTable::PUBLIC_FLAG_PRIVATE を与える（3.4 の場合は Member::getAge() 内に条件分岐を追加する）ようにするのがよいかと思います。
#8
Updated by Kousuke Ebihara over 13 years ago
- Assignee changed from Shogo Kawahara to Kousuke Ebihara
http://redmine.openpne.jp/issues/1872#note-7 で指摘した事項の対応について引き受けます。
#9
Updated by Kousuke Ebihara over 13 years ago
- Status changed from Rejected（差し戻し） to Pending Review（レビュー待ち）
更新履歴 22b4c0009546efaa9728d199a2676f38f0e34fc9 で適用されました。
#10
Updated by Kousuke Ebihara over 13 years ago
更新履歴 9bcca0767963f8b32f43d82d2118404bd8795858 で適用されました。
#11
Updated by Shogo Kawahara over 13 years ago
更新履歴 a5bafb22ac71a02ef407bb0356ba14e93c7c2cff で適用されました。
#12
Updated by Kousuke Ebihara over 13 years ago
すいません、 a5bafb22ac71a02ef407bb0356ba14e93c7c2cff, 9bcca0767963f8b32f43d82d2118404bd8795858 のコミットは stable-3.6.x への取り込みの際にひもづけるチケット番号を変更し忘れたものです
#13
Updated by Naoya Tozuka over 13 years ago
Updated by - Status changed from Pending Review（レビュー待ち） to Rejected（差し戻し）
修正点確認しました。問題ないと思われます。
一点だけ、お手数ですが、今回の lib/model/doctrine/Member.class.php の修正箇所の後で、$publicFlag と定数が比較されている部分について、$publicFlag が左辺に来ているのがコーディング規約に反しますので、直してもらっても宜しいでしょうか？
メモ¶
age_public_flag が参照されている箇所は他に- apps/mobile_frontend/modules/member/templates/profileSuccess.php
- apps/pc_frontend/modules/member/templates/_profileListBox.php
がありますが、どちらも ProfileTable::PUBLIC_FLAG_FRIEND との比較であり、本チケットのような問題は発生しないため現状のままとします。
#14
Updated by Kousuke Ebihara over 13 years ago
- Status changed from Rejected（差し戻し） to Pending Review（レビュー待ち）
更新履歴 63447f82eb12ec53addfa54776d7f7ccb091b8ce で適用されました。
#15
Updated by Naoya Tozuka over 13 years ago
- Status changed from Pending Review（レビュー待ち） to Pending Testing（テスト待ち）
- % Done changed from 50 to 70
確認しました。OKです。
#16
Updated by Kousuke Ebihara over 13 years ago
- Status changed from Pending Testing（テスト待ち） to Pending Review（レビュー待ち）
- % Done changed from 70 to 50
更新履歴 af98fa6c7949a39e860e2c24b360659bb97ffdf1 で適用されました。
#17
Updated by Shogo Kawahara over 13 years ago
更新履歴 30842fc56aa405ea0665938efdf893166d95c2c4 で適用されました。
#18
Updated by Naoya Tozuka over 13 years ago
- Status changed from Pending Review（レビュー待ち） to Pending Testing（テスト待ち）
- % Done changed from 50 to 70
#19
Updated by Minoru Takai about 13 years ago
Updated by このチケットではコミットメッセージ中のコミット番号が誤っていることと、3.6系において stable-3.6.x ブランチと 3.6beta11 ブランチで2回取り込まれていることにより、コミットがどのブランチに紐付くのか分かりにくくなっています。
このチケットでは master ブランチ向けに (1), (2), (3) の3個のコミットが行なわれていることを示しておきます。
#20
Updated by Yuma Sakata almost 13 years ago
Updated by - Status changed from Pending Testing（テスト待ち） to Fixed（完了）
- % Done changed from 70 to 100
テストOKです。
#21
Updated by kaoru n about 9 years ago
Updated by - 3.8 で発生するか set to Unknown (未調査)