Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UX: suspend forever time period messages #13776

Merged
merged 8 commits into from
Jul 20, 2021
7 changes: 1 addition & 6 deletions app/controllers/session_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -589,13 +589,8 @@ def admin_not_allowed_from_ip_address(user)
end

def failed_to_login(user)
message = user.suspend_reason ? "login.suspended_with_reason" : "login.suspended"

{
error: I18n.t(message,
date: I18n.l(user.suspended_till, format: :date_only),
reason: Rack::Utils.escape_html(user.suspend_reason)
),
error: user.suspended_message,
reason: 'suspended'
}
end
Expand Down
46 changes: 32 additions & 14 deletions app/mailers/user_notifications.rb
Original file line number Diff line number Diff line change
Expand Up @@ -153,27 +153,45 @@ def account_silenced(user, opts = nil)

return unless user_history = opts[:user_history]

build_email(
user.email,
template: "user_notifications.account_silenced",
locale: user_locale(user),
reason: user_history.details,
silenced_till: I18n.l(user.silenced_till, format: :long)
)
if user.silenced_forever?
build_email(
user.email,
template: "user_notifications.account_silenced_forever",
locale: user_locale(user),
reason: user_history.details
)
else
build_email(
user.email,
template: "user_notifications.account_silenced",
locale: user_locale(user),
reason: user_history.details,
silenced_till: I18n.l(user.silenced_till, format: :long)
)
end
end

def account_suspended(user, opts = nil)
opts ||= {}

return unless user_history = opts[:user_history]

build_email(
user.email,
template: "user_notifications.account_suspended",
locale: user_locale(user),
reason: user_history.details,
suspended_till: I18n.l(user.suspended_till, format: :long)
)
if user.suspended_forever?
build_email(
user.email,
template: "user_notifications.account_suspended_forever",
locale: user_locale(user),
reason: user_history.details
)
else
build_email(
user.email,
template: "user_notifications.account_suspended",
locale: user_locale(user),
reason: user_history.details,
suspended_till: I18n.l(user.suspended_till, format: :long)
)
end
end

def account_exists(user, opts = {})
Expand Down
25 changes: 25 additions & 0 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -958,6 +958,10 @@ def silenced_at
silenced_record.try(:created_at) if silenced?
end

def silenced_forever?
silenced_till > 100.years.from_now
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope Discourse lasts long enough that this becomes a bug!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That'll be a tremendous thing to celebrate :)

end

def suspend_record
UserHistory.for(self, :suspend_user).order('id DESC').first
end
Expand All @@ -974,6 +978,27 @@ def suspend_reason
nil
end

def suspended_message
return nil unless suspended?

message = "login.suspended"
if suspend_reason
if suspended_forever?
message = "login.suspended_with_reason_forever"
else
message = "login.suspended_with_reason"
end
end

I18n.t(message,
date: I18n.l(suspended_till, format: :date_only),
reason: Rack::Utils.escape_html(suspend_reason))
end

def suspended_forever?
suspended_till > 100.years.from_now
end

# Use this helper to determine if the user has a particular trust level.
# Takes into account admin, etc.
def has_trust_level?(level)
Expand Down
17 changes: 17 additions & 0 deletions config/locales/server.en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2515,6 +2515,7 @@ en:
reset_not_allowed_from_ip_address: "You can't request a password reset from that IP address."
suspended: "You can't log in until %{date}."
suspended_with_reason: "Account suspended until %{date}: %{reason}"
suspended_with_reason_forever: "Account suspended: %{reason}"
errors: "%{errors}"
not_available: "Not available. Try %{suggestion}?"
something_already_taken: "Something went wrong, perhaps the username or email is already registered. Try the forgot password link."
Expand Down Expand Up @@ -3703,6 +3704,14 @@ en:

Reason - %{reason}

account_suspended_forever:
title: "Account Suspended"
subject_template: "[%{email_prefix}] Your account has been suspended"
text_body_template: |
You have been suspended from the forum.

Reason - %{reason}

account_silenced:
title: "Account Silenced"
subject_template: "[%{email_prefix}] Your account has been silenced"
Expand All @@ -3711,6 +3720,14 @@ en:

Reason - %{reason}

account_silenced_forever:
title: "Account Silenced"
subject_template: "[%{email_prefix}] Your account has been silenced"
text_body_template: |
You have been silenced from the forum.

Reason - %{reason}

account_exists:
title: "Account already exists"
subject_template: "[%{email_prefix}] Account already exists"
Expand Down
3 changes: 1 addition & 2 deletions lib/auth/result.rb
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,7 @@ def to_client_hash
if user&.suspended?
return {
suspended: true,
suspended_message: I18n.t(user.suspend_reason ? "login.suspended_with_reason" : "login.suspended",
date: I18n.l(user.suspended_till, format: :date_only), reason: user.suspend_reason)
suspended_message: user.suspended_message
}
end

Expand Down
22 changes: 18 additions & 4 deletions spec/requests/session_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1401,11 +1401,25 @@ def login_with_sso_and_invite(invite_key = invite.invite_key)
login: user.username, password: 'myawesomepassword'
}

expected_message = I18n.t('login.suspended_with_reason',
date: I18n.l(user.suspended_till, format: :date_only),
reason: Rack::Utils.escape_html(user.suspend_reason))
expect(response.status).to eq(200)
expect(response.parsed_body['error']).to eq(I18n.t('login.suspended_with_reason',
date: I18n.l(user.suspended_till, format: :date_only),
reason: Rack::Utils.escape_html(user.suspend_reason)
))
expect(response.parsed_body['error']).to eq(expected_message)
end

it 'when suspended forever should return an error without suspended till date' do
user.suspended_till = 101.years.from_now
user.suspended_at = Time.now
user.save!
StaffActionLogger.new(user).log_user_suspend(user, "<strike>banned</strike>")

post "/session.json", params: {
login: user.username, password: 'myawesomepassword'
}

expected_message = I18n.t('login.suspended_with_reason_forever', reason: Rack::Utils.escape_html(user.suspend_reason))
expect(response.parsed_body['error']).to eq(expected_message)
end
end

Expand Down