Project

General

Profile

Bug(バグ) #2668

#2512 に対応した 3.6 系以上では、管理画面からデイリーニュースを無効に設定しても、メンバー側のメール設定にデイリーニュースの項目が表示されている

Added by Minoru Takai over 7 years ago. Updated over 2 years ago.

Status:
Won't fix(対応せず)
Priority:
Normal(通常)
Assignee:
Target version:
Start date:
2011-12-07
Due date:
% Done:

0%

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

Description

概要

メンバー側の「設定変更 > メール設定」のページには、

  • (1) フレンド承認完了メール
  • (2) フレンドの誕生日お知らせメール
  • (3) デイリー・ニュース

という設定項目があり、これに対応するように管理画面側でも、その項目の有効無効を設定できるようになっている。

現状では、管理画面側で (1), (2) を無効にするとメンバー側で (1), (2) の項目が非表示になるが、 (3) を無効にしても、メンバー側に (3) が表示されてしまっている。

これは #2512 の派生チケットです。

mail_config_frontend.png View (70.3 KB) Minoru Takai, 2011-12-07 15:39

mail_config_backend.png View (84.9 KB) Minoru Takai, 2011-12-07 15:39


Related issues

Related to OpenPNE 3 - Bug(バグ) #2512: 管理画面からデイリーニュースを無効に設定すると、メンバー側で「メール設定」のページが開けなくなる Fixed(完了) 2011-10-18
Related to OpenPNE 3 - Bug(バグ) #2694: 3.4 系で、管理画面からフレンドリンク承認完了メールを有効に設定しても、メンバー側のメール設定にフレンドリンク承認完了メールの項目が表示されない Won't fix(対応せず) 2011-12-15
Related to OpenPNE 3 - Backport(バックポート) #2722: #2512 に対応した 3.6 系以上では、管理画面からデイリーニュースを無効に設定しても、メンバー側のメール設定にデイリーニュースの項目が表示されている Fixed(完了) 2011-12-07
Related to OpenPNE 3 - Backport(バックポート) #3117: #2512 に対応した 3.6 系以上では、管理画面からデイリーニュースを無効に設定しても、メンバー側のメール設定にデイリーニュースの項目が表示されている Fixed(完了) 2011-12-07

Associated revisions

Revision 63563e3d (diff)
Added by Yuya Watanabe over 7 years ago

