Skip to content

Commit

Permalink
AO3-4728 Tidy and apply review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@weeklies
weeklies committed May 28, 2024
1 parent 12d6ac2 commit 4c737bb
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 33 deletions.
4 changes: 2 additions & 2 deletions app/models/subscription.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ class Subscription < ApplicationRecord
# if there's an invalid subscribable type
validates :subscribable, presence: true,
if: proc { |s| VALID_SUBSCRIBABLES.include?(s.subscribable_type) }

# Get the subscriptions associated with this work
# currently: users subscribed to work, users subscribed to creator of work
scope :for_work, lambda {|work|
Expand All @@ -32,7 +32,7 @@ def name
subscribable.name
elsif subscribable.respond_to?(:title)
subscribable.title
elsif subscribable.nil?
else
I18n.t("subscriptions.deleted")
end
end
Expand Down
43 changes: 23 additions & 20 deletions app/views/subscriptions/index.html.erb
View file
Original file line number Diff line number Diff line change
@@ -1,40 +1,43 @@
<!--Descriptive page name, messages and instructions-->
<h2 class="heading">
<%= params[:type].blank? ? ts('My Subscriptions') :
ts('My %{type} Subscriptions', type: params[:type].to_s.singularize.titleize)
%>
<% if params[:type].blank? %>
<%= t(".page_heading.general") %>
<% else %>
<%= t(".page_heading.type", type: params[:type].to_s.singularize.titleize) %>
<% end %>
</h2>
<!--/descriptions-->

<!--subnav-->
<h3 class="landmark heading"><%= ts('Navigation') %></h3>
<h3 class="landmark heading"><%= t("a11y.navigation") %></h3>
<ul class="navigation actions" role="navigation">
<li>
<%= span_if_current ts('All Subscriptions'),
user_subscriptions_path(@user), params[:type].blank? %>
<%= span_if_current(t(".navigation.all"),
user_subscriptions_path(@user),

Check failure on line 16 in app/views/subscriptions/index.html.erb

View workflow job for this annotation

GitHub Actions / ERB Lint runner 

[] reported by reviewdog 🐶
Layout/ArgumentAlignment: Use one level of indentation for arguments following the first line of a multi-line method call.

Raw Output:
app/views/subscriptions/index.html.erb:16:24: Layout/ArgumentAlignment: Use one level of indentation for arguments following the first line of a multi-line method call.
params[:type].blank?) %>

Check failure on line 17 in app/views/subscriptions/index.html.erb

View workflow job for this annotation

GitHub Actions / ERB Lint runner 

[] reported by reviewdog 🐶
Layout/ArgumentAlignment: Use one level of indentation for arguments following the first line of a multi-line method call.

Raw Output:
app/views/subscriptions/index.html.erb:17:24: Layout/ArgumentAlignment: Use one level of indentation for arguments following the first line of a multi-line method call.
</li>
<li>
<%= span_if_current ts('Series Subscriptions'),
user_subscriptions_path(@user, :type => "series"),
params[:type] == "series" %>
<%= span_if_current(t(".navigation.series"),
user_subscriptions_path(@user, type: "series"),

Check failure on line 21 in app/views/subscriptions/index.html.erb

View workflow job for this annotation

GitHub Actions / ERB Lint runner 

[] reported by reviewdog 🐶
Layout/ArgumentAlignment: Use one level of indentation for arguments following the first line of a multi-line method call.

Raw Output:
app/views/subscriptions/index.html.erb:21:24: Layout/ArgumentAlignment: Use one level of indentation for arguments following the first line of a multi-line method call.
params[:type] == "series") %>

Check failure on line 22 in app/views/subscriptions/index.html.erb

View workflow job for this annotation

GitHub Actions / ERB Lint runner 

[] reported by reviewdog 🐶
Layout/ArgumentAlignment: Use one level of indentation for arguments following the first line of a multi-line method call.

Raw Output:
app/views/subscriptions/index.html.erb:22:24: Layout/ArgumentAlignment: Use one level of indentation for arguments following the first line of a multi-line method call.
</li>
<li>
<%= span_if_current ts('User Subscriptions'),
user_subscriptions_path(@user, :type => "users"),
params[:type] == "users" %>
<%= span_if_current(t(".navigation.user"),
user_subscriptions_path(@user, type: "users"),

Check failure on line 26 in app/views/subscriptions/index.html.erb

View workflow job for this annotation

GitHub Actions / ERB Lint runner 

[] reported by reviewdog 🐶
Layout/ArgumentAlignment: Use one level of indentation for arguments following the first line of a multi-line method call.

Raw Output:
app/views/subscriptions/index.html.erb:26:24: Layout/ArgumentAlignment: Use one level of indentation for arguments following the first line of a multi-line method call.
params[:type] == "users") %>

Check failure on line 27 in app/views/subscriptions/index.html.erb

View workflow job for this annotation

GitHub Actions / ERB Lint runner 

[] reported by reviewdog 🐶
Layout/ArgumentAlignment: Use one level of indentation for arguments following the first line of a multi-line method call.

Raw Output:
app/views/subscriptions/index.html.erb:27:24: Layout/ArgumentAlignment: Use one level of indentation for arguments following the first line of a multi-line method call.
</li>
<li>
<%= span_if_current ts('Work Subscriptions'),
user_subscriptions_path(@user, :type => "works"),
params[:type] == "works" %>
<%= span_if_current(t(".navigation.work"),
user_subscriptions_path(@user, type: "works"),

Check failure on line 31 in app/views/subscriptions/index.html.erb

View workflow job for this annotation

GitHub Actions / ERB Lint runner 

[] reported by reviewdog 🐶
Layout/ArgumentAlignment: Use one level of indentation for arguments following the first line of a multi-line method call.

Raw Output:
app/views/subscriptions/index.html.erb:31:24: Layout/ArgumentAlignment: Use one level of indentation for arguments following the first line of a multi-line method call.
params[:type] == "works") %>

Check failure on line 32 in app/views/subscriptions/index.html.erb

View workflow job for this annotation

GitHub Actions / ERB Lint runner 

[] reported by reviewdog 🐶
Layout/ArgumentAlignment: Use one level of indentation for arguments following the first line of a multi-line method call.

Raw Output:
app/views/subscriptions/index.html.erb:32:24: Layout/ArgumentAlignment: Use one level of indentation for arguments following the first line of a multi-line method call.
</li>
</ul>
<!--/subnav-->

<!--main content-->
<%= will_paginate @subscriptions %>

<h3 class="landmark heading"><%= ts('List of Subscriptions') %></h3>
<h3 class="landmark heading"><%= t(".heading.landmark.list") %></h3>
<dl class="subscription index group">
<% @subscriptions.each do |subscription| %>
<dt>
Expand All @@ -49,17 +52,17 @@
<% if params[:type].blank? && subscription.subscribable_type == "Series" %>
<%= t(".series") %>
<% end %>
<% unless subscription.subscribable.nil? || subscription.subscribable_type == "User" %>
<%= t(".byline", :creators => byline(subscription.subscribable)).html_safe %>
<% if subscription.subscribable || subscription.subscribable_type == "User" %>
<%= t(".byline", creators: byline(subscription.subscribable)) %>
<% end %>
</dt>
<dd>
<%= form_for [current_user, subscription], :html => {:method => :delete} do |f| %>
<%= f.submit t(".unsubscribe", :name => subscription.name).html_safe %>
<%= f.submit t(".button", name: subscription.name) %>
<% end %>
</dd>
<% end %>
</dl>

<%= will_paginate @subscriptions %>
<!--/content-->
<!--/content-->
4 changes: 2 additions & 2 deletions config/locales/models/en.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -229,11 +229,11 @@ en:
other: Works
attributes:
ticket_number: Ticket ID
subscriptions:
deleted: "Deleted item"
errors:
attributes:
ticket_number:
closed_ticket: must not be closed.
invalid_department: must be in your department.
required: must exist and not be spam.
subscriptions:
deleted: Deleted item
23 changes: 17 additions & 6 deletions config/locales/views/en.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -404,12 +404,6 @@ en:
preferences_link_text: update your preferences
prompt_meme: Signing up for this challenge will allow any user who claims your prompt to gift you a work in response to your prompt regardless of your preference settings. If you wish to receive additional gifts from users who have not claimed your prompts, please %{preferences_link} to allow gifts from anyone. You can always %{refuse_link}.
refuse_link_text: refuse a gift
subscriptions:
index:
byline: "by %{creators}"
series: "(Series)"
work: "(Work)"
unsubscribe: "Unsubscribe from %{name}"
collection_items:
collection_item_form:
add: Add
Expand Down Expand Up @@ -786,6 +780,23 @@ en:
skins:
confirm_delete:
confirm_html: Are you sure you want to <strong><em>delete</em></strong> the skin "%{skin_title}"?
subscriptions:
index:
button: Unsubscribe from %{name}
byline: by %{creators}
heading:
landmark:
list: List of Subscriptions
navigation:
all: All Subscriptions
series: Series Subscriptions
user: User Subscriptions
work: Work Subscriptions
page_heading:
general: My Subscriptions
type: My %{type} Subscriptions
series: "(Series)"
work: "(Work)"
tag_wranglers:
show:
last_wrangled_html: "%{wrangler_login} last wrangled at %{time}."
Expand Down
6 changes: 3 additions & 3 deletions spec/controllers/subscriptions_controller_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,14 @@
let(:author) { create(:user) }
let(:work) { create(:work) }
let(:series) { create(:series) }
let!(:sub_series) { create(:subscription, user: user, subscribable_type: "Series", subscribable_id: series.id) }
let!(:sub_series) { create(:subscription, user: user, subscribable: series) }
let!(:sub_work) { create(:subscription, user: user, subscribable_type: "Work", subscribable_id: work.id) }
let!(:sub_user) { create(:subscription, user: user, subscribable_type: "User", subscribable_id: author.id) }

it "renders the user subscriptions" do
get :index, params: { user_id: user.login }
expect(response).to render_template("index")
expect(assigns(:subscriptions)).to satisfy { |subs| subs.size == 3 }
expect(assigns(:subscriptions)).to contain_exactly(sub_series, sub_work, sub_user)
end
end

Expand All @@ -42,7 +42,7 @@
it "renders the user subscriptions" do
get :index, params: { user_id: user.login }
expect(response).to render_template("index")

bad_sub = assigns(:subscriptions)[0]
expect(bad_sub.subscribable_id).to eq(-1)
expect(bad_sub.name).to eq("Deleted item")
Expand Down

0 comments on commit 4c737bb

Please sign in to comment.