Commit ea72d53e authored by Rémy Coutable's avatar Rémy Coutable Committed by rymai

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.
parent 5f95a5e0
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
...@@ -11,11 +11,11 @@ ...@@ -11,11 +11,11 @@
# #
class AbuseReport < ActiveRecord::Base class AbuseReport < ActiveRecord::Base
belongs_to :reporter, class_name: "User" belongs_to :reporter, class_name: 'User'
belongs_to :user belongs_to :user
validates :reporter, presence: true validates :reporter, presence: true
validates :user, presence: true validates :user, presence: true
validates :message, presence: true validates :message, presence: true
validates :user_id, uniqueness: { scope: :reporter_id } validates :user_id, uniqueness: true
end end
...@@ -97,9 +97,7 @@ class User < ActiveRecord::Base ...@@ -97,9 +97,7 @@ class User < ActiveRecord::Base
# Namespace for personal projects # Namespace for personal projects
has_one :namespace, -> { where type: nil }, dependent: :destroy, foreign_key: :owner_id, class_name: "Namespace" has_one :namespace, -> { where type: nil }, dependent: :destroy, foreign_key: :owner_id, class_name: "Namespace"
# Profile # Profile
has_one :abuse_report, dependent: :destroy
has_many :keys, dependent: :destroy has_many :keys, dependent: :destroy
has_many :emails, dependent: :destroy has_many :emails, dependent: :destroy
has_many :identities, dependent: :destroy, autosave: true has_many :identities, dependent: :destroy, autosave: true
...@@ -131,6 +129,7 @@ class User < ActiveRecord::Base ...@@ -131,6 +129,7 @@ class User < ActiveRecord::Base
has_many :assigned_issues, dependent: :destroy, foreign_key: :assignee_id, class_name: "Issue" 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 :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_many :oauth_applications, class_name: 'Doorkeeper::Application', as: :owner, dependent: :destroy
has_one :abuse_report, dependent: :destroy
# #
......
...@@ -19,18 +19,13 @@ ...@@ -19,18 +19,13 @@
= icon('user') = icon('user')
Profile settings Profile settings
- elsif current_user - elsif current_user
#report_abuse.pull-right .report_abuse.pull-right
%span.dropdown - if @user.abuse_report
- 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'}}
%span.light.dropdown-toggle.btn.btn-sm.btn-close{title: abuse_report_button_title(@user)} = icon('exclamation-circle')
= icon('exclamation-circle') - else
- 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'}}
%a.light.dropdown-toggle.btn.btn-sm{href: '#', "data-toggle" => "dropdown"} = icon('exclamation-circle')
= 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
.username .username
@#{@user.username} @#{@user.username}
......
...@@ -14,4 +14,4 @@ Feature: Abuse reports ...@@ -14,4 +14,4 @@ Feature: Abuse reports
And I click "Report abuse" button And I click "Report abuse" button
When I fill and submit abuse form When I fill and submit abuse form
And I visit "Mike" user page 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
...@@ -22,9 +22,8 @@ class Spinach::Features::AbuseReports < Spinach::FeatureSteps ...@@ -22,9 +22,8 @@ class Spinach::Features::AbuseReports < Spinach::FeatureSteps
user_mike user_mike
end end
step 'I should not see the "Remove abuse" dropdown / button' do step 'I should see a red "Report abuse" button' do
expect(find(:css, '#report_abuse')).not_to have_selector(:css, 'ul.dropdown-menu') expect(find(:css, '.report_abuse')).to have_selector(:css, 'span.btn-close')
expect(find(:css, '#report_abuse')).to have_selector(:css, '.btn-close')
end end
def user_mike def user_mike
......
require 'spec_helper' require 'spec_helper'
feature 'Users', feature: true do feature 'Users', feature: true do
let(:user) { create(:user, username: 'user1', name: 'User 1', email: 'user1@gitlab.com') } 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') }
scenario 'GET /users/sign_in creates a new user account' do scenario 'GET /users/sign_in creates a new user account' do
visit new_user_session_path visit new_user_session_path
...@@ -49,37 +48,4 @@ feature 'Users', feature: true do ...@@ -49,37 +48,4 @@ feature 'Users', feature: true do
page.find('#error_explanation').find('ul').all('li').count page.find('#error_explanation').find('ul').all('li').count
end 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 end
...@@ -16,4 +16,16 @@ RSpec.describe AbuseReport, type: :model do ...@@ -16,4 +16,16 @@ RSpec.describe AbuseReport, type: :model do
subject { create(:abuse_report) } subject { create(:abuse_report) }
it { expect(subject).to be_valid } 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 end
...@@ -73,7 +73,6 @@ describe User do ...@@ -73,7 +73,6 @@ describe User do
describe 'associations' do describe 'associations' do
it { is_expected.to have_one(:namespace) } 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(:snippets).class_name('Snippet').dependent(:destroy) }
it { is_expected.to have_many(:project_members).dependent(:destroy) } it { is_expected.to have_many(:project_members).dependent(:destroy) }
it { is_expected.to have_many(:groups) } it { is_expected.to have_many(:groups) }
...@@ -86,6 +85,7 @@ describe User do ...@@ -86,6 +85,7 @@ describe User do
it { is_expected.to have_many(:merge_requests).dependent(:destroy) } 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(:assigned_merge_requests).dependent(:destroy) }
it { is_expected.to have_many(:identities).dependent(:destroy) } it { is_expected.to have_many(:identities).dependent(:destroy) }
it { is_expected.to have_one(:abuse_report) }
end end
describe 'validations' do describe 'validations' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment