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-6765: Improve Skin Preview Message #4934

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+39 −39
Open

AO3-6765: Improve Skin Preview Message #4934

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
71 changes: 35 additions & 36 deletions app/controllers/skins_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
class SkinsController < ApplicationController

before_action :users_only, only: [:new, :create, :destroy]
before_action :load_skin, except: [:index, :new, :create, :unset]
before_action :check_title, only: [:create, :update]
Expand All @@ -13,34 +12,31 @@
# GET /skins
def index
is_work_skin = params[:skin_type] && params[:skin_type] == "WorkSkin"
if current_user && current_user.is_a?(User)
@preference = current_user.preference
end
@preference = current_user.preference if current_user && current_user.is_a?(User)

Check warning on line 15 in app/controllers/skins_controller.rb

View workflow job for this annotation

GitHub Actions / Rubocop 

[rubocop] reported by reviewdog 🐶
Use safe navigation (`&.`) instead of checking if an object exists before calling the method.

Raw Output:
app/controllers/skins_controller.rb:15:46: C: Style/SafeNavigation: Use safe navigation (`&.`) instead of checking if an object exists before calling the method.
if params[:user_id] && (@user = User.find_by(login: params[:user_id]))
redirect_to new_user_session_path and return unless logged_in?

if @user != current_user
flash[:error] = "You can only browse your own skins and approved public skins."
redirect_to skins_path and return
end
if is_work_skin
@skins = @user.work_skins.sort_by_recent
@title = ts('My Work Skins')
@title = ts("My Work Skins")

Check warning on line 25 in app/controllers/skins_controller.rb

View workflow job for this annotation

GitHub Actions / Rubocop 

[rubocop] reported by reviewdog 🐶
Prefer Rails built-in `t` helper over `ts` and move the text into the yml file. `ts` is not actually translatable. For more information, refer to https://github.com/otwcode/otwarchive/wiki/Internationalization-(i18n)-Standards

Raw Output:
app/controllers/skins_controller.rb:25:18: C: I18n/DeprecatedHelper: Prefer Rails built-in `t` helper over `ts` and move the text into the yml file. `ts` is not actually translatable. For more information, refer to https://github.com/otwcode/otwarchive/wiki/Internationalization-(i18n)-Standards
else
@skins = @user.skins.site_skins.sort_by_recent
@title = ts('My Site Skins')
@title = ts("My Site Skins")

Check warning on line 28 in app/controllers/skins_controller.rb

View workflow job for this annotation

GitHub Actions / Rubocop 

[rubocop] reported by reviewdog 🐶
Prefer Rails built-in `t` helper over `ts` and move the text into the yml file. `ts` is not actually translatable. For more information, refer to https://github.com/otwcode/otwarchive/wiki/Internationalization-(i18n)-Standards

Raw Output:
app/controllers/skins_controller.rb:28:18: C: I18n/DeprecatedHelper: Prefer Rails built-in `t` helper over `ts` and move the text into the yml file. `ts` is not actually translatable. For more information, refer to https://github.com/otwcode/otwarchive/wiki/Internationalization-(i18n)-Standards
end
elsif is_work_skin
@skins = WorkSkin.approved_skins.sort_by_recent_featured
@title = ts("Public Work Skins")

Check warning on line 32 in app/controllers/skins_controller.rb

View workflow job for this annotation

GitHub Actions / Rubocop 

[rubocop] reported by reviewdog 🐶
Prefer Rails built-in `t` helper over `ts` and move the text into the yml file. `ts` is not actually translatable. For more information, refer to https://github.com/otwcode/otwarchive/wiki/Internationalization-(i18n)-Standards

Raw Output:
app/controllers/skins_controller.rb:32:16: C: I18n/DeprecatedHelper: Prefer Rails built-in `t` helper over `ts` and move the text into the yml file. `ts` is not actually translatable. For more information, refer to https://github.com/otwcode/otwarchive/wiki/Internationalization-(i18n)-Standards
else
if is_work_skin
@skins = WorkSkin.approved_skins.sort_by_recent_featured
@title = ts('Public Work Skins')
else
if logged_in?
@skins = Skin.approved_skins.usable.site_skins.sort_by_recent_featured
else
@skins = Skin.approved_skins.usable.site_skins.cached.sort_by_recent_featured
end
@title = ts('Public Site Skins')
end
@skins = if logged_in?
Skin.approved_skins.usable.site_skins.sort_by_recent_featured
else
Skin.approved_skins.usable.site_skins.cached.sort_by_recent_featured
end
@title = ts("Public Site Skins")

