From c40e48116993e9a0b1701c40eded924dd496d5a7 Mon Sep 17 00:00:00 2001 From: Emelia Smith Date: Mon, 29 Jul 2024 17:49:44 +0200 Subject: [PATCH] Implement UI for Admin Search of Hashtags (#30880) --- app/controllers/admin/tags_controller.rb | 18 ++++- app/helpers/admin/tags_helper.rb | 15 ++++ app/javascript/styles/mastodon/accounts.scss | 4 + app/javascript/styles/mastodon/tables.scss | 4 + app/models/admin/tag_filter.rb | 74 +++++++++++++++++++ app/models/tag.rb | 9 ++- app/views/admin/tags/_tag.html.haml | 27 +++++++ app/views/admin/tags/index.html.haml | 39 ++++++++++ app/views/admin/tags/show.html.haml | 7 +- app/views/admin/trends/tags/_tag.html.haml | 4 +- config/locales/en.yml | 16 ++++ config/locales/simple_form.en.yml | 2 +- config/navigation.rb | 3 +- config/routes/admin.rb | 2 +- .../controllers/admin/tags_controller_spec.rb | 37 ++++++++++ spec/models/admin/tag_filter_spec.rb | 36 +++++++++ spec/models/tag_spec.rb | 30 ++++++++ 17 files changed, 316 insertions(+), 11 deletions(-) create mode 100644 app/helpers/admin/tags_helper.rb create mode 100644 app/models/admin/tag_filter.rb create mode 100644 app/views/admin/tags/_tag.html.haml create mode 100644 app/views/admin/tags/index.html.haml create mode 100644 spec/models/admin/tag_filter_spec.rb diff --git a/app/controllers/admin/tags_controller.rb b/app/controllers/admin/tags_controller.rb index 4f727c398a0..4759d15bc4b 100644 --- a/app/controllers/admin/tags_controller.rb +++ b/app/controllers/admin/tags_controller.rb @@ -2,7 +2,15 @@ module Admin class TagsController < BaseController - before_action :set_tag + before_action :set_tag, except: [:index] + + PER_PAGE = 20 + + def index + authorize :tag, :index? + + @tags = filtered_tags.page(params[:page]).per(PER_PAGE) + end def show authorize @tag, :show? @@ -31,5 +39,13 @@ module Admin def tag_params params.require(:tag).permit(:name, :display_name, :trendable, :usable, :listable) end + + def filtered_tags + TagFilter.new(filter_params.with_defaults(order: 'newest')).results + end + + def filter_params + params.slice(:page, *TagFilter::KEYS).permit(:page, *TagFilter::KEYS) + end end end diff --git a/app/helpers/admin/tags_helper.rb b/app/helpers/admin/tags_helper.rb new file mode 100644 index 00000000000..eb928a6db2b --- /dev/null +++ b/app/helpers/admin/tags_helper.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module Admin::TagsHelper + def admin_tags_moderation_options + [ + [t('admin.tags.moderation.reviewed'), 'reviewed'], + [t('admin.tags.moderation.review_requested'), 'review_requested'], + [t('admin.tags.moderation.unreviewed'), 'unreviewed'], + [t('admin.tags.moderation.trendable'), 'trendable'], + [t('admin.tags.moderation.not_trendable'), 'not_trendable'], + [t('admin.tags.moderation.usable'), 'usable'], + [t('admin.tags.moderation.not_usable'), 'not_usable'], + ] + end +end diff --git a/app/javascript/styles/mastodon/accounts.scss b/app/javascript/styles/mastodon/accounts.scss index e63601fa9f1..894332acb58 100644 --- a/app/javascript/styles/mastodon/accounts.scss +++ b/app/javascript/styles/mastodon/accounts.scss @@ -350,6 +350,10 @@ color: $primary-text-color; font-weight: 700; } + + .warning-hint { + font-weight: normal !important; + } } &__body { diff --git a/app/javascript/styles/mastodon/tables.scss b/app/javascript/styles/mastodon/tables.scss index eab79371507..65f42d0467b 100644 --- a/app/javascript/styles/mastodon/tables.scss +++ b/app/javascript/styles/mastodon/tables.scss @@ -286,6 +286,10 @@ a.table-action-link { padding: 0; } + &--padded { + padding: 12px 16px 16px; + } + &--with-image { display: flex; align-items: center; diff --git a/app/models/admin/tag_filter.rb b/app/models/admin/tag_filter.rb new file mode 100644 index 00000000000..6149c521758 --- /dev/null +++ b/app/models/admin/tag_filter.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +class Admin::TagFilter + KEYS = %i( + status + name + order + ).freeze + + attr_reader :params + + def initialize(params) + @params = params.to_h.symbolize_keys + end + + def results + scope = Tag.reorder(nil) + + params.each do |key, value| + next if key == :page + + scope.merge!(scope_for(key, value)) if value.present? + end + + scope + end + + private + + def scope_for(key, value) + case key + when :status + status_scope(value) + when :name + Tag.search_for(value.to_s.strip, params[:limit], params[:offset], exclude_unlistable: false) + when :order + order_scope(value) + else + raise Mastodon::InvalidParameterError, "Unknown filter: #{key}" + end + end + + def status_scope(value) + case value.to_s + when 'reviewed' + Tag.reviewed + when 'review_requested' + Tag.pending_review + when 'unreviewed' + Tag.unreviewed + when 'trendable' + Tag.trendable + when 'not_trendable' + Tag.not_trendable + when 'usable' + Tag.usable + when 'not_usable' + Tag.not_usable + else + raise Mastodon::InvalidParameterError, "Unknown status: #{value}" + end + end + + def order_scope(value) + case value.to_s + when 'newest' + Tag.order(created_at: :desc) + when 'oldest' + Tag.order(created_at: :asc) + else + raise Mastodon::InvalidParameterError, "Unknown order: #{value}" + end + end +end diff --git a/app/models/tag.rb b/app/models/tag.rb index dd1cb3027d2..9006e1f25d9 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -52,6 +52,7 @@ class Tag < ApplicationRecord scope :unreviewed, -> { where(reviewed_at: nil) } scope :pending_review, -> { unreviewed.where.not(requested_review_at: nil) } scope :usable, -> { where(usable: [true, nil]) } + scope :not_usable, -> { where(usable: false) } scope :listable, -> { where(listable: [true, nil]) } scope :trendable, -> { Setting.trendable_by_default ? where(trendable: [true, nil]) : where(trendable: true) } scope :not_trendable, -> { where(trendable: false) } @@ -74,6 +75,10 @@ class Tag < ApplicationRecord attributes['display_name'] || name end + def formatted_name + "##{display_name}" + end + def usable boolean_with_default('usable', true) end @@ -132,8 +137,10 @@ class Tag < ApplicationRecord def search_for(term, limit = 5, offset = 0, options = {}) stripped_term = term.strip + options.reverse_merge!({ exclude_unlistable: true, exclude_unreviewed: false }) - query = Tag.listable.matches_name(stripped_term) + query = Tag.matches_name(stripped_term) + query = query.merge(Tag.listable) if options[:exclude_unlistable] query = query.merge(matching_name(stripped_term).or(where.not(reviewed_at: nil))) if options[:exclude_unreviewed] query.order(Arel.sql('length(name) ASC, name ASC')) diff --git a/app/views/admin/tags/_tag.html.haml b/app/views/admin/tags/_tag.html.haml new file mode 100644 index 00000000000..322eee0407f --- /dev/null +++ b/app/views/admin/tags/_tag.html.haml @@ -0,0 +1,27 @@ +.batch-table__row{ class: [!tag.requires_review? && !tag.usable? && 'batch-table__row--muted'] } + .batch-table__row__content.batch-table__row__content--padded.pending-account + .pending-account__header + %strong + = link_to tag.formatted_name, admin_tag_path(tag.id) + + %br/ + + - if tag.usable? + = t('admin.tags.moderation.usable') + - else + = t('admin.tags.moderation.not_usable') + + · + - if tag.trendable? + = t('admin.tags.moderation.trendable') + - else + = t('admin.tags.moderation.not_trendable') + + - if tag.requested_review? || tag.requires_review? + · + - if tag.requested_review? + %span.negative-hint + = t('admin.tags.moderation.review_requested') + - else + %span.warning-hint + = t('admin.tags.moderation.pending_review') diff --git a/app/views/admin/tags/index.html.haml b/app/views/admin/tags/index.html.haml new file mode 100644 index 00000000000..8d76d8ffa74 --- /dev/null +++ b/app/views/admin/tags/index.html.haml @@ -0,0 +1,39 @@ +- content_for :page_title do + = t('admin.tags.title') + += form_with url: admin_tags_url, method: :get, class: :simple_form do |form| + .filters + .filter-subset.filter-subset--with-select + %strong= t('admin.tags.moderation.title') + .input.select.optional + = form.select :status, + options_for_select(admin_tags_moderation_options, params[:status]), + prompt: t('generic.all') + + .filter-subset.filter-subset--with-select + %strong= t 'generic.order_by' + .input.select + = form.select :order, + options_for_select([[t('admin.tags.newest'), 'newest'], [t('admin.tags.oldest'), 'oldest']], params[:order]) + + .fields-group + .input.string.optional + = form.text_field :name, + value: params[:name], + class: 'string optional', + placeholder: t('admin.tags.name') + + .actions + %button.button= t('admin.tags.search') + = link_to t('admin.tags.reset'), admin_tags_path, class: 'button negative' + +%hr.spacer/ + +.batch-table + .batch-table__body + - if @tags.empty? + = nothing_here 'nothing-here--under-tabs' + - else + = render partial: 'tag', collection: @tags + += paginate @tags diff --git a/app/views/admin/tags/show.html.haml b/app/views/admin/tags/show.html.haml index f2d87b54b04..f6155575ae2 100644 --- a/app/views/admin/tags/show.html.haml +++ b/app/views/admin/tags/show.html.haml @@ -1,12 +1,13 @@ - content_for :page_title do - = "##{@tag.display_name}" + = @tag.formatted_name -- if current_user.can?(:view_dashboard) - - content_for :heading_actions do +- content_for :heading_actions do + - if current_user.can?(:view_dashboard) = l(@time_period.first) = ' - ' = l(@time_period.last) +- if current_user.can?(:view_dashboard) .dashboard .dashboard__item = react_admin_component :counter, diff --git a/app/views/admin/trends/tags/_tag.html.haml b/app/views/admin/trends/tags/_tag.html.haml index 8cc0d713b9e..b1e714a9124 100644 --- a/app/views/admin/trends/tags/_tag.html.haml +++ b/app/views/admin/trends/tags/_tag.html.haml @@ -4,9 +4,7 @@ .batch-table__row__content.pending-account .pending-account__header - = link_to admin_tag_path(tag.id) do - = material_symbol 'tag' - = tag.display_name + = link_to tag.formatted_name, admin_tag_path(tag.id) %br/ diff --git a/config/locales/en.yml b/config/locales/en.yml index 322183f4ce9..aab8db18153 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -885,7 +885,23 @@ en: action: Check here for more information message_html: "Your object storage is misconfigured. The privacy of your users is at risk." tags: + moderation: + not_trendable: Not trendable + not_usable: Not usable + pending_review: Pending review + review_requested: Review requested + reviewed: Reviewed + title: Status + trendable: Trendable + unreviewed: Unreviewed + usable: Usable + name: Name + newest: Newest + oldest: Oldest + reset: Reset review: Review status + search: Search + title: Hashtags updated_msg: Hashtag settings updated successfully title: Administration trends: diff --git a/config/locales/simple_form.en.yml b/config/locales/simple_form.en.yml index 6bc7c6ac525..fee3a6151ad 100644 --- a/config/locales/simple_form.en.yml +++ b/config/locales/simple_form.en.yml @@ -314,7 +314,7 @@ en: listable: Allow this hashtag to appear in searches and suggestions name: Hashtag trendable: Allow this hashtag to appear under trends - usable: Allow posts to use this hashtag + usable: Allow posts to use this hashtag locally user: role: Role time_zone: Time zone diff --git a/config/navigation.rb b/config/navigation.rb index c1dd81501cf..f006f39a540 100644 --- a/config/navigation.rb +++ b/config/navigation.rb @@ -40,13 +40,14 @@ SimpleNavigation::Configuration.run do |navigation| n.item :trends, safe_join([fa_icon('fire fw'), t('admin.trends.title')]), admin_trends_statuses_path, if: -> { current_user.can?(:manage_taxonomies) && !self_destruct } do |s| s.item :statuses, safe_join([fa_icon('comments-o fw'), t('admin.trends.statuses.title')]), admin_trends_statuses_path, highlights_on: %r{/admin/trends/statuses} - s.item :tags, safe_join([fa_icon('hashtag fw'), t('admin.trends.tags.title')]), admin_trends_tags_path, highlights_on: %r{/admin/tags|/admin/trends/tags} + s.item :tags, safe_join([fa_icon('hashtag fw'), t('admin.trends.tags.title')]), admin_trends_tags_path, highlights_on: %r{/admin/trends/tags} s.item :links, safe_join([fa_icon('newspaper-o fw'), t('admin.trends.links.title')]), admin_trends_links_path, highlights_on: %r{/admin/trends/links} end n.item :moderation, safe_join([fa_icon('gavel fw'), t('moderation.title')]), nil, if: -> { current_user.can?(:manage_reports, :view_audit_log, :manage_users, :manage_invites, :manage_taxonomies, :manage_federation, :manage_blocks) && !self_destruct } do |s| s.item :reports, safe_join([fa_icon('flag fw'), t('admin.reports.title')]), admin_reports_path, highlights_on: %r{/admin/reports|admin/report_notes}, if: -> { current_user.can?(:manage_reports) } s.item :accounts, safe_join([fa_icon('users fw'), t('admin.accounts.title')]), admin_accounts_path(origin: 'local'), highlights_on: %r{/admin/accounts|admin/account_moderation_notes|/admin/pending_accounts|/admin/disputes|/admin/users}, if: -> { current_user.can?(:manage_users) } + s.item :tags, safe_join([fa_icon('hashtag fw'), t('admin.tags.title')]), admin_tags_path, highlights_on: %r{/admin/tags}, if: -> { current_user.can?(:manage_taxonomies) } s.item :invites, safe_join([fa_icon('user-plus fw'), t('admin.invites.title')]), admin_invites_path, if: -> { current_user.can?(:manage_invites) } s.item :follow_recommendations, safe_join([fa_icon('user-plus fw'), t('admin.follow_recommendations.title')]), admin_follow_recommendations_path, highlights_on: %r{/admin/follow_recommendations}, if: -> { current_user.can?(:manage_taxonomies) } s.item :instances, safe_join([fa_icon('cloud fw'), t('admin.instances.title')]), admin_instances_path(limited: limited_federation_mode? ? nil : '1'), highlights_on: %r{/admin/instances|/admin/domain_blocks|/admin/domain_allows}, if: -> { current_user.can?(:manage_federation) } diff --git a/config/routes/admin.rb b/config/routes/admin.rb index 207cb0580dc..50c4c10594a 100644 --- a/config/routes/admin.rb +++ b/config/routes/admin.rb @@ -163,7 +163,7 @@ namespace :admin do resources :roles, except: [:show] resources :account_moderation_notes, only: [:create, :destroy] resource :follow_recommendations, only: [:show, :update] - resources :tags, only: [:show, :update] + resources :tags, only: [:index, :show, :update] namespace :trends do resources :links, only: [:index] do diff --git a/spec/controllers/admin/tags_controller_spec.rb b/spec/controllers/admin/tags_controller_spec.rb index 4e06adaca67..1df2bc40030 100644 --- a/spec/controllers/admin/tags_controller_spec.rb +++ b/spec/controllers/admin/tags_controller_spec.rb @@ -9,6 +9,43 @@ RSpec.describe Admin::TagsController do sign_in Fabricate(:user, role: UserRole.find_by(name: 'Admin')) end + describe 'GET #index' do + before do + Fabricate(:tag) + + tag_filter = instance_double(Admin::TagFilter, results: Tag.all) + allow(Admin::TagFilter).to receive(:new).and_return(tag_filter) + end + + let(:params) { { order: 'newest' } } + + it 'returns http success' do + get :index + + expect(response).to have_http_status(200) + expect(response).to render_template(:index) + + expect(Admin::TagFilter) + .to have_received(:new) + .with(hash_including(params)) + end + + describe 'with filters' do + let(:params) { { order: 'newest', name: 'test' } } + + it 'returns http success' do + get :index, params: { name: 'test' } + + expect(response).to have_http_status(200) + expect(response).to render_template(:index) + + expect(Admin::TagFilter) + .to have_received(:new) + .with(hash_including(params)) + end + end + end + describe 'GET #show' do let!(:tag) { Fabricate(:tag) } diff --git a/spec/models/admin/tag_filter_spec.rb b/spec/models/admin/tag_filter_spec.rb new file mode 100644 index 00000000000..21dc28affb3 --- /dev/null +++ b/spec/models/admin/tag_filter_spec.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe Admin::TagFilter do + describe 'with invalid params' do + it 'raises with key error' do + filter = described_class.new(wrong: true) + + expect { filter.results }.to raise_error(/wrong/) + end + + it 'raises with status scope error' do + filter = described_class.new(status: 'unknown') + + expect { filter.results }.to raise_error(/Unknown status: unknown/) + end + + it 'raises with order value error' do + filter = described_class.new(order: 'unknown') + + expect { filter.results }.to raise_error(/Unknown order: unknown/) + end + end + + describe '#results' do + let(:listable_tag) { Fabricate(:tag, name: 'test1', listable: true) } + let(:not_listable_tag) { Fabricate(:tag, name: 'test2', listable: false) } + + it 'returns tags filtered by name' do + filter = described_class.new(name: 'test') + + expect(filter.results).to eq([listable_tag, not_listable_tag]) + end + end +end diff --git a/spec/models/tag_spec.rb b/spec/models/tag_spec.rb index a3cae4a3396..ff0a0551133 100644 --- a/spec/models/tag_spec.rb +++ b/spec/models/tag_spec.rb @@ -112,6 +112,18 @@ RSpec.describe Tag do end end + describe '#formatted_name' do + it 'returns name with a proceeding hash symbol' do + tag = Fabricate(:tag, name: 'foo') + expect(tag.formatted_name).to eq '#foo' + end + + it 'returns display_name with a proceeding hash symbol, if display name present' do + tag = Fabricate(:tag, name: 'foobar', display_name: 'FooBar') + expect(tag.formatted_name).to eq '#FooBar' + end + end + describe '.recently_used' do let(:account) { Fabricate(:account) } let(:other_person_status) { Fabricate(:status) } @@ -240,5 +252,23 @@ RSpec.describe Tag do expect(results).to eq [tag, similar_tag] end + + it 'finds only listable tags' do + tag = Fabricate(:tag, name: 'match') + _miss_tag = Fabricate(:tag, name: 'matchunlisted', listable: false) + + results = described_class.search_for('match') + + expect(results).to eq [tag] + end + + it 'finds non-listable tags as well via option' do + tag = Fabricate(:tag, name: 'match') + unlisted_tag = Fabricate(:tag, name: 'matchunlisted', listable: false) + + results = described_class.search_for('match', 5, 0, exclude_unlistable: false) + + expect(results).to eq [tag, unlisted_tag] + end end end