プロジェクト

全般

プロフィール

Bug(バグ) #2058

コミュニティのメンバー管理画面に戻るためのYes/Noフォームで「いいえ」を選ぶとアクセスエラーになる

Mutsumi Imamuraほぼ13年前に追加. ほぼ13年前に更新.

ステータス:
Fixed(完了)
優先度:
Normal(通常)
担当者:
対象バージョン:
開始日:
2011-05-06
期日:
進捗率:

100%

3.6 で発生するか:
Yes
3.8 で発生するか:
Unknown (未調査)

説明

現象

  1. 自分が管理者であるコミュニティページ(/community/:id)にアクセスする
  2. メンバー管理ページ(/community/member/manage?id=:id)にアクセスする
  3. メンバー管理画面に戻るためのYes/Noフォームがあるページへ遷移する
    • 副管理者に対する「副管理者から降格」リンクから遷移できるページ (/community/removeSubAdmin/id/1/member_id/2) あるいは、
    • コミュニティメンバーに対する「退会させる」リンクから遷移できるページ (/community/removeSubAdmin/id/1/member_id/2) のどちらでもよい。
  4. メンバー管理画面に戻るための「いいえ」ボタンを押下する
  5. /default/error エラーページへ遷移する

原因

#1004 での 修正 によって、以下のルーティングが指定されており、

community_memberManage:
  url: /community/member/manage
  param: { module: community, action: memberManage }

このルーティングにマッチした上で、クエリストリングとしてパラメータを含めようとする。 /community/member/manage?id=1 のようなURLとなる。

GET method のフォームに対して action 属性値に ?id=xxx の形のクエリストリングが付与されたURLを渡すと、action 属性値のパラメータを無視する実装のUAが多いらしい。 関連記事(「form action クエリ」や「form action querystring」で検索すると記事がいくつか見つかる) [1] [2]

上記のルーティングが定義されていない場合は lib/routing/opSymfonyDefaultRouteCollection.class.php で

    $this->routes['default'] = new opDeprecatedRoute(
      '/:module/:action/*'
    );

のように定義されているルーティングにマッチし、 /community/memberManage/id/123 のようなURLとなり、パラメータが渡っていたため想定通りに動作していた。

修正内容

PC版、携帯版ともに、ルーティングの定義を

community_memberManage:
  url: /community/member/manage
  param: { module: community, action: memberManage }

から、id パラメータを含めた形

community_memberManage:
  url: /community/member/manage/:id
  param: { module: community, action: memberManage }
  requirements: { id: \d+ }

に変更する。

当該のYes/Noフォームにおいて、action 属性値に含まれるクエリストリングを input 要素などを介して渡すことでこの問題を回避することも可能だが、本質的な解決策ではない。

補足

この問題は #1004 での修正における書き換え時のミスだと思われる。

  • id を付加する必要性について
    • community/memberManage アクションは、特定の id のコミュニティのメンバー管理画面を表示するためのアクションである。つまり id を受け取ることを前提としている。
      • lib/action/opCommunityAction.class.php の preExecute() メソッドおよび executeMemberManage($request) を見れば明らかである。
  • ルーティング定義を変更してしまうことによる副作用はないか
    • id を受け取らない community/memberManage の呼び出しは前述のとおり想定されていないため、もしそのようなリンクがあったとしてもエラーページに飛ばされるだけである。
    • community/memberManage アクションを指すURL /community/member/manage (idを含まないURL)が無くなっても問題はないと判断している。

関連するチケット

関連している OpenPNE 3 - Backport(バックポート) #2062: コミュニティのメンバー管理画面に戻るためのYes/Noフォームで「いいえ」を選ぶとアクセスエラーになる Fixed(完了) 2011-05-06
関連している OpenPNE 3 - Backport(バックポート) #2063: コミュニティのメンバー管理画面に戻るためのYes/Noフォームで「いいえ」を選ぶとアクセスエラーになる Invalid(無効) 2011-05-06
関連している OpenPNE 3 - Bug(バグ) #2250: コミュニティの副管理者降格画面の文言が不適切である Fixed(完了) 2011-06-28
次のチケットと重複 OpenPNE 3 - Bug(バグ) #1543: Becomes an access error when selected the "no" button in the member of community coercion leave form.(コミュニティのメンバーの強制退会画面(/community/dropMember/id/:id/member_id/:id)で「いいえ」を選ぶとアクセスエラーになる) Invalid(無効) 2010-08-31

関係しているリビジョン

リビジョン 032f641b (差分)
Masato Nagasawaほぼ13年前に追加