Check warning on line 39 in app/controllers/skins_controller.rb

View workflow job for this annotation

GitHub Actions / Rubocop 

[rubocop] reported by reviewdog 🐶
Prefer Rails built-in `t` helper over `ts` and move the text into the yml file. `ts` is not actually translatable. For more information, refer to https://github.com/otwcode/otwarchive/wiki/Internationalization-(i18n)-Standards

Raw Output:
app/controllers/skins_controller.rb:39:16: C: I18n/DeprecatedHelper: Prefer Rails built-in `t` helper over `ts` and move the text into the yml file. `ts` is not actually translatable. For more information, refer to https://github.com/otwcode/otwarchive/wiki/Internationalization-(i18n)-Standards
end
end

Expand All @@ -60,16 +56,16 @@

# POST /skins
def create
unless params[:skin_type].nil? || params[:skin_type] && %w(Skin WorkSkin).include?(params[:skin_type])
unless params[:skin_type].nil? || (params[:skin_type] && %w[Skin WorkSkin].include?(params[:skin_type]))
flash[:error] = ts("What kind of skin did you want to create?")
redirect_to :new and return
end
loaded = load_archive_parents unless params[:skin_type] && params[:skin_type] == 'WorkSkin'
if params[:skin_type] == "WorkSkin"
@skin = WorkSkin.new(skin_params)
else
@skin = Skin.new(skin_params)
end
loaded = load_archive_parents unless params[:skin_type] && params[:skin_type] == "WorkSkin"
@skin = if params[:skin_type] == "WorkSkin"
WorkSkin.new(skin_params)
else
Skin.new(skin_params)
end
@skin.author = current_user
if @skin.save
flash[:notice] = ts("Skin was successfully created.")
Expand All @@ -79,12 +75,10 @@
else
redirect_to skin_path(@skin)
end
elsif params[:wizard]
render :new_wizard
else
if params[:wizard]
render :new_wizard
else
render :new
end
render :new
end
end

Expand Down Expand Up @@ -120,9 +114,10 @@
def preview
flash[:notice] = []
flash[:notice] << ts("You are previewing the skin %{title}. This is a randomly chosen page.", title: @skin.title)
flash[:notice] << ts("Go back or click any link to remove the skin.")
flash[:notice] << ts("Tip: You can preview any archive page you want by tacking on '?site_skin=[skin_id]' like you can see in the url above.")
flash[:notice] << "<a href='#{skin_path(@skin)}' class='action' role='button'>".html_safe + ts("Return To Skin To Use") + "</a>".html_safe
flash[:notice] << ts("Go back or follow any link to remove the skin.")

Check warning on line 117 in app/controllers/skins_controller.rb

View workflow job for this annotation

GitHub Actions / Rubocop 

[rubocop] reported by reviewdog 🐶
Prefer Rails built-in `t` helper over `ts` and move the text into the yml file. `ts` is not actually translatable. For more information, refer to https://github.com/otwcode/otwarchive/wiki/Internationalization-(i18n)-Standards

Raw Output:
app/controllers/skins_controller.rb:117:23: C: I18n/DeprecatedHelper: Prefer Rails built-in `t` helper over `ts` and move the text into the yml file. `ts` is not actually translatable. For more information, refer to https://github.com/otwcode/otwarchive/wiki/Internationalization-(i18n)-Standards
flash[:notice] << (ts("Tip: You can preview any archive page you want by adding '?site_skin=%{skin_id}' to the end of the URL.", skin_id: @skin.id) +

Check warning on line 118 in app/controllers/skins_controller.rb

View workflow job for this annotation

GitHub Actions / Rubocop 

[rubocop] reported by reviewdog 🐶
Prefer Rails built-in `t` helper over `ts` and move the text into the yml file. `ts` is not actually translatable. For more information, refer to https://github.com/otwcode/otwarchive/wiki/Internationalization-(i18n)-Standards

Raw Output:
app/controllers/skins_controller.rb:118:24: C: I18n/DeprecatedHelper: Prefer Rails built-in `t` helper over `ts` and move the text into the yml file. `ts` is not actually translatable. For more information, refer to https://github.com/otwcode/otwarchive/wiki/Internationalization-(i18n)-Standards
"<p><a href='#{skin_path(@skin)}' class='action'>".html_safe + ts("Return to Skin to Use") + "</a></p>".html_safe)

