From 3e7db9d1b5f57c94179b0128c9aea8996b3070fa Mon Sep 17 00:00:00 2001 From: MaiteHdezRivas Date: Wed, 20 Apr 2016 16:40:58 +0200 Subject: [PATCH 1/3] Best practices code changes --- app/controllers/debates_controller.rb | 9 +++++++-- app/helpers/debates_helper.rb | 2 +- app/models/abilities/administrator.rb | 5 ++--- app/models/debate.rb | 2 +- app/views/debates/_actions.html.erb | 8 ++++---- app/views/debates/_featured_debates.html.erb | 2 +- app/views/debates/index.html.erb | 4 ++-- config/locales/admin.en.yml | 4 ++-- config/locales/admin.es.yml | 4 ++-- config/routes.rb | 4 ++-- 10 files changed, 24 insertions(+), 20 deletions(-) diff --git a/app/controllers/debates_controller.rb b/app/controllers/debates_controller.rb index 4a22d2e3e..cd213bd94 100644 --- a/app/controllers/debates_controller.rb +++ b/app/controllers/debates_controller.rb @@ -18,6 +18,11 @@ class DebatesController < ApplicationController helper_method :resource_model, :resource_name respond_to :html, :js + def index + super + @featured_debates = @debates.featured + end + def show super redirect_to debate_path(@debate), status: :moved_permanently if request.path != debate_path(@debate) @@ -28,12 +33,12 @@ class DebatesController < ApplicationController set_debate_votes(@debate) end - def remove_feature + def unmark_featured @debate.update_attribute(:featured_at, nil) redirect_to request.query_parameters.merge(action: :index) end - def feature + def mark_featured @debate.update_attribute(:featured_at, Time.now) redirect_to request.query_parameters.merge(action: :index) end diff --git a/app/helpers/debates_helper.rb b/app/helpers/debates_helper.rb index d12db7f78..3c1aa02a7 100644 --- a/app/helpers/debates_helper.rb +++ b/app/helpers/debates_helper.rb @@ -1,6 +1,6 @@ module DebatesHelper - def has_featured + def has_featured? Debate.all.featured.count > 0 end end \ No newline at end of file diff --git a/app/models/abilities/administrator.rb b/app/models/abilities/administrator.rb index 9727e8e35..55759959e 100644 --- a/app/models/abilities/administrator.rb +++ b/app/models/abilities/administrator.rb @@ -30,9 +30,8 @@ module Abilities can :confirm_hide, User cannot :confirm_hide, User, hidden_at: nil - can :feature, Debate - - can :remove_feature, Debate + can :mark_featured, Debate + can :unmark_featured, Debate can :comment_as_administrator, [Debate, Comment, Proposal] diff --git a/app/models/debate.rb b/app/models/debate.rb index 249ae0c44..cdae527f7 100644 --- a/app/models/debate.rb +++ b/app/models/debate.rb @@ -134,7 +134,7 @@ class Debate < ActiveRecord::Base end def featured? - ! self.featured_at.nil? + self.featured_at.present? end end diff --git a/app/views/debates/_actions.html.erb b/app/views/debates/_actions.html.erb index b88d4830c..1b61e045b 100644 --- a/app/views/debates/_actions.html.erb +++ b/app/views/debates/_actions.html.erb @@ -9,13 +9,13 @@ method: :put, data: { confirm: t('admin.actions.confirm') } %> <% end %> -<% if can? :feature, debate %> -  |  +<% if can? :mark_featured, debate %> +  |  <% if debate.featured? %> - <%= link_to t("admin.actions.remove_feature").capitalize, remove_feature_debate_path(debate), + <%= link_to t("admin.actions.unmark_featured").capitalize, unmark_featured_debate_path(debate), method: :put, data: { confirm: t('admin.actions.confirm') } %> <% else %> - <%= link_to t("admin.actions.feature").capitalize, feature_debate_path(debate), + <%= link_to t("admin.actions.mark_featured").capitalize, mark_featured_debate_path(debate), method: :put, data: { confirm: t('admin.actions.confirm') } %> <% end %> <% end %> diff --git a/app/views/debates/_featured_debates.html.erb b/app/views/debates/_featured_debates.html.erb index e678e2da8..577a6da77 100644 --- a/app/views/debates/_featured_debates.html.erb +++ b/app/views/debates/_featured_debates.html.erb @@ -4,7 +4,7 @@

<%= t("debates.index.featured_debates") %>

- <% @debates.featured.each do |debate| %> + <% @featured_debates.each do |debate| %>

<%= link_to debate.title, debate %>

