Bug(バグ) #3588
完了スマートフォンからコミュニティ画像を変更すると画像だけのコミュニティが作成される
0%
説明
概要¶
スマートフォンからコミュニティ画像を変更すると、参加者、コミュニティ名、説明文のないコミュニティが作成される。
このコミュニティは管理者が存在しないため削除を行うことは出来ない。
なお、スマートフォン版のコミュニティトップページには「コミュニティ編集」の導線はないため、直接URLを指定して編集を行った。
そもそもスマートフォンで「コミュニティ編集」画面に遷移できる事自体に問題があるのかもしれない。
再現環境¶
- OpenPNE 3.8.10
- OpenPNE 3.8.11
opCommunityTopicPlugin の有無は関連しない
再現手順¶
1. スマートフォンより任意のコミュニティAの管理者でログイン
2. コミュニティAのコミュニティ編集ページのURL(/community/edit?id=XX)を指定する
3. 写真を選択し、編集ボタンを押下
4. コミュニティAの画像は変更されない
5. 画像だけのコミュニティが作成されている
ファイル
Akihiro KOBAYASHI さんが10年以上前に更新
- ファイル スクリーンショット 2014-05-26 16.11.58.png スクリーンショット 2014-05-26 16.11.58.png を追加
- ステータス を New(新規) から Accepted(着手) に変更
- 担当者 を Akihiro KOBAYASHI にセット
とりあえず症状を確認しました(添付画像)。
ただ、コミュニティ画面で画像を設定して、「送信」ボタンを押した後の動作がまちまちで、この動作が起こる場合もあれば、「現在サーバーが混み合っているか〜〜〜」のエラーメッセージが出てくるのでもう少し具体的な調査を行います。
追記
「現在サーバーが混み合っているか〜〜〜」のエラーメッセージが出た以降は、スマホ版でのコミュニティ検索ができなくなる症状が出ました(PC版は普通に検索できる)
Akihiro KOBAYASHI さんが10年以上前に更新
症状まとめです
スマートフォンからコミュニティ画像を変更すると、
・参加者、コミュニティ名、説明文のないコミュニティが作成される
・スマホ版ではコミュニティ検索ができなくなる(/community/searchにはアクセスできるものの、ずっと検索結果表示待機状態のまま)
※PC版では検索が可能
・一度スマホからコミュニティの画像を変更する処理を行うと、それ以降同じ処理を行おうとしても「現在サーバーが混み合っているか〜〜〜」と表示される。さらに、参加者、コミュニティ名、説明文のないコミュニティは作成されない
という結果になりました。これらの情報を元に修正を行います。
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文を追加しました
Akihiro KOBAYASHI さんが10年以上前に更新
- ステータス を Accepted(着手) から Pending Review(レビュー待ち) に変更
- 進捗率 を 0 から 50 に変更
誠二 天重 さんが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; }
誠二 天重 さんが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'))
{
誠二 天重 さんが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年以上前に更新
- 優先度 を 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 にしておく。
isao sano さんが約10年前に更新
- コピー先 Backport(バックポート) #3729: スマートフォンからコミュニティ画像を変更すると画像だけのコミュニティが作成される を追加
Chiharu Nakajima さんがほぼ10年前に更新
- コピー先 Backport(バックポート) #3762: スマートフォンからコミュニティ画像を変更すると画像だけのコミュニティが作成される を追加
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 しているのがそもそも不自然な気がします。
isao sano さんが9年以上前に更新
- ステータス を Accepted(着手) から Pending Review(レビュー待ち) に変更
- 進捗率 を 0 から 50 に変更
https://github.com/openpne/OpenPNE3/pull/252
にてプルリクエストを行いました。
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())
各フォームから取得するほうがフォームのキー名が変わった時の対処もできるのでいいです。
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()); } } }
Shinichi Urabe さんが9年以上前に更新
- ステータス を Pending Review(レビュー待ち) から Pending Testing(テスト待ち) に変更
- 進捗率 を 50 から 70 に変更
レビューOK
Shinichi Urabe さんが9年以上前に更新
- ステータス を Pending Testing(テスト待ち) から Rejected(差し戻し) に変更
- 進捗率 を 70 から 50 に変更
backport 見たときに気づいたのですがフォームから名前取得しているので、ファイルについても同様にしたほうがいいですね$request->getFiles($this->communityFileForm->getName())
isao sano さんが9年以上前に更新
- ステータス を Accepted(着手) から Pending Review(レビュー待ち) に変更
- 進捗率 を 0 から 50 に変更
note-27 の指摘に関して修正しました。
https://github.com/openpne/OpenPNE3/pull/252
Shinichi Urabe さんが9年以上前に更新
- ステータス を Pending Review(レビュー待ち) から Pending Testing(テスト待ち) に変更
- 進捗率 を 50 から 70 に変更