BigW Consortium Gitlab

Skip to content

G
gitlab-ce
Commit a68a39e3 by Sean McGivern
Options

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

Resolve "Lock issue and merge request" Closes #18608 See merge request gitlab-org/gitlab-ce!14531
parents 8c4dea25 26f6065e
Showing with 1281 additions and 129 deletions
app/assets/javascripts/diff.js
......@@ -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));
......
app/assets/javascripts/notes/components/issue_comment_form.vue
......@@ -7,10 +7,12 @@
import TaskList from '../../task_list';
import * as constants from '../constants';
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 issueDiscussionLockedWidget from './issue_discussion_locked_widget.vue';
import markdownField from '../../vue_shared/components/markdown/field.vue';
import userAvatarLink from '../../vue_shared/components/user_avatar/user_avatar_link.vue';
import issuableStateMixin from '../mixins/issuable_state';
export default {
name: 'issueCommentForm',
......@@ -26,8 +28,9 @@
};
},
components: {
confidentialIssue,
issueWarning,
issueNoteSignedOutWidget,
issueDiscussionLockedWidget,
markdownField,
userAvatarLink,
},
......@@ -55,6 +58,9 @@
isIssueOpen() {
return this.issueState === constants.OPENED || this.issueState === constants.REOPENED;
},
canCreateNote() {
return this.getIssueData.current_user.can_create_note;
},
issueActionButtonTitle() {
if (this.note.length) {
const actionText = this.isIssueOpen ? 'close' : 'reopen';
......@@ -90,9 +96,6 @@
endpoint() {
return this.getIssueData.create_note_path;
},
isConfidentialIssue() {
return this.getIssueData.confidential;
},
},
methods: {
...mapActions([
......@@ -220,6 +223,9 @@
});
},
},
mixins: [
issuableStateMixin,
],
mounted() {
// jQuery is needed here because it is a custom event being dispatched with jQuery.
$(document).on('issuable:change', (e, isClosed) => {
......@@ -235,6 +241,7 @@
<template>
<div>
<issue-note-signed-out-widget v-if="!isLoggedIn" />
<issue-discussion-locked-widget v-else-if="!canCreateNote" />
<ul
v-else
class="notes notes-form timeline">
......@@ -253,15 +260,22 @@
<div class="timeline-content timeline-content-form">
<form
ref="commentForm"
class="new-note js-quick-submit common-note-form gfm-form js-main-target-form">
<confidentialIssue v-if="isConfidentialIssue" />
class="new-note js-quick-submit common-note-form gfm-form js-main-target-form"
>
<div class="error-alert"></div>
<issue-warning
v-if="hasWarning(getIssueData)"
:is-locked="isLocked(getIssueData)"
:is-confidential="isConfidential(getIssueData)"
/>
<markdown-field
:markdown-preview-path="markdownPreviewPath"
:markdown-docs-path="markdownDocsPath"
:quick-actions-docs-path="quickActionsDocsPath"
:add-spacing-classes="false"
:is-confidential-issue="isConfidentialIssue"
ref="markdownField">
<textarea
id="note-body"
......
app/assets/javascripts/notes/components/issue_discussion_locked_widget.vue 0 → 100644
<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>
app/assets/javascripts/notes/components/issue_note_form.vue
<script>
import { mapGetters } from 'vuex';
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 issuableStateMixin from '../mixins/issuable_state';
export default {
name: 'issueNoteForm',
......@@ -39,12 +40,13 @@
};
},
components: {
confidentialIssue,
issueWarning,
markdownField,
},
computed: {
...mapGetters([
'getDiscussionLastNote',
'getIssueData',
'getIssueDataByProp',
'getNotesDataByProp',
'getUserDataByProp',
......@@ -67,9 +69,6 @@
isDisabled() {
return !this.note.length || this.isSubmitting;
},
isConfidentialIssue() {
return this.getIssueDataByProp('confidential');
},
},
methods: {
handleUpdate() {
......@@ -95,6 +94,9 @@
this.$emit('cancelFormEdition', shouldConfirm, this.noteBody !== this.note);
},
},
mixins: [
issuableStateMixin,
],
mounted() {
this.$refs.textarea.focus();
},
......@@ -125,7 +127,13 @@
<div class="flash-container timeline-content"></div>
<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-preview-path="markdownPreviewPath"
:markdown-docs-path="markdownDocsPath"
......
app/assets/javascripts/notes/mixins/issuable_state.js 0 → 100644
export default {
methods: {
isConfidential(issue) {
return !!issue.confidential;
},
isLocked(issue) {
return !!issue.discussion_locked;
},
hasWarning(issue) {
return this.isConfidential(issue) || this.isLocked(issue);
},
},
};
app/assets/javascripts/sidebar/components/confidential/confidential_issue_sidebar.vue
......@@ -47,9 +47,9 @@ export default {
</script>
<template>
<div class="block confidentiality">
<div class="block issuable-sidebar-item confidentiality">
<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 class="title hide-collapsed">
Confidentiality
......@@ -62,19 +62,19 @@ export default {
Edit
</a>
</div>
<div class="value confidential-value hide-collapsed">
<div class="value sidebar-item-value hide-collapsed">
<editForm
v-if="edit"
:toggle-form="toggleForm"
:is-confidential="isConfidential"
:update-confidential-attribute="updateConfidentialAttribute"
/>
<div v-if="!isConfidential" class="no-value confidential-value">
<i class="fa fa-eye is-not-confidential"></i>
<div v-if="!isConfidential" class="no-value sidebar-item-value">
<i class="fa fa-eye sidebar-item-icon"></i>
Not confidential
</div>
<div v-else class="value confidential-value hide-collapsed">
<i aria-hidden="true" data-hidden="true" class="fa fa-eye-slash is-confidential"></i>
<div v-else class="value sidebar-item-value hide-collapsed">
<i aria-hidden="true" class="fa fa-eye-slash sidebar-item-icon is-active"></i>
This issue is confidential
</div>
</div>
......
app/assets/javascripts/sidebar/components/confidential/edit_form.vue
......@@ -2,9 +2,6 @@
import editFormButtons from './edit_form_buttons.vue';
export default {
components: {
editFormButtons,
},
props: {
isConfidential: {
required: true,
......@@ -19,12 +16,16 @@ export default {
type: Function,
},
},
components: {
editFormButtons,
},
};
</script>
<template>
<div class="dropdown open">
<div class="dropdown-menu confidential-warning-message">
<div class="dropdown-menu sidebar-item-warning-message">
<div>
<p v-if="!isConfidential">
You are going to turn on the confidentiality. This means that only team members with
......
app/assets/javascripts/sidebar/components/confidential/edit_form_buttons.vue
......@@ -15,7 +15,7 @@ export default {
},
},
computed: {
onOrOff() {
toggleButtonText() {
return this.isConfidential ? 'Turn Off' : 'Turn On';
},
updateConfidentialBool() {
......@@ -26,7 +26,7 @@ export default {
</script>
<template>
<div class="confidential-warning-message-actions">
<div class="sidebar-item-warning-message-actions">
<button
type="button"
class="btn btn-default append-right-10"
......@@ -39,7 +39,7 @@ export default {
class="btn btn-close"
@click.prevent="updateConfidentialAttribute(updateConfidentialBool)"
>
{{ onOrOff }}
{{ toggleButtonText }}
</button>
</div>
</template>
app/assets/javascripts/sidebar/components/lock/edit_form.vue 0 → 100644
<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>
app/assets/javascripts/sidebar/components/lock/edit_form_buttons.vue 0 → 100644
<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>
app/assets/javascripts/sidebar/components/lock/lock_issue_sidebar.vue 0 → 100644
<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>
app/assets/javascripts/sidebar/sidebar_bundle.js
import Vue from 'vue';
import sidebarTimeTracking from './components/time_tracking/sidebar_time_tracking';
import sidebarAssignees from './components/assignees/sidebar_assignees';
import confidential from './components/confidential/confidential_issue_sidebar.vue';
import SidebarTimeTracking from './components/time_tracking/sidebar_time_tracking';
import SidebarAssignees from './components/assignees/sidebar_assignees';
import ConfidentialIssueSidebar from './components/confidential/confidential_issue_sidebar.vue';
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';
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() {
const sidebarOptions = JSON.parse(document.querySelector('.js-sidebar-options').innerHTML);
const mediator = new Mediator(sidebarOptions);
mediator.fetch();
const sidebarAssigneesEl = document.querySelector('#js-vue-sidebar-assignees');
const confidentialEl = document.querySelector('#js-confidential-entry-point');
const sidebarAssigneesEl = document.getElementById('js-vue-sidebar-assignees');
// Only create the sidebarAssignees vue app if it is found in the DOM
// We currently do not use sidebarAssignees for the MR page
if (sidebarAssigneesEl) {
new Vue(sidebarAssignees).$mount(sidebarAssigneesEl);
new Vue(SidebarAssignees).$mount(sidebarAssigneesEl);
}
if (confidentialEl) {
const dataNode = document.getElementById('js-confidential-issue-data');
const initialData = JSON.parse(dataNode.innerHTML);
mountConfidentialComponent(mediator);
mountLockComponent(mediator);
const ConfidentialComp = Vue.extend(confidential);
new ConfidentialComp({
propsData: {
isConfidential: initialData.is_confidential,
isEditable: initialData.is_editable,
service: mediator.service,
},
}).$mount(confidentialEl);
new SidebarMoveIssue(
mediator,
$('.js-move-issue'),
$('.js-move-issue-confirmation-button'),
).init();
}
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);
......
app/assets/javascripts/sidebar/stores/sidebar_store.js
......@@ -15,6 +15,7 @@ export default class SidebarStore {
};
this.autocompleteProjects = [];
this.moveToProjectId = 0;
this.isLockDialogOpen = false;
SidebarStore.singleton = this;
}
......
app/assets/javascripts/vue_shared/components/issue/confidential_issue_warning.vue deleted 100644 → 0
<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>
app/assets/javascripts/vue_shared/components/issue/issue_warning.vue 0 → 100644
<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 icon"
: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>
app/assets/javascripts/vue_shared/mixins/issuable.js 0 → 100644
export default {
methods: {
issuableDisplayName(issuableType) {
const displayName = issuableType.replace(/_/, ' ');
return this.__ ? this.__(displayName) : displayName;
},
},
};
app/assets/stylesheets/framework/buttons.scss
......@@ -381,7 +381,11 @@
background: transparent;
border: 0;
&:hover,
&:active,
&:focus {
outline: 0;
background: transparent;
box-shadow: none;
}
}
app/assets/stylesheets/framework/variables.scss
......@@ -705,3 +705,9 @@ Project Templates Icons
$rails: #c00;
$node: #353535;
$java: #70ad51;
/*
Issuable warning
*/
$issuable-warning-size: 24px;
$issuable-warning-icon-margin: 4px;
app/assets/stylesheets/pages/issuable.scss
......@@ -5,27 +5,29 @@
margin-right: auto;
}
.is-confidential {
.issuable-warning-icon {
color: $orange-600;
background-color: $orange-100;
border-radius: $border-radius-default;
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;
&:first-of-type {
margin-right: $issuable-warning-icon-margin;
}
}
.is-not-confidential {
.sidebar-item-icon {
border-radius: $border-radius-default;
padding: 5px;
margin: 0 3px 0 -4px;
}
.confidentiality {
.is-not-confidential {
margin: auto;
}
.is-confidential {
margin: auto;
&.is-active {
color: $orange-600;
background-color: $orange-50;
}
}
......
app/assets/stylesheets/pages/note_form.scss
......@@ -101,7 +101,7 @@
}
}
.confidential-issue-warning {
.issuable-note-warning {
color: $orange-600;
background-color: $orange-100;
border-radius: $border-radius-default $border-radius-default 0 0;
......@@ -110,28 +110,52 @@
padding: 3px 12px;
margin: auto;
align-items: center;
.icon {
margin-right: $issuable-warning-icon-margin;
}
}
.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;
}
}
.confidential-value {
.sidebar-item-value {
.fa {
background-color: inherit;
}
}
.confidential-warning-message {
.sidebar-item-warning-message {
line-height: 1.5;
padding: 16px;
.confidential-warning-message-actions {
.text {
color: $text-color;
}
.sidebar-item-warning-message-actions {
display: flex;
button {
.btn {
flex-grow: 1;
}
}
}
.confidential-issue-warning + .md-area {
.issuable-note-warning + .md-area {
border-top-left-radius: 0;
border-top-right-radius: 0;
}
......
app/assets/stylesheets/pages/notes.scss
......@@ -703,6 +703,12 @@ ul.notes {
color: $note-disabled-comment-color;
padding: 90px 0;
&.discussion-locked {
border: none;
background-color: $white-light;
}
a {
color: $gl-link-color;
}
......
app/controllers/projects/issues_controller.rb
......@@ -278,6 +278,7 @@ class Projects::IssuesController < Projects::ApplicationController
state_event
task_num
lock_version
discussion_locked
] + [{ label_ids: [], assignee_ids: [] }]
end
......
app/controllers/projects/merge_requests/application_controller.rb
......@@ -34,6 +34,7 @@ class Projects::MergeRequests::ApplicationController < Projects::ApplicationCont
:target_project_id,
:task_num,
:title,
:discussion_locked,
label_ids: []
]
......
app/controllers/projects/notes_controller.rb
......@@ -66,7 +66,16 @@ class Projects::NotesController < Projects::ApplicationController
params.merge(last_fetched_at: last_fetched_at)
end
def authorize_admin_note!
return access_denied! unless can?(current_user, :admin_note, note)
end
def authorize_resolve_note!
return access_denied! unless can?(current_user, :resolve_note, note)
end
def authorize_create_note!
return unless noteable.lockable?
access_denied! unless can?(current_user, :create_note, noteable)
end
end
app/helpers/notes_helper.rb
......@@ -130,8 +130,12 @@ module NotesHelper
end
def can_create_note?
issuable = @issue || @merge_request
if @snippet.is_a?(PersonalSnippet)
can?(current_user, :comment_personal_snippet, @snippet)
elsif issuable
can?(current_user, :create_note, issuable)
else
can?(current_user, :create_note, @project)
end
......
app/helpers/system_note_helper.rb
......@@ -19,7 +19,9 @@ module SystemNoteHelper
'discussion' => 'comment',
'moved' => 'arrow-right',
'outdated' => 'pencil',
'duplicate' => 'issue-duplicate'
'duplicate' => 'issue-duplicate',
'locked' => 'lock',
'unlocked' => 'lock-open'
}.freeze
def system_note_icon_name(note)
......
app/models/concerns/noteable.rb
......@@ -74,4 +74,8 @@ module Noteable
def discussions_can_be_resolved_by?(user)
discussions_to_be_resolved.all? { |discussion| discussion.can_resolve?(user) }
end
def lockable?
[MergeRequest, Issue].include?(self.class)
end
end
app/models/system_note_metadata.rb
......@@ -2,7 +2,7 @@ class SystemNoteMetadata < ActiveRecord::Base
ICON_TYPES = %w[
commit description merge confidential visible label assignee cross_reference
title time_tracking branch milestone discussion task moved
opened closed merged duplicate
opened closed merged duplicate locked unlocked
outdated
].freeze
......
app/policies/issuable_policy.rb
class IssuablePolicy < BasePolicy
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"
condition(:assignee_or_author) do
@user && @subject.assignee_or_author?(@user)
......@@ -12,4 +16,12 @@ class IssuablePolicy < BasePolicy
enable :read_merge_request
enable :update_merge_request
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
app/policies/note_policy.rb
class NotePolicy < BasePolicy
delegate { @subject.project }
delegate { @subject.noteable if @subject.noteable.lockable? }
condition(:is_author) { @user && @subject.author == @user }
condition(:for_merge_request, scope: :subject) { @subject.for_merge_request? }
......@@ -8,6 +9,7 @@ class NotePolicy < BasePolicy
condition(:editable, scope: :subject) { @subject.editable? }
rule { ~editable | anonymous }.prevent :edit_note
rule { is_author | admin }.enable :edit_note
rule { can?(:master_access) }.enable :edit_note
......
app/serializers/issue_entity.rb
......@@ -3,6 +3,7 @@ class IssueEntity < IssuableEntity
expose :branch_name
expose :confidential
expose :discussion_locked
expose :assignees, using: API::Entities::UserBasic
expose :due_date
expose :moved_to_id
......@@ -14,7 +15,7 @@ class IssueEntity < IssuableEntity
expose :current_user do
expose :can_create_note do |issue|
can?(request.current_user, :create_note, issue.project)
can?(request.current_user, :create_note, issue)
end
expose :can_update do |issue|
......
app/services/issuable_base_service.rb
......@@ -43,6 +43,10 @@ class IssuableBaseService < BaseService
SystemNoteService.change_time_spent(issuable, issuable.project, current_user)
end
def create_discussion_lock_note(issuable)
SystemNoteService.discussion_lock(issuable, current_user)
end
def filter_params(issuable)
ability_name = :"admin_#{issuable.to_ability_name}"
......@@ -57,6 +61,7 @@ class IssuableBaseService < BaseService
params.delete(:due_date)
params.delete(:canonical_issue_id)
params.delete(:project)
params.delete(:discussion_locked)
end
filter_assignee(issuable)
......@@ -236,6 +241,7 @@ class IssuableBaseService < BaseService
handle_common_system_notes(issuable, old_labels: old_labels)
end
change_discussion_lock(issuable)
handle_changes(
issuable,
old_labels: old_labels,
......@@ -294,6 +300,12 @@ class IssuableBaseService < BaseService
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)
award = params.delete(:emoji_award)
if award
......
app/services/system_note_service.rb
......@@ -591,6 +591,13 @@ module SystemNoteService
create_note(NoteSummary.new(noteable, project, author, body, action: 'duplicate'))
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
def notes_for_mentioner(mentioner, noteable, notes)
......
app/views/projects/_md_preview.html.haml
- referenced_users = local_assigns.fetch(:referenced_users, nil)
- if defined?(@merge_request) && @merge_request.discussion_locked?
.issuable-note-warning
= icon('lock', class: 'icon')
%span
= _('This merge request is locked.')
= _('Only project members can comment.')
.md-area
.md-header
%ul.nav-links.clearfix
......
app/views/projects/issues/show.html.haml
......@@ -27,7 +27,9 @@
.issuable-meta
- 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-actions.js-issuable-actions
......
app/views/projects/merge_requests/_mr_title.html.haml
......@@ -15,6 +15,8 @@
= icon('angle-double-left')
.issuable-meta
- if @merge_request.discussion_locked?
= icon('lock', class: 'issuable-warning-icon')
= issuable_meta(@merge_request, @project, "Merge request")
.issuable-actions.js-issuable-actions
......
app/views/projects/merge_requests/show.html.haml
......@@ -83,7 +83,7 @@
#pipelines.pipelines.tab-pane
- if @pipelines.any?
= 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
.mr-loading-status
......
app/views/shared/issuable/_sidebar.html.haml
......@@ -119,6 +119,10 @@
%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
- 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)
- if current_user
- subscribed = issuable.subscribed?(current_user, @project)
......
app/views/shared/notes/_notes_with_form.html.haml
- issuable = @issue || @merge_request
- discussion_locked = issuable&.discussion_locked?
%ul#notes-list.notes.main-notes-list.timeline
= render "shared/notes/notes"
......@@ -21,5 +24,14 @@
or
= link_to "sign in", new_session_path(:user, redirect_to_referer: 'yes'), class: 'js-sign-in-link'
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
changelogs/unreleased/18608-lock-issues.yml 0 → 100644
title: Discussion lock for issues and merge requests
merge_request:
author:
type: added
db/migrate/20170815221154_add_discussion_locked_to_issuable.rb 0 → 100644
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
db/schema.rb
......@@ -677,6 +677,7 @@ ActiveRecord::Schema.define(version: 20171005130944) do
t.integer "cached_markdown_version"
t.datetime "last_edited_at"
t.integer "last_edited_by_id"
t.boolean "discussion_locked"
end
add_index "issues", ["assignee_id"], name: "index_issues_on_assignee_id", using: :btree
......@@ -900,6 +901,7 @@ ActiveRecord::Schema.define(version: 20171005130944) do
t.integer "head_pipeline_id"
t.boolean "ref_fetched"
t.string "merge_jid"
t.boolean "discussion_locked"
end
add_index "merge_requests", ["assignee_id"], name: "index_merge_requests_on_assignee_id", using: :btree
......
doc/api/issues.md
......@@ -110,7 +110,8 @@ Example response:
"human_time_estimate": null,
"human_total_time_spent": null
},
"confidential": false
"confidential": false,
"discussion_locked": false
}
]
```
......@@ -216,7 +217,8 @@ Example response:
"human_time_estimate": null,
"human_total_time_spent": null
},
"confidential": false
"confidential": false,
"discussion_locked": false
}
]
```
......@@ -323,7 +325,8 @@ Example response:
"human_time_estimate": null,
"human_total_time_spent": null
},
"confidential": false
"confidential": false,
"discussion_locked": false
}
]
```
......@@ -407,6 +410,7 @@ Example response:
"human_total_time_spent": null
},
"confidential": false,
"discussion_locked": false,
"_links": {
"self": "http://example.com/api/v4/projects/1/issues/2",
"notes": "http://example.com/api/v4/projects/1/issues/2/notes",
......@@ -482,6 +486,7 @@ Example response:
"human_total_time_spent": null
},
"confidential": false,
"discussion_locked": false,
"_links": {
"self": "http://example.com/api/v4/projects/1/issues/2",
"notes": "http://example.com/api/v4/projects/1/issues/2/notes",
......@@ -515,6 +520,8 @@ PUT /projects/:id/issues/:issue_iid
| `state_event` | string | no | The state event of an issue. Set `close` to close the issue and `reopen` to reopen it |
| `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` |
| `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
curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/4/issues/85?state_event=close
......@@ -558,6 +565,7 @@ Example response:
"human_total_time_spent": null
},
"confidential": false,
"discussion_locked": false,
"_links": {
"self": "http://example.com/api/v4/projects/1/issues/2",
"notes": "http://example.com/api/v4/projects/1/issues/2/notes",
......@@ -657,6 +665,7 @@ Example response:
"human_total_time_spent": null
},
"confidential": false,
"discussion_locked": false,
"_links": {
"self": "http://example.com/api/v4/projects/1/issues/2",
"notes": "http://example.com/api/v4/projects/1/issues/2/notes",
......@@ -735,6 +744,7 @@ Example response:
"human_total_time_spent": null
},
"confidential": false,
"discussion_locked": false,
"_links": {
"self": "http://example.com/api/v4/projects/1/issues/2",
"notes": "http://example.com/api/v4/projects/1/issues/2/notes",
......@@ -765,6 +775,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
```
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
Manually creates a todo for the current user on an issue. If
......@@ -857,7 +905,8 @@ Example response:
"downvotes": 0,
"due_date": null,
"web_url": "http://example.com/example/example/issues/110",
"confidential": false
"confidential": false,
"discussion_locked": false
},
"target_url": "https://gitlab.example.com/gitlab-org/gitlab-ci/issues/10",
"body": "Vel voluptas atque dicta mollitia adipisci qui at.",
......
doc/api/merge_requests.md
......@@ -201,6 +201,7 @@ Parameters:
"should_remove_source_branch": true,
"force_remove_source_branch": false,
"web_url": "http://example.com/example/example/merge_requests/1",
"discussion_locked": false,
"time_stats": {
"time_estimate": 0,
"total_time_spent": 0,
......@@ -276,6 +277,7 @@ Parameters:
"should_remove_source_branch": true,
"force_remove_source_branch": false,
"web_url": "http://example.com/example/example/merge_requests/1",
"discussion_locked": false,
"time_stats": {
"time_estimate": 0,
"total_time_spent": 0,
......@@ -387,6 +389,7 @@ Parameters:
"should_remove_source_branch": true,
"force_remove_source_branch": false,
"web_url": "http://example.com/example/example/merge_requests/1",
"discussion_locked": false,
"time_stats": {
"time_estimate": 0,
"total_time_spent": 0,
......@@ -480,6 +483,7 @@ POST /projects/:id/merge_requests
"should_remove_source_branch": true,
"force_remove_source_branch": false,
"web_url": "http://example.com/example/example/merge_requests/1",
"discussion_locked": false,
"time_stats": {
"time_estimate": 0,
"total_time_spent": 0,
......@@ -509,6 +513,7 @@ PUT /projects/:id/merge_requests/:merge_request_iid
| `labels` | string | no | Labels for MR as a comma-separated list |
| `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 |
| `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.
......@@ -563,6 +568,7 @@ Must include at least one non-required attribute from above.
"should_remove_source_branch": true,
"force_remove_source_branch": false,
"web_url": "http://example.com/example/example/merge_requests/1",
"discussion_locked": false,
"time_stats": {
"time_estimate": 0,
"total_time_spent": 0,
......@@ -667,6 +673,7 @@ Parameters:
"should_remove_source_branch": true,
"force_remove_source_branch": false,
"web_url": "http://example.com/example/example/merge_requests/1",
"discussion_locked": false,
"time_stats": {
"time_estimate": 0,
"total_time_spent": 0,
......@@ -743,6 +750,7 @@ Parameters:
"should_remove_source_branch": true,
"force_remove_source_branch": false,
"web_url": "http://example.com/example/example/merge_requests/1",
"discussion_locked": false,
"time_stats": {
"time_estimate": 0,
"total_time_spent": 0,
......@@ -1037,7 +1045,8 @@ Example response:
"id": 14,
"state": "active",
"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": {
"name": "Dr. Gabrielle Strosin",
......
doc/user/discussions/index.md
......@@ -153,12 +153,52 @@ comments in greater detail.
![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-7125]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7125
[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-8266]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/8266
[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-comment-button]: img/resolve_comment_button.png
[discussion-view]: img/discussion_view.png
......
doc/user/permissions.md
......@@ -25,6 +25,7 @@ The following table depicts the various user permission levels in a project.
| Create confidential issue | ✓ [^1] | ✓ | ✓ | ✓ | ✓ |
| View confidential issues | (✓) [^2] | ✓ | ✓ | ✓ | ✓ |
| Leave comments | ✓ [^1] | ✓ | ✓ | ✓ | ✓ |
| Lock comments | | | | ✓ | ✓ |
| See a list of jobs | ✓ [^3] | ✓ | ✓ | ✓ | ✓ |
| See a job log | ✓ [^3] | ✓ | ✓ | ✓ | ✓ |
| Download and browse job artifacts | ✓ [^3] | ✓ | ✓ | ✓ | ✓ |
......
lib/api/entities.rb
......@@ -368,6 +368,7 @@ module API
end
expose :due_date
expose :confidential
expose :discussion_locked
expose :web_url do |issue, options|
Gitlab::UrlBuilder.build(issue)
......@@ -464,6 +465,7 @@ module API
expose :diff_head_sha, as: :sha
expose :merge_commit_sha
expose :user_notes_count
expose :discussion_locked
expose :should_remove_source_branch?, as: :should_remove_source_branch
expose :force_remove_source_branch?, as: :force_remove_source_branch
......
lib/api/issues.rb
......@@ -48,6 +48,7 @@ module API
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 :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
params :issue_params do
......@@ -193,7 +194,7 @@ module API
desc: 'Date time when the issue was updated. Available only for admins and project owners.'
optional :state_event, type: String, values: %w[reopen close], desc: 'State of the issue'
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, :discussion_locked,
:labels, :created_at, :due_date, :confidential, :state_event
end
put ':id/issues/:issue_iid' do
......
lib/api/merge_requests.rb
......@@ -214,12 +214,14 @@ module API
:remove_source_branch,
:state_event,
:target_branch,
:title
:title,
:discussion_locked
]
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 :state_event, type: String, values: %w[close reopen],
desc: 'Status of the merge request'
optional :discussion_locked, type: Boolean, desc: 'Whether the MR discussion is locked'
use :optional_params
at_least_one_of(*at_least_one_of_ce)
......
lib/api/notes.rb
......@@ -78,6 +78,8 @@ module API
}
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)
opts[:created_at] = params[:created_at]
end
......
spec/controllers/projects/notes_controller_spec.rb
......@@ -266,6 +266,56 @@ describe Projects::NotesController do
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
describe 'DELETE destroy' do
......
spec/features/issuables/discussion_lock_spec.rb 0 → 100644
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
spec/features/issues_spec.rb
......@@ -609,14 +609,14 @@ describe 'Issues', :js do
visit project_issue_path(project, issue)
expect(page).to have_css('.confidential-issue-warning')
expect(page).to have_css('.is-confidential')
expect(page).not_to have_css('.is-not-confidential')
expect(page).to have_css('.issuable-note-warning')
expect(find('.issuable-sidebar-item.confidentiality')).to have_css('.is-active')
expect(find('.issuable-sidebar-item.confidentiality')).not_to have_css('.not-active')
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
end
......@@ -624,7 +624,7 @@ describe 'Issues', :js do
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
spec/features/merge_requests/discussion_lock_spec.rb 0 → 100644
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
spec/fixtures/api/schemas/public_api/v4/issues.json
......@@ -9,6 +9,7 @@
"title": { "type": "string" },
"description": { "type": ["string", "null"] },
"state": { "type": "string" },
"discussion_locked": { "type": ["boolean", "null"] },
"closed_at": { "type": "date" },
"created_at": { "type": "date" },
"updated_at": { "type": "date" },
......
spec/fixtures/api/schemas/public_api/v4/merge_requests.json
......@@ -72,6 +72,7 @@
"user_notes_count": { "type": "integer" },
"should_remove_source_branch": { "type": ["boolean", "null"] },
"force_remove_source_branch": { "type": ["boolean", "null"] },
"discussion_locked": { "type": ["boolean", "null"] },
"web_url": { "type": "uri" },
"time_stats": {
"time_estimate": { "type": "integer" },
......
spec/javascripts/sidebar/lock/edit_form_buttons_spec.js 0 → 100644
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);
});
});
spec/javascripts/sidebar/lock/edit_form_spec.js 0 → 100644
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);
});
});
spec/javascripts/sidebar/lock/lock_issue_sidebar_spec.js 0 → 100644
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();
});
});
});
spec/javascripts/vue_shared/components/issue/confidential_issue_warning_spec.js deleted 100644 → 0
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.');
});
});
spec/javascripts/vue_shared/components/issue/issue_warning_spec.js 0 → 100644
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 icon 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 icon 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.');
});
});
});
spec/lib/gitlab/import_export/safe_model_attributes.yml
......@@ -25,6 +25,7 @@ Issue:
- relative_position
- last_edited_at
- last_edited_by_id
- discussion_locked
Event:
- id
- target_type
......@@ -168,6 +169,7 @@ MergeRequest:
- last_edited_at
- last_edited_by_id
- head_pipeline_id
- discussion_locked
MergeRequestDiff:
- id
- state
......
spec/policies/issuable_policy_spec.rb 0 → 100644
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
spec/policies/note_policy_spec.rb 0 → 100644
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
spec/requests/api/notes_spec.rb
......@@ -302,6 +302,40 @@ describe API::Notes do
expect(private_issue.notes.reload).to be_empty
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
describe "POST /projects/:id/noteable/:noteable_id/notes to test observer on create" do
......
spec/services/issues/update_service_spec.rb
......@@ -48,7 +48,8 @@ describe Issues::UpdateService, :mailer do
assignee_ids: [user2.id],
state_event: 'close',
label_ids: [label.id],
due_date: Date.tomorrow
due_date: Date.tomorrow,
discussion_locked: true
}
end
......@@ -62,6 +63,7 @@ describe Issues::UpdateService, :mailer do
expect(issue).to be_closed
expect(issue.labels).to match_array [label]
expect(issue.due_date).to eq Date.tomorrow
expect(issue.discussion_locked).to be_truthy
end
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
expect(issue.labels).to be_empty
expect(issue.milestone).to be_nil
expect(issue.due_date).to be_nil
expect(issue.discussion_locked).to be_falsey
end
end
......@@ -148,6 +151,13 @@ describe Issues::UpdateService, :mailer do
expect(note).not_to be_nil
expect(note.note).to eq 'changed title from **{-Old-} title** to **{+New+} title**'
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
......
spec/services/merge_requests/update_service_spec.rb
......@@ -49,7 +49,8 @@ describe MergeRequests::UpdateService, :mailer do
state_event: 'close',
label_ids: [label.id],
target_branch: 'target',
force_remove_source_branch: '1'
force_remove_source_branch: '1',
discussion_locked: true
}
end
......@@ -73,6 +74,7 @@ describe MergeRequests::UpdateService, :mailer do
expect(@merge_request.labels.first.title).to eq(label.name)
expect(@merge_request.target_branch).to eq('target')
expect(@merge_request.merge_params['force_remove_source_branch']).to eq('1')
expect(@merge_request.discussion_locked).to be_truthy
end
it 'executes hooks with update action' do
......@@ -123,6 +125,13 @@ describe MergeRequests::UpdateService, :mailer do
expect(note.note).to eq 'changed target branch from `master` to `target`'
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
before do
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