From 380960fa75fe8985a73e5889bd233d917b843972 Mon Sep 17 00:00:00 2001 From: Colin Darie Date: Tue, 3 Jul 2018 20:11:52 +0200 Subject: [PATCH] Checks list: filters & sort --- Gemfile | 1 + Gemfile.lock | 4 + app/controllers/checks_controller.rb | 23 ++++- app/helpers/checks_helper.rb | 18 ++++ app/models/check.rb | 7 ++ app/views/checks/_table.html.erb | 12 ++- app/views/checks/index.html.erb | 15 ++- test/controllers/.rubocop.yml | 7 ++ test/controllers/checks_controller_test.rb | 108 +++++++++++++++++++++ 9 files changed, 189 insertions(+), 6 deletions(-) create mode 100644 test/controllers/.rubocop.yml diff --git a/Gemfile b/Gemfile index c050aba..e3665cb 100644 --- a/Gemfile +++ b/Gemfile @@ -43,6 +43,7 @@ gem 'whenever', require: false gem 'octicons' gem 'kaminari' +gem 'has_scope' # Reduces boot times through caching; required in config/boot.rb gem 'bootsnap', '>= 1.1.0', require: false diff --git a/Gemfile.lock b/Gemfile.lock index 7468329..5e091d7 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -131,6 +131,9 @@ GEM guard-minitest (2.4.6) guard-compat (~> 1.2) minitest (>= 3.0) + has_scope (0.7.2) + actionpack (>= 4.1) + activesupport (>= 4.1) i18n (1.0.1) concurrent-ruby (~> 1.0) io-like (0.3.0) @@ -350,6 +353,7 @@ DEPENDENCIES factory_bot_rails guard guard-minitest + has_scope jbuilder (~> 2.5) kaminari launchy diff --git a/app/controllers/checks_controller.rb b/app/controllers/checks_controller.rb index b592408..fff3482 100644 --- a/app/controllers/checks_controller.rb +++ b/app/controllers/checks_controller.rb @@ -4,8 +4,11 @@ class ChecksController < ApplicationController after_action :verify_authorized, except: :index after_action :verify_policy_scoped, only: :index + has_scope :kind + has_scope :by_domain + def index - @checks = policy_scope(Check).order(:domain_expires_at).page(params[:page]) + @checks = apply_scopes(policy_scope(Check)).order(current_sort).page(params[:page]) end def new @@ -79,4 +82,22 @@ class ChecksController < ApplicationController def build_empty_notification @check.notifications.build end + + def current_sort + @current_sort ||= clean_sort || Check.default_sort + end + helper_method :current_sort + + def clean_sort + return unless params[:sort].present? + field, _, direction = params[:sort].rpartition("_").map(&:to_sym) + + valid_fields = [:domain, :domain_expires_at] + valid_directions = [:asc, :desc] + + return unless valid_fields.include?(field) + return unless valid_directions.include?(direction) + + { field => direction } + end end diff --git a/app/helpers/checks_helper.rb b/app/helpers/checks_helper.rb index ae82a4c..84bf8e0 100644 --- a/app/helpers/checks_helper.rb +++ b/app/helpers/checks_helper.rb @@ -11,4 +11,22 @@ module ChecksHelper return "table-danger" if expiry_date <= 2.weeks.from_now return "table-warning" if expiry_date <= 30.days.from_now end + + def checks_sort_links(field) + current_sort_str = current_sort.to_a.join("_") + + %i[asc desc].map { |direction| + sort = "#{field}_#{direction}" + + icon = direction == :asc ? "chevron-up" : "chevron-down" + html = Octicons::Octicon.new(icon).to_svg.html_safe + + filter_params = current_criterias.merge(sort: sort) + link_to_unless sort == current_sort_str, html, checks_path(filter_params) + }.join + end + + def current_criterias + current_scopes.merge(sort: params[:sort]) + end end diff --git a/app/models/check.rb b/app/models/check.rb index 115119b..6d214a6 100644 --- a/app/models/check.rb +++ b/app/models/check.rb @@ -58,6 +58,13 @@ class Check < ApplicationRecord OR (last_success_at <= DATE_SUB(last_run_at, INTERVAL 5 MINUTE))") } + scope :kind, ->(kind) { where(kind: kind) } + scope :by_domain, ->(domain) { where("domain LIKE ?", "%#{domain}%") } + + def self.default_sort + { domain_expires_at: :asc } + end + private def domain_created_at_past diff --git a/app/views/checks/_table.html.erb b/app/views/checks/_table.html.erb index a0f05b0..94531a1 100644 --- a/app/views/checks/_table.html.erb +++ b/app/views/checks/_table.html.erb @@ -3,9 +3,15 @@ - Domain - Expiry date - Edit + + <%= t(".domain") %> + <%== checks_sort_links(:domain) %> + + + <%= t(".expiry_date") %> + <%== checks_sort_links(:domain_expires_at) %> + + <%= t(".edit") %> diff --git a/app/views/checks/index.html.erb b/app/views/checks/index.html.erb index f823b6f..606ee1f 100644 --- a/app/views/checks/index.html.erb +++ b/app/views/checks/index.html.erb @@ -1,13 +1,24 @@
- <% if @checks.empty? %> + <% if @checks.empty? && current_scopes.blank? %>
<%= t(".no_check_yet_html", new_domain_path: new_check_path(kind: :domain), new_ssl_path: new_check_path(kind: :ssl)) %>
<% else %>

<%= t(".title") %>

- <%= render "table", checks: @checks %> + <%= link_to("Domains", checks_path(current_criterias.merge(kind: :domain))) %> + <%= link_to("SSL", checks_path(current_criterias.merge(kind: :ssl))) %> + <%= form_tag(checks_path(current_scopes), method: :get) do %> + <%= search_field_tag :by_domain, current_scopes[:by_domain] %> + <%= button_tag t(".filter") %> + <% end %> + + <% if @checks.any? %> + <%= render "table", checks: @checks %> + <% else %> +
<%= t(".no_matching_check") %>
+ <% end %> <% end %>
diff --git a/test/controllers/.rubocop.yml b/test/controllers/.rubocop.yml new file mode 100644 index 0000000..9df3ff0 --- /dev/null +++ b/test/controllers/.rubocop.yml @@ -0,0 +1,7 @@ +inherit_from: ../../.rubocop.yml + +Metrics/ClassLength: + Enabled: false + +Metrics/BlockLength: + Enabled: false diff --git a/test/controllers/checks_controller_test.rb b/test/controllers/checks_controller_test.rb index 98035cf..54650cc 100644 --- a/test/controllers/checks_controller_test.rb +++ b/test/controllers/checks_controller_test.rb @@ -31,4 +31,112 @@ class ChecksControllerTest < ActionDispatch::IntegrationTest get new_check_path(kind: :invalid) assert_response :not_found end + + test "checks are ordered by default by expiry date sort" do + c1 = create(:check, user: @user, domain_expires_at: 20.days.from_now) + c2 = create(:check, user: @user, domain_expires_at: 10.days.from_now) + c3 = create(:check, user: @user, domain_expires_at: 1.day.from_now) + + get checks_path + assert_equal [c3, c2, c1], current_checks + end + + test "checks are ordered by expiry date asc" do + c1 = create(:check, user: @user, domain_expires_at: 20.days.from_now) + c2 = create(:check, user: @user, domain_expires_at: 10.days.from_now) + c3 = create(:check, user: @user, domain_expires_at: 1.day.from_now) + + get checks_path(sort: :domain_expires_at_asc) + assert_equal [c3, c2, c1], current_checks + end + + test "checks are ordered by reverse expiring date" do + c1 = create(:check, user: @user, domain_expires_at: 1.day.from_now) + c2 = create(:check, user: @user, domain_expires_at: 10.days.from_now) + c3 = create(:check, user: @user, domain_expires_at: 20.days.from_now) + + get checks_path(sort: :domain_expires_at_desc) + assert_equal [c3, c2, c1], current_checks + end + + test "checks are ordered by domain name asc" do + c1 = create(:check, user: @user, domain: "a") + c2 = create(:check, user: @user, domain: "b") + c3 = create(:check, user: @user, domain: "c") + + get checks_path(sort: :domain_asc) + assert_equal [c1, c2, c3], current_checks + end + + test "checks are ordered by domain name desc" do + c1 = create(:check, user: @user, domain: "a") + c2 = create(:check, user: @user, domain: "b") + c3 = create(:check, user: @user, domain: "c") + + get checks_path(sort: :domain_desc) + assert_equal [c3, c2, c1], current_checks + end + + test "invalid sort fallback to default sort" do + c1 = create(:check, user: @user, domain_expires_at: 20.days.from_now) + c2 = create(:check, user: @user, domain_expires_at: 10.days.from_now) + c3 = create(:check, user: @user, domain_expires_at: 1.day.from_now) + + get checks_path(sort: :invalid_sort_asc) + assert_equal [c3, c2, c1], current_checks + end + + test "checks are filtered by domain kind" do + c1 = create(:check, :domain, user: @user) + c2 = create(:check, :domain, user: @user) + create(:check, :ssl, user: @user) + + get checks_path(kind: :domain) + assert_equal [c1, c2], current_checks + end + + test "checks are filtered by ssl kind" do + create(:check, :domain, user: @user) + create(:check, :domain, user: @user) + c3 = create(:check, :ssl, user: @user) + + get checks_path(kind: :ssl) + assert_equal [c3], current_checks + end + + test "checks are filtered by domain name" do + c1 = create(:check, user: @user, domain: "abc") + c2 = create(:check, user: @user, domain: "bcde") + create(:check, user: @user, domain: "hijk") + + get checks_path(by_domain: "bc") + assert_equal [c1, c2], current_checks + + get checks_path(by_domain: "klm") + assert_empty current_checks + end + + test "checks are paginated" do + create_list(:check, 40, user: @user) + + get checks_path + assert_equal 1, current_checks.current_page + first_page = current_checks + + get checks_path(page: 2) + assert_equal 2, current_checks.current_page + assert_not_equal first_page, current_checks + end + + test "checks are scoped to current user" do + c1 = create(:check, user: @user) + create(:check) + + get checks_path + assert_equal [c1], current_checks + end + + def current_checks + @controller.instance_variable_get("@checks") + end end