Commit 4a1eab09 authored by Sean McGivern's avatar Sean McGivern

Merge branch 'ee-18608-lock-issues-v2' into 'master'

EE Port of "Lock discussion for issues and merge requests"

See merge request gitlab-org/gitlab-ee!3061
parents 65e847ac 31ad48c9
...@@ -17,7 +17,8 @@ class Diff { ...@@ -17,7 +17,8 @@ class Diff {
} }
}); });
FilesCommentButton.init($diffFile); const tab = document.getElementById('diffs');
if (!tab || (tab && tab.dataset && tab.dataset.isLocked !== '')) FilesCommentButton.init($diffFile);
$diffFile.each((index, file) => new gl.ImageFile(file)); $diffFile.each((index, file) => new gl.ImageFile(file));
......
...@@ -7,10 +7,12 @@ ...@@ -7,10 +7,12 @@
import TaskList from '../../task_list'; import TaskList from '../../task_list';
import * as constants from '../constants'; import * as constants from '../constants';
import eventHub from '../event_hub'; import eventHub from '../event_hub';
import confidentialIssue from '../../vue_shared/components/issue/confidential_issue_warning.vue'; import issueWarning from '../../vue_shared/components/issue/issue_warning.vue';
import issueNoteSignedOutWidget from './issue_note_signed_out_widget.vue'; import issueNoteSignedOutWidget from './issue_note_signed_out_widget.vue';
import issueDiscussionLockedWidget from './issue_discussion_locked_widget.vue';
import markdownField from '../../vue_shared/components/markdown/field.vue'; import markdownField from '../../vue_shared/components/markdown/field.vue';
import userAvatarLink from '../../vue_shared/components/user_avatar/user_avatar_link.vue'; import userAvatarLink from '../../vue_shared/components/user_avatar/user_avatar_link.vue';
import issuableStateMixin from '../mixins/issuable_state';
export default { export default {
name: 'issueCommentForm', name: 'issueCommentForm',
...@@ -26,8 +28,9 @@ ...@@ -26,8 +28,9 @@
}; };
}, },
components: { components: {
confidentialIssue, issueWarning,
issueNoteSignedOutWidget, issueNoteSignedOutWidget,
issueDiscussionLockedWidget,
markdownField, markdownField,
userAvatarLink, userAvatarLink,
}, },
...@@ -55,6 +58,9 @@ ...@@ -55,6 +58,9 @@
isIssueOpen() { isIssueOpen() {
return this.issueState === constants.OPENED || this.issueState === constants.REOPENED; return this.issueState === constants.OPENED || this.issueState === constants.REOPENED;
}, },
canCreateNote() {
return this.getIssueData.current_user.can_create_note;
},
issueActionButtonTitle() { issueActionButtonTitle() {
if (this.note.length) { if (this.note.length) {
const actionText = this.isIssueOpen ? 'close' : 'reopen'; const actionText = this.isIssueOpen ? 'close' : 'reopen';
...@@ -90,9 +96,6 @@ ...@@ -90,9 +96,6 @@
endpoint() { endpoint() {
return this.getIssueData.create_note_path; return this.getIssueData.create_note_path;
}, },
isConfidentialIssue() {
return this.getIssueData.confidential;
},
}, },
methods: { methods: {
...mapActions([ ...mapActions([
...@@ -220,6 +223,9 @@ ...@@ -220,6 +223,9 @@
}); });
}, },
}, },
mixins: [
issuableStateMixin,
],
mounted() { mounted() {
// jQuery is needed here because it is a custom event being dispatched with jQuery. // jQuery is needed here because it is a custom event being dispatched with jQuery.
$(document).on('issuable:change', (e, isClosed) => { $(document).on('issuable:change', (e, isClosed) => {
...@@ -235,6 +241,7 @@ ...@@ -235,6 +241,7 @@
<template> <template>
<div> <div>
<issue-note-signed-out-widget v-if="!isLoggedIn" /> <issue-note-signed-out-widget v-if="!isLoggedIn" />
<issue-discussion-locked-widget v-else-if="!canCreateNote" />
<ul <ul
v-else v-else
class="notes notes-form timeline"> class="notes notes-form timeline">
...@@ -253,15 +260,22 @@ ...@@ -253,15 +260,22 @@
<div class="timeline-content timeline-content-form"> <div class="timeline-content timeline-content-form">
<form <form
ref="commentForm" ref="commentForm"
class="new-note js-quick-submit common-note-form gfm-form js-main-target-form"> class="new-note js-quick-submit common-note-form gfm-form js-main-target-form"
<confidentialIssue v-if="isConfidentialIssue" /> >
<div class="error-alert"></div> <div class="error-alert"></div>
<issue-warning
v-if="hasWarning(getIssueData)"
:is-locked="isLocked(getIssueData)"
:is-confidential="isConfidential(getIssueData)"
/>
<markdown-field <markdown-field
:markdown-preview-path="markdownPreviewPath" :markdown-preview-path="markdownPreviewPath"
:markdown-docs-path="markdownDocsPath" :markdown-docs-path="markdownDocsPath"
:quick-actions-docs-path="quickActionsDocsPath" :quick-actions-docs-path="quickActionsDocsPath"
:add-spacing-classes="false" :add-spacing-classes="false"
:is-confidential-issue="isConfidentialIssue"
ref="markdownField"> ref="markdownField">
<textarea <textarea
id="note-body" id="note-body"
......
<script>
export default {
computed: {
lockIcon() {
return gl.utils.spriteIcon('lock');
},
},
};
</script>
<template>
<div class="disabled-comment text-center">
<span class="issuable-note-warning">
<span class="icon" v-html="lockIcon"></span>
<span>This issue is locked. Only <b>project members</b> can comment.</span>
</span>
</div>
</template>
<script> <script>
import { mapGetters } from 'vuex'; import { mapGetters } from 'vuex';
import eventHub from '../event_hub'; import eventHub from '../event_hub';
import confidentialIssue from '../../vue_shared/components/issue/confidential_issue_warning.vue'; import issueWarning from '../../vue_shared/components/issue/issue_warning.vue';
import markdownField from '../../vue_shared/components/markdown/field.vue'; import markdownField from '../../vue_shared/components/markdown/field.vue';
import issuableStateMixin from '../mixins/issuable_state';
export default { export default {
name: 'issueNoteForm', name: 'issueNoteForm',
...@@ -39,12 +40,13 @@ ...@@ -39,12 +40,13 @@
}; };
}, },
components: { components: {
confidentialIssue, issueWarning,
markdownField, markdownField,
}, },
computed: { computed: {
...mapGetters([ ...mapGetters([
'getDiscussionLastNote', 'getDiscussionLastNote',
'getIssueData',
'getIssueDataByProp', 'getIssueDataByProp',
'getNotesDataByProp', 'getNotesDataByProp',
'getUserDataByProp', 'getUserDataByProp',
...@@ -67,9 +69,6 @@ ...@@ -67,9 +69,6 @@
isDisabled() { isDisabled() {
return !this.note.length || this.isSubmitting; return !this.note.length || this.isSubmitting;
}, },
isConfidentialIssue() {
return this.getIssueDataByProp('confidential');
},
}, },
methods: { methods: {
handleUpdate() { handleUpdate() {
...@@ -95,6 +94,9 @@ ...@@ -95,6 +94,9 @@
this.$emit('cancelFormEdition', shouldConfirm, this.noteBody !== this.note); this.$emit('cancelFormEdition', shouldConfirm, this.noteBody !== this.note);
}, },
}, },
mixins: [
issuableStateMixin,
],
mounted() { mounted() {
this.$refs.textarea.focus(); this.$refs.textarea.focus();
}, },
...@@ -125,7 +127,13 @@ ...@@ -125,7 +127,13 @@
<div class="flash-container timeline-content"></div> <div class="flash-container timeline-content"></div>
<form <form
class="edit-note common-note-form js-quick-submit gfm-form"> class="edit-note common-note-form js-quick-submit gfm-form">
<confidentialIssue v-if="isConfidentialIssue" />
<issue-warning
v-if="hasWarning(getIssueData)"
:is-locked="isLocked(getIssueData)"
:is-confidential="isConfidential(getIssueData)"
/>
<markdown-field <markdown-field
:markdown-preview-path="markdownPreviewPath" :markdown-preview-path="markdownPreviewPath"
:markdown-docs-path="markdownDocsPath" :markdown-docs-path="markdownDocsPath"
......
export default {
methods: {
isConfidential(issue) {
return !!issue.confidential;
},
isLocked(issue) {
return !!issue.discussion_locked;
},
hasWarning(issue) {
return this.isConfidential(issue) || this.isLocked(issue);
},
},
};
...@@ -47,9 +47,9 @@ export default { ...@@ -47,9 +47,9 @@ export default {
</script> </script>
<template> <template>
<div class="block confidentiality"> <div class="block issuable-sidebar-item confidentiality">
<div class="sidebar-collapsed-icon"> <div class="sidebar-collapsed-icon">
<i class="fa" :class="faEye" aria-hidden="true" data-hidden="true"></i> <i class="fa" :class="faEye" aria-hidden="true"></i>
</div> </div>
<div class="title hide-collapsed"> <div class="title hide-collapsed">
Confidentiality Confidentiality
...@@ -62,19 +62,19 @@ export default { ...@@ -62,19 +62,19 @@ export default {
Edit Edit
</a> </a>
</div> </div>
<div class="value confidential-value hide-collapsed"> <div class="value sidebar-item-value hide-collapsed">
<editForm <editForm
v-if="edit" v-if="edit"
:toggle-form="toggleForm" :toggle-form="toggleForm"
:is-confidential="isConfidential" :is-confidential="isConfidential"
:update-confidential-attribute="updateConfidentialAttribute" :update-confidential-attribute="updateConfidentialAttribute"
/> />
<div v-if="!isConfidential" class="no-value confidential-value"> <div v-if="!isConfidential" class="no-value sidebar-item-value">
<i class="fa fa-eye is-not-confidential"></i> <i class="fa fa-eye sidebar-item-icon"></i>
Not confidential Not confidential
</div> </div>
<div v-else class="value confidential-value hide-collapsed"> <div v-else class="value sidebar-item-value hide-collapsed">
<i aria-hidden="true" data-hidden="true" class="fa fa-eye-slash is-confidential"></i> <i aria-hidden="true" class="fa fa-eye-slash sidebar-item-icon is-active"></i>
This issue is confidential This issue is confidential
</div> </div>
</div> </div>
......
...@@ -2,9 +2,6 @@ ...@@ -2,9 +2,6 @@
import editFormButtons from './edit_form_buttons.vue'; import editFormButtons from './edit_form_buttons.vue';
export default { export default {
components: {
editFormButtons,
},
props: { props: {
isConfidential: { isConfidential: {
required: true, required: true,
...@@ -19,12 +16,16 @@ export default { ...@@ -19,12 +16,16 @@ export default {
type: Function, type: Function,
}, },
}, },
components: {
editFormButtons,
},
}; };
</script> </script>
<template> <template>
<div class="dropdown open"> <div class="dropdown open">
<div class="dropdown-menu confidential-warning-message"> <div class="dropdown-menu sidebar-item-warning-message">
<div> <div>
<p v-if="!isConfidential"> <p v-if="!isConfidential">
You are going to turn on the confidentiality. This means that only team members with You are going to turn on the confidentiality. This means that only team members with
......
...@@ -15,7 +15,7 @@ export default { ...@@ -15,7 +15,7 @@ export default {
}, },
}, },
computed: { computed: {
onOrOff() { toggleButtonText() {
return this.isConfidential ? 'Turn Off' : 'Turn On'; return this.isConfidential ? 'Turn Off' : 'Turn On';
}, },
updateConfidentialBool() { updateConfidentialBool() {
...@@ -26,7 +26,7 @@ export default { ...@@ -26,7 +26,7 @@ export default {
</script> </script>
<template> <template>
<div class="confidential-warning-message-actions"> <div class="sidebar-item-warning-message-actions">
<button <button
type="button" type="button"
class="btn btn-default append-right-10" class="btn btn-default append-right-10"
...@@ -39,7 +39,7 @@ export default { ...@@ -39,7 +39,7 @@ export default {
class="btn btn-close" class="btn btn-close"
@click.prevent="updateConfidentialAttribute(updateConfidentialBool)" @click.prevent="updateConfidentialAttribute(updateConfidentialBool)"
> >
{{ onOrOff }} {{ toggleButtonText }}
</button> </button>
</div> </div>
</template> </template>
<script>
import editFormButtons from './edit_form_buttons.vue';
import issuableMixin from '../../../vue_shared/mixins/issuable';
export default {
props: {
isLocked: {
required: true,
type: Boolean,
},
toggleForm: {
required: true,
type: Function,
},
updateLockedAttribute: {
required: true,
type: Function,
},
issuableType: {
required: true,
type: String,
},
},
mixins: [
issuableMixin,
],
components: {
editFormButtons,
},
};
</script>
<template>
<div class="dropdown open">
<div class="dropdown-menu sidebar-item-warning-message">
<p class="text" v-if="isLocked">
Unlock this {{ issuableDisplayName(issuableType) }}?
<strong>Everyone</strong>
will be able to comment.
</p>
<p class="text" v-else>
Lock this {{ issuableDisplayName(issuableType) }}?
Only
<strong>project members</strong>
will be able to comment.
</p>
<edit-form-buttons
:is-locked="isLocked"
:toggle-form="toggleForm"
:update-locked-attribute="updateLockedAttribute"
/>
</div>
</div>
</template>
<script>
export default {
props: {
isLocked: {
required: true,
type: Boolean,
},
toggleForm: {
required: true,
type: Function,
},
updateLockedAttribute: {
required: true,
type: Function,
},
},
computed: {
buttonText() {
return this.isLocked ? this.__('Unlock') : this.__('Lock');
},
toggleLock() {
return !this.isLocked;
},
},
};
</script>
<template>
<div class="sidebar-item-warning-message-actions">
<button
type="button"
class="btn btn-default append-right-10"
@click="toggleForm"
>
{{ __('Cancel') }}
</button>
<button
type="button"
class="btn btn-close"
@click.prevent="updateLockedAttribute(toggleLock)"
>
{{ buttonText }}
</button>
</div>
</template>
<script>
/* global Flash */
import editForm from './edit_form.vue';
import issuableMixin from '../../../vue_shared/mixins/issuable';
export default {
props: {
isLocked: {
required: true,
type: Boolean,
},
isEditable: {
required: true,
type: Boolean,
},
mediator: {
required: true,
type: Object,
validator(mediatorObject) {
return mediatorObject.service && mediatorObject.service.update && mediatorObject.store;
},
},
issuableType: {
required: true,
type: String,
},
},
mixins: [
issuableMixin,
],
components: {
editForm,
},
computed: {
lockIconClass() {
return this.isLocked ? 'fa-lock' : 'fa-unlock';
},
isLockDialogOpen() {
return this.mediator.store.isLockDialogOpen;
},
},
methods: {
toggleForm() {
this.mediator.store.isLockDialogOpen = !this.mediator.store.isLockDialogOpen;
},
updateLockedAttribute(locked) {
this.mediator.service.update(this.issuableType, {
discussion_locked: locked,
})
.then(() => location.reload())
.catch(() => Flash(this.__(`Something went wrong trying to change the locked state of this ${this.issuableDisplayName(this.issuableType)}`)));
},
},
};
</script>
<template>
<div class="block issuable-sidebar-item lock">
<div class="sidebar-collapsed-icon">
<i
class="fa"
:class="lockIconClass"
aria-hidden="true"
></i>
</div>
<div class="title hide-collapsed">
Lock {{issuableDisplayName(issuableType) }}
<button
v-if="isEditable"
class="pull-right lock-edit btn btn-blank"
type="button"
@click.prevent="toggleForm"
>
{{ __('Edit') }}
</button>
</div>
<div class="value sidebar-item-value hide-collapsed">
<edit-form
v-if="isLockDialogOpen"
:toggle-form="toggleForm"
:is-locked="isLocked"
:update-locked-attribute="updateLockedAttribute"
:issuable-type="issuableType"
/>
<div
v-if="isLocked"
class="value sidebar-item-value"
>
<i
aria-hidden="true"
class="fa fa-lock sidebar-item-icon is-active"
></i>
{{ __('Locked') }}
</div>
<div
v-else
class="no-value sidebar-item-value hide-collapsed"
>
<i
aria-hidden="true"
class="fa fa-unlock sidebar-item-icon"
></i>
{{ __('Unlocked') }}
</div>
</div>
</div>
</template>
import Vue from 'vue'; import Vue from 'vue';
import sidebarTimeTracking from './components/time_tracking/sidebar_time_tracking'; import SidebarTimeTracking from './components/time_tracking/sidebar_time_tracking';
import sidebarAssignees from './components/assignees/sidebar_assignees'; import SidebarAssignees from './components/assignees/sidebar_assignees';
import confidential from './components/confidential/confidential_issue_sidebar.vue'; import ConfidentialIssueSidebar from './components/confidential/confidential_issue_sidebar.vue';
import SidebarMoveIssue from './lib/sidebar_move_issue'; import SidebarMoveIssue from './lib/sidebar_move_issue';
import LockIssueSidebar from './components/lock/lock_issue_sidebar.vue';
import Translate from '../vue_shared/translate';
import Mediator from './sidebar_mediator'; import Mediator from './sidebar_mediator';
Vue.use(Translate);
function mountConfidentialComponent(mediator) {
const el = document.getElementById('js-confidential-entry-point');
if (!el) return;
const dataNode = document.getElementById('js-confidential-issue-data');
const initialData = JSON.parse(dataNode.innerHTML);
const ConfidentialComp = Vue.extend(ConfidentialIssueSidebar);
new ConfidentialComp({
propsData: {
isConfidential: initialData.is_confidential,
isEditable: initialData.is_editable,
service: mediator.service,
},
}).$mount(el);
}
function mountLockComponent(mediator) {
const el = document.getElementById('js-lock-entry-point');
if (!el) return;
const dataNode = document.getElementById('js-lock-issue-data');
const initialData = JSON.parse(dataNode.innerHTML);
const LockComp = Vue.extend(LockIssueSidebar);
new LockComp({
propsData: {
isLocked: initialData.is_locked,
isEditable: initialData.is_editable,
mediator,
issuableType: gl.utils.isInIssuePage() ? 'issue' : 'merge_request',
},
}).$mount(el);
}
function domContentLoaded() { function domContentLoaded() {
const sidebarOptions = JSON.parse(document.querySelector('.js-sidebar-options').innerHTML); const sidebarOptions = JSON.parse(document.querySelector('.js-sidebar-options').innerHTML);
const mediator = new Mediator(sidebarOptions); const mediator = new Mediator(sidebarOptions);
mediator.fetch(); mediator.fetch();
const sidebarAssigneesEl = document.querySelector('#js-vue-sidebar-assignees'); const sidebarAssigneesEl = document.getElementById('js-vue-sidebar-assignees');
const confidentialEl = document.querySelector('#js-confidential-entry-point');
// Only create the sidebarAssignees vue app if it is found in the DOM // Only create the sidebarAssignees vue app if it is found in the DOM
// We currently do not use sidebarAssignees for the MR page // We currently do not use sidebarAssignees for the MR page
if (sidebarAssigneesEl) { if (sidebarAssigneesEl) {
new Vue(sidebarAssignees).$mount(sidebarAssigneesEl); new Vue(SidebarAssignees).$mount(sidebarAssigneesEl);
} }
if (confidentialEl) { mountConfidentialComponent(mediator);
const dataNode = document.getElementById('js-confidential-issue-data'); mountLockComponent(mediator);
const initialData = JSON.parse(dataNode.innerHTML);
const ConfidentialComp = Vue.extend(confidential); new SidebarMoveIssue(
mediator,
new ConfidentialComp({ $('.js-move-issue'),
propsData: { $('.js-move-issue-confirmation-button'),
isConfidential: initialData.is_confidential, ).init();
isEditable: initialData.is_editable,
service: mediator.service,
},
}).$mount(confidentialEl);
new SidebarMoveIssue(
mediator,
$('.js-move-issue'),
$('.js-move-issue-confirmation-button'),
).init();
}
new Vue(sidebarTimeTracking).$mount('#issuable-time-tracker'); new Vue(SidebarTimeTracking).$mount('#issuable-time-tracker');
} }
document.addEventListener('DOMContentLoaded', domContentLoaded); document.addEventListener('DOMContentLoaded', domContentLoaded);
......
...@@ -15,6 +15,7 @@ export default class SidebarStore { ...@@ -15,6 +15,7 @@ export default class SidebarStore {
}; };
this.autocompleteProjects = []; this.autocompleteProjects = [];
this.moveToProjectId = 0; this.moveToProjectId = 0;
this.isLockDialogOpen = false;
SidebarStore.singleton = this; SidebarStore.singleton = this;
} }
......
<script>
export default {
name: 'confidentialIssueWarning',
};
</script>
<template>
<div class="confidential-issue-warning">
<i
aria-hidden="true"
class="fa fa-eye-slash">
</i>
<span>
This is a confidential issue. Your comment will not be visible to the public.
</span>
</div>
</template>
<script>
export default {
props: {
isLocked: {
type: Boolean,
default: false,
required: false,
},
isConfidential: {
type: Boolean,
default: false,
required: false,
},
},
computed: {
iconClass() {
return {
'fa-eye-slash': this.isConfidential,
'fa-lock': this.isLocked,
};
},
isLockedAndConfidential() {
return this.isConfidential && this.isLocked;
},
},
};
</script>
<template>
<div class="issuable-note-warning">
<i
aria-hidden="true"
class="fa"
:class="iconClass"
v-if="!isLockedAndConfidential"
></i>
<span v-if="isLockedAndConfidential">
{{ __('This issue is confidential and locked.') }}
{{ __('People without permission will never get a notification and won\'t be able to comment.') }}
</span>
<span v-else-if="isConfidential">
{{ __('This is a confidential issue.') }}
{{ __('Your comment will not be visible to the public.') }}
</span>
<span v-else-if="isLocked">
{{ __('This issue is locked.') }}
{{ __('Only project members can comment.') }}
</span>
</div>
</template>
export default {
methods: {
issuableDisplayName(issuableType) {
const displayName = issuableType.replace(/_/, ' ');
return this.__ ? this.__(displayName) : displayName;
},
},
};
...@@ -385,7 +385,11 @@ ...@@ -385,7 +385,11 @@
background: transparent; background: transparent;
border: 0; border: 0;
&:hover,
&:active,
&:focus { &:focus {
outline: 0; outline: 0;
background: transparent;
box-shadow: none;
} }
} }
...@@ -720,3 +720,8 @@ Project Templates Icons ...@@ -720,3 +720,8 @@ Project Templates Icons
$rails: #c00; $rails: #c00;
$node: #353535; $node: #353535;
$java: #70ad51; $java: #70ad51;
/*
Issuable warning
*/
$issuable-warning-size: 24px;
...@@ -5,27 +5,25 @@ ...@@ -5,27 +5,25 @@
margin-right: auto; margin-right: auto;
} }
.is-confidential { .issuable-warning-icon {
color: $orange-600; color: $orange-600;
background-color: $orange-100; background-color: $orange-100;
border-radius: $border-radius-default; border-radius: $border-radius-default;
padding: 5px; padding: 5px;
margin: 0 3px 0 -4px; margin: 0 $btn-side-margin 0 0;
width: $issuable-warning-size;
height: $issuable-warning-size;
text-align: center;
} }
.is-not-confidential { .sidebar-item-icon {
border-radius: $border-radius-default; border-radius: $border-radius-default;
padding: 5px; padding: 5px;
margin: 0 3px 0 -4px; margin: 0 3px 0 -4px;
}
.confidentiality {
.is-not-confidential {
margin: auto;
}
.is-confidential { &.is-active {
margin: auto; color: $orange-600;
background-color: $orange-50;
} }
} }
......
...@@ -101,7 +101,7 @@ ...@@ -101,7 +101,7 @@
} }
} }
.confidential-issue-warning { .issuable-note-warning {
color: $orange-600; color: $orange-600;
background-color: $orange-100; background-color: $orange-100;
border-radius: $border-radius-default $border-radius-default 0 0; border-radius: $border-radius-default $border-radius-default 0 0;
...@@ -112,26 +112,46 @@ ...@@ -112,26 +112,46 @@
align-items: center; align-items: center;
} }
.confidential-value { .disabled-comment .issuable-note-warning {
border: none;
border-radius: $label-border-radius;
padding-top: $gl-vert-padding;
padding-bottom: $gl-vert-padding;
.icon svg {
position: relative;
top: 2px;
margin-right: $btn-xs-side-margin;
width: $gl-font-size;
height: $gl-font-size;
fill: $orange-600;
}
}
.sidebar-item-value {
.fa { .fa {
background-color: inherit; background-color: inherit;
} }
} }
.confidential-warning-message { .sidebar-item-warning-message {
line-height: 1.5; line-height: 1.5;
padding: 16px; padding: 16px;
.confidential-warning-message-actions { .text {
color: $text-color;
}
.sidebar-item-warning-message-actions {
display: flex; display: flex;
button { .btn {
flex-grow: 1; flex-grow: 1;
} }
} }
} }
.confidential-issue-warning + .md-area { .issuable-note-warning + .md-area {
border-top-left-radius: 0; border-top-left-radius: 0;
border-top-right-radius: 0; border-top-right-radius: 0;
} }
......
...@@ -703,6 +703,12 @@ ul.notes { ...@@ -703,6 +703,12 @@ ul.notes {
color: $note-disabled-comment-color; color: $note-disabled-comment-color;
padding: 90px 0; padding: 90px 0;
&.discussion-locked {
border: none;
background-color: $white-light;
}
a { a {
color: $gl-link-color; color: $gl-link-color;
} }
......
...@@ -280,6 +280,7 @@ class Projects::IssuesController < Projects::ApplicationController ...@@ -280,6 +280,7 @@ class Projects::IssuesController < Projects::ApplicationController
state_event state_event
task_num task_num
lock_version lock_version
discussion_locked
] + [{ label_ids: [], assignee_ids: [] }] ] + [{ label_ids: [], assignee_ids: [] }]
end end
......
...@@ -36,6 +36,7 @@ class Projects::MergeRequests::ApplicationController < Projects::ApplicationCont ...@@ -36,6 +36,7 @@ class Projects::MergeRequests::ApplicationController < Projects::ApplicationCont
:target_project_id, :target_project_id,
:task_num, :task_num,
:title, :title,
:discussion_locked,
label_ids: [] label_ids: []
] ]
......
...@@ -66,7 +66,16 @@ class Projects::NotesController < Projects::ApplicationController ...@@ -66,7 +66,16 @@ class Projects::NotesController < Projects::ApplicationController
params.merge(last_fetched_at: last_fetched_at) params.merge(last_fetched_at: last_fetched_at)
end end
def authorize_admin_note!
return access_denied! unless can?(current_user, :admin_note, note)
end
def authorize_resolve_note! def authorize_resolve_note!
return access_denied! unless can?(current_user, :resolve_note, note) return access_denied! unless can?(current_user, :resolve_note, note)
end end
def authorize_create_note!
return unless noteable.lockable?
access_denied! unless can?(current_user, :create_note, noteable)
end
end end
...@@ -130,8 +130,12 @@ module NotesHelper ...@@ -130,8 +130,12 @@ module NotesHelper
end end
def can_create_note? def can_create_note?
issuable = @issue || @merge_request
if @snippet.is_a?(PersonalSnippet) if @snippet.is_a?(PersonalSnippet)
can?(current_user, :comment_personal_snippet, @snippet) can?(current_user, :comment_personal_snippet, @snippet)
elsif issuable
can?(current_user, :create_note, issuable)
else else
can?(current_user, :create_note, @project) can?(current_user, :create_note, @project)
end end
......
...@@ -23,7 +23,9 @@ module SystemNoteHelper ...@@ -23,7 +23,9 @@ module SystemNoteHelper
'approved' => 'approval', 'approved' => 'approval',
'unapproved' => 'unapproval', 'unapproved' => 'unapproval',
'relate' => 'link', 'relate' => 'link',
'unrelate' => 'unlink' 'unrelate' => 'unlink',
'locked' => 'lock',
'unlocked' => 'lock-open'
}.freeze }.freeze
def system_note_icon_name(note) def system_note_icon_name(note)
......
...@@ -74,4 +74,8 @@ module Noteable ...@@ -74,4 +74,8 @@ module Noteable
def discussions_can_be_resolved_by?(user) def discussions_can_be_resolved_by?(user)
discussions_to_be_resolved.all? { |discussion| discussion.can_resolve?(user) } discussions_to_be_resolved.all? { |discussion| discussion.can_resolve?(user) }
end end
def lockable?
[MergeRequest, Issue].include?(self.class)
end
end end
...@@ -2,7 +2,7 @@ class SystemNoteMetadata < ActiveRecord::Base ...@@ -2,7 +2,7 @@ class SystemNoteMetadata < ActiveRecord::Base
ICON_TYPES = %w[ ICON_TYPES = %w[
commit description merge confidential visible label assignee cross_reference commit description merge confidential visible label assignee cross_reference
title time_tracking branch milestone discussion task moved title time_tracking branch milestone discussion task moved
opened closed merged duplicate opened closed merged duplicate locked unlocked
outdated outdated
approved unapproved relate unrelate approved unapproved relate unrelate
].freeze ].freeze
......
class IssuablePolicy < BasePolicy class IssuablePolicy < BasePolicy
delegate { @subject.project } delegate { @subject.project }
condition(:locked, scope: :subject, score: 0) { @subject.discussion_locked? }
condition(:is_project_member) { @user && @subject.project && @subject.project.team.member?(@user) }
desc "User is the assignee or author" desc "User is the assignee or author"
condition(:assignee_or_author) do condition(:assignee_or_author) do
@user && @subject.assignee_or_author?(@user) @user && @subject.assignee_or_author?(@user)
...@@ -12,4 +16,12 @@ class IssuablePolicy < BasePolicy ...@@ -12,4 +16,12 @@ class IssuablePolicy < BasePolicy
enable :read_merge_request enable :read_merge_request
enable :update_merge_request enable :update_merge_request
end end
rule { locked & ~is_project_member }.policy do
prevent :create_note
prevent :update_note
prevent :admin_note
prevent :resolve_note
prevent :edit_note
end
end end
class NotePolicy < BasePolicy class NotePolicy < BasePolicy
delegate { @subject.project } delegate { @subject.project }
delegate { @subject.noteable if @subject.noteable.lockable? }
condition(:is_author) { @user && @subject.author == @user } condition(:is_author) { @user && @subject.author == @user }
condition(:for_merge_request, scope: :subject) { @subject.for_merge_request? } condition(:for_merge_request, scope: :subject) { @subject.for_merge_request? }
...@@ -8,6 +9,7 @@ class NotePolicy < BasePolicy ...@@ -8,6 +9,7 @@ class NotePolicy < BasePolicy
condition(:editable, scope: :subject) { @subject.editable? } condition(:editable, scope: :subject) { @subject.editable? }
rule { ~editable | anonymous }.prevent :edit_note rule { ~editable | anonymous }.prevent :edit_note
rule { is_author | admin }.enable :edit_note rule { is_author | admin }.enable :edit_note
rule { can?(:master_access) }.enable :edit_note rule { can?(:master_access) }.enable :edit_note
......
...@@ -3,6 +3,7 @@ class IssueEntity < IssuableEntity ...@@ -3,6 +3,7 @@ class IssueEntity < IssuableEntity
expose :branch_name expose :branch_name
expose :confidential expose :confidential
expose :discussion_locked
expose :assignees, using: API::Entities::UserBasic expose :assignees, using: API::Entities::UserBasic
expose :due_date expose :due_date
expose :moved_to_id expose :moved_to_id
...@@ -17,7 +18,7 @@ class IssueEntity < IssuableEntity ...@@ -17,7 +18,7 @@ class IssueEntity < IssuableEntity
expose :current_user do expose :current_user do
expose :can_create_note do |issue| expose :can_create_note do |issue|
can?(request.current_user, :create_note, issue.project) can?(request.current_user, :create_note, issue)
end end
expose :can_update do |issue| expose :can_update do |issue|
......
...@@ -45,6 +45,10 @@ class IssuableBaseService < BaseService ...@@ -45,6 +45,10 @@ class IssuableBaseService < BaseService
SystemNoteService.change_time_spent(issuable, issuable.project, issuable.time_spent_user) SystemNoteService.change_time_spent(issuable, issuable.project, issuable.time_spent_user)
end end
def create_discussion_lock_note(issuable)
SystemNoteService.discussion_lock(issuable, current_user)
end
def filter_params(issuable) def filter_params(issuable)
ability_name = :"admin_#{issuable.to_ability_name}" ability_name = :"admin_#{issuable.to_ability_name}"
...@@ -59,6 +63,7 @@ class IssuableBaseService < BaseService ...@@ -59,6 +63,7 @@ class IssuableBaseService < BaseService
params.delete(:due_date) params.delete(:due_date)
params.delete(:canonical_issue_id) params.delete(:canonical_issue_id)
params.delete(:project) params.delete(:project)
params.delete(:discussion_locked)
end end
filter_assignee(issuable) filter_assignee(issuable)
...@@ -238,6 +243,7 @@ class IssuableBaseService < BaseService ...@@ -238,6 +243,7 @@ class IssuableBaseService < BaseService
handle_common_system_notes(issuable, old_labels: old_labels) handle_common_system_notes(issuable, old_labels: old_labels)
end end
change_discussion_lock(issuable)
handle_changes( handle_changes(
issuable, issuable,
old_labels: old_labels, old_labels: old_labels,
...@@ -296,6 +302,12 @@ class IssuableBaseService < BaseService ...@@ -296,6 +302,12 @@ class IssuableBaseService < BaseService
end end
end end
def change_discussion_lock(issuable)
if issuable.previous_changes.include?('discussion_locked')
create_discussion_lock_note(issuable)
end
end
def toggle_award(issuable) def toggle_award(issuable)
award = params.delete(:emoji_award) award = params.delete(:emoji_award)
if award if award
......
...@@ -645,6 +645,13 @@ module SystemNoteService ...@@ -645,6 +645,13 @@ module SystemNoteService
create_note(NoteSummary.new(noteable, project, author, body, action: 'duplicate')) create_note(NoteSummary.new(noteable, project, author, body, action: 'duplicate'))
end end
def discussion_lock(issuable, author)
action = issuable.discussion_locked? ? 'locked' : 'unlocked'
body = "#{action} this issue"
create_note(NoteSummary.new(issuable, issuable.project, author, body, action: action))
end
private private
def notes_for_mentioner(mentioner, noteable, notes) def notes_for_mentioner(mentioner, noteable, notes)
......
- referenced_users = local_assigns.fetch(:referenced_users, nil) - referenced_users = local_assigns.fetch(:referenced_users, nil)
- if defined?(@merge_request) && @merge_request.discussion_locked?
.issuable-note-warning
= icon('lock')
%span
= _('This merge request is locked.')
= _('Only project members can comment.')
.md-area .md-area
.md-header .md-header
%ul.nav-links.clearfix %ul.nav-links.clearfix
......
...@@ -30,7 +30,9 @@ ...@@ -30,7 +30,9 @@
.issuable-meta .issuable-meta
- if @issue.confidential - if @issue.confidential
= icon('eye-slash', class: 'is-confidential') = icon('eye-slash', class: 'issuable-warning-icon')
- if @issue.discussion_locked?
= icon('lock', class: 'issuable-warning-icon')
= issuable_meta(@issue, @project, "Issue") = issuable_meta(@issue, @project, "Issue")
.issuable-actions.js-issuable-actions .issuable-actions.js-issuable-actions
......
...@@ -15,6 +15,8 @@ ...@@ -15,6 +15,8 @@
= icon('angle-double-left') = icon('angle-double-left')
.issuable-meta .issuable-meta
- if @merge_request.discussion_locked?
= icon('lock', class: 'issuable-warning-icon')
= issuable_meta(@merge_request, @project, "Merge request") = issuable_meta(@merge_request, @project, "Merge request")
.issuable-actions.js-issuable-actions .issuable-actions.js-issuable-actions
......
...@@ -91,7 +91,7 @@ ...@@ -91,7 +91,7 @@
#pipelines.pipelines.tab-pane #pipelines.pipelines.tab-pane
- if @pipelines.any? - if @pipelines.any?
= render 'projects/commit/pipelines_list', disable_initialization: true, endpoint: pipelines_project_merge_request_path(@project, @merge_request) = render 'projects/commit/pipelines_list', disable_initialization: true, endpoint: pipelines_project_merge_request_path(@project, @merge_request)
#diffs.diffs.tab-pane #diffs.diffs.tab-pane{ data: { "is-locked" => @merge_request.discussion_locked? } }
-# This tab is always loaded via AJAX -# This tab is always loaded via AJAX
.mr-loading-status .mr-loading-status
......
...@@ -147,6 +147,10 @@ ...@@ -147,6 +147,10 @@
%script#js-confidential-issue-data{ type: "application/json" }= { is_confidential: @issue.confidential, is_editable: can_edit_issuable }.to_json.html_safe %script#js-confidential-issue-data{ type: "application/json" }= { is_confidential: @issue.confidential, is_editable: can_edit_issuable }.to_json.html_safe
#js-confidential-entry-point #js-confidential-entry-point
- if issuable.has_attribute?(:discussion_locked)
%script#js-lock-issue-data{ type: "application/json" }= { is_locked: issuable.discussion_locked?, is_editable: can_edit_issuable }.to_json.html_safe
#js-lock-entry-point
= render "shared/issuable/participants", participants: issuable.participants(current_user) = render "shared/issuable/participants", participants: issuable.participants(current_user)
- if current_user - if current_user
- subscribed = issuable.subscribed?(current_user, @project) - subscribed = issuable.subscribed?(current_user, @project)
......
- issuable = @issue || @merge_request
- discussion_locked = issuable&.discussion_locked?
%ul#notes-list.notes.main-notes-list.timeline %ul#notes-list.notes.main-notes-list.timeline
= render "shared/notes/notes" = render "shared/notes/notes"
...@@ -21,5 +24,14 @@ ...@@ -21,5 +24,14 @@
or or
= link_to "sign in", new_session_path(:user, redirect_to_referer: 'yes'), class: 'js-sign-in-link' = link_to "sign in", new_session_path(:user, redirect_to_referer: 'yes'), class: 'js-sign-in-link'
to comment to comment
- elsif discussion_locked
.disabled-comment.text-center.prepend-top-default
%span.issuable-note-warning
%span.icon= sprite_icon('lock', size: 14)
%span
This
= issuable.class.to_s.titleize.downcase
is locked. Only
%b project members
can comment.
%script.js-notes-data{ type: "application/json" }= initial_notes_data(autocomplete).to_json.html_safe %script.js-notes-data{ type: "application/json" }= initial_notes_data(autocomplete).to_json.html_safe
title: Discussion lock for issues and merge requests
merge_request:
author:
type: added
class AddDiscussionLockedToIssuable < ActiveRecord::Migration
DOWNTIME = false
def up
add_column(:merge_requests, :discussion_locked, :boolean)
add_column(:issues, :discussion_locked, :boolean)
end
def down
remove_column(:merge_requests, :discussion_locked)
remove_column(:issues, :discussion_locked)
end
end
...@@ -867,6 +867,7 @@ ActiveRecord::Schema.define(version: 20171002105019) do ...@@ -867,6 +867,7 @@ ActiveRecord::Schema.define(version: 20171002105019) do
t.integer "cached_markdown_version" t.integer "cached_markdown_version"
t.datetime "last_edited_at" t.datetime "last_edited_at"
t.integer "last_edited_by_id" t.integer "last_edited_by_id"
t.boolean "discussion_locked"
end end
add_index "issues", ["assignee_id"], name: "index_issues_on_assignee_id", using: :btree add_index "issues", ["assignee_id"], name: "index_issues_on_assignee_id", using: :btree
...@@ -1111,6 +1112,7 @@ ActiveRecord::Schema.define(version: 20171002105019) do ...@@ -1111,6 +1112,7 @@ ActiveRecord::Schema.define(version: 20171002105019) do
t.integer "head_pipeline_id" t.integer "head_pipeline_id"
t.boolean "ref_fetched" t.boolean "ref_fetched"
t.string "merge_jid" t.string "merge_jid"
t.boolean "discussion_locked"
end end
add_index "merge_requests", ["assignee_id"], name: "index_merge_requests_on_assignee_id", using: :btree add_index "merge_requests", ["assignee_id"], name: "index_merge_requests_on_assignee_id", using: :btree
......
...@@ -112,7 +112,8 @@ Example response: ...@@ -112,7 +112,8 @@ Example response:
"human_time_estimate": null, "human_time_estimate": null,
"human_total_time_spent": null "human_total_time_spent": null
}, },
"confidential": false "confidential": false,
"discussion_locked": false
} }
] ]
``` ```
...@@ -220,7 +221,8 @@ Example response: ...@@ -220,7 +221,8 @@ Example response:
"human_time_estimate": null, "human_time_estimate": null,
"human_total_time_spent": null "human_total_time_spent": null
}, },
"confidential": false "confidential": false,
"discussion_locked": false
} }
] ]
``` ```
...@@ -329,7 +331,8 @@ Example response: ...@@ -329,7 +331,8 @@ Example response:
"human_time_estimate": null, "human_time_estimate": null,
"human_total_time_spent": null "human_total_time_spent": null
}, },
"confidential": false "confidential": false,
"discussion_locked": false
} }
] ]
``` ```
...@@ -414,6 +417,7 @@ Example response: ...@@ -414,6 +417,7 @@ Example response:
}, },
"confidential": false, "confidential": false,
"weight": null, "weight": null,
"discussion_locked": false,
"_links": { "_links": {
"self": "http://example.com/api/v4/projects/1/issues/2", "self": "http://example.com/api/v4/projects/1/issues/2",
"notes": "http://example.com/api/v4/projects/1/issues/2/notes", "notes": "http://example.com/api/v4/projects/1/issues/2/notes",
...@@ -491,6 +495,7 @@ Example response: ...@@ -491,6 +495,7 @@ Example response:
}, },
"confidential": false, "confidential": false,
"weight": null, "weight": null,
"discussion_locked": false,
"_links": { "_links": {
"self": "http://example.com/api/v4/projects/1/issues/2", "self": "http://example.com/api/v4/projects/1/issues/2",
"notes": "http://example.com/api/v4/projects/1/issues/2/notes", "notes": "http://example.com/api/v4/projects/1/issues/2/notes",
...@@ -525,6 +530,7 @@ PUT /projects/:id/issues/:issue_iid ...@@ -525,6 +530,7 @@ PUT /projects/:id/issues/:issue_iid
| `updated_at` | string | no | Date time string, ISO 8601 formatted, e.g. `2016-03-11T03:45:40Z` (requires admin or project owner rights) | | `updated_at` | string | no | Date time string, ISO 8601 formatted, e.g. `2016-03-11T03:45:40Z` (requires admin or project owner rights) |
| `due_date` | string | no | Date time string in the format YEAR-MONTH-DAY, e.g. `2016-03-11` | | `due_date` | string | no | Date time string in the format YEAR-MONTH-DAY, e.g. `2016-03-11` |
| `weight` | integer | no | The weight of the issue in range 0 to 9 | | `weight` | integer | no | The weight of the issue in range 0 to 9 |
| `discussion_locked` | boolean | no | Flag indicating if the issue's discussion is locked. If the discussion is locked only project members can add or edit comments. |
```bash ```bash
curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/4/issues/85?state_event=close curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/4/issues/85?state_event=close
...@@ -569,6 +575,7 @@ Example response: ...@@ -569,6 +575,7 @@ Example response:
}, },
"confidential": false, "confidential": false,
"weight": null, "weight": null,
"discussion_locked": false,
"_links": { "_links": {
"self": "http://example.com/api/v4/projects/1/issues/2", "self": "http://example.com/api/v4/projects/1/issues/2",
"notes": "http://example.com/api/v4/projects/1/issues/2/notes", "notes": "http://example.com/api/v4/projects/1/issues/2/notes",
...@@ -669,6 +676,7 @@ Example response: ...@@ -669,6 +676,7 @@ Example response:
}, },
"confidential": false, "confidential": false,
"weight": null, "weight": null,
"discussion_locked": false,
"_links": { "_links": {
"self": "http://example.com/api/v4/projects/1/issues/2", "self": "http://example.com/api/v4/projects/1/issues/2",
"notes": "http://example.com/api/v4/projects/1/issues/2/notes", "notes": "http://example.com/api/v4/projects/1/issues/2/notes",
...@@ -748,6 +756,7 @@ Example response: ...@@ -748,6 +756,7 @@ Example response:
}, },
"confidential": false, "confidential": false,
"weight": null, "weight": null,
"discussion_locked": false,
"_links": { "_links": {
"self": "http://example.com/api/v4/projects/1/issues/2", "self": "http://example.com/api/v4/projects/1/issues/2",
"notes": "http://example.com/api/v4/projects/1/issues/2/notes", "notes": "http://example.com/api/v4/projects/1/issues/2/notes",
...@@ -778,6 +787,44 @@ POST /projects/:id/issues/:issue_iid/unsubscribe ...@@ -778,6 +787,44 @@ POST /projects/:id/issues/:issue_iid/unsubscribe
curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/issues/93/unsubscribe curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/issues/93/unsubscribe
``` ```
Example response:
```json
{
"id": 93,
"iid": 12,
"project_id": 5,
"title": "Incidunt et rerum ea expedita iure quibusdam.",
"description": "Et cumque architecto sed aut ipsam.",
"state": "opened",
"created_at": "2016-04-05T21:41:45.217Z",
"updated_at": "2016-04-07T13:02:37.905Z",
"labels": [],
"milestone": null,
"assignee": {
"name": "Edwardo Grady",
"username": "keyon",
"id": 21,
"state": "active",
"avatar_url": "http://www.gravatar.com/avatar/3e6f06a86cf27fa8b56f3f74f7615987?s=80&d=identicon",
"web_url": "https://gitlab.example.com/keyon"
},
"author": {
"name": "Vivian Hermann",
"username": "orville",
"id": 11,
"state": "active",
"avatar_url": "http://www.gravatar.com/avatar/5224fd70153710e92fb8bcf79ac29d67?s=80&d=identicon",
"web_url": "https://gitlab.example.com/orville"
},
"subscribed": false,
"due_date": null,
"web_url": "http://example.com/example/example/issues/12",
"confidential": false,
"discussion_locked": false
}
```
## Create a todo ## Create a todo
Manually creates a todo for the current user on an issue. If Manually creates a todo for the current user on an issue. If
...@@ -872,6 +919,7 @@ Example response: ...@@ -872,6 +919,7 @@ Example response:
"web_url": "http://example.com/example/example/issues/110", "web_url": "http://example.com/example/example/issues/110",
"confidential": false, "confidential": false,
"weight": null "weight": null
"discussion_locked": false
}, },
"target_url": "https://gitlab.example.com/gitlab-org/gitlab-ci/issues/10", "target_url": "https://gitlab.example.com/gitlab-org/gitlab-ci/issues/10",
"body": "Vel voluptas atque dicta mollitia adipisci qui at.", "body": "Vel voluptas atque dicta mollitia adipisci qui at.",
......
...@@ -194,6 +194,7 @@ Parameters: ...@@ -194,6 +194,7 @@ Parameters:
"force_remove_source_branch": false, "force_remove_source_branch": false,
"squash": false, "squash": false,
"web_url": "http://example.com/example/example/merge_requests/1", "web_url": "http://example.com/example/example/merge_requests/1",
"discussion_locked": false,
"time_stats": { "time_stats": {
"time_estimate": 0, "time_estimate": 0,
"total_time_spent": 0, "total_time_spent": 0,
...@@ -271,6 +272,7 @@ Parameters: ...@@ -271,6 +272,7 @@ Parameters:
"force_remove_source_branch": false, "force_remove_source_branch": false,
"squash": false, "squash": false,
"web_url": "http://example.com/example/example/merge_requests/1", "web_url": "http://example.com/example/example/merge_requests/1",
"discussion_locked": false,
"time_stats": { "time_stats": {
"time_estimate": 0, "time_estimate": 0,
"total_time_spent": 0, "total_time_spent": 0,
...@@ -384,6 +386,7 @@ Parameters: ...@@ -384,6 +386,7 @@ Parameters:
"force_remove_source_branch": false, "force_remove_source_branch": false,
"squash": false, "squash": false,
"web_url": "http://example.com/example/example/merge_requests/1", "web_url": "http://example.com/example/example/merge_requests/1",
"discussion_locked": false,
"time_stats": { "time_stats": {
"time_estimate": 0, "time_estimate": 0,
"total_time_spent": 0, "total_time_spent": 0,
...@@ -490,6 +493,7 @@ order for it to take effect: ...@@ -490,6 +493,7 @@ order for it to take effect:
"force_remove_source_branch": false, "force_remove_source_branch": false,
"squash": false, "squash": false,
"web_url": "http://example.com/example/example/merge_requests/1", "web_url": "http://example.com/example/example/merge_requests/1",
"discussion_locked": false,
"time_stats": { "time_stats": {
"time_estimate": 0, "time_estimate": 0,
"total_time_spent": 0, "total_time_spent": 0,
...@@ -520,8 +524,7 @@ PUT /projects/:id/merge_requests/:merge_request_iid ...@@ -520,8 +524,7 @@ PUT /projects/:id/merge_requests/:merge_request_iid
| `milestone_id` | integer | no | The ID of a milestone | | `milestone_id` | integer | no | The ID of a milestone |
| `remove_source_branch` | boolean | no | Flag indicating if a merge request should remove the source branch when merging | | `remove_source_branch` | boolean | no | Flag indicating if a merge request should remove the source branch when merging |
| `squash` | boolean| no | Squash commits into a single commit when merging | | `squash` | boolean| no | Squash commits into a single commit when merging |
| `discussion_locked` | boolean | no | Flag indicating if the merge request's discussion is locked. If the discussion is locked only project members can add, edit or resolve comments. |
Must include at least one non-required attribute from above.
Must include at least one non-required attribute from above. Must include at least one non-required attribute from above.
...@@ -578,6 +581,7 @@ Must include at least one non-required attribute from above. ...@@ -578,6 +581,7 @@ Must include at least one non-required attribute from above.
"force_remove_source_branch": false, "force_remove_source_branch": false,
"squash": false, "squash": false,
"web_url": "http://example.com/example/example/merge_requests/1", "web_url": "http://example.com/example/example/merge_requests/1",
"discussion_locked": false,
"time_stats": { "time_stats": {
"time_estimate": 0, "time_estimate": 0,
"total_time_spent": 0, "total_time_spent": 0,
...@@ -684,6 +688,7 @@ Parameters: ...@@ -684,6 +688,7 @@ Parameters:
"force_remove_source_branch": false, "force_remove_source_branch": false,
"squash": false, "squash": false,
"web_url": "http://example.com/example/example/merge_requests/1", "web_url": "http://example.com/example/example/merge_requests/1",
"discussion_locked": false,
"time_stats": { "time_stats": {
"time_estimate": 0, "time_estimate": 0,
"total_time_spent": 0, "total_time_spent": 0,
...@@ -888,6 +893,7 @@ Parameters: ...@@ -888,6 +893,7 @@ Parameters:
"force_remove_source_branch": false, "force_remove_source_branch": false,
"squash": false, "squash": false,
"web_url": "http://example.com/example/example/merge_requests/1", "web_url": "http://example.com/example/example/merge_requests/1",
"discussion_locked": false,
"time_stats": { "time_stats": {
"time_estimate": 0, "time_estimate": 0,
"total_time_spent": 0, "total_time_spent": 0,
...@@ -1183,7 +1189,8 @@ Example response: ...@@ -1183,7 +1189,8 @@ Example response:
"id": 14, "id": 14,
"state": "active", "state": "active",
"avatar_url": "http://www.gravatar.com/avatar/a7fa515d53450023c83d62986d0658a8?s=80&d=identicon", "avatar_url": "http://www.gravatar.com/avatar/a7fa515d53450023c83d62986d0658a8?s=80&d=identicon",
"web_url": "https://gitlab.example.com/francisca" "web_url": "https://gitlab.example.com/francisca",
"discussion_locked": false
}, },
"assignee": { "assignee": {
"name": "Dr. Gabrielle Strosin", "name": "Dr. Gabrielle Strosin",
......
...@@ -153,12 +153,52 @@ comments in greater detail. ...@@ -153,12 +153,52 @@ comments in greater detail.
![Discussion comment](img/discussion_comment.png) ![Discussion comment](img/discussion_comment.png)
## Locking discussions
> [Introduced][ce-14531] in GitLab 10.1.
There might be some cases where a discussion is better off if it's locked down.
For example:
- Discussions that are several years old and the issue/merge request is closed,
but people continue to try to resurrect the discussion.
- Discussions where someone or a group of people are trolling, are abusive, or
in-general are causing the discussion to be unproductive.
In locked discussions, only team members can write new comments and edit the old
ones.
To lock or unlock a discussion, you need to have at least Master [permissions]:
1. Find the "Lock" section in the sidebar and click **Edit**
1. In the dialog that will appear, you can choose to turn on or turn off the
discussion lock
1. Optionally, leave a comment to explain your reasoning behind that action
| Turn off discussion lock | Turn on discussion lock |
| :-----------: | :----------: |
| ![Turn off discussion lock](img/turn_off_lock.png) | ![Turn on discussion lock](img/turn_on_lock.png) |
Every change is indicated by a system note in the issue's or merge request's
comments.
![Discussion lock system notes](img/discussion_lock_system_notes.png)
Once an issue or merge request is locked, project members can see the indicator
in the comment area, whereas non project members can only see the information
that the discussion is locked.
| Team member | Not a member |
| :-----------: | :----------: |
| ![Comment form member](img/lock_form_member.png) | ![Comment form non-member](img/lock_form_non_member.png) |
[ce-5022]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5022 [ce-5022]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5022
[ce-7125]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7125 [ce-7125]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7125
[ce-7527]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7527 [ce-7527]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7527
[ce-7180]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7180 [ce-7180]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7180
[ce-8266]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/8266 [ce-8266]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/8266
[ce-14053]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14053 [ce-14053]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14053
[ce-14531]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14531
[resolve-discussion-button]: img/resolve_discussion_button.png [resolve-discussion-button]: img/resolve_discussion_button.png
[resolve-comment-button]: img/resolve_comment_button.png [resolve-comment-button]: img/resolve_comment_button.png
[discussion-view]: img/discussion_view.png [discussion-view]: img/discussion_view.png
......
...@@ -26,6 +26,7 @@ The following table depicts the various user permission levels in a project. ...@@ -26,6 +26,7 @@ The following table depicts the various user permission levels in a project.
| View confidential issues | (✓) [^2] | ✓ | ✓ | ✓ | ✓ | | View confidential issues | (✓) [^2] | ✓ | ✓ | ✓ | ✓ |
| Leave comments | ✓ [^1] | ✓ | ✓ | ✓ | ✓ | | Leave comments | ✓ [^1] | ✓ | ✓ | ✓ | ✓ |
| See related issues | ✓ | ✓ | ✓ | ✓ | ✓ | | See related issues | ✓ | ✓ | ✓ | ✓ | ✓ |
| Lock comments | | | | ✓ | ✓ |
| See a list of jobs | ✓ [^3] | ✓ | ✓ | ✓ | ✓ | | See a list of jobs | ✓ [^3] | ✓ | ✓ | ✓ | ✓ |
| See a job log | ✓ [^3] | ✓ | ✓ | ✓ | ✓ | | See a job log | ✓ [^3] | ✓ | ✓ | ✓ | ✓ |
| Download and browse job artifacts | ✓ [^3] | ✓ | ✓ | ✓ | ✓ | | Download and browse job artifacts | ✓ [^3] | ✓ | ✓ | ✓ | ✓ |
......
...@@ -398,6 +398,7 @@ module API ...@@ -398,6 +398,7 @@ module API
expose :due_date expose :due_date
expose :confidential expose :confidential
expose :weight, if: ->(issue, _) { issue.supports_weight? } expose :weight, if: ->(issue, _) { issue.supports_weight? }
expose :discussion_locked
expose :web_url do |issue, options| expose :web_url do |issue, options|
Gitlab::UrlBuilder.build(issue) Gitlab::UrlBuilder.build(issue)
...@@ -504,6 +505,7 @@ module API ...@@ -504,6 +505,7 @@ module API
expose :merge_commit_sha expose :merge_commit_sha
expose :user_notes_count expose :user_notes_count
expose :approvals_before_merge expose :approvals_before_merge
expose :discussion_locked
expose :should_remove_source_branch?, as: :should_remove_source_branch expose :should_remove_source_branch?, as: :should_remove_source_branch
expose :force_remove_source_branch?, as: :force_remove_source_branch expose :force_remove_source_branch?, as: :force_remove_source_branch
......
...@@ -48,6 +48,7 @@ module API ...@@ -48,6 +48,7 @@ module API
optional :labels, type: String, desc: 'Comma-separated list of label names' optional :labels, type: String, desc: 'Comma-separated list of label names'
optional :due_date, type: String, desc: 'Date string in the format YEAR-MONTH-DAY' optional :due_date, type: String, desc: 'Date string in the format YEAR-MONTH-DAY'
optional :confidential, type: Boolean, desc: 'Boolean parameter if the issue should be confidential' optional :confidential, type: Boolean, desc: 'Boolean parameter if the issue should be confidential'
optional :discussion_locked, type: Boolean, desc: " Boolean parameter indicating if the issue's discussion is locked"
end end
params :issue_params_ee do params :issue_params_ee do
...@@ -200,7 +201,7 @@ module API ...@@ -200,7 +201,7 @@ module API
use :issue_params use :issue_params
at_least_one_of :title, :description, :assignee_ids, :assignee_id, :milestone_id, at_least_one_of :title, :description, :assignee_ids, :assignee_id, :milestone_id,
:labels, :created_at, :due_date, :confidential, :state_event, :labels, :created_at, :due_date, :confidential, :state_event,
:weight :weight, :discussion_locked
end end
put ':id/issues/:issue_iid' do put ':id/issues/:issue_iid' do
issue = user_project.issues.find_by!(iid: params.delete(:issue_iid)) issue = user_project.issues.find_by!(iid: params.delete(:issue_iid))
......
...@@ -226,12 +226,14 @@ module API ...@@ -226,12 +226,14 @@ module API
:remove_source_branch, :remove_source_branch,
:state_event, :state_event,
:target_branch, :target_branch,
:title :title,
:discussion_locked
] ]
optional :title, type: String, allow_blank: false, desc: 'The title of the merge request' optional :title, type: String, allow_blank: false, desc: 'The title of the merge request'
optional :target_branch, type: String, allow_blank: false, desc: 'The target branch' optional :target_branch, type: String, allow_blank: false, desc: 'The target branch'
optional :state_event, type: String, values: %w[close reopen], optional :state_event, type: String, values: %w[close reopen],
desc: 'Status of the merge request' desc: 'Status of the merge request'
optional :discussion_locked, type: Boolean, desc: 'Whether the MR discussion is locked'
# EE # EE
at_least_one_of_ee = [ at_least_one_of_ee = [
......
...@@ -78,6 +78,8 @@ module API ...@@ -78,6 +78,8 @@ module API
} }
if can?(current_user, noteable_read_ability_name(noteable), noteable) if can?(current_user, noteable_read_ability_name(noteable), noteable)
authorize! :create_note, noteable
if params[:created_at] && (current_user.admin? || user_project.owner == current_user) if params[:created_at] && (current_user.admin? || user_project.owner == current_user)
opts[:created_at] = params[:created_at] opts[:created_at] = params[:created_at]
end end
......
...@@ -232,6 +232,56 @@ describe Projects::NotesController do ...@@ -232,6 +232,56 @@ describe Projects::NotesController do
end end
end end
end end
context 'when the merge request discussion is locked' do
before do
project.update_attribute(:visibility_level, Gitlab::VisibilityLevel::PUBLIC)
merge_request.update_attribute(:discussion_locked, true)
end
context 'when a noteable is not found' do
it 'returns 404 status' do
request_params[:note][:noteable_id] = 9999
post :create, request_params.merge(format: :json)
expect(response).to have_http_status(404)
end
end
context 'when a user is a team member' do
it 'returns 302 status for html' do
post :create, request_params
expect(response).to have_http_status(302)
end
it 'returns 200 status for json' do
post :create, request_params.merge(format: :json)
expect(response).to have_http_status(200)
end
it 'creates a new note' do
expect { post :create, request_params }.to change { Note.count }.by(1)
end
end
context 'when a user is not a team member' do
before do
project.project_member(user).destroy
end
it 'returns 404 status' do
post :create, request_params
expect(response).to have_http_status(404)
end
it 'does not create a new note' do
expect { post :create, request_params }.not_to change { Note.count }
end
end
end
end end
describe 'DELETE destroy' do describe 'DELETE destroy' do
......
require 'spec_helper'
describe 'Discussion Lock', :js do
let(:user) { create(:user) }
let(:issue) { create(:issue, project: project, author: user) }
let(:project) { create(:project, :public) }
before do
sign_in(user)
end
context 'when a user is a team member' do
before do
project.add_developer(user)
end
context 'when the discussion is unlocked' do
it 'the user can lock the issue' do
visit project_issue_path(project, issue)
expect(find('.issuable-sidebar')).to have_content('Unlocked')
page.within('.issuable-sidebar') do
find('.lock-edit').click
click_button('Lock')
end
expect(find('#notes')).to have_content('locked this issue')
end
end
context 'when the discussion is locked' do
before do
issue.update_attribute(:discussion_locked, true)
visit project_issue_path(project, issue)
end
it 'the user can unlock the issue' do
expect(find('.issuable-sidebar')).to have_content('Locked')
page.within('.issuable-sidebar') do
find('.lock-edit').click
click_button('Unlock')
end
expect(find('#notes')).to have_content('unlocked this issue')
expect(find('.issuable-sidebar')).to have_content('Unlocked')
end
it 'the user can create a comment' do
page.within('#notes .js-main-target-form') do
fill_in 'note[note]', with: 'Some new comment'
click_button 'Comment'
end
wait_for_requests
expect(find('div#notes')).to have_content('Some new comment')
end
end
end
context 'when a user is not a team member' do
context 'when the discussion is unlocked' do
before do
visit project_issue_path(project, issue)
end
it 'the user can not lock the issue' do
expect(find('.issuable-sidebar')).to have_content('Unlocked')
expect(find('.issuable-sidebar')).not_to have_selector('.lock-edit')
end
it 'the user can create a comment' do
page.within('#notes .js-main-target-form') do
fill_in 'note[note]', with: 'Some new comment'
click_button 'Comment'
end
wait_for_requests
expect(find('div#notes')).to have_content('Some new comment')
end
end
context 'when the discussion is locked' do
before do
issue.update_attribute(:discussion_locked, true)
visit project_issue_path(project, issue)
end
it 'the user can not unlock the issue' do
expect(find('.issuable-sidebar')).to have_content('Locked')
expect(find('.issuable-sidebar')).not_to have_selector('.lock-edit')
end
it 'the user can not create a comment' do
page.within('#notes') do
expect(page).not_to have_selector('js-main-target-form')
expect(page.find('.disabled-comment'))
.to have_content('This issue is locked. Only project members can comment.')
end
end
end
end
end
...@@ -645,14 +645,14 @@ describe 'Issues', :js do ...@@ -645,14 +645,14 @@ describe 'Issues', :js do
visit project_issue_path(project, issue) visit project_issue_path(project, issue)
expect(page).to have_css('.confidential-issue-warning') expect(page).to have_css('.issuable-note-warning')
expect(page).to have_css('.is-confidential') expect(find('.issuable-sidebar-item.confidentiality')).to have_css('.is-active')
expect(page).not_to have_css('.is-not-confidential') expect(find('.issuable-sidebar-item.confidentiality')).not_to have_css('.not-active')
find('.confidential-edit').click find('.confidential-edit').click
expect(page).to have_css('.confidential-warning-message') expect(page).to have_css('.sidebar-item-warning-message')
within('.confidential-warning-message') do within('.sidebar-item-warning-message') do
find('.btn-close').click find('.btn-close').click
end end
...@@ -660,7 +660,7 @@ describe 'Issues', :js do ...@@ -660,7 +660,7 @@ describe 'Issues', :js do
visit project_issue_path(project, issue) visit project_issue_path(project, issue)
expect(page).not_to have_css('.is-confidential') expect(page).not_to have_css('.is-active')
end end
end end
end end
require 'spec_helper'
describe 'Discussion Lock', :js do
let(:user) { create(:user) }
let(:merge_request) { create(:merge_request, source_project: project, author: user) }
let(:project) { create(:project, :public, :repository) }
before do
sign_in(user)
end
context 'when the discussion is locked' do
before do
merge_request.update_attribute(:discussion_locked, true)
end
context 'when a user is a team member' do
before do
project.add_developer(user)
visit project_merge_request_path(project, merge_request)
end
it 'the user can create a comment' do
page.within('.issuable-discussion #notes .js-main-target-form') do
fill_in 'note[note]', with: 'Some new comment'
click_button 'Comment'
end
wait_for_requests
expect(find('.issuable-discussion #notes')).to have_content('Some new comment')
end
end
context 'when a user is not a team member' do
before do
visit project_merge_request_path(project, merge_request)
end
it 'the user can not create a comment' do
page.within('.issuable-discussion #notes') do
expect(page).not_to have_selector('js-main-target-form')
expect(page.find('.disabled-comment'))
.to have_content('This merge request is locked. Only project members can comment.')
end
end
end
end
end
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
"title": { "type": "string" }, "title": { "type": "string" },
"description": { "type": ["string", "null"] }, "description": { "type": ["string", "null"] },
"state": { "type": "string" }, "state": { "type": "string" },
"discussion_locked": { "type": ["boolean", "null"] },
"closed_at": { "type": "date" }, "closed_at": { "type": "date" },
"created_at": { "type": "date" }, "created_at": { "type": "date" },
"updated_at": { "type": "date" }, "updated_at": { "type": "date" },
......
...@@ -72,6 +72,7 @@ ...@@ -72,6 +72,7 @@
"user_notes_count": { "type": "integer" }, "user_notes_count": { "type": "integer" },
"should_remove_source_branch": { "type": ["boolean", "null"] }, "should_remove_source_branch": { "type": ["boolean", "null"] },
"force_remove_source_branch": { "type": ["boolean", "null"] }, "force_remove_source_branch": { "type": ["boolean", "null"] },
"discussion_locked": { "type": ["boolean", "null"] },
"web_url": { "type": "uri" }, "web_url": { "type": "uri" },
"approvals_before_merge": { "type": ["integer", "null"] }, "approvals_before_merge": { "type": ["integer", "null"] },
"squash": { "type": "boolean" }, "squash": { "type": "boolean" },
......
import Vue from 'vue';
import editFormButtons from '~/sidebar/components/lock/edit_form_buttons.vue';
import mountComponent from '../../helpers/vue_mount_component_helper';
describe('EditFormButtons', () => {
let vm1;
let vm2;
beforeEach(() => {
const Component = Vue.extend(editFormButtons);
const toggleForm = () => { };
const updateLockedAttribute = () => { };
vm1 = mountComponent(Component, {
isLocked: true,
toggleForm,
updateLockedAttribute,
});
vm2 = mountComponent(Component, {
isLocked: false,
toggleForm,
updateLockedAttribute,
});
});
it('renders unlock or lock text based on locked state', () => {
expect(
vm1.$el.innerHTML.includes('Unlock'),
).toBe(true);
expect(
vm2.$el.innerHTML.includes('Lock'),
).toBe(true);
});
});
import Vue from 'vue';
import editForm from '~/sidebar/components/lock/edit_form.vue';
describe('EditForm', () => {
let vm1;
let vm2;
beforeEach(() => {
const Component = Vue.extend(editForm);
const toggleForm = () => { };
const updateLockedAttribute = () => { };
vm1 = new Component({
propsData: {
isLocked: true,
toggleForm,
updateLockedAttribute,
issuableType: 'issue',
},
}).$mount();
vm2 = new Component({
propsData: {
isLocked: false,
toggleForm,
updateLockedAttribute,
issuableType: 'merge_request',
},
}).$mount();
});
it('renders on the appropriate warning text', () => {
expect(
vm1.$el.innerHTML.includes('Unlock this issue?'),
).toBe(true);
expect(
vm2.$el.innerHTML.includes('Lock this merge request?'),
).toBe(true);
});
});
import Vue from 'vue';
import lockIssueSidebar from '~/sidebar/components/lock/lock_issue_sidebar.vue';
describe('LockIssueSidebar', () => {
let vm1;
let vm2;
beforeEach(() => {
const Component = Vue.extend(lockIssueSidebar);
const mediator = {
service: {
update: Promise.resolve(true),
},
store: {
isLockDialogOpen: false,
},
};
vm1 = new Component({
propsData: {
isLocked: true,
isEditable: true,
mediator,
issuableType: 'issue',
},
}).$mount();
vm2 = new Component({
propsData: {
isLocked: false,
isEditable: false,
mediator,
issuableType: 'merge_request',
},
}).$mount();
});
it('shows if locked and/or editable', () => {
expect(
vm1.$el.innerHTML.includes('Edit'),
).toBe(true);
expect(
vm1.$el.innerHTML.includes('Locked'),
).toBe(true);
expect(
vm2.$el.innerHTML.includes('Unlocked'),
).toBe(true);
});
it('displays the edit form when editable', (done) => {
expect(vm1.isLockDialogOpen).toBe(false);
vm1.$el.querySelector('.lock-edit').click();
expect(vm1.isLockDialogOpen).toBe(true);
vm1.$nextTick(() => {
expect(
vm1.$el
.innerHTML
.includes('Unlock this issue?'),
).toBe(true);
done();
});
});
});
import Vue from 'vue';
import confidentialIssue from '~/vue_shared/components/issue/confidential_issue_warning.vue';
describe('Confidential Issue Warning Component', () => {
let vm;
beforeEach(() => {
const Component = Vue.extend(confidentialIssue);
vm = new Component().$mount();
});
afterEach(() => {
vm.$destroy();
});
it('should render confidential issue warning information', () => {
expect(vm.$el.querySelector('i').className).toEqual('fa fa-eye-slash');
expect(vm.$el.querySelector('span').textContent.trim()).toEqual('This is a confidential issue. Your comment will not be visible to the public.');
});
});
import Vue from 'vue';
import issueWarning from '~/vue_shared/components/issue/issue_warning.vue';
import mountComponent from '../../../helpers/vue_mount_component_helper';
const IssueWarning = Vue.extend(issueWarning);
function formatWarning(string) {
// Replace newlines with a space then replace multiple spaces with one space
return string.trim().replace(/\n/g, ' ').replace(/\s\s+/g, ' ');
}
describe('Issue Warning Component', () => {
describe('isLocked', () => {
it('should render locked issue warning information', () => {
const vm = mountComponent(IssueWarning, {
isLocked: true,
});
expect(vm.$el.querySelector('i').className).toEqual('fa fa-lock');
expect(formatWarning(vm.$el.querySelector('span').textContent)).toEqual('This issue is locked. Only project members can comment.');
});
});
describe('isConfidential', () => {
it('should render confidential issue warning information', () => {
const vm = mountComponent(IssueWarning, {
isConfidential: true,
});
expect(vm.$el.querySelector('i').className).toEqual('fa fa-eye-slash');
expect(formatWarning(vm.$el.querySelector('span').textContent)).toEqual('This is a confidential issue. Your comment will not be visible to the public.');
});
});
describe('isLocked and isConfidential', () => {
it('should render locked and confidential issue warning information', () => {
const vm = mountComponent(IssueWarning, {
isLocked: true,
isConfidential: true,
});
expect(vm.$el.querySelector('i')).toBeFalsy();
expect(formatWarning(vm.$el.querySelector('span').textContent)).toEqual('This issue is confidential and locked. People without permission will never get a notification and won\'t be able to comment.');
});
});
});
...@@ -26,6 +26,7 @@ Issue: ...@@ -26,6 +26,7 @@ Issue:
- service_desk_reply_to - service_desk_reply_to
- last_edited_at - last_edited_at
- last_edited_by_id - last_edited_by_id
- discussion_locked
Event: Event:
- id - id
- target_type - target_type
...@@ -172,6 +173,7 @@ MergeRequest: ...@@ -172,6 +173,7 @@ MergeRequest:
- last_edited_at - last_edited_at
- last_edited_by_id - last_edited_by_id
- head_pipeline_id - head_pipeline_id
- discussion_locked
MergeRequestDiff: MergeRequestDiff:
- id - id
- state - state
......
require 'spec_helper'
describe IssuablePolicy, models: true do
describe '#rules' do
context 'when discussion is locked for the issuable' do
let(:user) { create(:user) }
let(:project) { create(:project, :public) }
let(:issue) { create(:issue, project: project, discussion_locked: true) }
let(:policies) { described_class.new(user, issue) }
context 'when the user is not a project member' do
it 'can not create a note' do
expect(policies).to be_disallowed(:create_note)
end
end
context 'when the user is a project member' do
before do
project.add_guest(user)
end
it 'can create a note' do
expect(policies).to be_allowed(:create_note)
end
end
end
end
end
require 'spec_helper'
describe NotePolicy, mdoels: true do
describe '#rules' do
let(:user) { create(:user) }
let(:project) { create(:project, :public) }
let(:issue) { create(:issue, project: project) }
def policies(noteable = nil)
return @policies if @policies
noteable ||= issue
note = create(:note, noteable: noteable, author: user, project: project)
@policies = described_class.new(user, note)
end
context 'when the project is public' do
context 'when the note author is not a project member' do
it 'can edit a note' do
expect(policies).to be_allowed(:update_note)
expect(policies).to be_allowed(:admin_note)
expect(policies).to be_allowed(:resolve_note)
expect(policies).to be_allowed(:read_note)
end
end
context 'when the noteable is a snippet' do
it 'can edit note' do
policies = policies(create(:project_snippet, project: project))
expect(policies).to be_allowed(:update_note)
expect(policies).to be_allowed(:admin_note)
expect(policies).to be_allowed(:resolve_note)
expect(policies).to be_allowed(:read_note)
end
end
context 'when a discussion is locked' do
before do
issue.update_attribute(:discussion_locked, true)
end
context 'when the note author is a project member' do
before do
project.add_developer(user)
end
it 'can edit a note' do
expect(policies).to be_allowed(:update_note)
expect(policies).to be_allowed(:admin_note)
expect(policies).to be_allowed(:resolve_note)
expect(policies).to be_allowed(:read_note)
end
end
context 'when the note author is not a project member' do
it 'can not edit a note' do
expect(policies).to be_disallowed(:update_note)
expect(policies).to be_disallowed(:admin_note)
expect(policies).to be_disallowed(:resolve_note)
end
it 'can read a note' do
expect(policies).to be_allowed(:read_note)
end
end
end
end
end
end
...@@ -302,6 +302,40 @@ describe API::Notes do ...@@ -302,6 +302,40 @@ describe API::Notes do
expect(private_issue.notes.reload).to be_empty expect(private_issue.notes.reload).to be_empty
end end
end end
context 'when the merge request discussion is locked' do
before do
merge_request.update_attribute(:discussion_locked, true)
end
context 'when a user is a team member' do
subject { post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/notes", user), body: 'Hi!' }
it 'returns 200 status' do
subject
expect(response).to have_http_status(201)
end
it 'creates a new note' do
expect { subject }.to change { Note.count }.by(1)
end
end
context 'when a user is not a team member' do
subject { post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/notes", private_user), body: 'Hi!' }
it 'returns 403 status' do
subject
expect(response).to have_http_status(403)
end
it 'does not create a new note' do
expect { subject }.not_to change { Note.count }
end
end
end
end end
describe "POST /projects/:id/noteable/:noteable_id/notes to test observer on create" do describe "POST /projects/:id/noteable/:noteable_id/notes to test observer on create" do
......
...@@ -48,7 +48,8 @@ describe Issues::UpdateService, :mailer do ...@@ -48,7 +48,8 @@ describe Issues::UpdateService, :mailer do
assignee_ids: [user2.id], assignee_ids: [user2.id],
state_event: 'close', state_event: 'close',
label_ids: [label.id], label_ids: [label.id],
due_date: Date.tomorrow due_date: Date.tomorrow,
discussion_locked: true
} }
end end
...@@ -62,6 +63,7 @@ describe Issues::UpdateService, :mailer do ...@@ -62,6 +63,7 @@ describe Issues::UpdateService, :mailer do
expect(issue).to be_closed expect(issue).to be_closed
expect(issue.labels).to match_array [label] expect(issue.labels).to match_array [label]
expect(issue.due_date).to eq Date.tomorrow expect(issue.due_date).to eq Date.tomorrow
expect(issue.discussion_locked).to be_truthy
end end
it 'refreshes the number of open issues when the issue is made confidential', :use_clean_rails_memory_store_caching do it 'refreshes the number of open issues when the issue is made confidential', :use_clean_rails_memory_store_caching do
...@@ -110,6 +112,7 @@ describe Issues::UpdateService, :mailer do ...@@ -110,6 +112,7 @@ describe Issues::UpdateService, :mailer do
expect(issue.labels).to be_empty expect(issue.labels).to be_empty
expect(issue.milestone).to be_nil expect(issue.milestone).to be_nil
expect(issue.due_date).to be_nil expect(issue.due_date).to be_nil
expect(issue.discussion_locked).to be_falsey
end end
end end
...@@ -148,6 +151,13 @@ describe Issues::UpdateService, :mailer do ...@@ -148,6 +151,13 @@ describe Issues::UpdateService, :mailer do
expect(note).not_to be_nil expect(note).not_to be_nil
expect(note.note).to eq 'changed title from **{-Old-} title** to **{+New+} title**' expect(note.note).to eq 'changed title from **{-Old-} title** to **{+New+} title**'
end end
it 'creates system note about discussion lock' do
note = find_note('locked this issue')
expect(note).not_to be_nil
expect(note.note).to eq 'locked this issue'
end
end end
end end
......
...@@ -49,7 +49,8 @@ describe MergeRequests::UpdateService, :mailer do ...@@ -49,7 +49,8 @@ describe MergeRequests::UpdateService, :mailer do
state_event: 'close', state_event: 'close',
label_ids: [label.id], label_ids: [label.id],
target_branch: 'target', target_branch: 'target',
force_remove_source_branch: '1' force_remove_source_branch: '1',
discussion_locked: true
} }
end end
...@@ -73,6 +74,7 @@ describe MergeRequests::UpdateService, :mailer do ...@@ -73,6 +74,7 @@ describe MergeRequests::UpdateService, :mailer do
expect(@merge_request.labels.first.title).to eq(label.name) expect(@merge_request.labels.first.title).to eq(label.name)
expect(@merge_request.target_branch).to eq('target') expect(@merge_request.target_branch).to eq('target')
expect(@merge_request.merge_params['force_remove_source_branch']).to eq('1') expect(@merge_request.merge_params['force_remove_source_branch']).to eq('1')
expect(@merge_request.discussion_locked).to be_truthy
end end
it 'executes hooks with update action' do it 'executes hooks with update action' do
...@@ -123,6 +125,13 @@ describe MergeRequests::UpdateService, :mailer do ...@@ -123,6 +125,13 @@ describe MergeRequests::UpdateService, :mailer do
expect(note.note).to eq 'changed target branch from `master` to `target`' expect(note.note).to eq 'changed target branch from `master` to `target`'
end end
it 'creates system note about discussion lock' do
note = find_note('locked this issue')
expect(note).not_to be_nil
expect(note.note).to eq 'locked this issue'
end
context 'when not including source branch removal options' do context 'when not including source branch removal options' do
before do before do
opts.delete(:force_remove_source_branch) opts.delete(:force_remove_source_branch)
......
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