From 40fe0f595f489ab5c8befef2089ebc71c73b07e4 Mon Sep 17 00:00:00 2001 From: evazion Date: Wed, 12 Apr 2017 22:38:27 -0500 Subject: [PATCH 1/9] /bans: color code expired/unexpired bans. --- app/assets/stylesheets/common/000_vars.scss | 3 +++ app/assets/stylesheets/specific/bans.scss | 19 +++++++++++++++++++ app/views/bans/index.html.erb | 6 +++--- 3 files changed, 25 insertions(+), 3 deletions(-) create mode 100644 app/assets/stylesheets/specific/bans.scss diff --git a/app/assets/stylesheets/common/000_vars.scss b/app/assets/stylesheets/common/000_vars.scss index 4122adde0..b0cb036f9 100644 --- a/app/assets/stylesheets/common/000_vars.scss +++ b/app/assets/stylesheets/common/000_vars.scss @@ -27,6 +27,9 @@ $preview_flagged_color: #F00; $preview_sample_warning_color: hsl(0, 100%, 90%); // light red $preview_quality_warning_color: hsl(50, 100%, 90%); // light yellow +$error_color: hsl(0, 100%, 95%); // light red +$success_color: hsl(120, 100%, 95%); // light green + @mixin border-radius($val) { -moz-border-radius: $val; -webkit-border-radius: $val; diff --git a/app/assets/stylesheets/specific/bans.scss b/app/assets/stylesheets/specific/bans.scss new file mode 100644 index 000000000..ae58d7a56 --- /dev/null +++ b/app/assets/stylesheets/specific/bans.scss @@ -0,0 +1,19 @@ +@import "../common/000_vars.scss"; + +#c-bans #a-index { + tr[data-expired="true"] { + background-color: $success_color !important; + + &:hover { + background-color: darken($success_color, 5%) !important; + } + } + + tr[data-expired="false"] { + background-color: $error_color !important; + + &:hover { + background-color: darken($error_color, 5%) !important; + } + } +} diff --git a/app/views/bans/index.html.erb b/app/views/bans/index.html.erb index e7808cf82..d1011f82b 100644 --- a/app/views/bans/index.html.erb +++ b/app/views/bans/index.html.erb @@ -1,5 +1,5 @@ -
-
+
+

Bans