(fixes #2668) fixed not to show daily news settings if pc_backend settings selected 'Don't Notify'

Revision 2f6c1fc7 (diff)
Added by Yuya Watanabe over 7 years ago

(fixes #2668) revert daily_new Default value to send default 'Send Everyday' setting

History

#1 Updated by Minoru Takai over 7 years ago

(3) を (1), (2) に合わせるには

(1), (2) は、メール配信の機能で lib/config/config/mail_template.yml に

130-  friendLinkComplete:
131:    caption: "Notification of Completion of Friend Link" 
132-    configurable: true
133-    member_configurable: true
134-    sample:
135-      en:
136-        - "{{ member.name }} accepted your {{ op_term.friend }} link request" 

213-  birthday:
214:    caption: "Notification of Friend Birthday" 
215-    configurable: true
216-    member_configurable: true
217-    sample:
218-      en:
219-        - "There is {{ member.name }}'s {{ op_term.my_friend }} that its birthday is coming soon" 

と記述されていて、ここでの configurable, member_configurable の設定で、メンバー側に設定変更のフォームが表示されるか否かを指定できている。これがあるため、管理画面側で無効にするとメンバー側では非表示になるようになっている。

デイリーニュースは member_configurable が記述されておらず次のようになっている。

248:  dailyNews:
249-    caption: "Daily News" 
250-    configurable: true
251-    sample:
252-      en:
253-        - "Daily News" 

しかし member_configurable を追加したところで note-5 を解消できなかった。というより、そもそもこれが記述されていなければ、 false を指定したのと同様に非表示になるはずである。

つまり、 (3) のデイリーニュースのメンバー側設定のフォームは、別のどこかで明示的に出力されているように思われるが、それが具体的にどこなのか追えていない。

修正方針

note-5 の指摘は尤もだが、これを仕様と考えることもできる。デイリーニュース設定に関しては、管理画面で無効にしてもメンバー側に表示されるということになる。

このチケットの問題は note-4 の修正で解消されていて、 note-5 は補足的な指摘に過ぎないが、これを対応したほうがよいか判断できていない(というか対応したくてもどこを直せばいいか追えていない)。

#2 Updated by Yuma Sakata over 7 years ago

  • 3.6 で発生するか set to No (いいえ)
  • 3.4 で発生するか set to No (いいえ)

#3 Updated by Minoru Takai over 7 years ago

  • Subject changed from 管理画面からデイリーニュースを無効に設定しても、メンバー側のメール設定にデイリーニュースの項目が表示されている to #2512 に対応した 3.6 系以上では、管理画面からデイリーニュースを無効に設定しても、メンバー側のメール設定にデイリーニュースの項目が表示されている
  • 3.6 で発生するか changed from No (いいえ) to Unknown (未調査)

このチケットは、 #2512 に対応した 3.6 系以上の問題です。 note-2 で「3.6 系で発生しない」という結果は間違っている可能性があるので、再調査する必要があります。

#4 Updated by Yuma Sakata over 7 years ago

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

再現確認できました。

Environment (再現バージョン)

OpenPNE3.6.1

Way to repro (再現手順)

1. 管理画面メール通知送信設定ページ(/pc_backend.php/mail)でデイリー・ニュースを通知しない設定にする
2. SNS側メール設定ページ(/member/config?category=mail)にアクセスする
3. 管理画面でデイリー・ニュースを無効に設定しても、SNS側のメール設定にデイリー・ニュースの項目が表示されている

Way to fix (修正内容)

管理画面でデイリー・ニュースを無効に設定した場合、SNS側のメール設定にデイリー・ニュースの項目が表示されないように修正お願いします。

#5 Updated by Yuya Watanabe over 7 years ago

  • Status changed from New(新規) to Accepted(着手)
  • Assignee set to Yuya Watanabe

#6 Updated by Yuya Watanabe over 7 years ago

原因

デイリーニュースのフォームのみ MemberConfigMailForm ではなく MemberConfigForm で扱われていたため,mail_template.yml での設定を反映しなかった.

修正する場合の注意点

MemberConfigMailForm か MemberConfigForm どちらかでの処理に統一したほうがよさそうだが,メールの設定を行うという処理の内容から考えて MemberConfigMailForm にまとめてしまうのが正しいと思われる.

ただし,それぞれMemberConfig の name が is_send_pc_dailyNews_mail と daily_news のように違う名前となるため,単純に設定をまとめてしまうとマイグレーションが発生する問題がある.

また,MemberConfigMailForm に統一する場合,MemberConfigForm では設定値によってフォームの種類を決定するロジックがあるが MemberConfigMailForm ではそのようなロジックがないため,独自に実装する必要がある.

実装案

下記のようものを考えたが,メンバ側の設定画面で「メール設定」の項目が設定可能なのに消えてしまう問題がある.

また,重複した処理をまとめることも必要である.

diff --git a/lib/config/config/mail_template.yml b/lib/config/config/mail_template.yml
index b1dea25..aa229ab 100644
--- a/lib/config/config/mail_template.yml
+++ b/lib/config/config/mail_template.yml
@@ -248,6 +248,7 @@ pc:
   dailyNews:
     caption: "Daily News" 
     configurable: true
+    member_configurable: true
     sample:
       en:
         - "Daily News" 
diff --git a/lib/config/config/member_config.yml b/lib/config/config/member_config.yml
index 3af30f9..00ac975 100644
--- a/lib/config/config/member_config.yml
+++ b/lib/config/config/member_config.yml
@@ -122,21 +122,6 @@ mail:
     caption: "Mail Configuration" 
     enable_pc:     true
     enable_mobile: true 
-  daily_news:
-    Name:       "daily_news" 
-    Caption:    "Daily News" 
-    FormType:   "radio" 
-    ValueType:  "string" 
-    IsRegist:   false
-    IsConfig:   true
-    IsUnique:   false
-    IsRequired: true
-    IsConfirm:  false
-    Default: 2
-    Choices:
-      - "Don't Send" 
-      - "Send At Intervals" 
-      - "Send Everyday" 

 language:
   _attributes:
diff --git a/lib/form/MemberConfigForm/MemberConfigMailForm.class.php b/lib/form/MemberConfigForm/MemberConfigMailForm.class.php
index 9bff3b7..2cf2706 100644
--- a/lib/form/MemberConfigForm/MemberConfigMailForm.class.php
+++ b/lib/form/MemberConfigForm/MemberConfigMailForm.class.php
@@ -22,21 +22,6 @@ class MemberConfigMailForm extends MemberConfigForm
   public function __construct(Member $member = null, $options = array(), $CSRFSecret = null)
   {
     parent::__construct($member, $options, $CSRFSecret);
-
-    $count = count(opConfig::get('daily_news_day'));
-
-    $i18n = sfContext::getInstance()->getI18N();
-    $translated = $i18n->__('[1]Send once a week (%2%)|[2]Send twice a week (%2%)|(2,+Inf]Send %1% times a week (%2%)', array(
-      '%1%' => $count,
-      '%2%' => implode(',', $this->generateDayList()))
-    );
-
-    $choice = new sfChoiceFormat();
-    $retval = $choice->format($translated, $count);
-
-    $options = $this->widgetSchema['daily_news']->getOptions();
-    $options['choices'][1] = $retval;
-    $this->widgetSchema['daily_news']->setOptions($options);
   }

   public function configure()
@@ -59,15 +44,34 @@ class MemberConfigMailForm extends MemberConfigForm
       if (isset($value['member_configurable']) && $value['member_configurable'])
       {
         $notification = Doctrine::getTable('NotificationMail')->findOneByName($app.'_'.$key);
+        $name  = 'is_send_'.$app.'_'.$key.'_mail';
         if (!$notification || $notification->getIsEnabled())
         {
-          $name  = 'is_send_'.$app.'_'.$key.'_mail';
-
-          $this->setWidget($name, new sfWidgetFormChoice(array('choices' => $choices, 'expanded' => true)));
-          $this->setValidator($name, new sfValidatorChoice(array('choices' => array_keys($choices), 'required' => true)));
-          $this->widgetSchema->setLabel($name, $value['caption']);
-
-          $this->setDefault($name, $this->member->getConfig($name, 1));
+          if ('dailyNews' !== $key)
+          {
+            $this->setWidget($name, new sfWidgetFormChoice(array('choices' => $choices, 'expanded' => true)));
+            $this->setValidator($name, new sfValidatorChoice(array('choices' => array_keys($choices), 'required' => true)));
+            $this->widgetSchema->setLabel($name, $value['caption']);
+
+            $this->setDefault($name, $this->member->getConfig($name, 1));
+          }
+          else
+          {
+            $name = 'daily_news';
+            $i18n = sfContext::getInstance()->getI18N();
+            $choice = new sfChoiceFormat();
+            $count = count(opConfig::get('daily_news_day'));
+            $translated = $choice->format($i18n->__('[1]Send once a week (%2%)|[2]Send twice a week (%2%)|(2,+Inf]Send %1% times a week (%2%)', array(
+              '%1%' => $count,
+              '%2%' => implode(',', $this->generateDayList()))
+            ), $count);
+            $dailyNewsChoices = array("Don't Send", $translated, "Send Everyday");
+            $this->setWidget($name, new sfWidgetFormChoice(array('choices' => $dailyNewsChoices, 'expanded' => true)));
+            $this->setValidator($name, new sfValidatorChoice(array('choices' => array_keys($dailyNewsChoices), 'required' => true)));
+            $this->widgetSchema->setLabel($name, $value['caption']);
+
+            $this->setDefault($name, $this->member->getConfig($name, 2));
+          }
         }
       }
     }

#7 Updated by Yuya Watanabe over 7 years ago

note-6 の内容に加えて,CSRFトークン以外のフォームが存在しない場合には「利用可能な設定はありません。」というメッセージを表示するように変更.

#8 Updated by Yuya Watanabe over 7 years ago

diff --git a/apps/mobile_frontend/i18n/messages.ja.xml b/apps/mobile_frontend/i18n/messages.ja.xml
index 02e4d5e..bb049cd 100644
--- a/apps/mobile_frontend/i18n/messages.ja.xml
+++ b/apps/mobile_frontend/i18n/messages.ja.xml
@@ -235,6 +235,10 @@
         <target>メニューから設定したい項目を選択してください。</target>
       </trans-unit>
       <trans-unit id="">
+        <source>There is no available settings.</source>
+        <target>利用可能な設定はありません。</target>
+      </trans-unit>
+      <trans-unit id="">
         <source>Successful in invite.</source>
         <target>送信が完了しました。</target>
       </trans-unit>
diff --git a/apps/mobile_frontend/modules/member/templates/configSuccess.php b/apps/mobile_frontend/modules/member/templates/configSuccess.php
index f27a9ee..3a20ffa 100644
--- a/apps/mobile_frontend/modules/member/templates/configSuccess.php
+++ b/apps/mobile_frontend/modules/member/templates/configSuccess.php
@@ -4,12 +4,14 @@
 <?php op_mobile_page_title(__('Settings')) ?>
 <?php endif; ?>

-<?php if ($categoryName): ?>
+<?php if ($categoryName && $form->count() > 1): // except CSRF token field ?>
 <?php op_include_form('configForm', $form, array(
   'url'    => url_for('member/config?category='.$categoryName),
   'align'  => 'center',
   'button' => __('Save')
 )) ?>
+<?php elseif ($categoryName && 1 === $form->count()) : ?>
+<?php echo __('There is no available settings.'); ?>
 <?php else: ?>
 <?php echo __('Please select the item from the menu.'); ?>
 <?php endif; ?>
diff --git a/apps/pc_frontend/i18n/messages.ja.xml b/apps/pc_frontend/i18n/messages.ja.xml
index 37db19b..efdbaa0 100644
--- a/apps/pc_frontend/i18n/messages.ja.xml
+++ b/apps/pc_frontend/i18n/messages.ja.xml
@@ -259,6 +259,10 @@
         <target>メニューから設定したい項目を選択してください。</target>
       </trans-unit>
       <trans-unit id="">
+        <source>There is no available settings.</source>
+        <target>利用可能な設定はありません。</target>
+      </trans-unit>
+      <trans-unit id="">
         <source>Edit profile</source>
         <target>プロフィール変更</target>
       </trans-unit>
diff --git a/apps/pc_frontend/modules/member/templates/configSuccess.php b/apps/pc_frontend/modules/member/templates/configSuccess.php
index f6f08a6..c5fd3e5 100644
--- a/apps/pc_frontend/modules/member/templates/configSuccess.php
+++ b/apps/pc_frontend/modules/member/templates/configSuccess.php
@@ -41,8 +41,10 @@ op_include_parts('pageNav', 'navForDelete', array('list' => $list));
 ?>
 <?php end_slot(); ?>

-<?php if ($categoryName): ?>
+<?php if ($categoryName && $form->count() > 1): // except CSRF token field ?>
 <?php op_include_form($categoryName.'Form', $form, array('title' => __($categoryCaptions[$categoryName]), 'url' => url_for('@member_config?category='.$categoryName))) ?>
+<?php elseif ($categoryName && 1 === $form->count()) : ?>
+<?php op_include_box('configInformation', __('There is no available settings.'), array('title' => __($categoryCaptions[$categoryName]))); ?>
 <?php else: ?>
 <?php op_include_box('configInformation', __('Please select the item that wants to be set from the menu.'), array('title' => __('Change Settings'))); ?>
 <?php endif; ?>
diff --git a/lib/config/config/mail_template.yml b/lib/config/config/mail_template.yml
index b1dea25..f8274a7 100644
--- a/lib/config/config/mail_template.yml
+++ b/lib/config/config/mail_template.yml
@@ -248,6 +248,7 @@ pc:
   dailyNews:
     caption: "Daily News" 
     configurable: true
+    member_configurable: true
     sample:
       en:
         - "Daily News" 
@@ -578,6 +579,7 @@ mobile:
   dailyNews:
     caption: "Daily News" 
     configurable: true
+    member_configurable: true
     sample:
       en:
         - "Daily News" 
diff --git a/lib/config/config/member_config.yml b/lib/config/config/member_config.yml
index 3af30f9..20d8880 100644
--- a/lib/config/config/member_config.yml
+++ b/lib/config/config/member_config.yml
@@ -123,20 +123,6 @@ mail:
     enable_pc:     true
     enable_mobile: true 
   daily_news:
-    Name:       "daily_news" 
-    Caption:    "Daily News" 
-    FormType:   "radio" 
-    ValueType:  "string" 
-    IsRegist:   false
-    IsConfig:   true
-    IsUnique:   false
-    IsRequired: true
-    IsConfirm:  false
-    Default: 2
-    Choices:
-      - "Don't Send" 
-      - "Send At Intervals" 
-      - "Send Everyday" 

 language:
   _attributes:
diff --git a/lib/form/MemberConfigForm/MemberConfigMailForm.class.php b/lib/form/MemberConfigForm/MemberConfigMailForm.class.php
index 9bff3b7..2cf2706 100644
--- a/lib/form/MemberConfigForm/MemberConfigMailForm.class.php
+++ b/lib/form/MemberConfigForm/MemberConfigMailForm.class.php
@@ -22,21 +22,6 @@ class MemberConfigMailForm extends MemberConfigForm
   public function __construct(Member $member = null, $options = array(), $CSRFSecret = null)
   {
     parent::__construct($member, $options, $CSRFSecret);
-
-    $count = count(opConfig::get('daily_news_day'));
-
-    $i18n = sfContext::getInstance()->getI18N();
-    $translated = $i18n->__('[1]Send once a week (%2%)|[2]Send twice a week (%2%)|(2,+Inf]Send %1% times a week (%2%)', array(
-      '%1%' => $count,
-      '%2%' => implode(',', $this->generateDayList()))
-    );
-
-    $choice = new sfChoiceFormat();
-    $retval = $choice->format($translated, $count);
-
-    $options = $this->widgetSchema['daily_news']->getOptions();
-    $options['choices'][1] = $retval;
-    $this->widgetSchema['daily_news']->setOptions($options);
   }

   public function configure()
@@ -59,15 +44,34 @@ class MemberConfigMailForm extends MemberConfigForm
       if (isset($value['member_configurable']) && $value['member_configurable'])
       {
         $notification = Doctrine::getTable('NotificationMail')->findOneByName($app.'_'.$key);
+        $name  = 'is_send_'.$app.'_'.$key.'_mail';
         if (!$notification || $notification->getIsEnabled())
         {
-          $name  = 'is_send_'.$app.'_'.$key.'_mail';
-
-          $this->setWidget($name, new sfWidgetFormChoice(array('choices' => $choices, 'expanded' => true)));
-          $this->setValidator($name, new sfValidatorChoice(array('choices' => array_keys($choices), 'required' => true)));
-          $this->widgetSchema->setLabel($name, $value['caption']);
-
-          $this->setDefault($name, $this->member->getConfig($name, 1));
+          if ('dailyNews' !== $key)
+          {
+            $this->setWidget($name, new sfWidgetFormChoice(array('choices' => $choices, 'expanded' => true)));
+            $this->setValidator($name, new sfValidatorChoice(array('choices' => array_keys($choices), 'required' => true)));
+            $this->widgetSchema->setLabel($name, $value['caption']);
+
+            $this->setDefault($name, $this->member->getConfig($name, 1));
+          }
+          else
+          {
+            $name = 'daily_news';
+            $i18n = sfContext::getInstance()->getI18N();
+            $choice = new sfChoiceFormat();
+            $count = count(opConfig::get('daily_news_day'));
+            $translated = $choice->format($i18n->__('[1]Send once a week (%2%)|[2]Send twice a week (%2%)|(2,+Inf]Send %1% times a week (%2%)', array(
+              '%1%' => $count,
+              '%2%' => implode(',', $this->generateDayList()))
+            ), $count);
+            $dailyNewsChoices = array("Don't Send", $translated, "Send Everyday");
+            $this->setWidget($name, new sfWidgetFormChoice(array('choices' => $dailyNewsChoices, 'expanded' => true)));
+            $this->setValidator($name, new sfValidatorChoice(array('choices' => array_keys($dailyNewsChoices), 'required' => true)));
+            $this->widgetSchema->setLabel($name, $value['caption']);
+
+            $this->setDefault($name, $this->member->getConfig($name, 2));
+          }
         }
       }
     }

