Project

General

Profile

Bug(バグ) #2058

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

Added by Mutsumi Imamura over 8 years ago. Updated over 8 years ago.

Status:
Fixed(完了)
Priority:
Normal(通常)
Assignee:
Target version:
Start date:
2011-05-06
Due date:
% Done:

100%

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

Description

現象

  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)が無くなっても問題はないと判断している。

Related issues

Related to OpenPNE 3 - Backport(バックポート) #2062: コミュニティのメンバー管理画面に戻るためのYes/Noフォームで「いいえ」を選ぶとアクセスエラーになる Fixed(完了) 2011-05-06
Related to OpenPNE 3 - Backport(バックポート) #2063: コミュニティのメンバー管理画面に戻るためのYes/Noフォームで「いいえ」を選ぶとアクセスエラーになる Invalid(無効) 2011-05-06
Related to OpenPNE 3 - Bug(バグ) #2250: コミュニティの副管理者降格画面の文言が不適切である Fixed(完了) 2011-06-28
Duplicates 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

Associated revisions

Revision 032f641b (diff)
Added by Masato Nagasawa over 8 years ago

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

Revision 043cb4ac (diff)
Added by Naoya Tozuka over 8 years ago

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

Revision 81b179d4 (diff)
Added by Minoru Takai over 8 years ago

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

History

#1 Updated by Masato Nagasawa over 8 years ago

  • Status changed from New(新規) to Accepted(着手)

#2 Updated by Masato Nagasawa over 8 years ago

テスト

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

#3 Updated by Masato Nagasawa over 8 years ago

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

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

#4 Updated by Naoya Tozuka over 8 years ago

  • Assignee deleted (Masato Nagasawa)

#5 Updated by Naoya Tozuka over 8 years ago

修正箇所確認しました。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 Updated by Shingo Yamada over 8 years ago

  • Assignee set to Naoya Tozuka

#7 Updated by Naoya Tozuka over 8 years ago

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

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

#8 Updated by Naoya Tozuka over 8 years ago

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

#9 Updated by Naoya Tozuka over 8 years ago

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

#10 Updated by Naoya Tozuka over 8 years ago

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

レビューお願いします。

#11 Updated by Minoru Takai over 8 years ago

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

  • Status changed from Rejected(差し戻し) to Accepted(着手)
  • Assignee changed from Naoya Tozuka to Minoru Takai

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

#13 Updated by Minoru Takai over 8 years ago

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

#14 Updated by Minoru Takai over 8 years ago

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

#15 Updated by Minoru Takai over 8 years ago

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

#16 Updated by Minoru Takai over 8 years ago

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

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

#17 Updated by Naoya Tozuka over 8 years ago

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

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

#18 Updated by Minoru Takai over 8 years ago

  • Status changed from Pending Testing(テスト待ち) to Fixed(完了)
  • % Done changed from 70 to 100

動作テスト

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

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

完了とします。

Also available in: Atom PDF