From ea72d53ec083676ee1171e97c0869132f360d0c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Tue, 29 Sep 2015 18:08:55 +0200 Subject: [PATCH] Streamline the "Report button" This simplifies the "Report button" to not use open a dropdown and adds a tooltip on this button. This also removes an extra spec and adds missing specs. --- app/helpers/user_helper.rb | 11 ---------- app/models/abuse_report.rb | 4 ++-- app/models/user.rb | 3 +-- app/views/users/show.html.haml | 19 +++++++---------- features/abuse_report.feature | 2 +- features/steps/abuse_reports.rb | 5 ++--- spec/features/users_spec.rb | 36 +------------------------------- spec/models/abuse_report_spec.rb | 12 +++++++++++ spec/models/user_spec.rb | 2 +- 9 files changed, 27 insertions(+), 67 deletions(-) delete mode 100644 app/helpers/user_helper.rb diff --git a/app/helpers/user_helper.rb b/app/helpers/user_helper.rb deleted file mode 100644 index 058cff85135..00000000000 --- a/app/helpers/user_helper.rb +++ /dev/null @@ -1,11 +0,0 @@ -module UserHelper - - def abuse_report_button_title(user) - if user.abuse_report - "#{user.username} has already been reported for abuse." - else - "Report #{user.username} for abuse." - end - end - -end diff --git a/app/models/abuse_report.rb b/app/models/abuse_report.rb index 07c87a7fe87..89b3116b9f2 100644 --- a/app/models/abuse_report.rb +++ b/app/models/abuse_report.rb @@ -11,11 +11,11 @@ # class AbuseReport < ActiveRecord::Base - belongs_to :reporter, class_name: "User" + belongs_to :reporter, class_name: 'User' belongs_to :user validates :reporter, presence: true validates :user, presence: true validates :message, presence: true - validates :user_id, uniqueness: { scope: :reporter_id } + validates :user_id, uniqueness: true end diff --git a/app/models/user.rb b/app/models/user.rb index a3b149ff992..9ea7cabff15 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -97,9 +97,7 @@ class User < ActiveRecord::Base # Namespace for personal projects has_one :namespace, -> { where type: nil }, dependent: :destroy, foreign_key: :owner_id, class_name: "Namespace" - # Profile - has_one :abuse_report, dependent: :destroy has_many :keys, dependent: :destroy has_many :emails, dependent: :destroy has_many :identities, dependent: :destroy, autosave: true @@ -131,6 +129,7 @@ class User < ActiveRecord::Base has_many :assigned_issues, dependent: :destroy, foreign_key: :assignee_id, class_name: "Issue" has_many :assigned_merge_requests, dependent: :destroy, foreign_key: :assignee_id, class_name: "MergeRequest" has_many :oauth_applications, class_name: 'Doorkeeper::Application', as: :owner, dependent: :destroy + has_one :abuse_report, dependent: :destroy # diff --git a/app/views/users/show.html.haml b/app/views/users/show.html.haml index 0661d8d06a2..11beb3e3239 100644 --- a/app/views/users/show.html.haml +++ b/app/views/users/show.html.haml @@ -19,18 +19,13 @@ = icon('user') Profile settings - elsif current_user - #report_abuse.pull-right - %span.dropdown - - if @user.abuse_report - %span.light.dropdown-toggle.btn.btn-sm.btn-close{title: abuse_report_button_title(@user)} - = icon('exclamation-circle') - - else - %a.light.dropdown-toggle.btn.btn-sm{href: '#', "data-toggle" => "dropdown"} - = icon('exclamation-circle') - %ul.dropdown-menu.dropdown-menu-right - %li - = link_to new_abuse_report_path(user_id: @user.id), title: abuse_report_button_title(@user) do - Report abuse + .report_abuse.pull-right + - if @user.abuse_report + %span#report_abuse_btn.light.btn.btn-sm.btn-close{title: 'Already reported for abuse', data: {toggle: 'tooltip', placement: 'right', container: 'body'}} + = icon('exclamation-circle') + - else + %a.light.btn.btn-sm{href: new_abuse_report_path(user_id: @user.id), title: 'Report abuse', data: {toggle: 'tooltip', placement: 'right', container: 'body'}} + = icon('exclamation-circle') .username @#{@user.username} diff --git a/features/abuse_report.feature b/features/abuse_report.feature index fc80f0aa399..212972a762a 100644 --- a/features/abuse_report.feature +++ b/features/abuse_report.feature @@ -14,4 +14,4 @@ Feature: Abuse reports And I click "Report abuse" button When I fill and submit abuse form And I visit "Mike" user page - Then I should not see the "Remove abuse" dropdown / button + Then I should see a red "Report abuse" button diff --git a/features/steps/abuse_reports.rb b/features/steps/abuse_reports.rb index 623807dac82..56652ff6f05 100644 --- a/features/steps/abuse_reports.rb +++ b/features/steps/abuse_reports.rb @@ -22,9 +22,8 @@ class Spinach::Features::AbuseReports < Spinach::FeatureSteps user_mike end - step 'I should not see the "Remove abuse" dropdown / button' do - expect(find(:css, '#report_abuse')).not_to have_selector(:css, 'ul.dropdown-menu') - expect(find(:css, '#report_abuse')).to have_selector(:css, '.btn-close') + step 'I should see a red "Report abuse" button' do + expect(find(:css, '.report_abuse')).to have_selector(:css, 'span.btn-close') end def user_mike diff --git a/spec/features/users_spec.rb b/spec/features/users_spec.rb index 633616241f1..c1248162031 100644 --- a/spec/features/users_spec.rb +++ b/spec/features/users_spec.rb @@ -1,8 +1,7 @@ require 'spec_helper' feature 'Users', feature: true do - let(:user) { create(:user, username: 'user1', name: 'User 1', email: 'user1@gitlab.com') } - let(:user2) { create(:user, username: 'user2', name: 'User 2', email: 'user2@gitlab.com') } + let(:user) { create(:user, username: 'user1', name: 'User 1', email: 'user1@gitlab.com') } scenario 'GET /users/sign_in creates a new user account' do visit new_user_session_path @@ -49,37 +48,4 @@ feature 'Users', feature: true do page.find('#error_explanation').find('ul').all('li').count end - context 'With a logged-in user' do - before do - login_as(user) - end - - describe 'Abuse report button' do - context 'User has never been reported for abuse' do - it 'enables the "Report abuse" button / dropdown' do - visit user_path(user2) - - expect(page.find('#report_abuse').find('ul.dropdown-menu').all('li').count).to be(1) - expect(page.find('#report_abuse').all('.btn-close').count).to be(0) - end - end - - context 'User has already been reported for abuse' do - before do - @abuse_report = AbuseReport.new(user: user2, message: 'Foo bar') - @abuse_report.reporter = user - @abuse_report.save! - end - - it 'disables the "Report abuse" button' do - visit user_path(user2) - - expect(page.find('#report_abuse').all('ul.dropdown-menu').count).to be(0) - expect(page.find('#report_abuse').all('.btn-close').count).to be(1) - end - end - end - - end - end diff --git a/spec/models/abuse_report_spec.rb b/spec/models/abuse_report_spec.rb index 635a6e2518c..d45319b25d4 100644 --- a/spec/models/abuse_report_spec.rb +++ b/spec/models/abuse_report_spec.rb @@ -16,4 +16,16 @@ RSpec.describe AbuseReport, type: :model do subject { create(:abuse_report) } it { expect(subject).to be_valid } + + describe 'associations' do + it { is_expected.to belong_to(:reporter).class_name('User') } + it { is_expected.to belong_to(:user) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:reporter) } + it { is_expected.to validate_presence_of(:user) } + it { is_expected.to validate_presence_of(:message) } + it { is_expected.to validate_uniqueness_of(:user_id) } + end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 6342c3b8d13..b7b525bfca2 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -73,7 +73,6 @@ describe User do describe 'associations' do it { is_expected.to have_one(:namespace) } - it { is_expected.to have_one(:abuse_report) } it { is_expected.to have_many(:snippets).class_name('Snippet').dependent(:destroy) } it { is_expected.to have_many(:project_members).dependent(:destroy) } it { is_expected.to have_many(:groups) } @@ -86,6 +85,7 @@ describe User do it { is_expected.to have_many(:merge_requests).dependent(:destroy) } it { is_expected.to have_many(:assigned_merge_requests).dependent(:destroy) } it { is_expected.to have_many(:identities).dependent(:destroy) } + it { is_expected.to have_one(:abuse_report) } end describe 'validations' do -- GitLab