fixed moved get parameter to input tag (fixes #2058)

リビジョン 043cb4ac (差分)
Naoya Tozukaほぼ13年前に追加

corrected some text to use 'demote' instead of 'demotion' where the verb is required; (fixes #2058)

リビジョン 81b179d4 (差分)
Minoru Takaiほぼ13年前に追加

(fixed #2058) redefined routing url of @community_memberManage to contain id parameter into url.

履歴

#1 Masato Nagasawaほぼ13年前に更新

  • ステータスNew(新規) から Accepted(着手) に変更

#2 Masato Nagasawaほぼ13年前に更新

テスト

区分 期待結果 結果
携帯側のコミュニティ副管理者を降格画面で「いいえ」を選択 メンバー管理画面へ戻る
PC側のコミュニティ副管理者を降格画面で「いいえ」を選択 メンバー管理画面へ戻る

#3 Masato Nagasawaほぼ13年前に更新

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

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

#4 Naoya Tozukaほぼ13年前に更新

  • 担当者 を削除 (Masato Nagasawa)

#5 Naoya Tozukaほぼ13年前に更新

修正箇所確認しました。OKです。

【メモ】「副管理者から降格」に対応する英文の誤りについて

今回指摘されている問題とは異なりますが、「副管理者から降格」に対応する英文

"Do you demotion %0% from this %community%'s sub-administrator?"  // %0% には副管理者の名前が、%community% にはコミュニティ名が入ります。
において demotion は名詞なので、これは demote に置き換えるべきと思われます。

修正すべき箇所は、

  • apps/mobile_frontend/i18n/messages.ja.xml
  • apps/mobile_frontend/modules/community/templates/memberManageSuccess.php
  • apps/mobile_frontend/modules/community/templates/removeSubAdminInput.php
  • apps/pc_frontend/i18n/messages.ja.xml
  • apps/pc_frontend/modules/community/templates/memberManageSuccess.php
  • apps/pc_frontend/modules/community/templates/removeSubAdminInput.php

になります。

#6 Shingo Yamadaほぼ13年前に更新

  • 担当者Naoya Tozuka にセット

#7 Naoya Tozukaほぼ13年前に更新

takai: @tozuka その間違った文言のコミットに対して、修正すべきは戸塚さんが指摘した文言のみでしょうか。もし間違った文言が他にもあるのであれば全面的に修正した方が好ましい気がしました。間違った文言のコミットは 43ae2a74 らしいですね(masterブランチ)。 http://redmine.openpne.jp/issues/228 の4個目のコミット。

tozuka: @takai なるほど( 43ae2a74 みました)。「副管理者から降格」に射影される英文が2つあるので両方辿ってみます。

#8 Naoya Tozukaほぼ13年前に更新

更新履歴 commit:"054285bf3cd48d0f963e8becc51914e24afcc13a" で適用されました。043cb4ac に統合のため削除

#9 Naoya Tozukaほぼ13年前に更新

更新履歴 commit:"dd065c59f1674bbf5bf0d26e7f87609c10838587" で適用されました。043cb4ac に統合のため削除

#10 Naoya Tozukaほぼ13年前に更新

高井さんの指摘を受け、 043cb4ac にて他の間違った文言も修正しました。

レビューお願いします。

#11 Minoru Takaiほぼ13年前に更新

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

このチケットに対して

このチケット自体をレビューします。

3. 適当なメンバーで「退会させる」を押下:/community/removeSubAdmin/id/19/member_id/2

これは『3. 副管理者に対して「副管理者から降格」を押下』の誤りでしょうか。

原因

get の form のaction では action="URL?id=○○" のパラメータが引き継げないので URL? ページに遷移してしまう

『get の form のaction では action="URL?id=○○" のパラメータが引き継げない』この理由・原因について可能であれば、コメントかチケットに記述してください。修正方針が妥当かどうか、他の箇所について修正する必要がないかを判断するのに有用です。

note-3 での修正内容について

修正内容

inputタグでパラメータを引き継ぐように修正。

この修正方針が妥当であれば、修正内容は問題ないと思います。

-  'no_url'    => url_for('@community_memberManage?id='.$community->getId()),
+  'no_url'    => url_for('@community_memberManage'),
   'no_method' => 'get',
+  'no_form'   => '<input type="hidden" name="id" value="'.$community->getId().'"/>',
 )) ?>

PC版の修正について、追加している input 要素タグの末尾のスラッシュの直前に空白がありませんが、本質的な問題ではないためこの修正内容で問題ないと思います。

note-10 での修正内容について

修正内容自体はOKです。

チケット自体に文言について修正する旨が記述されていないため、note-3 の修正に併せて、文言についての修正も扱ったことが分かるようチケットを書き換える必要がありそうです。

また、バックポートチケットでも同じ修正を取り込むか、あるいは取り込まないことが分かるようになっているべきです。3.4.x 向けのBPチケット #2063 は、この親チケットが完了する前にクローズされているため、 note-10 での修正は #2063 の派生チケット #2093 で扱うようにした方がよいかと思います。

一旦、担当者に差し戻します。

#12 Minoru Takaiほぼ13年前に更新

  • ステータスRejected(差し戻し) から Accepted(着手) に変更
  • 担当者Naoya Tozuka から Minoru Takai に変更

一先ず、チケットの概要(現象・原因・修正内容)を書き換えました。

#13 Minoru Takaiほぼ13年前に更新

action 属性値のクエリストリングを input 要素に渡す方針から、ルーティング定義を適切なものにする修正方針に改めます。また、原因を見れば明らかですが、この問題はルーティング定義が適切でないことに因るものであり、 #1543 と同一の問題(つまり #1543#2058 の問題が同時に解消するもの)です。

#14 Minoru Takaiほぼ13年前に更新

note-10 の文言に関する修正についてですが、チケットの関連をより分かりやすくするために別チケット対応としました。文言の修正については #2250 で扱います。

#15 Minoru Takaiほぼ13年前に更新

  • 題名コミュニティの副管理者降格画面(/community/removeSubAdmin/id/:id/member_id/:id)で「いいえ」を選ぶとアクセスエラーになる から コミュニティのメンバー管理画面に戻るためのYes/Noフォームで「いいえ」を選ぶとアクセスエラーになる に変更

#16 Minoru Takaiほぼ13年前に更新

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

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

#17 Naoya Tozukaほぼ13年前に更新

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

新しい修正方針に基づいた修正を確認しました。レビューOKです。
チケットの整理どうもありがとうございます。

#18 Minoru Takaiほぼ13年前に更新

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

動作テスト

実装時に動作テストを行っており、原理的にもこの修正による副作用はないものと判断しています。

このチケットで挙げられた当初の問題点については改善できていることが確認できており、修正内容も妥当であると判断しているため、動作上も問題がないと判断します。

完了とします。

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