FIX: display correct status on unsubscribe page (#10294)

FIX: display correct status on unsubscribe page (#10294)

There was a bug that even when email_digest was set to false but digest_after_minutes was positive, we were not displaying correct status.

In addition, the message is improved when the user is unsubscribed + unsubscribe from all is hidden.

diff --git a/app/controllers/email_controller.rb b/app/controllers/email_controller.rb
index deabbf1..1ae099f 100644
--- a/app/controllers/email_controller.rb
+++ b/app/controllers/email_controller.rb
@@ -24,6 +24,8 @@ class EmailController < ApplicationController
 
         watching = TopicUser.notification_levels[:watching]
 
+        @unsubscribed_from_all = @user.user_option.unsubscribed_from_all?
+
         if @topic
           @watching_topic = TopicUser.exists?(user_id: @user.id, notification_level: watching, topic_id: @topic.id)
           if @topic.category_id
@@ -132,16 +134,17 @@ class EmailController < ApplicationController
 
   def digest_frequencies(user)
     frequency_in_minutes = user.user_option.digest_after_minutes
+    email_digests = user.user_option.email_digests
     frequencies = DigestEmailSiteSetting.values.dup
     never = frequencies.delete_at(0)
     allowed_frequencies = %w[never weekly every_month every_six_months]
 
     result = frequencies.reduce(frequencies: [], current: nil, selected: nil, take_next: false) do |memo, v|
-      memo[:current] = v[:name] if v[:value] == frequency_in_minutes
+      memo[:current] = v[:name] if v[:value] == frequency_in_minutes && email_digests
       next(memo) unless allowed_frequencies.include?(v[:name])
 
       memo.tap do |m|
-        m[:selected] = v[:value] if m[:take_next]
+        m[:selected] = v[:value] if m[:take_next] && email_digests
         m[:frequencies] << [I18n.t("unsubscribe.digest_frequency.#{v[:name]}"), v[:value]]
         m[:take_next] = !m[:take_next] && m[:current]
       end
diff --git a/app/models/user_option.rb b/app/models/user_option.rb
index f3b48c6..7a68727 100644
--- a/app/models/user_option.rb
+++ b/app/models/user_option.rb
@@ -182,6 +182,13 @@ class UserOption < ActiveRecord::Base
     self.title_count_mode_key = UserOption.title_count_modes[value.to_sym]
   end
 
+  def unsubscribed_from_all?
+    !mailing_list_mode &&
+      !email_digests &&
+      email_level == UserOption.email_level_types[:never] &&
+      email_messages_level == UserOption.email_level_types[:never]
+  end
+
   private
 
   def update_tracked_topics
diff --git a/app/views/email/unsubscribe.html.erb b/app/views/email/unsubscribe.html.erb
index 4bc4a3d..2125d30 100644
--- a/app/views/email/unsubscribe.html.erb
+++ b/app/views/email/unsubscribe.html.erb
@@ -57,10 +57,14 @@
 
           <% if @digest_frequencies[:current] %>
             <h3>
-            <%= t(
-              'unsubscribe.digest_frequency.title',
-              frequency: t("unsubscribe.digest_frequency.#{@digest_frequencies[:current]}")
-            ) %>
+              <% if @digest_frequencies[:current] == 'never' %>
+                <%= t('unsubscribe.digest_frequency.never_title') %> 
+              <% else %>
+                <%= t(
+                  'unsubscribe.digest_frequency.title',
+                  frequency: t("unsubscribe.digest_frequency.#{@digest_frequencies[:current]}")
+                ) %>
+            <% end %>
             </h3>
             <br/>
           <% end %>
@@ -74,12 +78,14 @@
           </p>
       <% end %>
 
-      <p>
-      <label>
-      <%= check_box_tag 'unsubscribe_all', 1, @type=="all" %>
-      <%= t 'unsubscribe.all', sitename: SiteSetting.title %>
-      </label>
-      </p>
+      <% unless @unsubscribed_from_all %>
+        <p>
+        <label>
+          <%= check_box_tag 'unsubscribe_all', 1, @type=="all" %>
+          <%= t 'unsubscribe.all', sitename: SiteSetting.title %>
+        </label>
+        </p>
+      <% end %>
 
       <br/>
       <% text = @type=='digest' ? t('unsubscribe.submit') : t('unsubscribe.title') %>
diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index 151fd71..8c3c14f 100644
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@@ -1028,6 +1028,7 @@ en:
     submit: "Save preferences"
     digest_frequency:
       title: "You are receiving summary emails %{frequency}"
+      never_title: "You are not receiving summary emails"
       select_title: "Set summary emails frequency to:"
 
       never: "never"
diff --git a/spec/requests/email_controller_spec.rb b/spec/requests/email_controller_spec.rb
index e0764c6..a0d0b4b 100644
--- a/spec/requests/email_controller_spec.rb
+++ b/spec/requests/email_controller_spec.rb
@@ -191,6 +191,24 @@ RSpec.describe EmailController do
         expect(response.body).to include(I18n.t("unsubscribe.different_user_description"))
       end
 
+      it 'displays correct label when email_digests is set to false' do
+        user.user_option.update!(email_digests: false, digest_after_minutes: 10080)
+
+        navigate_to_unsubscribe
+
+        expect(body).to include("You are not receiving summary emails")
+        expect(body).to include("Don&#39;t send me any mail from Discourse")
+      end
+
+      it 'hides unsubscribe from all checkbox when user already unsubscribed' do
+        user.user_option.update!(email_digests: false, mailing_list_mode: false, email_level: 2, email_messages_level: 2)
+
+        navigate_to_unsubscribe
+
+        expect(body).to include("You are not receiving summary emails")
+        expect(body).not_to include("Don&#39;t send me any mail from Discourse")
+      end
+
       it 'correctly handles mailing list mode' do
         user.user_option.update_columns(mailing_list_mode: true)
 

GitHub sha: 4b053462

1 Like

This commit appears in #10294 which was merged by lis2.