#9 Updated by Yuya Watanabe over 7 years ago

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

更新履歴 63563e3d8710a4aaca6059617382cfed0d731a64 で適用されました。

#10 Updated by Kousuke Ebihara over 7 years ago

  • Status changed from Pending Review(レビュー待ち) to Rejected(差し戻し)

lib/config/config/member_config.yml の daily_news の設定値は lib/task/openpneSendDailyNewsTask.class.php において、初期値を得るために使われているようです。

$dailyNewsConfig = $member->getConfig('daily_news');
if (null !== $dailyNewsConfig && 0 === (int)$dailyNewsConfig)
{
  continue;
}
if (1 === (int)$dailyNewsConfig && !$this->isDailyNewsDay())
{
  continue;
}

daily_news_day の設定値が存在しない場合に、 daily_news の設定値がデフォルト値の 2 であれば、毎日デイリーニュースを送信するという実装になっているようです。

このチケットの変更によって、 daily_news_day の設定値が存在しない場合はデイリーニュースが送信されなくなってしまうように思います。確認よろしくお願いします。

#11 Updated by Yuya Watanabe over 7 years ago

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

#12 Updated by Yuya Watanabe over 7 years ago

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

更新履歴 2f6c1fc7aeecd9afbfba99b4046259961f7f741d で適用されました。

#13 Updated by Kousuke Ebihara over 7 years ago

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

#14 Updated by Shouta Kashiwagi over 7 years ago

  • Target version changed from OpenPNE 3.7.0 to 252

#15 Updated by Shouta Kashiwagi over 7 years ago

  • Target version changed from 252 to OpenPNE 3.8.x

#16 Updated by 開 石切山 almost 7 years ago

  • Target version changed from OpenPNE 3.8.x to OpenPNE 3.8.1

#17 Updated by Yuya Watanabe almost 7 years ago

  • Target version changed from OpenPNE 3.8.1 to OpenPNE 3.9.0-old

こちらは master 側のチケットなので対象バージョンのみを変更します.

#18 Updated by kaoru n about 4 years ago

  • 3.8 で発生するか set to Unknown (未調査)

#19 Updated by isao sano over 2 years ago

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

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

Also available in: Atom PDF