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.

Sign up for GitHub

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

Jump to bottom

AO3-6308 fix user suspension end time #4338

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
+47 −11
Open

AO3-6308 fix user suspension end time #4338

Changes from 3 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
e54b1e2
fix user suspension end time
Cesium-Ice Sep 14, 2022
09cf24f
Merge branch 'master' into AO3-6308-correct-user-suspension-end-time-…
Cesium-Ice Dec 28, 2022
9824485
please hound
Cesium-Ice Dec 28, 2022
a4854d4
merge from master
Cesium-Ice May 21, 2024
4f4a914
date localization suggestion
Cesium-Ice May 21, 2024
ea83a19
avoid html in translation string
Cesium-Ice May 21, 2024
ed87454
fix translation file issues
Cesium-Ice May 22, 2024
7b4f89d
Merge branch 'master' into AO3-6308-correct-user-suspension-end-time-…
Cesium-Ice Sep 10, 2024
8aa302f
fix merge conflict part 2
Cesium-Ice Sep 10, 2024
1be34dd
address linter
Cesium-Ice Sep 10, 2024
6180e3f
add unit test
Cesium-Ice Sep 14, 2024
293c305
make reset_blocked_html use different locale key
Cesium-Ice Sep 22, 2024
fef4613
Update app/controllers/application_controller.rb
Cesium-Ice Sep 22, 2024
e0dcfc8
Update app/controllers/application_controller.rb
Cesium-Ice Sep 22, 2024
ba42b57
Update features/users/suspensions.feature
Cesium-Ice Sep 22, 2024
0018798
Update features/users/suspensions.feature
Cesium-Ice Sep 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion app/controllers/application_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,16 @@ def use_caching?
def check_user_status
if current_user.is_a?(User) && (current_user.suspended? || current_user.banned?)
if current_user.suspended?
flash[:error] = t('suspension_notice', default: "Your account has been suspended until %{suspended_until}. You may not add or edit content until your suspension has been resolved. Please <a href=\"#{new_abuse_report_path}\">contact Abuse</a> for more information.", suspended_until: localize(current_user.suspended_until)).html_safe
suspension_end = current_user.suspended_until

# Unban threshold is 6:51pm, 12 hours after the unsuspend_users rake task located in schedule.rb is run at 6:51am
unban_theshold = DateTime.new(suspension_end.year, suspension_end.month, suspension_end.day, 18, 51, 0, "+00:00")

# If the stated suspension end date is after the unban threshold we need to advance a day
suspension_end = suspension_end.next_day(1) if suspension_end > unban_theshold
localized_suspension_end = DateTime.parse(localize(suspension_end)).strftime("%Y-%m-%d")
brianjaustin marked this conversation as resolved.
Show resolved Hide resolved
flash[:error] = t("suspension_notice", default: "Your account has been suspended until %{suspended_until}. You may not add or edit content until your suspension has been resolved. Please <a href=\"#{new_abuse_report_path}\">contact Abuse</a> for more information.", suspended_until: localized_suspension_end).html_safe
Copy link
Member

Choose a reason for hiding this comment

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

We're trying to avoid HTML in translation strings when possible; could you update the string here to use a variable (like "%{abuse_link}") and the link_to method? The code in https://github.com/otwcode/otwarchive/blob/master/app/controllers/users/passwords_controller.rb#L11 does something similar

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've edited line 441 to be like so

flash[:error] = t(".users.status.suspension_notice", suspended_until: localized_suspension_end, contact_abuse_link: view_context.link_to(t(".contact_abuse"), new_abuse_report_path)).html_safe

and added the following to en.yml:

status:
      ban_notice: Your account has been banned. You are not permitted to add or edit archive content. Please %{contact_abuse_link} for more information.
      suspension_notice: Your account has been suspended until %{suspended_until}. You may not add or edit content until your suspension has been resolved. Please %{contact_abuse_link} for more information.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

However I still get missing translations and unused key errors, so something is wrong with the link between the controller and the translation file. Do you know what I'm doing wrong?

Copy link
Member

Choose a reason for hiding this comment

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

It looks like the test is expecting the following absolute/fully-qualified keys

+--------+----------------------------------------------------+--------------------------------------------------------+
| Locale | Key                                                | Value in other locales or source                       |
+--------+----------------------------------------------------+--------------------------------------------------------+
|   en   | application.check_user_not_suspended.contact_abuse | app/controllers/application_controller.rb:423          |
|   en   | application.check_user_status.contact_abuse        | app/controllers/application_controller.rb:401 (1 more) |
+--------+----------------------------------------------------+--------------------------------------------------------+

If you wanted to use a key like

otwarchive/config/locales/controllers/en.yml

Line 93 in ea83a19

contact_abuse: contact Policy & Abuse
, it would need to be absolute (users.passwords.create.contact_abuse) -- although I would recommend extracting that key to a common location if it's used in multiple controllers


else
flash[:error] = t('ban_notice', default: "Your account has been banned. You are not permitted to add or edit archive content. Please <a href=\"#{new_abuse_report_path}\">contact Abuse</a> for more information.").html_safe
end
Expand Down