プロジェクト

全般

プロフィール

Bug(バグ) #3588

スマートフォンからコミュニティ画像を変更すると画像だけのコミュニティが作成される

isao sanoほぼ10年前に追加. 約7年前に更新.

ステータス:
Won't fix(対応せず)
優先度:
High(高め)
担当者:
対象バージョン:
開始日:
2014-04-09
期日:
進捗率:

0%

3.6 で発生するか:
No (いいえ)
3.8 で発生するか:
Yes (はい)

説明

概要

スマートフォンからコミュニティ画像を変更すると、参加者、コミュニティ名、説明文のないコミュニティが作成される。
このコミュニティは管理者が存在しないため削除を行うことは出来ない。

なお、スマートフォン版のコミュニティトップページには「コミュニティ編集」の導線はないため、直接URLを指定して編集を行った。
そもそもスマートフォンで「コミュニティ編集」画面に遷移できる事自体に問題があるのかもしれない。

再現環境

  • OpenPNE 3.8.10
  • OpenPNE 3.8.11

opCommunityTopicPlugin の有無は関連しない

再現手順

1. スマートフォンより任意のコミュニティAの管理者でログイン
2. コミュニティAのコミュニティ編集ページのURL(/community/edit?id=XX)を指定する
3. 写真を選択し、編集ボタンを押下
4. コミュニティAの画像は変更されない
5. 画像だけのコミュニティが作成されている

画像だけコミュニティ.png 表示 (75.1 KB) isao sano, 2014-04-09 13:52

スクリーンショット 2014-05-26 16.11.58.png 表示 (620 KB) Akihiro KOBAYASHI, 2014-05-26 16:27

t-3588.patch 表示 (3.47 KB) Shinichi Urabe, 2015-06-09 04:18


関連するチケット

コピー先 OpenPNE 3 - Backport(バックポート) #3729: スマートフォンからコミュニティ画像を変更すると画像だけのコミュニティが作成される Fixed(完了) 2014-04-09
コピー先 OpenPNE 3 - Backport(バックポート) #3762: スマートフォンからコミュニティ画像を変更すると画像だけのコミュニティが作成される Invalid(無効) 2014-04-09

履歴

#1 isao sanoほぼ10年前に更新

  • 説明 を更新 (diff)

#2 isao sanoほぼ10年前に更新

  • 3.6 で発生するかUnknown (未調査) から No (いいえ) に変更
  • 3.8 で発生するかUnknown (未調査) から Yes (はい) に変更

#3 Akihiro KOBAYASHIほぼ10年前に更新

とりあえず症状を確認しました(添付画像)。
ただ、コミュニティ画面で画像を設定して、「送信」ボタンを押した後の動作がまちまちで、この動作が起こる場合もあれば、「現在サーバーが混み合っているか〜〜〜」のエラーメッセージが出てくるのでもう少し具体的な調査を行います。

追記
「現在サーバーが混み合っているか〜〜〜」のエラーメッセージが出た以降は、スマホ版でのコミュニティ検索ができなくなる症状が出ました(PC版は普通に検索できる)

#4 Akihiro KOBAYASHIほぼ10年前に更新

症状まとめです

スマートフォンからコミュニティ画像を変更すると、
・参加者、コミュニティ名、説明文のないコミュニティが作成される
・スマホ版ではコミュニティ検索ができなくなる(/community/searchにはアクセスできるものの、ずっと検索結果表示待機状態のまま)
※PC版では検索が可能
・一度スマホからコミュニティの画像を変更する処理を行うと、それ以降同じ処理を行おうとしても「現在サーバーが混み合っているか〜〜〜」と表示される。さらに、参加者、コミュニティ名、説明文のないコミュニティは作成されない

という結果になりました。これらの情報を元に修正を行います。

#5 Akihiro KOBAYASHIほぼ10年前に更新

プルリクエストしました
https://github.com/openpne/OpenPNE3/pull/138

元々、スマホではコミュニティの編集ができないことから、スマホでのアクセスをいかなる形を用いても制限することでデータベースが破壊されることを防ぎます。

