From 4d42a389540690b32886f2a38af1f86aee617d27 Mon Sep 17 00:00:00 2001 From: abcang Date: Tue, 18 Jul 2017 23:38:22 +0900 Subject: [PATCH] Improve admin page (#4121) * Improve admin page * Fix test * Add spec * Improve select style --- .../admin/reported_statuses_controller.rb | 15 ++- app/controllers/admin/reports_controller.rb | 4 +- app/controllers/admin/statuses_controller.rb | 69 +++++++++++ app/javascript/packs/admin.js | 40 +++++++ app/javascript/styles/admin.scss | 45 +++++++- app/models/form/status_batch.rb | 39 +++++++ app/views/admin/accounts/show.html.haml | 4 +- app/views/admin/reports/show.html.haml | 34 ++++-- app/views/admin/statuses/index.html.haml | 47 ++++++++ config/locales/en.yml | 15 +++ config/locales/ja.yml | 15 +++ config/routes.rb | 3 +- .../reported_statuses_controller_spec.rb | 38 ++++++- .../admin/statuses_controller_spec.rb | 107 ++++++++++++++++++ spec/models/form/status_batch_spec.rb | 52 +++++++++ 15 files changed, 508 insertions(+), 19 deletions(-) create mode 100644 app/controllers/admin/statuses_controller.rb create mode 100644 app/javascript/packs/admin.js create mode 100644 app/models/form/status_batch.rb create mode 100644 app/views/admin/statuses/index.html.haml create mode 100644 spec/controllers/admin/statuses_controller_spec.rb create mode 100644 spec/models/form/status_batch_spec.rb diff --git a/app/controllers/admin/reported_statuses_controller.rb b/app/controllers/admin/reported_statuses_controller.rb index 32434d30f..5a31adecf 100644 --- a/app/controllers/admin/reported_statuses_controller.rb +++ b/app/controllers/admin/reported_statuses_controller.rb @@ -5,7 +5,14 @@ module Admin include Authorization before_action :set_report - before_action :set_status + before_action :set_status, only: [:update, :destroy] + + def create + @form = Form::StatusBatch.new(form_status_batch_params) + flash[:alert] = t('admin.statuses.failed_to_execute') unless @form.save + + redirect_to admin_report_path(@report) + end def update @status.update(status_params) @@ -15,7 +22,7 @@ module Admin def destroy authorize @status, :destroy? RemovalWorker.perform_async(@status.id) - redirect_to admin_report_path(@report) + render json: @status end private @@ -24,6 +31,10 @@ module Admin params.require(:status).permit(:sensitive) end + def form_status_batch_params + params.require(:form_status_batch).permit(:action, status_ids: []) + end + def set_report @report = Report.find(params[:report_id]) end diff --git a/app/controllers/admin/reports_controller.rb b/app/controllers/admin/reports_controller.rb index 2d8c3c820..226467739 100644 --- a/app/controllers/admin/reports_controller.rb +++ b/app/controllers/admin/reports_controller.rb @@ -8,7 +8,9 @@ module Admin @reports = filtered_reports.page(params[:page]) end - def show; end + def show + @form = Form::StatusBatch.new + end def update process_report diff --git a/app/controllers/admin/statuses_controller.rb b/app/controllers/admin/statuses_controller.rb new file mode 100644 index 000000000..50712f0dd --- /dev/null +++ b/app/controllers/admin/statuses_controller.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true + +module Admin + class StatusesController < BaseController + include Authorization + + helper_method :current_params + + before_action :set_account + before_action :set_status, only: [:update, :destroy] + + PAR_PAGE = 20 + + def index + @statuses = @account.statuses + if params[:media] + account_media_status_ids = @account.media_attachments.attached.reorder(nil).select(:status_id).distinct + @statuses.merge!(Status.where(id: account_media_status_ids)) + end + @statuses = @statuses.preload(:media_attachments, :mentions).page(params[:page]).per(PAR_PAGE) + + @form = Form::StatusBatch.new + end + + def create + @form = Form::StatusBatch.new(form_status_batch_params) + flash[:alert] = t('admin.statuses.failed_to_execute') unless @form.save + + redirect_to admin_account_statuses_path(@account.id, current_params) + end + + def update + @status.update(status_params) + redirect_to admin_account_statuses_path(@account.id, current_params) + end + + def destroy + authorize @status, :destroy? + RemovalWorker.perform_async(@status.id) + render json: @status + end + + private + + def status_params + params.require(:status).permit(:sensitive) + end + + def form_status_batch_params + params.require(:form_status_batch).permit(:action, status_ids: []) + end + + def set_status + @status = @account.statuses.find(params[:id]) + end + + def set_account + @account = Account.find(params[:account_id]) + end + + def current_params + page = (params[:page] || 1).to_i + { + media: params[:media], + page: page > 1 && page, + }.select { |_, value| value.present? } + end + end +end diff --git a/app/javascript/packs/admin.js b/app/javascript/packs/admin.js new file mode 100644 index 000000000..993827db5 --- /dev/null +++ b/app/javascript/packs/admin.js @@ -0,0 +1,40 @@ +import { delegate } from 'rails-ujs'; + +function handleDeleteStatus(event) { + const [data] = event.detail; + const element = document.querySelector(`[data-id="${data.id}"]`); + if (element) { + element.parentNode.removeChild(element); + } +} + +[].forEach.call(document.querySelectorAll('.trash-button'), (content) => { + content.addEventListener('ajax:success', handleDeleteStatus); +}); + +const batchCheckboxClassName = '.batch-checkbox input[type="checkbox"]'; + +delegate(document, '#batch_checkbox_all', 'change', ({ target }) => { + [].forEach.call(document.querySelectorAll(batchCheckboxClassName), (content) => { + content.checked = target.checked; + }); +}); + +delegate(document, batchCheckboxClassName, 'change', () => { + const checkAllElement = document.querySelector('#batch_checkbox_all'); + if (checkAllElement) { + checkAllElement.checked = [].every.call(document.querySelectorAll(batchCheckboxClassName), (content) => content.checked); + } +}); + +delegate(document, '.media-spoiler-show-button', 'click', () => { + [].forEach.call(document.querySelectorAll('.activity-stream .media-spoiler-wrapper'), (content) => { + content.classList.add('media-spoiler-wrapper__visible'); + }); +}); + +delegate(document, '.media-spoiler-hide-button', 'click', () => { + [].forEach.call(document.querySelectorAll('.activity-stream .media-spoiler-wrapper'), (content) => { + content.classList.remove('media-spoiler-wrapper__visible'); + }); +}); diff --git a/app/javascript/styles/admin.scss b/app/javascript/styles/admin.scss index 3bc713566..4c3bbdfc5 100644 --- a/app/javascript/styles/admin.scss +++ b/app/javascript/styles/admin.scss @@ -253,7 +253,8 @@ } } -.report-status { +.report-status, +.account-status { display: flex; margin-bottom: 10px; @@ -263,7 +264,8 @@ } } -.report-status__actions { +.report-status__actions, +.account-status__actions { flex: 0 0 auto; display: flex; flex-direction: column; @@ -275,3 +277,42 @@ margin-bottom: 10px; } } + +.batch-form-box { + display: flex; + margin-bottom: 10px; + + #form_status_batch_action { + margin-right: 5px; + font-size: 14px; + } + + .media-spoiler-toggle-buttons { + margin-left: auto; + + .button { + overflow: visible; + } + } +} + +.batch-checkbox, +.batch-checkbox-all { + display: flex; + align-items: center; + margin-right: 5px; +} + +.back-link { + margin-bottom: 10px; + font-size: 14px; + + a { + color: $classic-highlight-color; + text-decoration: none; + + &:hover { + text-decoration: underline; + } + } +} diff --git a/app/models/form/status_batch.rb b/app/models/form/status_batch.rb new file mode 100644 index 000000000..a97b4aa28 --- /dev/null +++ b/app/models/form/status_batch.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +class Form::StatusBatch + include ActiveModel::Model + + attr_accessor :status_ids, :action + + ACTION_TYPE = %w(nsfw_on nsfw_off delete).freeze + + def save + case action + when 'nsfw_on', 'nsfw_off' + change_sensitive(action == 'nsfw_on') + when 'delete' + delete_statuses + end + end + + private + + def change_sensitive(sensitive) + media_attached_status_ids = MediaAttachment.where(status_id: status_ids).pluck(:status_id) + ApplicationRecord.transaction do + Status.where(id: media_attached_status_ids).find_each do |status| + status.update!(sensitive: sensitive) + end + end + true + rescue ActiveRecord::RecordInvalid + false + end + + def delete_statuses + Status.where(id: status_ids).find_each do |status| + RemovalWorker.perform_async(status.id) + end + true + end +end diff --git a/app/views/admin/accounts/show.html.haml b/app/views/admin/accounts/show.html.haml index d91ba9c78..5ad1fd6ee 100644 --- a/app/views/admin/accounts/show.html.haml +++ b/app/views/admin/accounts/show.html.haml @@ -53,11 +53,11 @@ %td= @account.followers_count %tr %th= t('admin.accounts.statuses') - %td= @account.statuses_count + %td= link_to @account.statuses_count, admin_account_statuses_path(@account.id) %tr %th= t('admin.accounts.media_attachments') %td - = @account.media_attachments.count + = link_to @account.media_attachments.count, admin_account_statuses_path(@account.id, { media: true }) = surround '(', ')' do = number_to_human_size @account.media_attachments.sum('file_file_size') %tr diff --git a/app/views/admin/reports/show.html.haml b/app/views/admin/reports/show.html.haml index 44486cb42..5747cc274 100644 --- a/app/views/admin/reports/show.html.haml +++ b/app/views/admin/reports/show.html.haml @@ -1,3 +1,6 @@ +- content_for :header_tags do + = javascript_pack_tag 'admin', integrity: true, async: true, crossorigin: 'anonymous' + - content_for :page_title do = t('admin.reports.report', id: @report.id) @@ -19,16 +22,27 @@ - unless @report.statuses.empty? %hr/ - - @report.statuses.each do |status| - .report-status - .activity-stream.activity-stream-headless - .entry= render 'stream_entries/simple_status', status: status - .report-status__actions - - unless status.media_attachments.empty? - = link_to admin_report_reported_status_path(@report, status, status: { sensitive: !status.sensitive }), method: :patch, class: 'icon-button nsfw-button', title: t("admin.reports.nsfw.#{!status.sensitive}") do - = fa_icon status.sensitive? ? 'eye' : 'eye-slash' - = link_to admin_report_reported_status_path(@report, status), method: :delete, class: 'icon-button trash-button', title: t('admin.reports.delete'), data: { confirm: t('admin.reports.are_you_sure') } do - = fa_icon 'trash' + = form_for(@form, url: admin_report_reported_statuses_path(@report.id)) do |f| + .batch-form-box + .batch-checkbox-all + = check_box_tag :batch_checkbox_all, nil, false + = f.select :action, Form::StatusBatch::ACTION_TYPE.map{|action| [t("admin.statuses.batch.#{action}"), action]} + = f.submit t('admin.statuses.execute'), data: { confirm: t('admin.reports.are_you_sure') }, class: 'button' + .media-spoiler-toggle-buttons + .media-spoiler-show-button.button= t('admin.statuses.media.show') + .media-spoiler-hide-button.button= t('admin.statuses.media.hide') + - @report.statuses.each do |status| + .report-status{ data: { id: status.id } } + .batch-checkbox + = f.check_box :status_ids, { multiple: true, include_hidden: false }, status.id + .activity-stream.activity-stream-headless + .entry= render 'stream_entries/simple_status', status: status + .report-status__actions + - unless status.media_attachments.empty? + = link_to admin_report_reported_status_path(@report, status, status: { sensitive: !status.sensitive }), method: :put, class: 'icon-button nsfw-button', title: t("admin.reports.nsfw.#{!status.sensitive}") do + = fa_icon status.sensitive? ? 'eye' : 'eye-slash' + = link_to admin_report_reported_status_path(@report, status), method: :delete, class: 'icon-button trash-button', title: t('admin.reports.delete'), data: { confirm: t('admin.reports.are_you_sure') }, remote: true do + = fa_icon 'trash' %hr/ diff --git a/app/views/admin/statuses/index.html.haml b/app/views/admin/statuses/index.html.haml new file mode 100644 index 000000000..fe2581527 --- /dev/null +++ b/app/views/admin/statuses/index.html.haml @@ -0,0 +1,47 @@ +- content_for :header_tags do + = javascript_pack_tag 'admin', integrity: true, async: true, crossorigin: 'anonymous' + +- content_for :page_title do + = t('admin.statuses.title') + +.back-link + = link_to admin_account_path(@account.id) do + %i.fa.fa-chevron-left.fa-fw + = t('admin.statuses.back_to_account') + +.filters + .filter-subset + %strong= t('admin.statuses.media.title') + %ul + %li= link_to t('admin.statuses.no_media'), admin_account_statuses_path(@account.id, current_params.merge(media: nil)), class: !params[:media] && 'selected' + %li= link_to t('admin.statuses.with_media'), admin_account_statuses_path(@account.id, current_params.merge(media: true)), class: params[:media] && 'selected' + +- if @statuses.empty? + .accounts-grid + = render 'accounts/nothing_here' +- else + = form_for(@form, url: admin_account_statuses_path(@account.id)) do |f| + = hidden_field_tag :page, params[:page] + = hidden_field_tag :media, params[:media] + .batch-form-box + .batch-checkbox-all + = check_box_tag :batch_checkbox_all, nil, false + = f.select :action, Form::StatusBatch::ACTION_TYPE.map{|action| [t("admin.statuses.batch.#{action}"), action]} + = f.submit t('admin.statuses.execute'), data: { confirm: t('admin.reports.are_you_sure') }, class: 'button' + .media-spoiler-toggle-buttons + .media-spoiler-show-button.button= t('admin.statuses.media.show') + .media-spoiler-hide-button.button= t('admin.statuses.media.hide') + - @statuses.each do |status| + .account-status{ data: { id: status.id } } + .batch-checkbox + = f.check_box :status_ids, { multiple: true, include_hidden: false }, status.id + .activity-stream.activity-stream-headless + .entry= render 'stream_entries/simple_status', status: status + .account-status__actions + - unless status.media_attachments.empty? + = link_to admin_account_status_path(@account.id, status, current_params.merge(status: { sensitive: !status.sensitive })), method: :patch, class: 'icon-button nsfw-button', title: t("admin.reports.nsfw.#{!status.sensitive}") do + = fa_icon status.sensitive? ? 'eye' : 'eye-slash' + = link_to admin_account_status_path(@account.id, status), method: :delete, class: 'icon-button trash-button', title: t('admin.reports.delete'), data: { confirm: t('admin.reports.are_you_sure') }, remote: true do + = fa_icon 'trash' + += paginate @statuses diff --git a/config/locales/en.yml b/config/locales/en.yml index be1f15e25..4cb536223 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -185,6 +185,21 @@ en: desc_html: Display public timeline on landing page title: Timeline preview title: Site Settings + statuses: + back_to_account: Back to account page + batch: + delete: Delete + nsfw_off: NSFW OFF + nsfw_on: NSFW ON + execute: Execute + failed_to_execute: Failed to execute + media: + hide: Hide media + show: Show media + title: Media + no_media: No media + with_media: With media + title: Account statuses subscriptions: callback_url: Callback URL confirmed: Confirmed diff --git a/config/locales/ja.yml b/config/locales/ja.yml index fda87526d..2897e864f 100644 --- a/config/locales/ja.yml +++ b/config/locales/ja.yml @@ -171,6 +171,21 @@ ja: desc_html: ランディングページに公開タイムラインを表示します title: タイムラインプレビュー title: サイト設定 + statuses: + back_to_account: アカウントページに戻る + batch: + delete: 削除 + nsfw_off: NSFW オフ + nsfw_on: NSFW オン + execute: 実行 + failed_to_execute: 実行に失敗しました + media: + hide: メディアを隠す + show: メディアを表示 + title: メディア + no_media: メディアなし + with_media: メディアあり + title: トゥート一覧 subscriptions: callback_url: コールバックURL confirmed: 確認済み diff --git a/config/routes.rb b/config/routes.rb index dda3534eb..60234a9e6 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -89,7 +89,7 @@ Rails.application.routes.draw do resources :instances, only: [:index] resources :reports, only: [:index, :show, :update] do - resources :reported_statuses, only: [:update, :destroy] + resources :reported_statuses, only: [:create, :update, :destroy] end resources :accounts, only: [:index, :show] do @@ -103,6 +103,7 @@ Rails.application.routes.draw do resource :silence, only: [:create, :destroy] resource :suspension, only: [:create, :destroy] resource :confirmation, only: [:create] + resources :statuses, only: [:index, :create, :update, :destroy] end resources :users, only: [] do diff --git a/spec/controllers/admin/reported_statuses_controller_spec.rb b/spec/controllers/admin/reported_statuses_controller_spec.rb index 80c69e8d1..297807d41 100644 --- a/spec/controllers/admin/reported_statuses_controller_spec.rb +++ b/spec/controllers/admin/reported_statuses_controller_spec.rb @@ -11,6 +11,42 @@ describe Admin::ReportedStatusesController do sign_in user, scope: :user end + describe 'POST #create' do + subject do + -> { post :create, params: { report_id: report, form_status_batch: { action: action, status_ids: status_ids } } } + end + + let(:action) { 'nsfw_on' } + let(:status_ids) { [status.id] } + let(:status) { Fabricate(:status, sensitive: !sensitive) } + let(:sensitive) { true } + let!(:media_attachment) { Fabricate(:media_attachment, status: status) } + + context 'updates sensitive column to true' do + it 'updates sensitive column' do + is_expected.to change { + status.reload.sensitive + }.from(false).to(true) + end + end + + context 'updates sensitive column to false' do + let(:action) { 'nsfw_off' } + let(:sensitive) { false } + + it 'updates sensitive column' do + is_expected.to change { + status.reload.sensitive + }.from(true).to(false) + end + end + + it 'redirects to report page' do + subject.call + expect(response).to redirect_to(admin_report_path(report)) + end + end + describe 'PATCH #update' do subject do -> { patch :update, params: { report_id: report, id: status, status: { sensitive: sensitive } } } @@ -48,7 +84,7 @@ describe Admin::ReportedStatusesController do allow(RemovalWorker).to receive(:perform_async) delete :destroy, params: { report_id: report, id: status } - expect(response).to redirect_to(admin_report_path(report)) + expect(response).to have_http_status(:success) expect(RemovalWorker). to have_received(:perform_async).with(status.id) end diff --git a/spec/controllers/admin/statuses_controller_spec.rb b/spec/controllers/admin/statuses_controller_spec.rb new file mode 100644 index 000000000..1515e299b --- /dev/null +++ b/spec/controllers/admin/statuses_controller_spec.rb @@ -0,0 +1,107 @@ +require 'rails_helper' + +describe Admin::StatusesController do + render_views + + let(:user) { Fabricate(:user, admin: true) } + let(:account) { Fabricate(:account) } + let!(:status) { Fabricate(:status, account: account) } + let(:media_attached_status) { Fabricate(:status, account: account, sensitive: !sensitive) } + let!(:media_attachment) { Fabricate(:media_attachment, account: account, status: media_attached_status) } + let(:sensitive) { true } + + before do + sign_in user, scope: :user + end + + describe 'GET #index' do + it 'returns http success with no media' do + get :index, params: { account_id: account.id } + + statuses = assigns(:statuses).to_a + expect(statuses.size).to eq 2 + expect(response).to have_http_status(:success) + end + + it 'returns http success with media' do + get :index, params: { account_id: account.id , media: true } + + statuses = assigns(:statuses).to_a + expect(statuses.size).to eq 1 + expect(response).to have_http_status(:success) + end + end + + describe 'POST #create' do + subject do + -> { post :create, params: { account_id: account.id, form_status_batch: { action: action, status_ids: status_ids } } } + end + + let(:action) { 'nsfw_on' } + let(:status_ids) { [media_attached_status.id] } + + context 'updates sensitive column to true' do + it 'updates sensitive column' do + is_expected.to change { + media_attached_status.reload.sensitive + }.from(false).to(true) + end + end + + context 'updates sensitive column to false' do + let(:action) { 'nsfw_off' } + let(:sensitive) { false } + + it 'updates sensitive column' do + is_expected.to change { + media_attached_status.reload.sensitive + }.from(true).to(false) + end + end + + it 'redirects to account statuses page' do + subject.call + expect(response).to redirect_to(admin_account_statuses_path(account.id)) + end + end + + describe 'PATCH #update' do + subject do + -> { patch :update, params: { account_id: account.id, id: media_attached_status, status: { sensitive: sensitive } } } + end + + context 'updates sensitive column to true' do + it 'updates sensitive column' do + is_expected.to change { + media_attached_status.reload.sensitive + }.from(false).to(true) + end + end + + context 'updates sensitive column to false' do + let(:sensitive) { false } + + it 'updates sensitive column' do + is_expected.to change { + media_attached_status.reload.sensitive + }.from(true).to(false) + end + end + + it 'redirects to account statuses page' do + subject.call + expect(response).to redirect_to(admin_account_statuses_path(account.id)) + end + end + + describe 'DELETE #destroy' do + it 'removes a status' do + allow(RemovalWorker).to receive(:perform_async) + + delete :destroy, params: { account_id: account.id, id: status } + expect(response).to have_http_status(:success) + expect(RemovalWorker). + to have_received(:perform_async).with(status.id) + end + end +end diff --git a/spec/models/form/status_batch_spec.rb b/spec/models/form/status_batch_spec.rb new file mode 100644 index 000000000..00c790a11 --- /dev/null +++ b/spec/models/form/status_batch_spec.rb @@ -0,0 +1,52 @@ +require 'rails_helper' + +describe Form::StatusBatch do + let(:form) { Form::StatusBatch.new(action: action, status_ids: status_ids) } + let(:status) { Fabricate(:status) } + + describe 'with nsfw action' do + let(:status_ids) { [status.id, nonsensitive_status.id, sensitive_status.id] } + let(:nonsensitive_status) { Fabricate(:status, sensitive: false) } + let(:sensitive_status) { Fabricate(:status, sensitive: true) } + let!(:shown_media_attachment) { Fabricate(:media_attachment, status: nonsensitive_status) } + let!(:hidden_media_attachment) { Fabricate(:media_attachment, status: sensitive_status) } + + context 'nsfw_on' do + let(:action) { 'nsfw_on' } + + it { expect(form.save).to be true } + it { expect { form.save }.to change { nonsensitive_status.reload.sensitive }.from(false).to(true) } + it { expect { form.save }.not_to change { sensitive_status.reload.sensitive } } + it { expect { form.save }.not_to change { status.reload.sensitive } } + end + + context 'nsfw_off' do + let(:action) { 'nsfw_off' } + + it { expect(form.save).to be true } + it { expect { form.save }.to change { sensitive_status.reload.sensitive }.from(true).to(false) } + it { expect { form.save }.not_to change { nonsensitive_status.reload.sensitive } } + it { expect { form.save }.not_to change { status.reload.sensitive } } + end + end + + describe 'with delete action' do + let(:status_ids) { [status.id] } + let(:action) { 'delete' } + let!(:another_status) { Fabricate(:status) } + + before do + allow(RemovalWorker).to receive(:perform_async) + end + + it 'call RemovalWorker' do + form.save + expect(RemovalWorker).to have_received(:perform_async).with(status.id) + end + + it 'do not call RemovalWorker' do + form.save + expect(RemovalWorker).not_to have_received(:perform_async).with(another_status.id) + end + end +end