Prevent awarding emoji when a project is archived

This prevents performing the requests, and disables all emoji reaction buttons
This commit is contained in:
Bob Van Landuyt 2018-04-06 20:19:37 +02:00
parent 71ccfde322
commit 04c7d0d555
24 changed files with 231 additions and 26 deletions

View file

@ -40,6 +40,10 @@ export default {
type: Boolean,
required: true,
},
canAwardEmoji: {
type: Boolean,
required: true,
},
canDelete: {
type: Boolean,
required: true,
@ -74,9 +78,6 @@ export default {
shouldShowActionsDropdown() {
return this.currentUserId && (this.canEdit || this.canReportAsAbuse);
},
canAddAwardEmoji() {
return this.currentUserId;
},
isAuthoredByCurrentUser() {
return this.authorId === this.currentUserId;
},
@ -149,7 +150,7 @@ export default {
</button>
</div>
<div
v-if="canAddAwardEmoji"
v-if="canAwardEmoji"
class="note-actions-item">
<a
v-tooltip

View file

@ -28,6 +28,10 @@ export default {
type: Number,
required: true,
},
canAwardEmoji: {
type: Boolean,
required: true,
},
},
computed: {
...mapGetters(['getUserData']),
@ -67,9 +71,6 @@ export default {
isAuthoredByMe() {
return this.noteAuthorId === this.getUserData.id;
},
isLoggedIn() {
return this.getUserData.id;
},
},
created() {
this.emojiSmiling = emojiSmiling;
@ -156,7 +157,7 @@ export default {
return title;
},
handleAward(awardName) {
if (!this.isLoggedIn) {
if (!this.canAwardEmoji) {
return;
}
@ -208,7 +209,7 @@ export default {
</span>
</button>
<div
v-if="isLoggedIn"
v-if="canAwardEmoji"
class="award-menu-holder">
<button
v-tooltip

View file

@ -112,6 +112,7 @@ export default {
:note-author-id="note.author.id"
:awards="note.award_emoji"
:toggle-award-path="note.toggle_award_path"
:can-award-emoji="note.current_user.can_award_emoji"
/>
<note-attachment
v-if="note.attachment"

View file

@ -177,6 +177,7 @@ export default {
:note-id="note.id"
:access-level="note.human_access"
:can-edit="note.current_user.can_edit"
:can-award-emoji="note.current_user.can_award_emoji"
:can-delete="note.current_user.can_edit"
:can-report-as-abuse="canReportAsAbuse"
:report-abuse-path="note.report_abuse_path"

View file

@ -82,8 +82,8 @@ module IssuesHelper
names.to_sentence
end
def award_state_class(awards, current_user)
if !current_user
def award_state_class(awardable, awards, current_user)
if !can?(current_user, :award_emoji, awardable)
"disabled"
elsif current_user && awards.find { |a| a.user_id == current_user.id }
"active"

View file

@ -79,11 +79,7 @@ module Awardable
end
def user_can_award?(current_user, name)
if user_authored?(current_user)
!awardable_votes?(normalize_name(name))
else
true
end
awardable_by_user?(current_user, name) && Ability.allowed?(current_user, :award_emoji, self)
end
def user_authored?(current_user)
@ -119,4 +115,12 @@ module Awardable
def normalize_name(name)
Gitlab::Emoji.normalize_emoji_name(name)
end
def awardable_by_user?(current_user, name)
if user_authored?(current_user)
!awardable_votes?(normalize_name(name))
else
true
end
end
end

View file

@ -1,6 +1,6 @@
class NotePolicy < BasePolicy
delegate { @subject.project }
delegate { @subject.noteable if @subject.noteable.lockable? }
delegate { @subject.noteable if DeclarativePolicy.has_policy?(@subject.noteable) }
condition(:is_author) { @user && @subject.author == @user }
condition(:is_noteable_author) { @user && @subject.noteable.author_id == @user.id }

View file

@ -25,4 +25,6 @@ class PersonalSnippetPolicy < BasePolicy
end
rule { anonymous }.prevent :comment_personal_snippet
rule { can?(:comment_personal_snippet) }.enable :award_emoji
end

View file

@ -155,6 +155,7 @@ class ProjectPolicy < BasePolicy
enable :create_note
enable :upload_file
enable :read_cycle_analytics
enable :award_emoji
end
# These abilities are not allowed to admins that are not members of the project,
@ -253,6 +254,7 @@ class ProjectPolicy < BasePolicy
prevent :resolve_note
prevent :create_merge_request_from
prevent :create_merge_request_in
prevent :award_emoji
READONLY_FEATURES_WHEN_ARCHIVED.each do |feature|
prevent(*create_update_admin_destroy(feature))

View file

@ -29,6 +29,10 @@ class IssueEntity < IssuableEntity
expose :can_update do |issue|
can?(request.current_user, :update_issue, issue)
end
expose :can_award_emoji do |issue|
can?(request.current_user, :award_emoji, issue)
end
end
expose :create_note_path do |issue|

View file

@ -17,6 +17,10 @@ class NoteEntity < API::Entities::Note
expose :can_edit do |note|
Ability.allowed?(request.current_user, :admin_note, note)
end
expose :can_award_emoji do |note|
Ability.allowed?(request.current_user, :award_emoji, note)
end
end
expose :resolved?, as: :resolved

View file

@ -3,13 +3,13 @@
.awards.js-awards-block{ class: ("hidden" if !inline && grouped_emojis.empty?), data: { award_url: toggle_award_url(awardable) } }
- awards_sort(grouped_emojis).each do |emoji, awards|
%button.btn.award-control.js-emoji-btn.has-tooltip{ type: "button",
class: [(award_state_class(awards, current_user)), (award_user_authored_class(emoji) if user_authored)],
class: [(award_state_class(awardable, awards, current_user)), (award_user_authored_class(emoji) if user_authored)],
data: { placement: "bottom", title: award_user_list(awards, current_user) } }
= emoji_icon(emoji)
%span.award-control-text.js-counter
= awards.count
- if current_user
- if can?(current_user, :award_emoji, awardable)
.award-menu-holder.js-award-holder
%button.btn.award-control.has-tooltip.js-add-award{ type: 'button',
'aria-label': 'Add reaction',

View file

@ -36,7 +36,7 @@
%template{ 'v-else' => '' }
= render 'shared/icons/icon_resolve_discussion.svg'
- if current_user
- if can?(current_user, :award_emoji, note)
- if note.emoji_awardable?
- user_authored = note.user_authored?(current_user)
.note-actions-item

View file

@ -4,6 +4,10 @@ FactoryBot.define do
user
awardable factory: :issue
after(:create) do |award, evaluator|
award.awardable.project.add_guest(evaluator.user)
end
trait :upvote
trait :downvote do
name "thumbsdown"

View file

@ -35,6 +35,14 @@ describe 'Merge request > User awards emoji', :js do
expect(page).to have_selector('.emoji-menu', count: 1)
end
describe 'the project is archived' do
let(:project) { create(:project, :public, :repository, archived: true) }
it 'does not see award menu button' do
expect(page).not_to have_selector('.js-award-holder')
end
end
end
describe 'logged out' do

View file

@ -101,4 +101,72 @@ describe 'User interacts with awards in an issue', :js do
expect(page).to have_selector('gl-emoji[data-name="smile"]')
end
context 'when a project is archived' do
let(:project) { create(:project, archived: true) }
it 'hides the add award button' do
page.within('.awards') do
expect(page).not_to have_css('.js-add-award')
end
end
end
context 'awards on a note' do
let!(:note) { create(:note, noteable: issue, project: issue.project) }
let!(:award_emoji) { create(:award_emoji, awardable: note, name: '100') }
it 'shows the award on the note' do
page.within('.note-awards') do
expect(page).to have_selector('gl-emoji[data-name="100"]')
end
end
it 'allows adding a vote to an award' do
page.within('.note-awards') do
find('gl-emoji[data-name="100"]').click
end
wait_for_requests
expect(note.reload.award_emoji.size).to eq(2)
end
it 'allows adding a new emoji' do
page.within('.note-actions') do
find('a.js-add-award').click
end
page.within('.emoji-menu-content') do
find('gl-emoji[data-name="8ball"]').click
end
wait_for_requests
page.within('.note-awards') do
expect(page).to have_selector('gl-emoji[data-name="8ball"]')
end
expect(note.reload.award_emoji.size).to eq(2)
end
context 'when the project is archived' do
let(:project) { create(:project, archived: true) }
it 'hides the buttons for adding new emoji' do
page.within('.note-awards') do
expect(page).not_to have_css('.award-menu-holder')
end
page.within('.note-actions') do
expect(page).not_to have_css('a.js-add-award')
end
end
it 'does not allow toggling existing emoji' do
page.within('.note-awards') do
find('gl-emoji[data-name="100"]').click
end
wait_for_requests
expect(note.reload.award_emoji.size).to eq(1)
end
end
end
end

View file

@ -96,13 +96,32 @@ describe IssuesHelper do
describe '#award_state_class' do
let!(:upvote) { create(:award_emoji) }
let(:awardable) { upvote.awardable }
let(:user) { upvote.user }
before do
allow(helper).to receive(:can?) do |*args|
Ability.allowed?(*args)
end
end
it "returns disabled string for unauthenticated user" do
expect(award_state_class(AwardEmoji.all, nil)).to eq("disabled")
expect(helper.award_state_class(awardable, AwardEmoji.all, nil)).to eq("disabled")
end
it "returns disabled for a user that does not have access to the awardable" do
expect(helper.award_state_class(awardable, AwardEmoji.all, build(:user))).to eq("disabled")
end
it "returns active string for author" do
expect(award_state_class(AwardEmoji.all, upvote.user)).to eq("active")
expect(helper.award_state_class(awardable, AwardEmoji.all, upvote.user)).to eq("active")
end
it "is blank for a user that has access to the awardable" do
user = build(:user)
expect(helper).to receive(:can?).with(user, :award_emoji, awardable).and_return(true)
expect(helper.award_state_class(awardable, AwardEmoji.all, user)).to be_blank
end
end

View file

@ -3,7 +3,7 @@ import store from '~/notes/stores';
import noteActions from '~/notes/components/note_actions.vue';
import { userDataMock } from '../mock_data';
describe('issse_note_actions component', () => {
describe('issue_note_actions component', () => {
let vm;
let Component;
@ -24,6 +24,7 @@ describe('issse_note_actions component', () => {
authorId: 26,
canDelete: true,
canEdit: true,
canAwardEmoji: true,
canReportAsAbuse: true,
noteId: 539,
reportAbusePath: '/abuse_reports/new?ref_url=http%3A%2F%2Flocalhost%3A3000%2Fgitlab-org%2Fgitlab-ce%2Fissues%2F7%23note_539&user_id=26',
@ -70,6 +71,7 @@ describe('issse_note_actions component', () => {
authorId: 26,
canDelete: false,
canEdit: false,
canAwardEmoji: false,
canReportAsAbuse: false,
noteId: 539,
reportAbusePath: '/abuse_reports/new?ref_url=http%3A%2F%2Flocalhost%3A3000%2Fgitlab-org%2Fgitlab-ce%2Fissues%2F7%23note_539&user_id=26',

View file

@ -29,6 +29,7 @@ describe('note_awards_list component', () => {
awards: awardsMock,
noteAuthorId: 2,
noteId: 545,
canAwardEmoji: true,
toggleAwardPath: '/gitlab-org/gitlab-ce/notes/545/toggle_award_emoji',
},
}).$mount();
@ -43,14 +44,45 @@ describe('note_awards_list component', () => {
expect(vm.$el.querySelector('.js-awards-block button [data-name="cartwheel_tone3"]')).toBeDefined();
});
it('should be possible to remove awareded emoji', () => {
it('should be possible to remove awarded emoji', () => {
spyOn(vm, 'handleAward').and.callThrough();
spyOn(vm, 'toggleAwardRequest').and.callThrough();
vm.$el.querySelector('.js-awards-block button').click();
expect(vm.handleAward).toHaveBeenCalledWith('flag_tz');
expect(vm.toggleAwardRequest).toHaveBeenCalled();
});
it('should be possible to add new emoji', () => {
expect(vm.$el.querySelector('.js-add-award')).toBeDefined();
});
describe('when the user cannot award emoji', () => {
beforeEach(() => {
const Component = Vue.extend(awardsNote);
vm = new Component({
store,
propsData: {
awards: awardsMock,
noteAuthorId: 2,
noteId: 545,
canAwardEmoji: false,
toggleAwardPath: '/gitlab-org/gitlab-ce/notes/545/toggle_award_emoji',
},
}).$mount();
});
it('should not be possible to remove awarded emoji', () => {
spyOn(vm, 'toggleAwardRequest').and.callThrough();
vm.$el.querySelector('.js-awards-block button').click();
expect(vm.toggleAwardRequest).not.toHaveBeenCalled();
});
it('should not be possible to add new emoji', () => {
expect(vm.$el.querySelector('.js-add-award')).toBeNull();
});
});
});

View file

@ -18,6 +18,7 @@ describe('issue_note_body component', () => {
propsData: {
note,
canEdit: true,
canAwardEmoji: true,
},
}).$mount();
});

View file

@ -9,6 +9,7 @@ export const notesDataMock = {
totalNotes: 1,
closePath: '/twitter/flight/issues/9.json?issue%5Bstate_event%5D=close',
reopenPath: '/twitter/flight/issues/9.json?issue%5Bstate_event%5D=reopen',
canAwardEmoji: true,
};
export const userDataMock = {
@ -30,6 +31,7 @@ export const noteableDataMock = {
current_user: {
can_create_note: true,
can_update: true,
can_award_emoji: true,
},
description: '',
due_date: null,
@ -86,7 +88,10 @@ export const individualNote = {
human_access: 'Owner',
note: 'sdfdsaf',
note_html: "<p dir='auto'>sdfdsaf</p>",
current_user: { can_edit: true },
current_user: {
can_edit: true,
can_award_emoji: true,
},
discussion_id: '0fb4e0e3f9276e55ff32eb4195add694aece4edd',
emoji_awardable: true,
award_emoji: [
@ -129,6 +134,7 @@ export const note = {
note_html: '<p dir="auto">Vel id placeat reprehenderit sit numquam.</p>',
current_user: {
can_edit: true,
can_award_emoji: true,
},
discussion_id: 'd3842a451b7f3d9a5dfce329515127b2d29a4cd0',
emoji_awardable: true,
@ -187,6 +193,7 @@ export const discussionMock = {
note_html: "<p dir='auto'>THIS IS A DICUSSSION!</p>",
current_user: {
can_edit: true,
can_award_emoji: true,
},
discussion_id: '9e3bd2f71a01de45fd166e6719eb380ad9f270b1',
emoji_awardable: true,
@ -231,6 +238,7 @@ export const discussionMock = {
},
current_user: {
can_edit: true,
can_award_emoji: true,
},
discussion_id: '9e3bd2f71a01de45fd166e6719eb380ad9f270b1',
emoji_awardable: true,
@ -275,6 +283,7 @@ export const discussionMock = {
},
current_user: {
can_edit: true,
can_award_emoji: true,
},
discussion_id: '9e3bd2f71a01de45fd166e6719eb380ad9f270b1',
emoji_awardable: true,
@ -365,6 +374,7 @@ export const INDIVIDUAL_NOTE_RESPONSE_MAP = {
note_html: '\u003cp dir="auto"\u003esdfdsaf\u003c/p\u003e',
current_user: {
can_edit: true,
can_award_emoji: true,
},
discussion_id: '0fb4e0e3f9276e55ff32eb4195add694aece4edd',
emoji_awardable: true,
@ -425,6 +435,7 @@ export const INDIVIDUAL_NOTE_RESPONSE_MAP = {
note_html: '\u003cp dir="auto"\u003eNew note!\u003c/p\u003e',
current_user: {
can_edit: true,
can_award_emoji: true,
},
discussion_id: '70d5c92a4039a36c70100c6691c18c27e4b0a790',
emoji_awardable: true,
@ -478,6 +489,7 @@ export const INDIVIDUAL_NOTE_RESPONSE_MAP = {
},
current_user: {
can_edit: true,
can_award_emoji: true,
},
discussion_id: 'a3ed36e29b1957efb3b68c53e2d7a2b24b1df052',
emoji_awardable: true,
@ -527,6 +539,7 @@ export const DISCUSSION_NOTE_RESPONSE_MAP = {
note_html: '\u003cp dir="auto"\u003eAdding a comment\u003c/p\u003e',
current_user: {
can_edit: true,
can_award_emoji: true,
},
discussion_id: 'a3ed36e29b1957efb3b68c53e2d7a2b24b1df052',
emoji_awardable: true,

View file

@ -46,6 +46,31 @@ describe Awardable do
end
end
describe '#user_can_award?' do
let(:user) { create(:user) }
before do
issue.project.add_guest(user)
end
it 'does not allow upvoting or downvoting your own issue' do
issue.update!(author: user)
expect(issue.user_can_award?(user, AwardEmoji::DOWNVOTE_NAME)).to be_falsy
expect(issue.user_can_award?(user, AwardEmoji::UPVOTE_NAME)).to be_falsy
end
it 'is truthy when the user is allowed to award emoji' do
expect(issue.user_can_award?(user, AwardEmoji::UPVOTE_NAME)).to be_truthy
end
it 'is falsy when the project is archived' do
issue.project.update!(archived: true)
expect(issue.user_can_award?(user, AwardEmoji::UPVOTE_NAME)).to be_falsy
end
end
describe "#toggle_award_emoji" do
it "adds an emoji if it isn't awarded yet" do
expect { issue.toggle_award_emoji("thumbsup", award_emoji.user) }.to change { AwardEmoji.count }.by(1)

View file

@ -27,6 +27,7 @@ describe PersonalSnippetPolicy do
it do
is_expected.to be_allowed(:read_personal_snippet)
is_expected.to be_disallowed(:comment_personal_snippet)
is_expected.to be_disallowed(:award_emoji)
is_expected.to be_disallowed(*author_permissions)
end
end
@ -37,6 +38,7 @@ describe PersonalSnippetPolicy do
it do
is_expected.to be_allowed(:read_personal_snippet)
is_expected.to be_allowed(:comment_personal_snippet)
is_expected.to be_allowed(:award_emoji)
is_expected.to be_disallowed(*author_permissions)
end
end
@ -47,6 +49,7 @@ describe PersonalSnippetPolicy do
it do
is_expected.to be_allowed(:read_personal_snippet)
is_expected.to be_allowed(:comment_personal_snippet)
is_expected.to be_allowed(:award_emoji)
is_expected.to be_allowed(*author_permissions)
end
end
@ -61,6 +64,7 @@ describe PersonalSnippetPolicy do
it do
is_expected.to be_disallowed(:read_personal_snippet)
is_expected.to be_disallowed(:comment_personal_snippet)
is_expected.to be_disallowed(:award_emoji)
is_expected.to be_disallowed(*author_permissions)
end
end
@ -71,6 +75,7 @@ describe PersonalSnippetPolicy do
it do
is_expected.to be_allowed(:read_personal_snippet)
is_expected.to be_allowed(:comment_personal_snippet)
is_expected.to be_allowed(:award_emoji)
is_expected.to be_disallowed(*author_permissions)
end
end
@ -81,6 +86,7 @@ describe PersonalSnippetPolicy do
it do
is_expected.to be_disallowed(:read_personal_snippet)
is_expected.to be_disallowed(:comment_personal_snippet)
is_expected.to be_disallowed(:award_emoji)
is_expected.to be_disallowed(*author_permissions)
end
end
@ -91,6 +97,7 @@ describe PersonalSnippetPolicy do
it do
is_expected.to be_allowed(:read_personal_snippet)
is_expected.to be_allowed(:comment_personal_snippet)
is_expected.to be_allowed(:award_emoji)
is_expected.to be_allowed(*author_permissions)
end
end
@ -105,6 +112,7 @@ describe PersonalSnippetPolicy do
it do
is_expected.to be_disallowed(:read_personal_snippet)
is_expected.to be_disallowed(:comment_personal_snippet)
is_expected.to be_disallowed(:award_emoji)
is_expected.to be_disallowed(*author_permissions)
end
end
@ -115,6 +123,7 @@ describe PersonalSnippetPolicy do
it do
is_expected.to be_disallowed(:read_personal_snippet)
is_expected.to be_disallowed(:comment_personal_snippet)
is_expected.to be_disallowed(:award_emoji)
is_expected.to be_disallowed(*author_permissions)
end
end
@ -125,6 +134,7 @@ describe PersonalSnippetPolicy do
it do
is_expected.to be_disallowed(:read_personal_snippet)
is_expected.to be_disallowed(:comment_personal_snippet)
is_expected.to be_disallowed(:award_emoji)
is_expected.to be_disallowed(*author_permissions)
end
end
@ -135,6 +145,7 @@ describe PersonalSnippetPolicy do
it do
is_expected.to be_allowed(:read_personal_snippet)
is_expected.to be_allowed(:comment_personal_snippet)
is_expected.to be_allowed(:award_emoji)
is_expected.to be_allowed(*author_permissions)
end
end

View file

@ -15,6 +15,7 @@ describe ProjectPolicy do
read_project_for_iids read_issue_iid read_merge_request_iid read_label
read_milestone read_project_snippet read_project_member read_note
create_project create_issue create_note upload_file create_merge_request_in
award_emoji
]
end
@ -166,6 +167,7 @@ describe ProjectPolicy do
request_access
upload_file
resolve_note
award_emoji
]
end
@ -193,7 +195,7 @@ describe ProjectPolicy do
context 'when a project has pending invites' do
let(:group) { create(:group, :public) }
let(:project) { create(:project, :public, namespace: group) }
let(:user_permissions) { [:create_merge_request_in, :create_project, :create_issue, :create_note, :upload_file] }
let(:user_permissions) { [:create_merge_request_in, :create_project, :create_issue, :create_note, :upload_file, :award_emoji] }
let(:anonymous_permissions) { guest_permissions - user_permissions }
subject { described_class.new(nil, project) }