lib/action/opCommunityAction.class.php内
public function executeEdit(opWebRequest $request) {
$this->forward404If($this->id && !$this->isEditCommunity);

ここでスマホでもアクセスできないようにという条件を追加することで対応しました

具体的には
public function executeEdit(opWebRequest $request) {
if ($request->isSmartphone()) {
$this->forward404();
}
$this->forward404If($this->id && !$this->isEditCommunity);

という風にif文を追加しました

#6 Akihiro KOBAYASHIほぼ10年前に更新

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

#7 誠二 天重ほぼ10年前に更新

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

元々、スマホではコミュニティの編集ができない

この仕様の根拠となるものはなんでしょうか?
以下のソースコードを見る限り、「スマホでもコミュニティを編集できる」という仕様が想定されているようにおもわれます。
apps/pc_frontend/modules/community/actions/actions.class.php 75行目以下

  public function executeSmtEdit(opWebRequest $request)
  {
    $result = parent::executeEdit($request);

    if ($this->community->isNew())
    {
      $this->setLayout('smtLayoutHome');
    }
    else
    {
      opSmartphoneLayoutUtil::setLayoutParameters(array('community' => $this->community));
    }

    return $result;
  }

#8 誠二 天重ほぼ10年前に更新

メモ:関連しそうなチケット
http://trac.openpne.jp/ticket/4230
この修正が入っているコミットは 319c599e25d822d36183f4fff9c57aa160e037c5 で以下のような修正が入っている。

diff --git a/apps/pc_frontend/modules/community/actions/actions.class.php b/apps/pc_frontend/modules/community/actions/actions.class.php
index 92dd419..cca9328 100644
--- a/apps/pc_frontend/modules/community/actions/actions.class.php
+++ b/apps/pc_frontend/modules/community/actions/actions.class.php
@@ -24,13 +24,13 @@ class communityActions extends sfOpenPNECommunityAction
   */
   public function executeEdit($request)
   {
+    $this->enableImage = true;
     $result = parent::executeEdit($request);

     if ($this->community->isNew()) {
       sfConfig::set('sf_nav_type', 'default');
     }

-    $this->communityFileForm   = new CommunityFileForm(array(), array('community' => $this->community));

     return $result;
   }
diff --git a/lib/action/sfOpenPNECommunityAction.class.php b/lib/action/sfOpenPNECommunityAction.class.php
index a51a6f4..0b303c7 100644
--- a/lib/action/sfOpenPNECommunityAction.class.php
+++ b/lib/action/sfOpenPNECommunityAction.class.php
@@ -74,6 +74,9 @@ abstract class sfOpenPNECommunityAction extends sfActions

     $this->communityForm       = new CommunityForm($this->community);
     $this->communityConfigForm = new CommunityConfigForm(array(), array('community' => $this->community));
+    $this->communityFileForm = isset($this->enableImage) && $this->enableImage ?
+      new CommunityFileForm(array(), array('community' => $this->community)) :
+      new CommunityFileForm();

     if ($request->isMethod('post'))
     {

#9 誠二 天重ほぼ10年前に更新

検証

該当する箇所を以下のようにtry-catchで囲ってみたが、エラー発生せず。

    if ($request->isMethod('post'))
    {
      $conn = opDoctrineQuery::getMasterConnectionDirect();

      try
      {
        $params = $request->getParameter('community');
        $params['id'] = $this->id;
        $this->communityForm->bind($params);
        $this->communityConfigForm->bind($request->getParameter('community_config'));
        $this->communityFileForm->bind($request->getParameter('community_file'), $request->getFiles('community_file'));
        if ($this->communityForm->isValid() && $this->communityConfigForm->isValid() && $this->communityFileForm->isValid())
        {
          $this->communityForm->save($conn);
          $this->communityConfigForm->save($conn);
          $this->communityFileForm->save($conn);

          $this->redirect('@community_home?id='.$this->community->getId());
        }
      }
      catch (Exception $e)
      {
        $conn->rollback();
        $this->forward404();
      }
    }

現象

現象としては、正確には以下である。
  • スマートフォンから画像を設定してコミュニティ作成を行うと、画像が設定されていない正常なコミュニティと、画像が設定されているだけで他の情報を持たないコミュニティの二つのコミュニティができる。

上記の検証から、CommunityForm, CommunityConfigForm, CommunityFileForm の全てがバリデーションが正しいと判定されているが故に、それぞれのフォームがsaveメソッドを実行していることになる。
問題としては、CommunityConfigFormはCommunityFormと紐付いてデータが保存されるが、CommunityFileFormが紐付いていない状態で保存されるため、新たにデータが空の状態のコミュニティが作成される、ということである。

原因

一度の投稿で二つのコミュニティ(片方は画像以外の情報を持たない)が登録されてしまうのは、CommunityFileFormにコミュニティオブジェクトが設定されていなくてもエラーとならないことが根本的な原因である。
CommunityFileForm のconfigureは以下になっている。

  public function configure()
  {
    $this->setCommunity($this->getOption('community'));


CommunityFileFormを初期化する箇所は lib/action/opCommunityAction.class.php 内の以下。
75    $this->communityConfigForm = new CommunityConfigForm(array(), array('community' => $this->community));
76    $this->communityFileForm = isset($this->enableImage) && $this->enableImage ?
77      new CommunityFileForm(array(), array('community' => $this->community)) :
78      new CommunityFileForm();

また、PC版のcommunity/editアクションとスマホ版のそれは以下。
55  public function executeEdit(opWebRequest $request)
56  {
57    $this->forwardIf($request->isSmartphone(), 'community', 'smtEdit');
58
59    $this->enableImage = true;
60    $result = parent::executeEdit($request);
...
75  public function executeSmtEdit(opWebRequest $request)
76  {
77    $result = parent::executeEdit($request);

修正方針

もっとも簡単な解決としては、スマホ版のeditアクションでも enableImage に true をセットすれば解決する。
だが、その場合 lib/action/opCommunityAction.class.php の78行目を通ることはないのだから、 enableImage の値で分岐する理由がそもそもない。
もともと、enableImage という変数名から、コミュニティ作成時に画像を設定できるかどうかの設定値を保持するような想定だったのかもしれない。
現状でその実装がなされていないことと、すでに当初想定されていた仕様が不明であるため、enableImage の値による分岐自体を削除するというのが妥当な修正方針であると判断する。

#10 誠二 天重ほぼ10年前に更新

  • ステータスRejected(差し戻し) から Accepted(着手) に変更
  • 担当者Akihiro KOBAYASHI から 誠二 天重 に変更
  • 進捗率50 から 0 に変更

#11 誠二 天重ほぼ10年前に更新

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

https://github.com/openpne/OpenPNE3/pull/140
にてプルリクエスト。

#12 誠二 天重ほぼ10年前に更新

  • 優先度Normal(通常) から High(高め) に変更
なお、ここで報告されているデータが一度できてしまうと以下の現象が同時に発生する。
  • 管理画面のコミュニティ管理で Fatal Error が発生する(そのため管理画面から削除できない)
  • frontend側から community/:id と直接 id を指定しても Fatal Error が発生する。
  • community/search の api が Fatal Error で処理がとまる(スマホ版のコミュニティ検索ができない)。

そのため、このデータは一度できてしまうとデータを直接削除しなければ、機能復帰しない。

データの削除方法は以下。

//名前が登録されていないコミュニティを検索する
$ php symfony doctrine:dql 'FROM Community WHERE name = "" '
//検索結果のコミュニティの id が該当のデータであれば、以下で削除
$ php symfony doctrine:dql 'DELETE FROM Community WHERE name = "" '

データを直接編集するしか復旧方法がないこと、影響範囲が大きいことなど、致命的な問題であると判断できるため、優先度を High にしておく。

#13 isao sano9年以上前に更新

#14 Chiharu Nakajima約9年前に更新

#15 Rimpei Ogawa約9年前に更新

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

note-8 に関連情報として書いてある以下のバグが再発します。

#4230 (携帯版でコミュニティ写真が登録されている場合にコミュニティ編集をすると500エラー) - OpenPNE - Trac
http://trac.openpne.jp/ticket/4230

mobile_frontend に 'default/formEditImage' のテンプレートがないのに呼び出そうとしているエラーです。
pc, smt, mobile の3つで共有している部分なのですべて動く状態で修正してください。

mobile_frontend で CommunityFileForm 作って bind, save しているのがそもそも不自然な気がします。

#16 Chiharu Nakajima約9年前に更新

  • 担当者 を削除 (誠二 天重)

担当者不在のため

#17 isao sanoほぼ9年前に更新

  • 担当者isao sano にセット

#18 Shinichi Urabeほぼ9年前に更新

修正案を添付

#19 isao sanoほぼ9年前に更新

  • ステータスRejected(差し戻し) から Accepted(着手) に変更
  • 進捗率50 から 0 に変更

#20 isao sanoほぼ9年前に更新

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

https://github.com/openpne/OpenPNE3/pull/252
にてプルリクエストを行いました。

#21 Shinichi Urabeほぼ9年前に更新

  • ステータスPending Review(レビュー待ち) から Rejected(差し戻し) に変更
  • $this->communityForm $this->communityConfigForm の isValid(), save() を別々で行っていますが、共通化できると思います。 #note-18 のような修正もできます

Ex

if ($this->communityForm->isValid() ... snip ... && (!$this->communityFileForm || $this->communityFileForm->isValid())
{
  ... snip ..
  $this->communityConfigForm->save();
  if ($this->communityFileForm)
  {
    $this->communityFileForm->save();
  } 

  $this->redirect('@community_home?id='.$this->community->getId());

  ... snip ...

  • 元々の問題ですが、各フォームのパラメータを取得する際に直接名前を指定していますが、 $request->getParameter($xxxForm->getName()) 各フォームから取得するほうがフォームのキー名が変わった時の対処もできるのでいいです。

#22 isao sanoほぼ9年前に更新

#note-18 の指摘にともない以下のように修正してみたのですが、mobileでコミュニティ編集画面を開き「確定」ボタンを押下しても画面が遷移しません。なにが原因でしょうか。

    if ($request->isMethod('post'))
    {
      $params = $request->getParameter('community');
      $params['id'] = $this->id;
      $this->communityForm->bind($params);
      $this->communityConfigForm->bind($request->getParameter('community_config'));
      if($this->communityFileForm)
      {
        $this->communityFileForm->bind($request->getParameter('community_file'), $request->getFiles('community_file'));
        if ($this->communityForm->isValid() && $this->communityConfigForm->isValid() && (!$this->communityFileForm || $this->communityFileForm->isValid()))
        {
          $this->communityForm->save();
          $this->communityConfigForm->save();
          if ($this->communityFileForm)
          {
          $this->communityFileForm->save();
          }

          $this->redirect('@community_home?id='.$this->community->getId());
        }
      }
    }

#23 isao sanoほぼ9年前に更新

  • ステータスRejected(差し戻し) から Accepted(着手) に変更
  • 進捗率50 から 0 に変更

#24 isao sanoほぼ9年前に更新

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

https://github.com/openpne/OpenPNE3/pull/252
#note-21 に関して修正しました。

#25 Shinichi Urabeほぼ9年前に更新

  • 対象バージョンOpenPNE 3.9.0-old にセット

#26 Shinichi Urabeほぼ9年前に更新

  • ステータスPending Review(レビュー待ち) から Pending Testing(テスト待ち) に変更
  • 進捗率50 から 70 に変更

レビューOK

#27 Shinichi Urabeほぼ9年前に更新

  • ステータスPending Testing(テスト待ち) から Rejected(差し戻し) に変更
  • 進捗率70 から 50 に変更

backport 見たときに気づいたのですがフォームから名前取得しているので、ファイルについても同様にしたほうがいいですね
$request->getFiles($this->communityFileForm->getName())

#28 isao sanoほぼ9年前に更新

  • ステータスRejected(差し戻し) から Accepted(着手) に変更
  • 進捗率50 から 0 に変更

#29 isao sanoほぼ9年前に更新

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

note-27 の指摘に関して修正しました。
https://github.com/openpne/OpenPNE3/pull/252

#30 Shinichi Urabeほぼ9年前に更新

  • ステータスPending Review(レビュー待ち) から Pending Testing(テスト待ち) に変更
  • 進捗率50 から 70 に変更

#32 kaoru n約7年前に更新

  • ステータスPending Testing(テスト待ち) から Won't fix(対応せず) に変更
  • 対象バージョンOpenPNE 3.9.0-old から OpenPNE 3.9.0 に変更
  • 進捗率70 から 0 に変更

OpenPNE 3.8.16 にて対応済みであったため、対応せずとします。

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