Check warning on line 119 in app/controllers/skins_controller.rb

View workflow job for this annotation

GitHub Actions / Rubocop 

[rubocop] reported by reviewdog 🐶
Prefer Rails built-in `t` helper over `ts` and move the text into the yml file. `ts` is not actually translatable. For more information, refer to https://github.com/otwcode/otwarchive/wiki/Internationalization-(i18n)-Standards

Raw Output:
app/controllers/skins_controller.rb:119:86: C: I18n/DeprecatedHelper: Prefer Rails built-in `t` helper over `ts` and move the text into the yml file. `ts` is not actually translatable. For more information, refer to https://github.com/otwcode/otwarchive/wiki/Internationalization-(i18n)-Standards
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Apparently the linter had a field day with this file when I saved it - the three lines above are the only three deliberately edited in this file, the rest was the auto-linting, sorry.


tag = FilterCount.where("public_works_count BETWEEN 10 AND 20").random_order.first.filter
redirect_to tag_works_path(tag, site_skin: @skin.id)
end
Expand Down Expand Up @@ -157,15 +152,19 @@
begin
@skin.destroy
flash[:notice] = ts("The skin was deleted.")
rescue
rescue StandardError
flash[:error] = ts("We couldn't delete that right now, sorry! Please try again later.")
end

if current_user && current_user.is_a?(User) && current_user.preference.skin_id == @skin.id
current_user.preference.skin_id = AdminSetting.default_skin_id
current_user.preference.save
end
redirect_to user_skins_path(current_user) rescue redirect_to skins_path
begin
redirect_to user_skins_path(current_user)
rescue StandardError
redirect_to skins_path
end
end

private
Expand Down Expand Up @@ -211,7 +210,7 @@
if params[:add_site_parents]
params[:skin][:skin_parents_attributes] ||= ActionController::Parameters.new
archive_parents = Skin.get_current_site_skin.get_all_parents
skin_parent_titles = params[:skin][:skin_parents_attributes].values.map { |v| v[:parent_skin_title] }
skin_parent_titles = params[:skin][:skin_parents_attributes].values.pluck(:parent_skin_title)
skin_parents = skin_parent_titles.empty? ? [] : Skin.where(title: skin_parent_titles).pluck(:id)
skin_parents += @skin.get_all_parents.collect(&:id) if @skin
unless (skin_parents.uniq & archive_parents.map(&:id)).empty?
Expand All @@ -222,7 +221,7 @@
archive_parents.each do |parent_skin|
last_position += 1
new_skin_parent_hash = ActionController::Parameters.new({ position: last_position, parent_skin_id: parent_skin.id })
params[:skin][:skin_parents_attributes].merge!({last_position.to_s => new_skin_parent_hash})
params[:skin][:skin_parents_attributes].merge!({ last_position.to_s => new_skin_parent_hash })
end
return true
end
Expand Down
7 changes: 4 additions & 3 deletions features/other_b/skin_public.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,8 @@ Feature: Public skins
And I follow "Preview"
Then I should be on the works tagged "Major Crimes"
And I should see "You are previewing the skin Usable Skin. This is a randomly chosen page."
And I should see "Go back or click any link to remove the skin"
And I should see "Tip: You can preview any archive page you want by tacking on '?site_skin=[skin_id]' like you can see in the url above."
When I follow "Return To Skin To Use"
And I should see "Go back or follow any link to remove the skin"
And I should see "Tip: You can preview any archive page you want by adding '?site_skin="
And I should see "' to the end of the URL."
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'm not familiar enough with Ruby test suites to have been able to figure out how to put the actual test-skin ID into this string, so split it up into the before and after parts. If there's a better way to do that I would love to learn :)

When I follow "Return to Skin to Use"
Then I should be on "Usable Skin" skin page
Loading