<%= link_to debate.author.name, user_path(debate.author)%> diff --git a/app/views/debates/index.html.erb b/app/views/debates/index.html.erb index 7ee9d12d4..ab0a137c7 100644 --- a/app/views/debates/index.html.erb +++ b/app/views/debates/index.html.erb @@ -25,8 +25,8 @@ <% end %>
- <% unless @tag_filter || @search_terms || !has_featured %> - <%= render "featured_debates", debate: @debates.featured %> + <% unless @tag_filter || @search_terms || !has_featured? %> + <%= render "featured_debates" %> <% end %> <%= render "shared/advanced_search", search_path: debates_path(page: 1) %> diff --git a/config/locales/admin.en.yml b/config/locales/admin.en.yml index 024c0140d..60b615562 100755 --- a/config/locales/admin.en.yml +++ b/config/locales/admin.en.yml @@ -7,8 +7,8 @@ en: hide: Hide hide_author: Hide author restore: Restore - feature: Feature - remove_feature: Remove feature + mark_featured: Featured + unmark_featured: Unmark featured activity: show: action: Action diff --git a/config/locales/admin.es.yml b/config/locales/admin.es.yml index 0493cedeb..c2a3669d5 100644 --- a/config/locales/admin.es.yml +++ b/config/locales/admin.es.yml @@ -7,8 +7,8 @@ es: hide: Ocultar hide_author: Bloquear al autor restore: Volver a mostrar - feature: Destacar - remove_feature: Quitar destacado + mark_featured: Destacar + unmark_featured: Quitar destacado activity: show: action: Acción diff --git a/config/routes.rb b/config/routes.rb index 79aa167e5..b9cdbdd3a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -36,8 +36,8 @@ Rails.application.routes.draw do post :vote put :flag put :unflag - put :remove_feature - put :feature + put :mark_featured + put :unmark_featured end collection do get :map From f771a61a50b1c92e387bfd0c6eaeddee1a8b6bc0 Mon Sep 17 00:00:00 2001 From: MaiteHdezRivas Date: Thu, 21 Apr 2016 11:13:58 +0200 Subject: [PATCH 2/3] Refactoring method on debates controller --- .../concerns/commentable_actions.rb | 6 ++++ app/controllers/debate_links_controller.rb | 31 ------------------- app/controllers/debates_controller.rb | 9 +++--- 3 files changed, 10 insertions(+), 36 deletions(-) delete mode 100644 app/controllers/debate_links_controller.rb diff --git a/app/controllers/concerns/commentable_actions.rb b/app/controllers/concerns/commentable_actions.rb index 01066c8ad..a948948a2 100644 --- a/app/controllers/concerns/commentable_actions.rb +++ b/app/controllers/concerns/commentable_actions.rb @@ -9,6 +9,7 @@ module CommentableActions @resources = @resources.tagged_with(@tag_filter) if @tag_filter @resources = @resources.page(params[:page]).for_render.send("sort_by_#{@current_order}") index_customization if index_customization.present? + featured_debates if featured_debates.present? @tag_cloud = tag_cloud set_resource_votes(@resources) @@ -152,4 +153,9 @@ module CommentableActions def index_customization nil end + + def featured_debates + nil + end + end diff --git a/app/controllers/debate_links_controller.rb b/app/controllers/debate_links_controller.rb deleted file mode 100644 index f6f7c3af8..000000000 --- a/app/controllers/debate_links_controller.rb +++ /dev/null @@ -1,31 +0,0 @@ -class DebateLinksController < ApplicationController - include FeatureFlags - include CommentableActions - - before_action :authenticate_user!, except: [:show] - - load_and_authorize_resource class: "Debate" - - feature_flag :debates - - respond_to :html, :js - - private - - def create_params - params.require(:debate).permit(:title, :external_link, :tag_list, :terms_of_service, :captcha, :captcha_key).merge(link_required: true) - end - - def debate_params - params.require(:debate).permit(:title, :external_link, :tag_list, :terms_of_service, :captcha, :captcha_key).merge(link_required: true) - end - - def after_create_path - debate_path(@resource) - end - - def resource_model - Debate - end - -end diff --git a/app/controllers/debates_controller.rb b/app/controllers/debates_controller.rb index cd213bd94..c743d58a7 100644 --- a/app/controllers/debates_controller.rb +++ b/app/controllers/debates_controller.rb @@ -18,11 +18,6 @@ class DebatesController < ApplicationController helper_method :resource_model, :resource_name respond_to :html, :js - def index - super - @featured_debates = @debates.featured - end - def show super redirect_to debate_path(@debate), status: :moved_permanently if request.path != debate_path(@debate) @@ -43,6 +38,10 @@ class DebatesController < ApplicationController redirect_to request.query_parameters.merge(action: :index) end + def featured_debates + @featured_debates = @debates.featured + end + private def debate_params From c86f3516db59028e008864e5006646d4c79b630c Mon Sep 17 00:00:00 2001 From: MaiteHdezRivas Date: Thu, 21 Apr 2016 11:30:45 +0200 Subject: [PATCH 3/3] correct text in the test --- spec/features/debates_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/features/debates_spec.rb b/spec/features/debates_spec.rb index b7cc3ea3a..9db200f11 100644 --- a/spec/features/debates_spec.rb +++ b/spec/features/debates_spec.rb @@ -1017,10 +1017,10 @@ feature 'Debates' do debate2 = create(:debate, featured_at: Time.now) visit debate_path(debate1) - expect(page).to have_content("Feature") + expect(page).to have_content("Featured") visit debate_path(debate2) - expect(page).to have_content("Remove feature") + expect(page).to have_content("Unmark featured") end