OpenPNE3 Plugins » opRankingPlugin

コミュニティのランキングは、
メンバーの参加人数から結果を算出していますが、
今回の問題は community_member に存在しない
community_id が含まれていたことによって発生したものと考えられます。
本来であればcommunityが削除される際に
リレーション情報に基づいて community_member のレコードも削除されるはずですが、
何らかの原因によってレコードが残ってしまっていたと考えられます。

ソース上で存在しないコミュニティidの対応は可能なので、対応を行います。

https://github.com/nothan/opRankingPlugin/commit/43218a74d4156f632d290442bc39e63359ce4978 にて対応しました。

OpenPNE-3.6.0 にバンドルするプラグインとして、対応されたチケットをレビューします。

このプラグインはもともと長澤氏（github.com/nothan）の元で管理されていましたが、現在は github.com/tejimaya で管理されています。次のコミットを確認します。 https://github.com/tejimaya/opRankingPlugin/commit/43218a74d4156f632d290442bc39e63359ce4978

https://github.com/tejimaya/opRankingPlugin/commit/43218a74d4156f632d290442bc39e63359ce4978

この修正についてレビューしました。

• (1) まず、この修正後には、修正前に生じていた Fatal error は解消されていると思われます。
• (2) この修正後には、 Fatal error は解消されていますが、「参加人数No1コミュニティ」で表示されるアイテムについて、順位が飛び飛びになる（1位の次に3位が表示されるなど）可能性があるかもしれません。これは http://redmine.openpne.jp/issues/2310#note-10 で指摘されています。
• (3) !$community->getId() という条件式がありますが、ここで取得している $community に対して getId() の返り値の真偽値を確認している意図がよく分かりません。 Community のオブジェクトが取得できていれば getId() の値は常に取得でき、その値は常に真になるはずです。 http://redmine.openpne.jp/issues/2310#note-8 でも同じ内容に関する指摘がされています。
  • しかし、この記述が残ったままであっても、無意味な記述があるだけで動作には影響しません。
  • 補足： $community の真偽値が真であることをチェックした後に $community->getId() が真であることをチェックする必要がある場面は、 Community クラスに関して Null Object パターンを採用している場合が考えられます。例えば http://redmine.openpne.jp/issues/1944#note-22 ではそのような記述が有効な場合について記述しています。しかし Null Object パターンを実装している場合でも Null Object かどうかを判断するためのメソッドを記述した方が見通しがよいはずで、 getId() の値を真偽値として扱うのは好ましくないと思います。

関連する問題は #2310 で修正されており、そこでは (1) と (3) が修正されています。このチケットでは (1) のみが修正されています。

#2310 と同じ判断基準でよければ (2) については（仮に問題が発生していても別チケット扱いでよいため）ここでは扱わず、(3) については修正が必須なものではない（無駄な記述があるだけ）と判断できるため、このチケットでの修正内容はこのままで問題なしと判断します。

なお、加えられた修正に関してエンバグしていないかなどをチェックしましたが、このチケットに対する修正として適切かどうかはチェックしていません（妥当そうだというチェックをしたまでです）。場合によっては他にも修正すべき箇所があるといったことがあるかもしれません。バグが直っているかどうかについてはテストのフェーズできちんと確認してください。

レビューOKとします。

テストOKです。