@@ -13,7 +13,7 @@ <% @bans.each do |ban| %> - + From 715dcc491b6159cb79298a1d60c3b57d23085056 Mon Sep 17 00:00:00 2001 From: evazion Date: Wed, 12 Apr 2017 22:40:10 -0500 Subject: [PATCH 2/9] /bans: size columns to avoid unnecessary wrapping. Sizes columns such that they automatically shrink to fit. This fixes problems with usernames and dates wrapping in the middle. --- app/assets/stylesheets/common/tables.scss | 16 ++++++++++++++++ app/views/bans/index.html.erb | 4 ++-- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/app/assets/stylesheets/common/tables.scss b/app/assets/stylesheets/common/tables.scss index 32877aa73..551e9f650 100644 --- a/app/assets/stylesheets/common/tables.scss +++ b/app/assets/stylesheets/common/tables.scss @@ -40,6 +40,22 @@ table.striped { } } +/* + * A table where one column expands to fill the screen, while the + * other columns shrink to fit their contents. + */ +table.autofit { + td, th, .col-fit { + white-space: nowrap; + padding-right: 2em; + } + + .col-expand { + white-space: normal; + width: 100%; + } +} + table.search { tr { height: 2em; diff --git a/app/views/bans/index.html.erb b/app/views/bans/index.html.erb index d1011f82b..4ab5c8fff 100644 --- a/app/views/bans/index.html.erb +++ b/app/views/bans/index.html.erb @@ -2,7 +2,7 @@

Bans

-
<%= link_to_user(ban.user) %> <%= ban.expires_at %> <%= format_text ban.reason, :ragel => true %>
+
@@ -16,7 +16,7 @@ - + + + @@ -15,6 +17,8 @@ <% @bans.each do |ban| %> + + - + @@ -19,7 +19,7 @@ - + <% @bans.each do |ban| %> - - + + From 06f2ed685e698c6061f9460c8486b8a1c95dfc0f Mon Sep 17 00:00:00 2001 From: evazion Date: Wed, 12 Apr 2017 23:52:16 -0500 Subject: [PATCH 6/9] /bans: add `reason_matches`, `expired`, `order` search params. --- app/controllers/bans_controller.rb | 4 ++-- app/models/ban.rb | 28 +++++++++++++++++++++++++++- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/app/controllers/bans_controller.rb b/app/controllers/bans_controller.rb index 5b9d1372a..b3eba6c9c 100644 --- a/app/controllers/bans_controller.rb +++ b/app/controllers/bans_controller.rb @@ -11,8 +11,8 @@ class BansController < ApplicationController end def index - @search = Ban.search(params[:search]).order("id desc") - @bans = @search.paginate(params[:page], :limit => params[:limit]) + @bans = Ban.search(params[:search]) + @bans = @bans.paginate(params[:page], :limit => params[:limit]) respond_with(@bans) end diff --git a/app/models/ban.rb b/app/models/ban.rb index 6cbf0d39b..289abf564 100644 --- a/app/models/ban.rb +++ b/app/models/ban.rb @@ -10,13 +10,23 @@ class Ban < ActiveRecord::Base validates_presence_of :user_id, :reason, :duration before_validation :initialize_banner_id, :on => :create + scope :unexpired, -> { where("bans.expires_at > ?", Time.now) } + scope :expired, -> { where("bans.expires_at <= ?", Time.now) } + def self.is_banned?(user) exists?(["user_id = ? AND expires_at > ?", user.id, Time.now]) end + def self.reason_matches(query) + if query =~ /\*/ + where("lower(bans.reason) LIKE ?", query.to_escaped_for_sql_like) + else + where("bans.reason @@ plainto_tsquery(?)", query) + end + end + def self.search(params) q = where("true") - return q if params.blank? if params[:banner_name] q = q.where("banner_id = (select _.id from users _ where lower(_.name) = ?)", params[:banner_name].mb_chars.downcase) @@ -34,6 +44,22 @@ class Ban < ActiveRecord::Base q = q.where("user_id = ?", params[:user_id].to_i) end + if params[:reason_matches].present? + q = q.reason_matches(params[:reason_matches]) + end + + case params[:expired] + when "true" then q = q.expired + when "false" then q = q.unexpired + end + + case params[:order] + when "expires_at_desc" + q = q.order("bans.expires_at desc") + else + q = q.order("bans.id desc") + end + q end From 9bf1c893571226f187474020c1838fccb570d27a Mon Sep 17 00:00:00 2001 From: evazion Date: Wed, 12 Apr 2017 23:51:26 -0500 Subject: [PATCH 7/9] /bans: add search form. --- app/views/bans/_search.html.erb | 8 ++++++++ app/views/bans/index.html.erb | 2 ++ 2 files changed, 10 insertions(+) create mode 100644 app/views/bans/_search.html.erb diff --git a/app/views/bans/_search.html.erb b/app/views/bans/_search.html.erb new file mode 100644 index 000000000..8d7068d70 --- /dev/null +++ b/app/views/bans/_search.html.erb @@ -0,0 +1,8 @@ +<%= simple_form_for(:search, method: :get, url: bans_path, defaults: { required: false }, html: { class: "inline-form" }) do |f| %> + <%= f.input :user_name, label: "User", input_html: { value: params[:search][:user_name] } %> + <%= f.input :banner_name, label: "Banner", input_html: { value: params[:search][:banner_name] } %> + <%= f.input :reason_matches, label: "Reason", hint: "Use * for wildcard", input_html: { value: params[:search][:reason_matches] } %> + <%= f.input :expired, label: "Expired?", collection: [["Yes", true], ["No", false]], include_blank: true, selected: params[:search][:expired] %> + <%= f.input :order, include_blank: false, collection: [%w[Created id_desc], %w[Expiration expires_at_desc]], selected: params[:search][:order] %> + <%= f.submit "Search" %> +<% end %> diff --git a/app/views/bans/index.html.erb b/app/views/bans/index.html.erb index 5db68240c..d98afeabf 100644 --- a/app/views/bans/index.html.erb +++ b/app/views/bans/index.html.erb @@ -2,6 +2,8 @@

Bans

+ <%= render "search" %> +
User
<%= link_to_user(ban.user) %> <%= ban.expires_at %><%= format_text ban.reason, :ragel => true %><%= format_text ban.reason, :ragel => true %> <% if CurrentUser.is_moderator? %> <%= link_to "Edit", edit_ban_path(ban) %> From 1bc4eda12b505368e92c80e2a45ccfbbf0fcccc9 Mon Sep 17 00:00:00 2001 From: evazion Date: Wed, 12 Apr 2017 22:45:21 -0500 Subject: [PATCH 3/9] /bans: add Banner, Banned at columns. --- app/views/bans/index.html.erb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/views/bans/index.html.erb b/app/views/bans/index.html.erb index 4ab5c8fff..3c5f981c8 100644 --- a/app/views/bans/index.html.erb +++ b/app/views/bans/index.html.erb @@ -6,6 +6,8 @@
UserBannerBanned Expires Reason
<%= link_to_user(ban.user) %><%= link_to_user(ban.banner) %><%= time_ago_in_words_tagged(ban.created_at) %> <%= ban.expires_at %> <%= format_text ban.reason, :ragel => true %> From 198b6db507c8dc3106c0f9075500b8887bdfefd1 Mon Sep 17 00:00:00 2001 From: evazion Date: Wed, 12 Apr 2017 22:48:09 -0500 Subject: [PATCH 4/9] /bans: replace Expires column with Duration. --- app/helpers/application_helper.rb | 8 ++++++++ app/views/bans/index.html.erb | 4 ++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 2283393a0..379c46a38 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -81,6 +81,14 @@ module ApplicationHelper content_tag(:time, content || datetime, :datetime => datetime, :title => time.to_formatted_s) end + def humanized_duration(from, to) + duration = distance_of_time_in_words(from, to) + datetime = from.iso8601 + "/" + to.iso8601 + title = "#{from.strftime("%Y-%m-%d %H:%M")} to #{to.strftime("%Y-%m-%d %H:%M")}" + + raw content_tag(:time, duration, datetime: datetime, title: title) + end + def time_ago_in_words_tagged(time) raw time_tag(time_ago_in_words(time) + " ago", time) end diff --git a/app/views/bans/index.html.erb b/app/views/bans/index.html.erb index 3c5f981c8..89139c237 100644 --- a/app/views/bans/index.html.erb +++ b/app/views/bans/index.html.erb @@ -8,7 +8,7 @@ User Banner BannedExpiresDuration Reason
<%= link_to_user(ban.user) %> <%= link_to_user(ban.banner) %> <%= time_ago_in_words_tagged(ban.created_at) %><%= ban.expires_at %><%= humanized_duration(ban.created_at, ban.expires_at) %> <%= format_text ban.reason, :ragel => true %> <% if CurrentUser.is_moderator? %> From 84c075e4bff97c0b341322cbcc687b6a03a28174 Mon Sep 17 00:00:00 2001 From: evazion Date: Wed, 12 Apr 2017 23:49:26 -0500 Subject: [PATCH 5/9] =?UTF-8?q?/bans:=20add=20"=C2=BB"=20links=20for=20nar?= =?UTF-8?q?rowing=20search=20by=20user.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/views/bans/index.html.erb | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/app/views/bans/index.html.erb b/app/views/bans/index.html.erb index 89139c237..5db68240c 100644 --- a/app/views/bans/index.html.erb +++ b/app/views/bans/index.html.erb @@ -16,8 +16,14 @@
<%= link_to_user(ban.user) %><%= link_to_user(ban.banner) %> + <%= link_to_user(ban.user) %> + <%= link_to "»", bans_path(search: params[:search].merge(user_name: ban.user.name)) %> + + <%= link_to_user(ban.banner) %> + <%= link_to "»", bans_path(search: params[:search].merge(banner_name: ban.banner.name)) %> + <%= time_ago_in_words_tagged(ban.created_at) %> <%= humanized_duration(ban.created_at, ban.expires_at) %> <%= format_text ban.reason, :ragel => true %>
From f6fff16e759ec206c9ea4151cc7f903dc2306384 Mon Sep 17 00:00:00 2001 From: evazion Date: Thu, 13 Apr 2017 00:06:02 -0500 Subject: [PATCH 8/9] /bans: avoid N+1 queries for user, banner. Avoids an N+1 issue when rendering users with link_to_user. --- app/controllers/bans_controller.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/app/controllers/bans_controller.rb b/app/controllers/bans_controller.rb index b3eba6c9c..9ce26ebba 100644 --- a/app/controllers/bans_controller.rb +++ b/app/controllers/bans_controller.rb @@ -11,9 +11,10 @@ class BansController < ApplicationController end def index - @bans = Ban.search(params[:search]) - @bans = @bans.paginate(params[:page], :limit => params[:limit]) - respond_with(@bans) + @bans = Ban.search(params[:search]).paginate(params[:page], :limit => params[:limit]) + respond_with(@bans) do |fmt| + fmt.html { @bans = @bans.includes(:user, :banner) } + end end def show From 94e548cfe12f361d96290eaf72f43f08043b6e78 Mon Sep 17 00:00:00 2001 From: evazion Date: Wed, 19 Apr 2017 17:55:02 -0500 Subject: [PATCH 9/9] /bans: add test for searching bans. --- app/models/user_feedback.rb | 2 +- test/unit/ban_test.rb | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/app/models/user_feedback.rb b/app/models/user_feedback.rb index 143c88dd2..deb6f2666 100644 --- a/app/models/user_feedback.rb +++ b/app/models/user_feedback.rb @@ -74,7 +74,7 @@ class UserFeedback < ActiveRecord::Base extend SearchMethods def initialize_creator - self.creator_id = CurrentUser.id + self.creator_id ||= CurrentUser.id end def user_name diff --git a/test/unit/ban_test.rb b/test/unit/ban_test.rb index a66ca41e3..dae6b6eac 100644 --- a/test/unit/ban_test.rb +++ b/test/unit/ban_test.rb @@ -165,6 +165,26 @@ class BanTest < ActiveSupport::TestCase end context "Searching for a ban" do + should "find a given ban" do + CurrentUser.user = FactoryGirl.create(:admin_user) + CurrentUser.ip_addr = "127.0.0.1" + + user = FactoryGirl.create(:user) + ban = FactoryGirl.create(:ban, user: user) + params = { + user_name: user.name, + banner_name: ban.banner.name, + reason: ban.reason, + expired: false, + order: :id_desc + } + + bans = Ban.search(params) + + assert_equal(1, bans.length) + assert_equal(ban.id, bans.first.id) + end + context "by user id" do setup do @admin = FactoryGirl.create(:admin_user)