Backport（バックポート） #2294
完了
When a value of MemberConfig's "age_public_flag" is not exists, the label of "Settings" is inconsistent with behavior (MemberConfigの "age_public_flag" の値が存在しないとき、「設定変更」の表示と実際の挙動が異なる)
Yuma Sakata さんが13年以上前に追加.
13年以上前に更新.
説明
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 〜
関連するチケット
2 (0件未完了 — 2件完了)
- 担当者 を Shinichi Urabe にセット
コンフリクトの修正について
- Default:1は外部公開機能の c615f5d4f578043b1cdc3bcaa52e438793857f5a で混入されたもの （外部公開機能はないが、デフォルト値を設定する必要があるため、Default: 3 を設定）
- Member::getConfig() の第二引数にデフォルト値がセット出来るのは3.6の機能で 3.4には存在しないので、nullと結果を比較する対処した
- ステータス を New（新規） から Pending Review（レビュー待ち） に変更
- 進捗率 を 0 から 50 に変更
- ステータス を Pending Review（レビュー待ち） から Rejected（差し戻し） に変更
レビューしました。
修正内容としては問題が解消しているためこれで OK としても良いのですが、以下について修正可能であれば対応して頂きたいです。
差し戻し内容¶
2 個目の修正内容について¶
http://redmine.openpne.jp/issues/1872#note-7 では「（3.4 の場合は Member::getAge() 内に条件分岐を追加する）」と示されていますが、
$publicFlag = $this->getConfig('age_public_flag');
$publicFlag = null === $publicFlag ? ProfileTable::PUBLIC_FLAG_PRIVATE : $publicFlag;
if (!$viewableCheck || $publicFlag == ProfileTable::PUBLIC_FLAG_SNS)
この箇所で、このように条件演算子を用いるとこの部分の意味が分かりにくいので、 if 文で記述した方がよいと思います。
$publicFlag = $this->getConfig('age_public_flag');
if (null === $publicFlag)
{
$publicFlag = ProfileTable::PUBLIC_FLAG_PRIVATE;
}
if (!$viewableCheck || $publicFlag == ProfileTable::PUBLIC_FLAG_SNS)
3 個目の修正が取り込まれていません¶
親チケットでは 3 個のコミットが行なわれていますが、このチケットでは 3 個目のコミット（コーディング規約違反を修正する内容）が取り込まれていません。取り込む必要がないと判断したのであればそれでもよいですが、コミットを見逃している可能性があるので確認しておきます。参考 http://redmine.openpne.jp/issues/1872#note-19
上記の修正を行うか、修正する必要がないと判断した旨をコメントするかの対応をお願いします。
- ステータス を Rejected（差し戻し） から Pending Review（レビュー待ち） に変更
コーディング規約違反の修正を取り込みにあたり、正常系動作のみ再テストしました。
問題ありません。
- ステータス を Pending Review（レビュー待ち） から Pending Testing（テスト待ち） に変更
- 進捗率 を 50 から 70 に変更
note-6, note-7 での取り込みをレビューしました。
親チケット 3 個目のコミットの取り込みについて、親チケットでは 3 箇所が修正されていますが、本チケットでは 2 箇所しか修正されておらずコンフリクトが生じています。しかしながら、これは 3 箇所目の修正部分が #667 でのコミット c615f5d4 (3.5.0 時点) で追加されたものであるためであり、取り込みは問題ないと判断できます。
親チケット 2 個目のコミットで、条件演算子（三項演算子）ではなく if 文を用いたほうがよいという指摘に対する修正について、修正が適切に行なわれていると判断できます。
ちなみに note-6 のコミットメッセージを拝見しましたが、一般的に expr ? val : val の演算子は「条件演算子」または「三項演算子（条件演算子以外に三項の演算子がないため）」と呼ばれており、英語では Conditional Operator （条件演算子）または Ternary Operator （三項演算子 : see PHP Manual ）と表記されます。
レビュー OK とします。
- ステータス を Pending Testing（テスト待ち） から Fixed（完了） に変更
- 進捗率 を 70 から 100 に変更
他の形式にエクスポート: Atom
PDF
