プロジェクト

全般

プロフィール

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 Sakata7年以上前に追加. 7年以上前に更新.

ステータス:
Fixed(完了)
優先度:
High(高め)
担当者:
対象バージョン:
開始日:
2011-01-17
期日:
進捗率:

100%


説明

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 〜


関連するチケット

関連している OpenPNE 3 - 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" の値が存在しないとき、「設定変更」の表示と実際の挙動が異なる) Fixed(完了) 2011-01-17
関連している OpenPNE 3 - Backport(バックポート) #1881: When a value of MemberConfig's "age_public_flag" is not exists, the label of "Settings" is inconsistent with behavior (MemberConfigの "age_public_flag" の値が存在しないとき、「設定変更」の表示と実際の挙動が異なる) Fixed(完了) 2011-01-17 2011-06-24

関係しているリビジョン

リビジョン c5e9121e (差分)
Shinichi Urabe7年以上前に追加

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 (refs #2294, BP from #1872)

Conflicts:

lib/config/config/member_config.yml

リビジョン 1be0f847 (差分)
Shinichi Urabe7年以上前に追加

Member::getAge() now never handles null as age_public_flag value (refs #2294, BP from #1872)

Conflicts:

lib/model/doctrine/Member.class.php

リビジョン c977c64d
Shinichi Urabe7年以上前に追加

Merge branch 't2294' into stable-3.4.x (fixes #2294, BP from #1872)

リビジョン 895d027d (差分)
Kousuke Ebihara7年以上前に追加

fixed comparing ProfileTable::PUBLIC_FLAG_* to a value to follow conding standard in the Member::getAge() (fixes #2294, BP from #1872)

Conflicts:

lib/model/doctrine/Member.class.php

リビジョン ce11fcb0 (差分)
Shinichi Urabe7年以上前に追加

I changed the Clause 3 operator to the description not to use. (fixes #2294)

履歴

#1 Shinichi Urabe7年以上前に更新

  • 担当者Shinichi Urabe にセット

#2 Shinichi Urabe7年以上前に更新

コンフリクトの修正について

c5e9121ebf2c95bb1b2e でのコンフリクトについて

  • Default:1は外部公開機能の c615f5d4f578043b1cdc3bcaa52e438793857f5a で混入されたもの (外部公開機能はないが、デフォルト値を設定する必要があるため、Default: 3 を設定)

c5e9121ebf2c95bb1b2e でのコンフリクトについて

  • Member::getConfig() の第二引数にデフォルト値がセット出来るのは3.6の機能で 3.4には存在しないので、nullと結果を比較する対処した

#3 Shinichi Urabe7年以上前に更新

  • ステータスNew(新規) から Pending Review(レビュー待ち) に変更
  • 進捗率0 から 50 に変更

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

#4 Yuma Sakata7年以上前に更新

テストOKです。

#5 Minoru Takai7年以上前に更新

  • ステータス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


上記の修正を行うか、修正する必要がないと判断した旨をコメントするかの対応をお願いします。

#6 Shinichi Urabe7年以上前に更新

  • ステータスRejected(差し戻し) から Pending Review(レビュー待ち) に変更

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

#7 Kousuke Ebihara7年以上前に更新

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

#8 Yuma Sakata7年以上前に更新

コーディング規約違反の修正を取り込みにあたり、正常系動作のみ再テストしました。
問題ありません。

#9 Minoru Takai7年以上前に更新

  • ステータス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 とします。

#10 Yuma Sakata7年以上前に更新

  • ステータスPending Testing(テスト待ち) から Fixed(完了) に変更
  • 進捗率70 から 100 に変更

他の形式にエクスポート: Atom PDF