Project

General

Profile

Bug(バグ) #3588

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

Added by isao sano over 5 years ago. Updated over 2 years ago.

Status:
Won't fix(対応せず)
Priority:
High(高め)
Assignee:
Target version:
Start date:
2014-04-09
Due date:
% Done:

0%

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

Description

概要

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

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

再現環境

  • OpenPNE 3.8.10
  • OpenPNE 3.8.11

opCommunityTopicPlugin の有無は関連しない

再現手順

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

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

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

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


Related issues

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

History

#1 Updated by isao sano over 5 years ago

  • Description updated (diff)

#2 Updated by isao sano over 5 years ago

  • 3.6 で発生するか changed from Unknown (未調査) to No (いいえ)
  • 3.8 で発生するか changed from Unknown (未調査) to Yes (はい)

#3 Updated by Akihiro KOBAYASHI about 5 years ago

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

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

#4 Updated by Akihiro KOBAYASHI about 5 years ago

症状まとめです

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

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

#5 Updated by Akihiro KOBAYASHI about 5 years ago

プルリクエストしました
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 Updated by Akihiro KOBAYASHI about 5 years ago

  • Status changed from Accepted(着手) to Pending Review(レビュー待ち)
  • % Done changed from 0 to 50

#7 Updated by 誠二 天重 about 5 years ago

  • Status changed from Pending Review(レビュー待ち) to 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 Updated by 誠二 天重 about 5 years ago

メモ:関連しそうなチケット
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 Updated by 誠二 天重 about 5 years ago

検証

該当する箇所を以下のように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 Updated by 誠二 天重 about 5 years ago

  • Status changed from Rejected(差し戻し) to Accepted(着手)
  • Assignee changed from Akihiro KOBAYASHI to 誠二 天重
  • % Done changed from 50 to 0

#11 Updated by 誠二 天重 about 5 years ago

  • Status changed from Accepted(着手) to Pending Review(レビュー待ち)
  • % Done changed from 0 to 50

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

#12 Updated by 誠二 天重 about 5 years ago

  • Priority changed from Normal(通常) to 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 Updated by isao sano almost 5 years ago

#14 Updated by Chiharu Nakajima over 4 years ago

#15 Updated by Rimpei Ogawa over 4 years ago

  • Status changed from Pending Review(レビュー待ち) to 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 Updated by Chiharu Nakajima over 4 years ago

  • Assignee deleted (誠二 天重)

担当者不在のため

#17 Updated by isao sano over 4 years ago

  • Assignee set to isao sano

#18 Updated by Shinichi Urabe about 4 years ago

修正案を添付

#19 Updated by isao sano about 4 years ago

  • Status changed from Rejected(差し戻し) to Accepted(着手)
  • % Done changed from 50 to 0

#20 Updated by isao sano about 4 years ago

  • Status changed from Accepted(着手) to Pending Review(レビュー待ち)
  • % Done changed from 0 to 50

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

#21 Updated by Shinichi Urabe about 4 years ago

  • Status changed from Pending Review(レビュー待ち) to 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 Updated by isao sano about 4 years ago

#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 Updated by isao sano about 4 years ago

  • Status changed from Rejected(差し戻し) to Accepted(着手)
  • % Done changed from 50 to 0

#24 Updated by isao sano about 4 years ago

  • Status changed from Accepted(着手) to Pending Review(レビュー待ち)
  • % Done changed from 0 to 50

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

#25 Updated by Shinichi Urabe about 4 years ago

  • Target version set to OpenPNE 3.9.0-old

#26 Updated by Shinichi Urabe about 4 years ago

  • Status changed from Pending Review(レビュー待ち) to Pending Testing(テスト待ち)
  • % Done changed from 50 to 70

レビューOK

#27 Updated by Shinichi Urabe about 4 years ago

  • Status changed from Pending Testing(テスト待ち) to Rejected(差し戻し)
  • % Done changed from 70 to 50

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

#28 Updated by isao sano about 4 years ago

  • Status changed from Rejected(差し戻し) to Accepted(着手)
  • % Done changed from 50 to 0

#29 Updated by isao sano about 4 years ago

  • Status changed from Accepted(着手) to Pending Review(レビュー待ち)
  • % Done changed from 0 to 50

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

#30 Updated by Shinichi Urabe about 4 years ago

  • Status changed from Pending Review(レビュー待ち) to Pending Testing(テスト待ち)
  • % Done changed from 50 to 70

#32 Updated by kaoru n over 2 years ago

  • Status changed from Pending Testing(テスト待ち) to Won't fix(対応せず)
  • Target version changed from OpenPNE 3.9.0-old to OpenPNE 3.9.0
  • % Done changed from 70 to 0

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

Also available in: Atom PDF