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.
This commit is contained in:
Krzysztof Kotlarek 2020-07-23 16:20:10 +10:00 committed by GitHub
parent e027acd367
commit 4b053462c0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 47 additions and 12 deletions

View File

@ -24,6 +24,8 @@ class EmailController < ApplicationController
watching = TopicUser.notification_levels[:watching] watching = TopicUser.notification_levels[:watching]
@unsubscribed_from_all = @user.user_option.unsubscribed_from_all?
if @topic if @topic
@watching_topic = TopicUser.exists?(user_id: @user.id, notification_level: watching, topic_id: @topic.id) @watching_topic = TopicUser.exists?(user_id: @user.id, notification_level: watching, topic_id: @topic.id)
if @topic.category_id if @topic.category_id
@ -132,16 +134,17 @@ class EmailController < ApplicationController
def digest_frequencies(user) def digest_frequencies(user)
frequency_in_minutes = user.user_option.digest_after_minutes frequency_in_minutes = user.user_option.digest_after_minutes
email_digests = user.user_option.email_digests
frequencies = DigestEmailSiteSetting.values.dup frequencies = DigestEmailSiteSetting.values.dup
never = frequencies.delete_at(0) never = frequencies.delete_at(0)
allowed_frequencies = %w[never weekly every_month every_six_months] allowed_frequencies = %w[never weekly every_month every_six_months]
result = frequencies.reduce(frequencies: [], current: nil, selected: nil, take_next: false) do |memo, v| 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]) next(memo) unless allowed_frequencies.include?(v[:name])
memo.tap do |m| 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[:frequencies] << [I18n.t("unsubscribe.digest_frequency.#{v[:name]}"), v[:value]]
m[:take_next] = !m[:take_next] && m[:current] m[:take_next] = !m[:take_next] && m[:current]
end end

View File

@ -182,6 +182,13 @@ class UserOption < ActiveRecord::Base
self.title_count_mode_key = UserOption.title_count_modes[value.to_sym] self.title_count_mode_key = UserOption.title_count_modes[value.to_sym]
end 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 private
def update_tracked_topics def update_tracked_topics

View File

@ -57,10 +57,14 @@
<% if @digest_frequencies[:current] %> <% if @digest_frequencies[:current] %>
<h3> <h3>
<%= t( <% if @digest_frequencies[:current] == 'never' %>
'unsubscribe.digest_frequency.title', <%= t('unsubscribe.digest_frequency.never_title') %>
frequency: t("unsubscribe.digest_frequency.#{@digest_frequencies[:current]}") <% else %>
) %> <%= t(
'unsubscribe.digest_frequency.title',
frequency: t("unsubscribe.digest_frequency.#{@digest_frequencies[:current]}")
) %>
<% end %>
</h3> </h3>
<br/> <br/>
<% end %> <% end %>
@ -74,12 +78,14 @@
</p> </p>
<% end %> <% end %>
<p> <% unless @unsubscribed_from_all %>
<label> <p>
<%= check_box_tag 'unsubscribe_all', 1, @type=="all" %> <label>
<%= t 'unsubscribe.all', sitename: SiteSetting.title %> <%= check_box_tag 'unsubscribe_all', 1, @type=="all" %>
</label> <%= t 'unsubscribe.all', sitename: SiteSetting.title %>
</p> </label>
</p>
<% end %>
<br/> <br/>
<% text = @type=='digest' ? t('unsubscribe.submit') : t('unsubscribe.title') %> <% text = @type=='digest' ? t('unsubscribe.submit') : t('unsubscribe.title') %>

View File

@ -1028,6 +1028,7 @@ en:
submit: "Save preferences" submit: "Save preferences"
digest_frequency: digest_frequency:
title: "You are receiving summary emails %{frequency}" title: "You are receiving summary emails %{frequency}"
never_title: "You are not receiving summary emails"
select_title: "Set summary emails frequency to:" select_title: "Set summary emails frequency to:"
never: "never" never: "never"

View File

@ -191,6 +191,24 @@ RSpec.describe EmailController do
expect(response.body).to include(I18n.t("unsubscribe.different_user_description")) expect(response.body).to include(I18n.t("unsubscribe.different_user_description"))
end 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 it 'correctly handles mailing list mode' do
user.user_option.update_columns(mailing_list_mode: true) user.user_option.update_columns(mailing_list_mode: true)