From 9bd93602bac6c81309843f4bb5ad9c7a22743164 Mon Sep 17 00:00:00 2001 From: Joshua Wehner Date: Mon, 18 Jul 2022 14:17:45 +0200 Subject: [PATCH] Admin can attach user via username (#18056) * Admin can attach user via username * Formatting * Match @username as seen elsewhere * Matching for accessibility Co-authored-by: Suzanne Aitchison * Formatting cleanup after merge Co-authored-by: Suzanne Aitchison --- .../admin/response_templates_controller.rb | 8 ++ app/models/response_template.rb | 6 ++ app/queries/admin/users_query.rb | 19 +++- .../admin/response_templates/_form.html.erb | 4 +- .../admin/response_templates/index.html.erb | 4 +- spec/queries/admin/users_query_spec.rb | 86 ++++++++++++++----- 6 files changed, 98 insertions(+), 29 deletions(-) diff --git a/app/controllers/admin/response_templates_controller.rb b/app/controllers/admin/response_templates_controller.rb index 36e548837..f83f30c0e 100644 --- a/app/controllers/admin/response_templates_controller.rb +++ b/app/controllers/admin/response_templates_controller.rb @@ -20,6 +20,7 @@ module Admin def create @response_template = ResponseTemplate.new(permitted_params) + @response_template.user = find_user_via_identifier params[:response_template][:user_identifier] if @response_template.save flash[:success] = I18n.t("admin.response_templates_controller.saved", @@ -38,6 +39,7 @@ module Admin def update @response_template = ResponseTemplate.find(params[:id]) + @response_template.user = find_user_via_identifier params[:response_template][:user_identifier] if @response_template.update(permitted_attributes(ResponseTemplate)) flash[:success] = @@ -66,6 +68,12 @@ module Admin private + def find_user_via_identifier(identifier) + return if identifier.blank? + + UsersQuery.find identifier + end + def permitted_params params.require(:response_template).permit(:body_markdown, :user_id, :content, :title, :type_of, :content_type) end diff --git a/app/models/response_template.rb b/app/models/response_template.rb index 3b859df6a..6efc2a0c6 100644 --- a/app/models/response_template.rb +++ b/app/models/response_template.rb @@ -27,6 +27,12 @@ class ResponseTemplate < ApplicationRecord validate :user_nil_only_for_user_nil_types validate :template_count + attribute :user_identifier, :string + + def user_identifier + user&.username + end + private def user_nil_only_for_user_nil_types diff --git a/app/queries/admin/users_query.rb b/app/queries/admin/users_query.rb index a67134ba2..063b987d2 100644 --- a/app/queries/admin/users_query.rb +++ b/app/queries/admin/users_query.rb @@ -1,8 +1,19 @@ module Admin class UsersQuery - QUERY_CLAUSE = "users.name ILIKE :search OR " \ - "users.email ILIKE :search OR " \ - "users.username ILIKE :search".freeze + SEARCH_CLAUSE = "users.name ILIKE :search OR " \ + "users.email ILIKE :search OR " \ + "users.username ILIKE :search".freeze + + # @api public + # @param relation [ActiveRecord::Relation] + # @param identifier [String, nil] + def self.find(identifier, relation: User) + return if identifier.blank? + + relation.where(id: identifier) + .or(relation.where(username: identifier)) + .or(relation.where(email: identifier)).first + end # @api public # @param relation [ActiveRecord::Relation] @@ -46,7 +57,7 @@ module Admin end def self.search_relation(relation, search) - relation.where(QUERY_CLAUSE, search: "%#{search.strip}%") + relation.where(SEARCH_CLAUSE, search: "%#{search.strip}%") end def self.filter_joining_date(relation:, joining_start:, joining_end:, date_format:) diff --git a/app/views/admin/response_templates/_form.html.erb b/app/views/admin/response_templates/_form.html.erb index 380cb13a1..8ea82bb31 100644 --- a/app/views/admin/response_templates/_form.html.erb +++ b/app/views/admin/response_templates/_form.html.erb @@ -18,8 +18,8 @@ <%= f.select :content_type, options_for_select(content_type_options, response_template.content_type || "plain_text"), class: "form-control" %>
- <%= f.label :user_id, "User ID - Add ID to restrict usage for a single user" %> - <%= f.text_field :user_id, value: response_template.user_id, class: "form-control" %> + <%= f.label :user_identifier, "User - Specify a username, email or ID to restrict usage for a single user" %> + <%= f.text_field :user_identifier, value: response_template.user_identifier, class: "form-control" %>
<%= f.submit class: "btn btn-primary" %> <% end %> diff --git a/app/views/admin/response_templates/index.html.erb b/app/views/admin/response_templates/index.html.erb index c5ff82794..649d3e038 100644 --- a/app/views/admin/response_templates/index.html.erb +++ b/app/views/admin/response_templates/index.html.erb @@ -10,7 +10,7 @@ Title Type Of - User ID + User @@ -24,7 +24,7 @@ <% if response_template.user_id.present? %> - <%= response_template.user_id %> + <%= link_to "@#{response_template.user.username}", user_url(response_template.user), target: "_blank", rel: :noopener %> <% end %> diff --git a/spec/queries/admin/users_query_spec.rb b/spec/queries/admin/users_query_spec.rb index df9142b1d..36628b4a9 100644 --- a/spec/queries/admin/users_query_spec.rb +++ b/spec/queries/admin/users_query_spec.rb @@ -1,12 +1,6 @@ require "rails_helper" RSpec.describe Admin::UsersQuery, type: :query do - subject do - described_class.call(search: search, role: role, roles: roles, organizations: organizations, - joining_start: joining_start, joining_end: joining_end, date_format: date_format, - statuses: statuses) - end - let(:role) { nil } let(:roles) { [] } let(:statuses) { [] } @@ -14,27 +8,77 @@ RSpec.describe Admin::UsersQuery, type: :query do let(:search) { [] } let(:joining_start) { nil } let(:joining_end) { nil } - let(:date_format) { "DD/MM/YYYY" } - let!(:org1) { create(:organization, name: "Org1") } - let!(:org2) { create(:organization, name: "Org2") } + describe ".find" do + let!(:user1) { create :user, username: "user1" } + let!(:user2) { create :user, username: "user12" } - let!(:user) { create(:user, :trusted, name: "Greg", registered_at: "2020-05-06T13:09:47+0000") } - let!(:user2) { create(:user, :trusted, name: "Gregory", registered_at: "2020-05-08T13:09:47+0000") } - let!(:user3) { create(:user, :tag_moderator, name: "Paul", registered_at: "2020-05-10T13:09:47+0000") } - let!(:user4) { create(:user, :admin, name: "Susi", registered_at: "2020-10-05T13:09:47+0000") } - let!(:user5) { create(:user, :trusted, :admin, name: "Beth", registered_at: "2020-10-07T13:09:47+0000") } - let!(:user6) { create(:user, :super_admin, name: "Jean", registered_at: "2020-10-08T13:09:47+0000") } - let!(:user7) do - create(:user, registered_at: "2020-12-05T13:09:47+0000").tap do |u| - u.add_role(:single_resource_admin, DataUpdateScript) + context "when identifier is blank" do + it "returns nil" do + expect(described_class.find("")).to be_nil + end + end + + context "when identifier is an int id" do + let(:id) { user1.id.to_i } + + it "returns user by id" do + expect(described_class.find(id)).to eq(user1) + end + end + + context "when identifier is a string 'id'" do + let(:id) { user2.id.to_s } + + it "returns user by id" do + expect(described_class.find(id)).to eq(user2) + end + end + + context "when identifier is a username" do + let(:id) { user1.username } + + it "returns user by id" do + expect(described_class.find(id)).to eq(user1) + end + end + + context "when identifier is an email" do + let(:id) { user2.email } + + it "returns user by id" do + expect(described_class.find(id)).to eq(user2) + end end end - let!(:user8) { create(:user, :comment_suspended, name: "Bob", registered_at: "2020-10-08T13:09:47+0000") } - let!(:user9) { create(:user, name: "Lucia", registered_at: "2020-10-08T13:09:47+0000") } - let!(:user10) { create(:user, :warned, name: "Billie", registered_at: "2020-10-08T13:09:47+0000") } describe ".call" do + subject do + described_class.call(search: search, role: role, roles: roles, organizations: organizations, + joining_start: joining_start, joining_end: joining_end, date_format: date_format, + statuses: statuses) + end + + let(:date_format) { "DD/MM/YYYY" } + + let!(:org1) { create(:organization, name: "Org1") } + let!(:org2) { create(:organization, name: "Org2") } + + let!(:user) { create(:user, :trusted, name: "Greg", registered_at: "2020-05-06T13:09:47+0000") } + let!(:user2) { create(:user, :trusted, name: "Gregory", registered_at: "2020-05-08T13:09:47+0000") } + let!(:user3) { create(:user, :tag_moderator, name: "Paul", registered_at: "2020-05-10T13:09:47+0000") } + let!(:user4) { create(:user, :admin, name: "Susi", registered_at: "2020-10-05T13:09:47+0000") } + let!(:user5) { create(:user, :trusted, :admin, name: "Beth", registered_at: "2020-10-07T13:09:47+0000") } + let!(:user6) { create(:user, :super_admin, name: "Jean", registered_at: "2020-10-08T13:09:47+0000") } + let!(:user7) do + create(:user, registered_at: "2020-12-05T13:09:47+0000").tap do |u| + u.add_role(:single_resource_admin, DataUpdateScript) + end + end + let!(:user8) { create(:user, :comment_suspended, name: "Bob", registered_at: "2020-10-08T13:09:47+0000") } + let!(:user9) { create(:user, name: "Lucia", registered_at: "2020-10-08T13:09:47+0000") } + let!(:user10) { create(:user, :warned, name: "Billie", registered_at: "2020-10-08T13:09:47+0000") } + context "when no arguments are given" do it "returns all users" do expect(described_class.call).to eq([user10, user9, user8, user7, user6, user5, user4, user3, user2, user])