diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 8fa46a457012d86676fd4a799863080264dc89c9..6970bb4e7c8eca54b88fee8365d42d0f50180c5b 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -62,7 +62,7 @@ Lint/UnusedMethodArgument: # Offense count: 112 # Configuration parameters: CountComments. Metrics/BlockLength: - Max: 303 + Enabled: false # Offense count: 4 # Cop supports --auto-correct. @@ -125,7 +125,7 @@ RSpec/MessageSpies: # Offense count: 3278 RSpec/MultipleExpectations: - Max: 37 + Enabled: false # Offense count: 2287 RSpec/NamedSubject: diff --git a/app/assets/images/mailers/approval/icon-merge-request-gray.gif b/app/assets/images/mailers/approval/icon-merge-request-gray.gif new file mode 100644 index 0000000000000000000000000000000000000000..30cbe66980ffb906ffeaadaf5ded34d82d293637 Binary files /dev/null and b/app/assets/images/mailers/approval/icon-merge-request-gray.gif differ diff --git a/app/assets/images/mailers/approval/icon-x-orange-inverted.gif b/app/assets/images/mailers/approval/icon-x-orange-inverted.gif new file mode 100644 index 0000000000000000000000000000000000000000..7fbf1c413845c1befcd3314a8ea6b74c4e594ff5 Binary files /dev/null and b/app/assets/images/mailers/approval/icon-x-orange-inverted.gif differ diff --git a/app/assets/javascripts/generic_bundles/vue_resource.js.es6 b/app/assets/javascripts/generic_bundles/vue_resource.js.es6 new file mode 100644 index 0000000000000000000000000000000000000000..eff1dcabfa2d5b381b3c519711c9cc90f704f84a --- /dev/null +++ b/app/assets/javascripts/generic_bundles/vue_resource.js.es6 @@ -0,0 +1,2 @@ +//= require vue +//= require vue-resource diff --git a/app/assets/javascripts/merge_request_widget/approvals/approvals_api.js.es6 b/app/assets/javascripts/merge_request_widget/approvals/approvals_api.js.es6 new file mode 100644 index 0000000000000000000000000000000000000000..4f181e02f3939afb356e76f38bfaccb02ef36b3a --- /dev/null +++ b/app/assets/javascripts/merge_request_widget/approvals/approvals_api.js.es6 @@ -0,0 +1,36 @@ +/* global Vue, Flash */ +//= require ./approvals_store + +(() => { + class ApprovalsApi { + constructor(endpoint) { + gl.ApprovalsApi = this; + this.init(endpoint); + } + + init(mergeRequestEndpoint) { + this.baseEndpoint = `${mergeRequestEndpoint}/approvals`; + Vue.http.headers.common['X-CSRF-Token'] = $.rails.csrfToken(); + } + + fetchApprovals() { + const flashErrorMessage = 'An error occured while retrieving approval data for this merge request.'; + + return Vue.http.get(this.baseEndpoint).catch(() => new Flash(flashErrorMessage)); + } + + approveMergeRequest() { + const flashErrorMessage = 'An error occured while submitting your approval.'; + + return Vue.http.post(this.baseEndpoint).catch(() => new Flash(flashErrorMessage)); + } + + unapproveMergeRequest() { + const flashErrorMessage = 'An error occured while removing your approval.'; + + return Vue.http.delete(this.baseEndpoint).catch(() => new Flash(flashErrorMessage)); + } + } + + gl.ApprovalsApi = ApprovalsApi; +})(); diff --git a/app/assets/javascripts/merge_request_widget/approvals/approvals_bundle.js.es6 b/app/assets/javascripts/merge_request_widget/approvals/approvals_bundle.js.es6 new file mode 100644 index 0000000000000000000000000000000000000000..e5173098f9356a9bdfa71d89afda859eaca89d48 --- /dev/null +++ b/app/assets/javascripts/merge_request_widget/approvals/approvals_bundle.js.es6 @@ -0,0 +1,3 @@ +//= require ./approvals_store +//= require ./approvals_api +//= require_directory ./components diff --git a/app/assets/javascripts/merge_request_widget/approvals/approvals_store.js.es6 b/app/assets/javascripts/merge_request_widget/approvals/approvals_store.js.es6 new file mode 100644 index 0000000000000000000000000000000000000000..1c96109656b6094eb470ec10f0fe09d0060788e8 --- /dev/null +++ b/app/assets/javascripts/merge_request_widget/approvals/approvals_store.js.es6 @@ -0,0 +1,65 @@ +/* global Vue */ +//= require ./approvals_api + +(() => { + let singleton; + + class MergeRequestApprovalsStore { + constructor(rootStore) { + if (!singleton) { + singleton = this; + this.init(rootStore); + } + return singleton; + } + + init(rootStore) { + this.rootStore = rootStore; + this.api = new gl.ApprovalsApi(rootStore.rootEl.dataset.endpoint); + this.state = { + fetching: false, + }; + } + + initStoreOnce() { + const state = this.state; + if (!state.fetching) { + state.fetching = true; + return this.fetch() + .then(() => { + state.fetching = false; + this.assignToRootStore('showApprovals', true); + }); + } + return Promise.resolve(); + } + + fetch() { + return this.api.fetchApprovals() + .then(res => this.assignToRootStore('approvals', res.data)) + .then(data => this.setMergeRequestAcceptanceStatus(data.approvals_left)); + } + + approve() { + return this.api.approveMergeRequest() + .then(res => this.assignToRootStore('approvals', res.data)) + .then(data => this.setMergeRequestAcceptanceStatus(data.approvals_left)); + } + + unapprove() { + return this.api.unapproveMergeRequest() + .then(res => this.assignToRootStore('approvals', res.data)) + .then(data => this.setMergeRequestAcceptanceStatus(data.approvals_left)); + } + + setMergeRequestAcceptanceStatus(approvalsLeft) { + return this.rootStore.assignToData('disableAcceptance', !!approvalsLeft); + } + + assignToRootStore(key, data) { + return this.rootStore.assignToData(key, data); + } + } + gl.MergeRequestApprovalsStore = MergeRequestApprovalsStore; +})(window.gl || (window.gl = {})); + diff --git a/app/assets/javascripts/merge_request_widget/approvals/components/approvals_body.js.es6 b/app/assets/javascripts/merge_request_widget/approvals/components/approvals_body.js.es6 new file mode 100644 index 0000000000000000000000000000000000000000..7451639de35c2839e7e1ad202dc7d98ad300d5ef --- /dev/null +++ b/app/assets/javascripts/merge_request_widget/approvals/components/approvals_body.js.es6 @@ -0,0 +1,94 @@ +/* global Vue */ +//= require ../approvals_store +//= require ../approvals_api + +(() => { + Vue.component('approvals-body', { + name: 'approvals-body', + props: { + approvedBy: { + type: Array, + required: false, + }, + approvalsLeft: { + type: Number, + required: false, + }, + userCanApprove: { + type: Boolean, + required: false, + }, + userHasApproved: { + type: Boolean, + required: false, + }, + suggestedApprovers: { + type: Array, + required: false, + }, + }, + data() { + return { + approving: false, + }; + }, + computed: { + approvalsRequiredStringified() { + const baseString = `${this.approvalsLeft} more approval`; + return this.approvalsLeft === 1 ? baseString : `${baseString}s`; + }, + approverNamesStringified() { + const approvers = this.suggestedApprovers; + + if (!approvers) { + return ''; + } + + return approvers.length === 1 ? approvers[0].name : + approvers.reduce((memo, curr, index) => { + const nextMemo = `${memo}${curr.name}`; + + if (index === approvers.length - 2) { // second to last index + return `${nextMemo} or `; + } else if (index === approvers.length - 1) { // last index + return nextMemo; + } + + return `${nextMemo}, `; + }, ''); + }, + showApproveButton() { + return this.userCanApprove && !this.userHasApproved; + }, + showSuggestedApprovers() { + return this.suggestedApprovers && this.suggestedApprovers.length; + }, + }, + methods: { + approveMergeRequest() { + this.approving = true; + return gl.ApprovalsStore.approve().then(() => { + this.approving = false; + }); + }, + }, + beforeCreate() { + gl.ApprovalsStore.initStoreOnce(); + }, + template: ` +
+

Requires {{ approvalsRequiredStringified }} + (from {{ approverNamesStringified }}) +

+
+ +
+
+ `, + }); +})(); diff --git a/app/assets/javascripts/merge_request_widget/approvals/components/approvals_footer.js.es6 b/app/assets/javascripts/merge_request_widget/approvals/components/approvals_footer.js.es6 new file mode 100644 index 0000000000000000000000000000000000000000..dfa9c62e3440d9c2c9835a9248b79460082aee77 --- /dev/null +++ b/app/assets/javascripts/merge_request_widget/approvals/components/approvals_footer.js.es6 @@ -0,0 +1,93 @@ +/* global Vue */ +//= require ../approvals_store +//= require vue_common_component/link_to_member_avatar + +(() => { + Vue.component('approvals-footer', { + name: 'approvals-footer', + props: { + approvedBy: { + type: Array, + required: false, + }, + approvalsLeft: { + type: Number, + required: false, + }, + userCanApprove: { + type: Boolean, + required: false, + }, + userHasApproved: { + type: Boolean, + required: false, + }, + suggestedApprovers: { + type: Array, + required: false, + }, + pendingAvatarSvg: { + type: String, + required: true, + }, + checkmarkSvg: { + type: String, + required: true, + }, + }, + data() { + return { + unapproving: false, + }; + }, + computed: { + showUnapproveButton() { + return this.userHasApproved && !this.userCanApprove; + }, + }, + methods: { + unapproveMergeRequest() { + this.unapproving = true; + gl.ApprovalsStore.unapprove().then(() => { + this.unapproving = false; + }); + }, + }, + beforeCreate() { + gl.ApprovalsStore.initStoreOnce(); + }, + template: ` +
+ Approved by + + + + + + + + + + + +
+ `, + }); +})(); + diff --git a/app/assets/javascripts/merge_request_widget/widget_bundle.js.es6 b/app/assets/javascripts/merge_request_widget/widget_bundle.js.es6 new file mode 100644 index 0000000000000000000000000000000000000000..0986ae242ecd34ec022f3d267743b30196cf30d6 --- /dev/null +++ b/app/assets/javascripts/merge_request_widget/widget_bundle.js.es6 @@ -0,0 +1,15 @@ +/* global Vue */ +//= require ./widget_store +//= require ./approvals/approvals_bundle + +(() => { + $(() => { + const rootEl = document.getElementById('merge-request-widget-app'); + const widgetSharedStore = new gl.MergeRequestWidgetStore(rootEl); + + gl.MergeRequestWidgetApp = new Vue({ + el: rootEl, + data: widgetSharedStore.data, + }); + }); +})(window.gl || (window.gl = {})); diff --git a/app/assets/javascripts/merge_request_widget/widget_store.js.es6 b/app/assets/javascripts/merge_request_widget/widget_store.js.es6 new file mode 100644 index 0000000000000000000000000000000000000000..a934712f5b92459bcb51d831895d368872d69320 --- /dev/null +++ b/app/assets/javascripts/merge_request_widget/widget_store.js.es6 @@ -0,0 +1,40 @@ +//= require ./approvals/approvals_store + +(() => { + let singleton; + + class MergeRequestWidgetStore { + constructor(rootEl) { + if (!singleton) { + singleton = gl.MergeRequestWidget.Store = this; + this.init(rootEl); + } + return singleton; + } + + init(rootEl) { + this.rootEl = rootEl; + this.data = {}; + + // init other widget stores here + this.initWidgetState(); + this.initApprovals(); + } + + initWidgetState() { + this.assignToData('showApprovals', false); + this.assignToData('disableAcceptance', Boolean(this.rootEl.dataset.approvalPending)); + } + + initApprovals() { + gl.ApprovalsStore = new gl.MergeRequestApprovalsStore(this); + this.assignToData('approvals', {}); + } + + assignToData(key, val) { + this.data[key] = val; + return val; + } + } + gl.MergeRequestWidgetStore = MergeRequestWidgetStore; +})(window.gl || (window.gl = {})); diff --git a/app/assets/javascripts/vue_common_component/link_to_member_avatar.js.es6 b/app/assets/javascripts/vue_common_component/link_to_member_avatar.js.es6 new file mode 100644 index 0000000000000000000000000000000000000000..f36591c6181cd9c506b22d3d76f133a8edafc3e6 --- /dev/null +++ b/app/assets/javascripts/vue_common_component/link_to_member_avatar.js.es6 @@ -0,0 +1,92 @@ +/* global Vue */ +// Analogue of link_to_member_avatar in app/helpers/projects_helper.rb + +(() => { + Vue.component('link-to-member-avatar', { + props: { + avatarUrl: { + type: String, + required: false, + default: '/assets/no_avatar.png', + }, + profileUrl: { + type: String, + required: false, + default: '', + }, + displayName: { + type: String, + required: false, + }, + extraAvatarClass: { + type: String, + default: '', + required: false, + }, + extraLinkClass: { + type: String, + default: '', + required: false, + }, + showTooltip: { + type: Boolean, + required: false, + default: true, + }, + clickable: { + type: Boolean, + default: true, + required: false, + }, + tooltipContainer: { + type: String, + required: false, + }, + avatarHtml: { + type: String, + required: false, + }, + avatarSize: { + type: Number, + required: false, + default: 32, + }, + }, + data() { + return { + avatarBaseClass: 'avatar avatar-inline', + }; + }, + computed: { + avatarSizeClass() { + return `s${this.avatarSize}`; + }, + avatarHtmlClass() { + return `${this.avatarSizeClass} ${this.avatarBaseClass}`; + }, + tooltipClass() { + return this.showTooltip ? 'has-tooltip' : ''; + }, + avatarClass() { + return `${this.avatarBaseClass} ${this.avatarSizeClass} ${this.extraAvatarClass}`; + }, + disabledClass() { + return !this.clickable ? 'disabled' : ''; + }, + linkClass() { + return `author_link ${this.tooltipClass} ${this.extraLinkClass} ${this.disabledClass}`; + }, + tooltipContainerAttr() { + return this.tooltipContainer || 'body'; + }, + }, + template: ` +
+ + + + +
+ `, + }); +})(); diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss index 45ff9f7ff5f34343bff8f0625270953273a680a0..e6b66036f9496b5ec0acd5a825d1e92830f33646 100644 --- a/app/assets/stylesheets/pages/merge_requests.scss +++ b/app/assets/stylesheets/pages/merge_requests.scss @@ -440,3 +440,93 @@ padding-right: 0; } } + +#merge-request-widget-app .loading { + padding-top: 5px; +} + +#merge-request-widget-app .loading, +.approvals-components { + border-top: 1px solid $well-inner-border; +} + +.approvals-body { + @media (max-width: $screen-xs-max) { + text-align: center; + } + + .approve-btn { + margin-top: 10px; + } +} + +.approvals-footer { + display: flex; + + // vertically centers all children + > span { + align-self: center; + } + + .hide-asset { + img { + display: none; + } + + svg { + margin-bottom: -7px; // makes up for border removed + border: none; + } + } + + .approvers-prefix { + margin-right: 5px; + } + + .unapprove-btn-wrap { + border-left: 1px solid $gray-darker; + padding-left: 5px; + margin-left: 10px; + } + + .unapprove-btn { + border: none; + background: transparent; + cursor: pointer; + + &:hover { + color: $gl-text-color-secondary; + text-decoration: none; + } + + &:focus { + outline: none; + } + } + + // styles for approver avatar checkmark + .approver-avatar { + position: relative; + display: inline-block; + + svg.avatar { + position: absolute; + top: 12%; + right: 4%; + height: 45%; + width: 45%; + } + } +} + +.link-to-member-avatar { + .disabled { + pointer-events: none; + cursor: default; + } + + .avatar { + margin-bottom: -2px; + margin-right: 3px; + } +} diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 44cd2466d483f98ce0760681fec6a3bb42b4be5a..0d0f81f93ce402b8c4cd8040bd821b9534089105 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -11,7 +11,8 @@ class Projects::MergeRequestsController < Projects::ApplicationController before_action :merge_request, only: [ :edit, :update, :show, :diffs, :commits, :conflicts, :conflict_for_path, :pipelines, :merge, :merge_check, :ci_status, :ci_environments_status, :toggle_subscription, :cancel_merge_when_build_succeeds, :remove_wip, :resolve_conflicts, :assign_related_issues, - :approve, :rebase + # EE + :approve, :approvals, :unapprove, :rebase ] before_action :validates_merge_request, only: [:show, :diffs, :commits, :pipelines] before_action :define_show_vars, only: [:show, :diffs, :commits, :conflicts, :conflict_for_path, :builds, :pipelines] @@ -481,15 +482,38 @@ def approve return render_404 end - MergeRequests::ApprovalService. + ::MergeRequests::ApprovalService. new(project, current_user). execute(@merge_request) - redirect_to merge_request_path(@merge_request) + render_approvals_json + end + + def approvals + render_approvals_json + end + + def unapprove + if @merge_request.has_approved?(current_user) + ::MergeRequests::RemoveApprovalService. + new(project, current_user). + execute(@merge_request) + end + + render_approvals_json end protected + def render_approvals_json + respond_to do |format| + format.json do + entity = API::Entities::MergeRequestApprovals.new(@merge_request, current_user: current_user) + render json: entity + end + end + end + def selected_target_project if @project.id.to_s == params[:target_project_id] || @project.forked_project_link.nil? @project diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb index 759d85fdb318e88559f516a2841e50142c409074..568dabb1e4b5591e7f4ed07053490dc037d69684 100644 --- a/app/helpers/merge_requests_helper.rb +++ b/app/helpers/merge_requests_helper.rb @@ -90,6 +90,7 @@ def render_items_list(items, separator = "and") end end + # This may be able to be removed with associated specs def render_require_section(merge_request) str = if merge_request.approvals_left == 1 "Requires one more approval" diff --git a/app/mailers/emails/merge_requests.rb b/app/mailers/emails/merge_requests.rb index 5bd8c37ed7b3ae08d3dfd9989e22f613923e7088..73dea209b88879b5853172781a18f0b9dc3054ce 100644 --- a/app/mailers/emails/merge_requests.rb +++ b/app/mailers/emails/merge_requests.rb @@ -54,11 +54,18 @@ def add_merge_request_approver_email(recipient_id, merge_request_id, updated_by_ mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, recipient_id)) end - def approved_merge_request_email(recipient_id, merge_request_id, updated_by_user_id) + def approved_merge_request_email(recipient_id, merge_request_id, approved_by_user_id) setup_merge_request_mail(merge_request_id, recipient_id) - @approved_by_users = @merge_request.approved_by_users.map(&:name) - mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, recipient_id)) + @approved_by = User.find(approved_by_user_id) + mail_answer_thread(@merge_request, merge_request_thread_options(approved_by_user_id, recipient_id)) + end + + def unapproved_merge_request_email(recipient_id, merge_request_id, unapproved_by_user_id) + setup_merge_request_mail(merge_request_id, recipient_id) + + @unapproved_by = User.find(unapproved_by_user_id) + mail_answer_thread(@merge_request, merge_request_thread_options(unapproved_by_user_id, recipient_id)) end def resolved_all_discussions_email(recipient_id, merge_request_id, resolved_by_user_id) diff --git a/app/models/concerns/approvable.rb b/app/models/concerns/approvable.rb index 561a38392766b5e4ff7536ee2b728c6f4872839d..a34eac68f23ebc42d7d4ecd8fb507f5fc75b95d7 100644 --- a/app/models/concerns/approvable.rb +++ b/app/models/concerns/approvable.rb @@ -123,6 +123,12 @@ def can_approve?(user) any_approver_allowed? && approvals.where(user: user).empty? end + def has_approved?(user) + return false unless user + + approved_by_users.include?(user) + end + # Once there are fewer approvers left in the list than approvals required, allow other # project members to approve the MR. # diff --git a/app/services/merge_requests/remove_approval_service.rb b/app/services/merge_requests/remove_approval_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..8ae6651e59bdd11134c0517f21d4d5046dbd92b2 --- /dev/null +++ b/app/services/merge_requests/remove_approval_service.rb @@ -0,0 +1,31 @@ +module MergeRequests + class RemoveApprovalService < MergeRequests::BaseService + def execute(merge_request) + # paranoid protection against running wrong deletes + return unless merge_request.id && current_user.id + + approval = merge_request.approvals.where(user: current_user) + + currently_approved = merge_request.approved? + + if approval.destroy_all + # bust the cache here, otherwise will show results from + # before the deletion + merge_request.approvals(true) + + create_note(merge_request) + + if currently_approved + notification_service.unapprove_mr(merge_request, current_user) + execute_hooks(merge_request, 'unapproved') + end + end + end + + private + + def create_note(merge_request) + SystemNoteService.unapprove_mr(merge_request, current_user) + end + end +end diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index 89396156d0c66fc779f67fc43978aef12d543c6c..34982ff416795baf80a03bb9229bfcb47ac6ff01 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -162,6 +162,10 @@ def approve_mr(merge_request, current_user) approve_mr_email(merge_request, merge_request.target_project, current_user) end + def unapprove_mr(merge_request, current_user) + unapprove_mr_email(merge_request, merge_request.target_project, current_user) + end + def resolve_all_discussions(merge_request, current_user) recipients = build_recipients(merge_request, merge_request.target_project, current_user, action: "resolve_all_discussions") @@ -608,6 +612,14 @@ def approve_mr_email(merge_request, project, current_user) end end + def unapprove_mr_email(merge_request, project, current_user) + recipients = build_recipients(merge_request, project, current_user) + + recipients.each do |recipient| + mailer.unapproved_merge_request_email(recipient.id, merge_request.id, current_user.id).deliver_later + end + end + def add_mr_approvers_email(merge_request, approvers, current_user) approvers.each do |approver| recipient = approver.user diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb index c6c9d006809f17538b85a2be93871e762b7de1f1..9e6a3c8f31d566159691297d74259fc0bac26638 100644 --- a/app/services/system_note_service.rb +++ b/app/services/system_note_service.rb @@ -474,6 +474,11 @@ def approve_mr(noteable, user) create_note(noteable: noteable, project: noteable.project, author: user, note: body) end + def unapprove_mr(noteable, user) + body = "Unapproved this merge request" + create_note(noteable: noteable, project: noteable.project, author: user, note: body) + end + private def notes_for_mentioner(mentioner, noteable, notes) diff --git a/app/views/notify/approved_merge_request_email.html.haml b/app/views/notify/approved_merge_request_email.html.haml index de2c3818494f0cfdd9512d32439e52f64c1db515..16b9df65f5ec120ce975f0dca94c1aa6d8cb2221 100644 --- a/app/views/notify/approved_merge_request_email.html.haml +++ b/app/views/notify/approved_merge_request_email.html.haml @@ -1,2 +1,146 @@ -%p - = "Merge Request #{@merge_request.to_reference} was approved by #{@approved_by_users.to_sentence}" + +%html{ lang: "en" } + %head + %meta{ content: "text/html; charset=UTF-8", "http-equiv" => "Content-Type" }/ + %meta{ content: "width=device-width, initial-scale=1", name: "viewport" }/ + %meta{ content: "IE=edge", "http-equiv" => "X-UA-Compatible" }/ + %title= message.subject + :css + /* CLIENT-SPECIFIC STYLES */ + body, table, td, a { -webkit-text-size-adjust: 100%; -ms-text-size-adjust: 100%; } + table, td { mso-table-lspace: 0pt; mso-table-rspace: 0pt; } + img { -ms-interpolation-mode: bicubic; } + + /* iOS BLUE LINKS */ + a[x-apple-data-detectors] { + color: inherit !important; + text-decoration: none !important; + font-size: inherit !important; + font-family: inherit !important; + font-weight: inherit !important; + line-height: inherit !important; + } + + /* ANDROID MARGIN HACK */ + body { margin:0 !important; } + div[style*="margin: 16px 0"] { margin:0 !important; } + + @media only screen and (max-width: 639px) { + body, #body { + min-width: 320px !important; + } + table.wrapper { + width: 100% !important; + min-width: 320px !important; + } + table.wrapper > tbody > tr > td { + border-left: 0 !important; + border-right: 0 !important; + border-radius: 0 !important; + padding-left: 10px !important; + padding-right: 10px !important; + } + } + %body{ style: "background-color:#fafafa;margin:0;padding:0;text-align:center;min-width:640px;width:100%;height:100%;font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;" } + %table#body{ border: "0", cellpadding: "0", cellspacing: "0", style: "background-color:#fafafa;margin:0;padding:0;text-align:center;min-width:640px;width:100%;" } + %tbody + %tr.line + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;background-color:#6b4fbb;height:4px;font-size:4px;line-height:4px;" } + %tr.header + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;padding:25px 0;font-size:13px;line-height:1.6;color:#5c5c5c;" } + %img{ alt: "GitLab", height: "50", src: image_url('mailers/ci_pipeline_notif_v1/gitlab-logo.gif'), width: "55" }/ + %tr + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;" } + %table.wrapper{ border: "0", cellpadding: "0", cellspacing: "0", style: "width:640px;margin:0 auto;border-collapse:separate;border-spacing:0;" } + %tbody + %tr + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;background-color:#ffffff;text-align:left;padding:18px 25px;border:1px solid #ededed;border-radius:3px;overflow:hidden;" } + %table.content{ border: "0", cellpadding: "0", cellspacing: "0", style: "width:100%;border-collapse:separate;border-spacing:0;" } + %tbody + %tr.success + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;padding:10px;border-radius:3px;font-size:14px;line-height:1.3;text-align:center;overflow:hidden;color:#ffffff;background-color:#31af64;" } + %table.img{ border: "0", cellpadding: "0", cellspacing: "0", style: "border-collapse:collapse;margin:0 auto;" } + %tbody + %tr + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;vertical-align:middle;color:#ffffff;text-align:center;padding-right:5px;" } + %img{ alt: "✓", height: "13", src: image_url('mailers/ci_pipeline_notif_v1/icon-check-green-inverted.gif'), style: "display:block;", width: "13" }/ + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;vertical-align:middle;color:#ffffff;text-align:center;" } + %span Merge request was approved (#{@merge_request.approvals.count}/#{@merge_request.approvals_required}) + %tr.spacer + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;height:18px;font-size:18px;line-height:18px;" } +   + %tr.section + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;line-height:1.4;text-align:center;padding:0 15px;border:1px solid #ededed;border-radius:3px;overflow:hidden;" } + %table.img{ border: "0", cellpadding: "0", cellspacing: "0", style: "border-collapse:collapse;width:100%;" } + %tbody + %tr{ style: 'width:100%;' } + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;color:#8c8c8c;font-weight:300;padding:14px 0;margin:0;text-align:center;" } + %img{ src: image_url('mailers/approval/icon-merge-request-gray.gif'), style: "height:18px;width:18px;margin-bottom:-4px;", alt: "Merge request icon" } + %span{ style: "font-weight: bold;color:#333333;" } Merge request + %a{ href: merge_request_url(@merge_request), style: "font-weight: bold;color:#3777b0;text-decoration:none" } #{@merge_request.to_reference} + %span was approved by + %img.avatar{ height: "24", src: avatar_icon(@approved_by, 24), style: "border-radius:12px;margin:-7px 0 -7px 3px;", width: "24", alt: "Avatar" }/ + %a.muted{ href: user_url(@approved_by), style: "color:#333333;text-decoration:none;" } + = @approved_by.name + %tr.spacer + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;height:18px;font-size:18px;line-height:18px;" } +   + %tr.section + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;padding:0 15px;border:1px solid #ededed;border-radius:3px;overflow:hidden;" } + %table.info{ border: "0", cellpadding: "0", cellspacing: "0", style: "width:100%;" } + %tbody + %tr + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;color:#8c8c8c;font-weight:300;padding:14px 0;margin:0;" } Project + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;color:#8c8c8c;font-weight:300;padding:14px 0;margin:0;color:#333333;font-weight:400;width:75%;padding-left:5px;" } + - namespace_name = @project.group ? @project.group.name : @project.namespace.owner.name + - namespace_url = @project.group ? group_url(@project.group) : user_url(@project.namespace.owner) + %a.muted{ href: namespace_url, style: "color:#333333;text-decoration:none;" } + = namespace_name + \/ + %a.muted{ href: project_url(@project), style: "color:#333333;text-decoration:none;" } + = @project.name + %tr + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;color:#8c8c8c;font-weight:300;padding:14px 0;margin:0;border-top:1px solid #ededed;" } Branch + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;color:#8c8c8c;font-weight:300;padding:14px 0;margin:0;color:#333333;font-weight:400;width:75%;padding-left:5px;border-top:1px solid #ededed;" } + %table.img{ border: "0", cellpadding: "0", cellspacing: "0", style: "border-collapse:collapse;" } + %tbody + %tr + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;padding-right:5px;" } + %img{ height: "13", src: image_url('mailers/ci_pipeline_notif_v1/icon-branch-gray.gif'), style: "display:block;", width: "13", alt: "Branch icon" }/ + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;" } + %span.muted{ style: "color:#333333;text-decoration:none;" } + = @merge_request.source_branch + %tr + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;color:#8c8c8c;font-weight:300;padding:14px 0;margin:0;border-top:1px solid #ededed;" } Author + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;color:#8c8c8c;font-weight:300;padding:14px 0;margin:0;color:#333333;font-weight:400;width:75%;padding-left:5px;border-top:1px solid #ededed;" } + %table.img{ border: "0", cellpadding: "0", cellspacing: "0", style: "border-collapse:collapse;" } + %tbody + %tr + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;padding-right:5px;" } + %img.avatar{ height: "24", src: avatar_icon(@merge_request.author, 24), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "Avatar" }/ + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;" } + %a.muted{ href: user_url(@merge_request.author), style: "color:#333333;text-decoration:none;" } + = @merge_request.author.name + %tr + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;color:#8c8c8c;font-weight:300;padding:14px 0;margin:0;border-top:1px solid #ededed;" } Assignee + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;color:#8c8c8c;font-weight:300;padding:14px 0;margin:0;color:#333333;font-weight:400;width:75%;padding-left:5px;border-top:1px solid #ededed;" } + %table.img{ border: "0", cellpadding: "0", cellspacing: "0", style: "border-collapse:collapse;" } + %tbody + %tr + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;padding-right:5px;" } + %img.avatar{ height: "24", src: avatar_icon(@merge_request.assignee, 24), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "Avatar" }/ + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;" } + %a.muted{ href: user_url(@merge_request.assignee), style: "color:#333333;text-decoration:none;" } + = @merge_request.assignee.name + + %tr.footer + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;padding:25px 0;font-size:13px;line-height:1.6;color:#5c5c5c;" } + %img{ alt: "GitLab", height: "33", src: image_url('mailers/ci_pipeline_notif_v1/gitlab-logo-full-horizontal.gif'), style: "display:block;margin:0 auto 1em;", width: "90" }/ + %div + %a{ href: profile_notifications_url, style: "color:#3777b0;text-decoration:none;" } Manage all notifications + · + %a{ href: help_url, style: "color:#3777b0;text-decoration:none;" } Help + %div + You're receiving this email because of your account on + = succeed "." do + %a{ href: root_url, style: "color:#3777b0;text-decoration:none;" }= Gitlab.config.gitlab.host diff --git a/app/views/notify/approved_merge_request_email.text.haml b/app/views/notify/approved_merge_request_email.text.haml index 5038a70e3864f9510ceab266e752994e507f1bfa..1b7af953b102d409ffcece988cd050d706c35687 100644 --- a/app/views/notify/approved_merge_request_email.text.haml +++ b/app/views/notify/approved_merge_request_email.text.haml @@ -1,4 +1,4 @@ -= "Merge Request #{@merge_request.to_reference} was approved by #{@approved_by_users.to_sentence}" += "Merge Request #{@merge_request.to_reference} was approved by #{@approved_by_user}" Merge Request url: #{namespace_project_merge_request_url(@merge_request.target_project.namespace, @merge_request.target_project, @merge_request)} diff --git a/app/views/notify/unapproved_merge_request_email.html.haml b/app/views/notify/unapproved_merge_request_email.html.haml new file mode 100644 index 0000000000000000000000000000000000000000..b7c5e9a89a96d49a7d3f4b92aa32fda74e8c2872 --- /dev/null +++ b/app/views/notify/unapproved_merge_request_email.html.haml @@ -0,0 +1,146 @@ + +%html{ lang: "en" } + %head + %meta{ content: "text/html; charset=UTF-8", "http-equiv" => "Content-Type" }/ + %meta{ content: "width=device-width, initial-scale=1", name: "viewport" }/ + %meta{ content: "IE=edge", "http-equiv" => "X-UA-Compatible" }/ + %title= message.subject + :css + /* CLIENT-SPECIFIC STYLES */ + body, table, td, a { -webkit-text-size-adjust: 100%; -ms-text-size-adjust: 100%; } + table, td { mso-table-lspace: 0pt; mso-table-rspace: 0pt; } + img { -ms-interpolation-mode: bicubic; } + + /* iOS BLUE LINKS */ + a[x-apple-data-detectors] { + color: inherit !important; + text-decoration: none !important; + font-size: inherit !important; + font-family: inherit !important; + font-weight: inherit !important; + line-height: inherit !important; + } + + /* ANDROID MARGIN HACK */ + body { margin:0 !important; } + div[style*="margin: 16px 0"] { margin:0 !important; } + + @media only screen and (max-width: 639px) { + body, #body { + min-width: 320px !important; + } + table.wrapper { + width: 100% !important; + min-width: 320px !important; + } + table.wrapper > tbody > tr > td { + border-left: 0 !important; + border-right: 0 !important; + border-radius: 0 !important; + padding-left: 10px !important; + padding-right: 10px !important; + } + } + %body{ style: "background-color:#fafafa;margin:0;padding:0;text-align:center;min-width:640px;width:100%;height:100%;font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;" } + %table#body{ border: "0", cellpadding: "0", cellspacing: "0", style: "background-color:#fafafa;margin:0;padding:0;text-align:center;min-width:640px;width:100%;" } + %tbody + %tr.line + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;background-color:#6b4fbb;height:4px;font-size:4px;line-height:4px;" } + %tr.header + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;padding:25px 0;font-size:13px;line-height:1.6;color:#5c5c5c;" } + %img{ alt: "GitLab", height: "50", src: image_url('mailers/ci_pipeline_notif_v1/gitlab-logo.gif'), width: "55" }/ + %tr + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;" } + %table.wrapper{ border: "0", cellpadding: "0", cellspacing: "0", style: "width:640px;margin:0 auto;border-collapse:separate;border-spacing:0;" } + %tbody + %tr + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;background-color:#ffffff;text-align:left;padding:18px 25px;border:1px solid #ededed;border-radius:3px;overflow:hidden;" } + %table.content{ border: "0", cellpadding: "0", cellspacing: "0", style: "width:100%;border-collapse:separate;border-spacing:0;" } + %tbody + %tr.success + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;padding:10px;border-radius:3px;font-size:14px;line-height:1.3;text-align:center;overflow:hidden;color:#ffffff;background-color:#FC6D26;" } + %table.img{ border: "0", cellpadding: "0", cellspacing: "0", style: "border-collapse:collapse;margin:0 auto;" } + %tbody + %tr + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;vertical-align:middle;color:#ffffff;text-align:center;padding-right:5px;" } + %img{ alt: "✗", height: "13", src: image_url('mailers/approval/icon-x-orange-inverted.gif'), style: "display:block;", width: "13" }/ + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;vertical-align:middle;color:#ffffff;text-align:center;" } + %span Merge request was unapproved (#{@merge_request.approvals.count}/#{@merge_request.approvals_required}) + %tr.spacer + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;height:18px;font-size:18px;line-height:18px;" } +   + %tr.section + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;line-height:1.4;text-align:center;padding:0 15px;border:1px solid #ededed;border-radius:3px;overflow:hidden;" } + %table.img{ border: "0", cellpadding: "0", cellspacing: "0", style: "border-collapse:collapse;width:100%;" } + %tbody + %tr{ style: 'width:100%;' } + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;color:#8c8c8c;font-weight:300;padding:14px 0;margin:0;text-align:center;" } + %img{ src: image_url('mailers/approval/icon-merge-request-gray.gif'), style: "height:18px;width:18px;margin-bottom:-4px;", alt: "Merge request icon" } + %span{ style: "font-weight: bold;color:#333333;" } Merge request + %a{ href: merge_request_url(@merge_request), style: "font-weight: bold;color:#3777b0;text-decoration:none" } #{@merge_request.to_reference} + %span was unapproved by + %img.avatar{ height: "24", src: avatar_icon(@unapproved_by, 24), style: "border-radius:12px;margin:-7px 0 -7px 3px;", width: "24", alt: "Avatar" }/ + %a.muted{ href: user_url(@unapproved_by), style: "color:#333333;text-decoration:none;" } + = @unapproved_by.name + %tr.spacer + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;height:18px;font-size:18px;line-height:18px;" } +   + %tr.section + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;padding:0 15px;border:1px solid #ededed;border-radius:3px;overflow:hidden;" } + %table.info{ border: "0", cellpadding: "0", cellspacing: "0", style: "width:100%;" } + %tbody + %tr + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;color:#8c8c8c;font-weight:300;padding:14px 0;margin:0;" } Project + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;color:#8c8c8c;font-weight:300;padding:14px 0;margin:0;color:#333333;font-weight:400;width:75%;padding-left:5px;" } + - namespace_name = @project.group ? @project.group.name : @project.namespace.owner.name + - namespace_url = @project.group ? group_url(@project.group) : user_url(@project.namespace.owner) + %a.muted{ href: namespace_url, style: "color:#333333;text-decoration:none;" } + = namespace_name + \/ + %a.muted{ href: project_url(@project), style: "color:#333333;text-decoration:none;" } + = @project.name + %tr + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;color:#8c8c8c;font-weight:300;padding:14px 0;margin:0;border-top:1px solid #ededed;" } Branch + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;color:#8c8c8c;font-weight:300;padding:14px 0;margin:0;color:#333333;font-weight:400;width:75%;padding-left:5px;border-top:1px solid #ededed;" } + %table.img{ border: "0", cellpadding: "0", cellspacing: "0", style: "border-collapse:collapse;" } + %tbody + %tr + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;padding-right:5px;" } + %img{ height: "13", src: image_url('mailers/ci_pipeline_notif_v1/icon-branch-gray.gif'), style: "display:block;", width: "13", alt: "Branch icon" }/ + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;" } + %span.muted{ style: "color:#333333;text-decoration:none;" } + = @merge_request.source_branch + %tr + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;color:#8c8c8c;font-weight:300;padding:14px 0;margin:0;border-top:1px solid #ededed;" } Author + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;color:#8c8c8c;font-weight:300;padding:14px 0;margin:0;color:#333333;font-weight:400;width:75%;padding-left:5px;border-top:1px solid #ededed;" } + %table.img{ border: "0", cellpadding: "0", cellspacing: "0", style: "border-collapse:collapse;" } + %tbody + %tr + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;padding-right:5px;" } + %img.avatar{ height: "24", src: avatar_icon(@merge_request.author, 24), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "Avatar" }/ + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;" } + %a.muted{ href: user_url(@merge_request.author), style: "color:#333333;text-decoration:none;" } + = @merge_request.author.name + %tr + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;color:#8c8c8c;font-weight:300;padding:14px 0;margin:0;border-top:1px solid #ededed;" } Assignee + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;color:#8c8c8c;font-weight:300;padding:14px 0;margin:0;color:#333333;font-weight:400;width:75%;padding-left:5px;border-top:1px solid #ededed;" } + %table.img{ border: "0", cellpadding: "0", cellspacing: "0", style: "border-collapse:collapse;" } + %tbody + %tr + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;padding-right:5px;" } + %img.avatar{ height: "24", src: avatar_icon(@merge_request.assignee, 24), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "Avatar" }/ + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;" } + %a.muted{ href: user_url(@merge_request.assignee), style: "color:#333333;text-decoration:none;" } + = @merge_request.assignee.name + + %tr.footer + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;padding:25px 0;font-size:13px;line-height:1.6;color:#5c5c5c;" } + %img{ alt: "GitLab", height: "33", src: image_url('mailers/ci_pipeline_notif_v1/gitlab-logo-full-horizontal.gif'), style: "display:block;margin:0 auto 1em;", width: "90" }/ + %div + %a{ href: profile_notifications_url, style: "color:#3777b0;text-decoration:none;" } Manage all notifications + · + %a{ href: help_url, style: "color:#3777b0;text-decoration:none;" } Help + %div + You're receiving this email because of your account on + = succeed "." do + %a{ href: root_url, style: "color:#3777b0;text-decoration:none;" }= Gitlab.config.gitlab.host diff --git a/app/views/notify/unapproved_merge_request_email.text.haml b/app/views/notify/unapproved_merge_request_email.text.haml new file mode 100644 index 0000000000000000000000000000000000000000..7e31b2f45e6c2ae3605af76aeb33b06dfc9c45d0 --- /dev/null +++ b/app/views/notify/unapproved_merge_request_email.text.haml @@ -0,0 +1,8 @@ += "Merge Request #{@merge_request.to_reference} was unapproved by #{@unapproved_by_user}" + +Merge Request url: #{namespace_project_merge_request_url(@merge_request.target_project.namespace, @merge_request.target_project, @merge_request)} + += merge_path_description(@merge_request, 'to') + +Author: #{@merge_request.author_name} +Assignee: #{@merge_request.assignee_name} diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml index 0516801cdf38fffa7afb7d478d101ac37e66c2ab..24ca7bc3fd111967aea0939a95b7a676c7171f28 100644 --- a/app/views/projects/merge_requests/_show.html.haml +++ b/app/views/projects/merge_requests/_show.html.haml @@ -108,8 +108,8 @@ = render "projects/commit/change", type: 'cherry-pick', commit: @merge_request.merge_commit, title: @merge_request.title :javascript - var merge_request; - - merge_request = new MergeRequest({ - action: "#{controller.action_name}" + $(function () { + new MergeRequest({ + action: "#{controller.action_name}" + }); }); diff --git a/app/views/projects/merge_requests/widget/_open.html.haml b/app/views/projects/merge_requests/widget/_open.html.haml index bfc096cfe6382b2bbfe3f36083a2d1d3148908f3..a2127549a903b1cc27be1e4dc6001420039ff412 100644 --- a/app/views/projects/merge_requests/widget/_open.html.haml +++ b/app/views/projects/merge_requests/widget/_open.html.haml @@ -1,4 +1,9 @@ -.mr-state-widget +- content_for :page_specific_javascripts do + = page_specific_javascript_tag('merge_request_widget/widget_bundle.js') + +- approval_pending = @merge_request.requires_approve? && !@merge_request.approved? + +#merge-request-widget-app.mr-state-widget{ 'data-endpoint'=> merge_request_path(@merge_request), 'data-approval-pending' => approval_pending } = render 'projects/merge_requests/widget/heading' .mr-widget-body -# After conflicts are resolved, the user is redirected back to the MR page. @@ -23,8 +28,6 @@ = render 'projects/merge_requests/widget/open/conflicts' - elsif @merge_request.work_in_progress? = render 'projects/merge_requests/widget/open/wip' - - elsif @merge_request.requires_approve? && !@merge_request.approved? - = render 'projects/merge_requests/widget/open/approve' - elsif @merge_request.merge_when_build_succeeds? = render 'projects/merge_requests/widget/open/merge_when_build_succeeds' - elsif !@merge_request.can_be_merged_by?(current_user) @@ -52,8 +55,12 @@ != markdown issues_sentence(mr_issues_mentioned_but_not_closing), pipeline: :gfm, author: @merge_request.author #{mr_issues_mentioned_but_not_closing.size > 1 ? 'are' : 'is'} mentioned but will not be closed. - - if @merge_request.approvals.any? - .mr-widget-footer.approved-by-users - Approved by - - @merge_request.approved_by_users.each do |user| - = link_to_member(@project, user, name: false, size: 24) + - if @merge_request.requires_approve? + .mr-widget-footer{ 'v-show' => '!showApprovals' } + = icon("spinner spin") + Checking approval status for this merge request. + .approvals-components{ 'v-show' => 'showApprovals' } + = render 'projects/merge_requests/widget/open/approvals_body' + = render 'projects/merge_requests/widget/open/approvals_footer' + + diff --git a/app/views/projects/merge_requests/widget/open/_accept.html.haml b/app/views/projects/merge_requests/widget/open/_accept.html.haml index 81125aa0dac00ccfee21e2ba8894c27a7dc15cea..5daeb6da6fdaddb910f167d5b18a68437a3116aa 100644 --- a/app/views/projects/merge_requests/widget/open/_accept.html.haml +++ b/app/views/projects/merge_requests/widget/open/_accept.html.haml @@ -28,7 +28,7 @@ = icon('warning fw') Merge Immediately - else - = f.button class: "btn btn-create btn-grouped js-merge-button accept_merge_request #{status_class}" do + = f.button class: "btn btn-create btn-grouped js-merge-button accept_merge_request #{status_class}", ':disabled' => 'disableAcceptance' do Accept Merge Request - if @merge_request.force_remove_source_branch? .accept-control diff --git a/app/views/projects/merge_requests/widget/open/_approvals_body.html.haml b/app/views/projects/merge_requests/widget/open/_approvals_body.html.haml new file mode 100644 index 0000000000000000000000000000000000000000..60d1bf166cc0f8952e2bc7d94a3a5f5c3d55e23f --- /dev/null +++ b/app/views/projects/merge_requests/widget/open/_approvals_body.html.haml @@ -0,0 +1 @@ +%approvals-body{ ':user-can-approve' => 'approvals.user_can_approve', ':user-has-approved' => 'approvals.user_has_approved', ':approved-by' => 'approvals.approved_by', ':approvals-left':'approvals.approvals_left', ':suggested-approvers' => 'approvals.suggested_approvers' } diff --git a/app/views/projects/merge_requests/widget/open/_approvals_footer.html.haml b/app/views/projects/merge_requests/widget/open/_approvals_footer.html.haml new file mode 100644 index 0000000000000000000000000000000000000000..87c584127cdc357a15de6c5269c8656285ec636d --- /dev/null +++ b/app/views/projects/merge_requests/widget/open/_approvals_footer.html.haml @@ -0,0 +1 @@ +%approvals-footer{ 'pending-avatar-svg' => custom_icon('icon_dotted_circle'), 'checkmark-svg' => custom_icon('icon_checkmark'), ':user-can-approve' => 'approvals.user_can_approve', ':user-has-approved' => 'approvals.user_has_approved', ':approved-by' => 'approvals.approved_by', ':approvals-left':'approvals.approvals_left' } diff --git a/app/views/projects/merge_requests/widget/open/_approve.html.haml b/app/views/projects/merge_requests/widget/open/_approve.html.haml deleted file mode 100644 index c762c8597ba43e01d9fd4f282b7ed672455846a3..0000000000000000000000000000000000000000 --- a/app/views/projects/merge_requests/widget/open/_approve.html.haml +++ /dev/null @@ -1,7 +0,0 @@ -%div - %h4 - = render_require_section(@merge_request) - - if @merge_request.can_approve?(current_user) - .append-bottom-10 - = form_for [:approve, @project.namespace.becomes(Namespace), @project, @merge_request], method: :post do |f| - = f.submit "Approve Merge Request", class: "btn btn-primary approve-btn" diff --git a/app/views/projects/merge_requests/widget/open/_rebase.html.haml b/app/views/projects/merge_requests/widget/open/_rebase.html.haml index d6d804cb32a33ac7e142fb9ba0e87c63a3cbd8d4..e524f05a7c836e038303c6e4f8bbf48bde8cb274 100644 --- a/app/views/projects/merge_requests/widget/open/_rebase.html.haml +++ b/app/views/projects/merge_requests/widget/open/_rebase.html.haml @@ -1,14 +1,13 @@ +- content_for :page_specific_javascripts do + = page_specific_javascript_tag('merge_request_widget/ci_bundle.js') + - if @merge_request.rebase_in_progress? || (defined?(rebase_in_progress) && rebase_in_progress) - %h4 + %h4.rebase-in-progress = icon("spinner spin") Rebase in progress… %p This merge request is in the process of being rebased. - :javascript - $(function() { - merge_request_widget.rebaseInProgress() - }); - elsif !can_push_branch?(@merge_request.source_project, @merge_request.source_branch) %h4 = icon("exclamation-triangle") @@ -26,12 +25,3 @@ Rebase onto #{@merge_request.target_branch} .accept-control Fast-forward merge is not possible. Rebase the source branch onto the target branch or merge target branch into source branch to allow this merge request to be merged. - - :javascript - $('.rebase-mr-form').on('ajax:send', function() { - $('.rebase-mr-form :input').disable(); - }); - - $('.js-rebase-button').on('click', function() { - $('.js-rebase-button').html(" Rebase in progress"); - }); diff --git a/app/views/shared/icons/_icon_checkmark.svg b/app/views/shared/icons/_icon_checkmark.svg new file mode 100644 index 0000000000000000000000000000000000000000..d01e24d4b68db82a0c707c32d937904d87d90ce2 --- /dev/null +++ b/app/views/shared/icons/_icon_checkmark.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/app/views/shared/icons/_icon_dotted_circle.svg b/app/views/shared/icons/_icon_dotted_circle.svg new file mode 100644 index 0000000000000000000000000000000000000000..fee95e1cab5ccbbec957f2ea25f60bc47665ca3f --- /dev/null +++ b/app/views/shared/icons/_icon_dotted_circle.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/config/application.rb b/config/application.rb index 183b546d17a75fb66ec5d9041f18b3067fd4cf86..a08b30174a922c5d2fae205927bfd92e2ecf5bcb 100644 --- a/config/application.rb +++ b/config/application.rb @@ -101,6 +101,9 @@ class Application < Rails::Application config.assets.precompile << "profile/profile_bundle.js" config.assets.precompile << "protected_branches/protected_branches_bundle.js" config.assets.precompile << "diff_notes/diff_notes_bundle.js" + config.assets.precompile << "lib/vue_resource.js" + config.assets.precompile << "merge_request_widget/widget_bundle.js" + config.assets.precompile << "merge_request_widget/ci_bundle.js" config.assets.precompile << "issuable/issuable_bundle.js" config.assets.precompile << "merge_request_widget/ci_bundle.js" config.assets.precompile << "boards/boards_bundle.js" diff --git a/config/routes/project.rb b/config/routes/project.rb index 9a9ecae4fc8ae6f4740c2c55a43ba13646ddb957..b819d6c62ad0009f621a572e8ae408c04b0a873b 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -106,7 +106,10 @@ post :toggle_subscription ## EE-specific - post :approve + get :approvals + post :approvals, action: :approve + delete :approvals, action: :unapprove + post :rebase ## EE-specific diff --git a/features/project/merge_requests.feature b/features/project/merge_requests.feature index eeaec2f3ec87ff53cb9056ddf4cdb499a13cac78..f6c101189d453edacbb51167e6aef388d3556696 100644 --- a/features/project/merge_requests.feature +++ b/features/project/merge_requests.feature @@ -329,19 +329,21 @@ Feature: Project Merge Requests And I click link "Close" Then I should see closed merge request "Bug NS-04" + @javascript Scenario: Developer can approve merge request Given I am a "Shop" developer And I visit project "Shop" merge requests page And merge request 'Bug NS-04' must be approved And I click link "Bug NS-04" - And I should not see merge button + And I should see the merge button disabled When I click link "Approve" Then I should see approved merge request "Bug NS-04" + @javascript Scenario: I can not approve merge request if I am not an approver Given merge request 'Bug NS-04' must be approved by some user And I click link "Bug NS-04" - And I should not see merge button + And I should see the merge button disabled When I should not see Approve button And I should see message that MR require an approval diff --git a/features/steps/project/merge_requests.rb b/features/steps/project/merge_requests.rb index 1058697f0411c8e8704245a797f7ca0967479a36..6b528ea6beb98e325dd8f0698911d47ffad43680 100644 --- a/features/steps/project/merge_requests.rb +++ b/features/steps/project/merge_requests.rb @@ -604,13 +604,20 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps step 'I click link "Approve"' do page.within '.mr-state-widget' do + wait_for_ajax click_button 'Approve Merge Request' end end + step 'I should see the merge button disabled' do + page.within '.mr-state-widget' do + expect(page).to have_button('Accept Merge Request', disabled: true) + end + end + step 'I should not see merge button' do page.within '.mr-state-widget' do - expect(page).not_to have_button("Accept Merge Request") + expect(page).not_to have_button('Accept Merge Request') end end @@ -622,7 +629,7 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps step 'I should see approved merge request "Bug NS-04"' do page.within '.mr-state-widget' do - expect(page).to have_button("Accept Merge Request") + expect(page).to have_button('Accept Merge Request', disabled: false) end end @@ -634,13 +641,13 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps step 'I should see message that MR require an approval from me' do page.within '.mr-state-widget' do - expect(page).to have_content("Requires one more approval (from #{current_user.name})") + expect(page).to have_content("Requires 1 more approval (from #{current_user.name})") end end step 'I should see message that MR require an approval' do page.within '.mr-state-widget' do - expect(page).to have_content("Requires one more approval") + expect(page).to have_content("Requires 1 more approval") end end diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 68f63fef422efde558526f7096da08e12fd4bb13..6505e7995ec0fff59047eb6c1a53a2aed54cf869 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -340,6 +340,15 @@ class MergeRequestApprovals < ProjectEntity expose :approvals_required expose :approvals_left expose :approvals, as: :approved_by, using: Entities::Approvals + expose :approvers_left, as: :suggested_approvers, using: Entities::UserBasic + + expose :user_has_approved do |merge_request, options| + merge_request.has_approved?(options[:current_user]) + end + + expose :user_can_approve do |merge_request, options| + merge_request.can_approve?(options[:current_user]) + end end class MergeRequestDiff < Grape::Entity diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index fb77691e714d7296c926e8a6ea0135001c80dd7e..bafb1e350c0e16f19e92cdf126d27ac6fb61640f 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -313,6 +313,18 @@ def handle_merge_request_errors!(errors) present merge_request, with: Entities::MergeRequestApprovals, current_user: current_user end + + delete "#{path}/unapprove" do + merge_request = user_project.merge_requests.find(params[:merge_request_id]) + + not_found! unless merge_request.has_approved?(current_user) + + ::MergeRequests::RemoveApprovalService + .new(user_project, current_user) + .execute(merge_request) + + present merge_request, with: Entities::MergeRequestApprovals, current_user: current_user + end end end end diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index 73242f7a2f19450b8ad39a605cbc52e1f7e2ee72..a20108ace92d4fea3fde395a05360107cddaa47a 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -125,6 +125,83 @@ def create_merge_request(overrides = {}) end end + context 'approvals' do + def json_response + JSON.load(response.body) + end + + let(:approver) { create(:user) } + + before do + merge_request.update_attribute :approvals_before_merge, 2 + project.team << [approver, :developer] + project.approver_ids = [user, approver].map(&:id).join(',') + end + + describe 'approve' do + before do + post :approve, + namespace_id: project.namespace.to_param, + project_id: project.to_param, + id: merge_request.iid, + format: :json + end + + it 'approves the merge request' do + expect(response).to be_success + expect(json_response['approvals_left']).to eq 1 + expect(json_response['approved_by'].size).to eq 1 + expect(json_response['approved_by'][0]['user']['username']).to eq user.username + expect(json_response['user_has_approved']).to be true + expect(json_response['user_can_approve']).to be false + expect(json_response['suggested_approvers'].size).to eq 1 + expect(json_response['suggested_approvers'][0]['username']).to eq approver.username + end + end + + describe 'approvals' do + before do + merge_request.approvals.create(user: approver) + get :approvals, + namespace_id: project.namespace.to_param, + project_id: project.to_param, + id: merge_request.iid, + format: :json + end + + it 'shows approval information' do + expect(response).to be_success + expect(json_response['approvals_left']).to eq 1 + expect(json_response['approved_by'].size).to eq 1 + expect(json_response['approved_by'][0]['user']['username']).to eq approver.username + expect(json_response['user_has_approved']).to be false + expect(json_response['user_can_approve']).to be true + expect(json_response['suggested_approvers'].size).to eq 1 + expect(json_response['suggested_approvers'][0]['username']).to eq user.username + end + end + + describe 'unapprove' do + before do + merge_request.approvals.create(user: user) + delete :unapprove, + namespace_id: project.namespace.to_param, + project_id: project.to_param, + id: merge_request.iid, + format: :json + end + + it 'unapproves the merge request' do + expect(response).to be_success + expect(json_response['approvals_left']).to eq 2 + expect(json_response['approved_by']).to be_empty + expect(json_response['user_has_approved']).to be false + expect(json_response['user_can_approve']).to be true + expect(json_response['suggested_approvers'].size).to eq 2 + end + end + end + shared_examples "loads labels" do |action| it "loads labels into the @labels variable" do get action, diff --git a/spec/features/merge_requests/approvals_spec.rb b/spec/features/merge_requests/approvals_spec.rb index d30eb17c5ccbffdf1045d17e86518bb20d86718b..cf87f0bacd2183115d28c697bd413d75e1f18245 100644 --- a/spec/features/merge_requests/approvals_spec.rb +++ b/spec/features/merge_requests/approvals_spec.rb @@ -75,7 +75,9 @@ find('.select2-results').click click_on("Submit merge request") - expect(page).to have_content("Requires one more approval (from #{other_user.name})") + + find('.approvals-components') + expect(page).to have_content("Requires 1 more approval (from #{other_user.name})") end it 'allows delete approvers group when it is set in project' do @@ -94,7 +96,10 @@ expect(page).to have_css('.approver-list li', count: 1) click_on("Submit merge request") - expect(page).not_to have_content("Requires one more approval (from #{other_user.name})") + + wait_for_ajax + find('.approvals-components') + expect(page).not_to have_content("Requires 1 more approval (from #{other_user.name})") end end @@ -123,7 +128,9 @@ find('.select2-results').click click_on("Save changes") - expect(page).to have_content("Requires one more approval") + wait_for_ajax + find('.approvals-components') + expect(page).to have_content("Requires 1 more approval") end it 'allows delete approvers group when it`s set in project' do @@ -142,7 +149,9 @@ expect(page).to have_css('.approver-list li', count: 1) click_on("Save changes") - expect(page).to have_content("Requires one more approval (from #{approver.name})") + + find('.approvals-components') + expect(page).to have_content("Requires 1 more approval (from #{approver.name})") end end end @@ -161,32 +170,57 @@ login_as(user) end - context 'when group is assigned to a project' do - it 'I am able to approve' do + context 'when group is assigned to a project', js: true do + before do create :approver_group, group: group, target: project - visit namespace_project_merge_request_path(project.namespace, project, merge_request) + end - page.within '.mr-state-widget' do - click_button 'Approve Merge Request' - end + it 'I am able to approve' do + approve_merge_request + expect(page).to have_content('Approved by') + expect(page).to have_css('.approver-avatar') + end - expect(page).to have_content("Approved by") + it 'I am able to unapprove' do + approve_merge_request + unapprove_merge_request + expect(page).to have_no_css('.approver-avatar') end end - context 'when group is assigned to a merge request' do - it 'I am able to approve' do + context 'when group is assigned to a merge request', js: true do + before do create :approver_group, group: group, target: merge_request - visit namespace_project_merge_request_path(project.namespace, project, merge_request) + end - page.within '.mr-state-widget' do - click_button 'Approve Merge Request' - end + it 'I am able to approve' do + approve_merge_request + wait_for_ajax + expect(page).to have_content('Approved by') + expect(page).to have_css('.approver-avatar') + end - expect(page).to have_content("Approved by") + it 'I am able to unapprove' do + approve_merge_request + unapprove_merge_request + expect(page).to have_no_css('.approver-avatar') end end end end + +def approve_merge_request + page.within '.mr-state-widget' do + click_button 'Approve Merge Request' + end + wait_for_ajax +end + +def unapprove_merge_request + page.within '.mr-state-widget' do + find('.unapprove-btn-wrap').click + end + wait_for_ajax +end diff --git a/spec/features/merge_requests/merge_when_pipeline_succeeds_spec.rb b/spec/features/merge_requests/merge_when_pipeline_succeeds_spec.rb index aa24a9050016ca096232142f9d708346b5101ed4..04441d1fbea4ee2ce53bc58fe90d77ba3c21b61e 100644 --- a/spec/features/merge_requests/merge_when_pipeline_succeeds_spec.rb +++ b/spec/features/merge_requests/merge_when_pipeline_succeeds_spec.rb @@ -32,7 +32,7 @@ expect(page).to have_button "Merge When Pipeline Succeeds" end - context "Merge When Pipeline Succeeds enabled" do + context "Merge When Pipeline Succeeds enabled", js: true do before do click_button "Merge When Pipeline Succeeds" end diff --git a/spec/javascripts/approvals/approvals_body_spec.js.es6 b/spec/javascripts/approvals/approvals_body_spec.js.es6 new file mode 100644 index 0000000000000000000000000000000000000000..c3baa5d274cf1a1ceadde99ab523ac32931afea7 --- /dev/null +++ b/spec/javascripts/approvals/approvals_body_spec.js.es6 @@ -0,0 +1,131 @@ +/* global Vue eslint-disable fixture */ +//= require vue +//= require underscore +//= require jquery +//= require merge_request_widget/approvals/components/approvals_body + +(() => { + gl.ApprovalsStore = { + data: {}, + initStoreOnce() { + return { + then() {}, + }; + }, + }; + + function initApprovalsBodyComponent() { + fixture.set(` +
+
+
+ `); + + this.initialData = { + suggestedApprovers: [{ name: 'Approver 1' }], + userCanApprove: false, + userHasApproved: true, + approvedBy: [], + approvalsLeft: 1, + pendingAvatarSvg: '', + checkmarkSvg: '', + }; + + const ApprovalsBodyComponent = Vue.component('approvals-body'); + + this.approvalsBody = new ApprovalsBodyComponent({ + el: '#mock-container', + propsData: this.initialData, + }); + } + + describe('Approvals Body Component', function () { + beforeEach(function () { + initApprovalsBodyComponent.call(this); + }); + + it('should correctly set component props', function () { + const approvalsBody = this.approvalsBody; + _.each(approvalsBody, (propValue, propKey) => { + if (this.initialData[propKey]) { + expect(approvalsBody[propKey]).toBe(this.initialData[propKey]); + } + }); + }); + + describe('Computed properties', function () { + describe('approvalsRequiredStringified', function () { + it('should display the correct string for 1 possible approver', function () { + const correctText = '1 more approval'; + expect(this.approvalsBody.approvalsRequiredStringified).toBe(correctText); + }); + + it('should display the correct string for 2 possible approver', function (done) { + this.approvalsBody.approvalsLeft = 2; + this.approvalsBody.suggestedApprovers.push({ name: 'Approver 2' }); + + Vue.nextTick(() => { + const correctText = '2 more approvals'; + expect(this.approvalsBody.approvalsRequiredStringified).toBe(correctText); + done(); + }); + }); + }); + + describe('approverNamesStringified', function () { + // Preceded by: Requires {1 more approval} required from _____ + it('should display the correct string for 1 possible approver name', function (done) { + const correctText = 'Approver 1'; + Vue.nextTick(() => { + expect(this.approvalsBody.approverNamesStringified).toBe(correctText); + done(); + }); + }); + + it('should display the correct string for 2 possible approver names', function (done) { + this.approvalsBody.approvalsLeft = 2; + this.approvalsBody.suggestedApprovers.push({ name: 'Approver 2' }); + + Vue.nextTick(() => { + const correctText = 'Approver 1 or Approver 2'; + expect(this.approvalsBody.approverNamesStringified).toBe(correctText); + done(); + }); + }); + + it('should display the correct string for 3 possible approver names', function (done) { + this.approvalsBody.approvalsLeft = 3; + this.approvalsBody.suggestedApprovers.push({ name: 'Approver 2' }); + this.approvalsBody.suggestedApprovers.push({ name: 'Approver 3' }); + + Vue.nextTick(() => { + const correctText = 'Approver 1, Approver 2 or Approver 3'; + expect(this.approvalsBody.approverNamesStringified).toBe(correctText); + done(); + }); + }); + }); + + describe('showApproveButton', function () { + it('should not be true when the user cannot approve', function (done) { + this.approvalsBody.userCanApprove = false; + this.approvalsBody.userHasApproved = true; + Vue.nextTick(() => { + expect(this.approvalsBody.showApproveButton).toBe(false); + done(); + }); + }); + + it('should be true when the user can approve', function (done) { + this.approvalsBody.userCanApprove = true; + this.approvalsBody.userHasApproved = false; + Vue.nextTick(() => { + expect(this.approvalsBody.showApproveButton).toBe(true); + done(); + }); + }); + }); + }); + }); +})(window.gl || (window.gl = {})); + diff --git a/spec/javascripts/approvals/approvals_footer_spec.js.es6 b/spec/javascripts/approvals/approvals_footer_spec.js.es6 new file mode 100644 index 0000000000000000000000000000000000000000..c6f98cbd25d989b430912f00d815e87696508ff5 --- /dev/null +++ b/spec/javascripts/approvals/approvals_footer_spec.js.es6 @@ -0,0 +1,72 @@ +/* global Vue eslint-disable fixture */ +//= require vue +//= require underscore +//= require jquery +//= require merge_request_widget/approvals/components/approvals_footer + +(() => { + gl.ApprovalsStore = { + data: {}, + initStoreOnce() { + return { + then() {}, + }; + }, + }; + + function initApprovalsFooterComponent() { + fixture.set(` +
+
+
+ `); + + this.initialData = { + userCanApprove: false, + userHasApproved: true, + approvedBy: [], + approvalsLeft: 1, + pendingAvatarSvg: '', + checkmarkSvg: '', + }; + + const ApprovalsFooterComponent = Vue.component('approvals-footer'); + + this.approvalsFooter = new ApprovalsFooterComponent({ + el: '#mock-container', + propsData: this.initialData, + beforeCreate() {}, + }); + } + + describe('Approvals Footer Component', function () { + beforeEach(function () { + initApprovalsFooterComponent.call(this); + }); + + it('should correctly set component props', function () { + const approvalsFooter = this.approvalsFooter; + _.each(approvalsFooter, (propValue, propKey) => { + if (this.initialData[propKey]) { + expect(approvalsFooter[propKey]).toBe(this.initialData[propKey]); + } + }); + }); + + describe('Computed properties', function () { + it('should correctly set showUnapproveButton when the user can unapprove', function () { + expect(this.approvalsFooter.showUnapproveButton).toBe(true); + }); + + it('should correctly set showUnapproveButton when the user can not unapprove', function (done) { + this.approvalsFooter.userCanApprove = true; + + Vue.nextTick(() => { + expect(this.approvalsFooter.showUnapproveButton).toBe(false); + done(); + }); + }); + }); + }); +})(window.gl || (window.gl = {})); + diff --git a/spec/javascripts/approvals/approvals_store_spec.js.es6 b/spec/javascripts/approvals/approvals_store_spec.js.es6 new file mode 100644 index 0000000000000000000000000000000000000000..585a88b65dbb63a2b7aa26102e4d97fa1b5a894b --- /dev/null +++ b/spec/javascripts/approvals/approvals_store_spec.js.es6 @@ -0,0 +1,67 @@ +//= require es6-promise.auto +//= require jquery +//= require vue +//= require vue-resource +//= require merge_request_widget/approvals/approvals_store + +$.rails = { + csrfToken() {}, +}; + +(() => { + // stand in for promise returned by api calls + const mockThenable = { + then() { + return { + catch() {}, + }; + }, + catch() {}, + }; + + const mockRootStore = { + data: {}, + rootEl: { + dataset: { + endpoint: 'gitlab/myendpoint/', + }, + }, + assignToData(key, val) { + return { key, val }; + }, + }; + + describe('Approvals Store', function () { + beforeEach(function () { + this.rootStore = mockRootStore; + this.approvalsStore = new gl.MergeRequestApprovalsStore(this.rootStore); + }); + + it('should define all needed approval api calls', function () { + expect(this.approvalsStore.fetch).toBeDefined(); + expect(this.approvalsStore.approve).toBeDefined(); + expect(this.approvalsStore.unapprove).toBeDefined(); + }); + + it('should only init the store once', function () { + spyOn(this.approvalsStore, 'fetch').and.callFake(() => mockThenable); + + this.approvalsStore.initStoreOnce(); + this.approvalsStore.initStoreOnce(); + this.approvalsStore.initStoreOnce(); + + expect(this.approvalsStore.fetch.calls.count()).toBe(1); + }); + + it('should be able to write to the rootStore', function () { + const dataToStore = { myMockData: 'string' }; + + spyOn(this.rootStore, 'assignToData'); + + this.approvalsStore.assignToRootStore('approvals', dataToStore); + + expect(this.rootStore.assignToData).toHaveBeenCalled(); + expect(this.rootStore.assignToData).toHaveBeenCalledWith('approvals', dataToStore); + }); + }); +})(window.gl || (window.gl = {})); diff --git a/spec/javascripts/labels_issue_sidebar_spec.js.es6 b/spec/javascripts/labels_issue_sidebar_spec.js.es6 index e3146559a4a44f1bf4f636091d8d8a3761ce98ba..c79992dd9293ad1b5932aae3e572e7130cc5d8bd 100644 --- a/spec/javascripts/labels_issue_sidebar_spec.js.es6 +++ b/spec/javascripts/labels_issue_sidebar_spec.js.es6 @@ -1,4 +1,4 @@ -/* eslint-disable no-new, no-plusplus, object-curly-spacing, prefer-const, semi */ +/* eslint-disable no-new, no-plusplus, object-curly-spacing, prefer-const, semi, new-cap */ /* global IssuableContext */ /* global LabelsSelect */ diff --git a/spec/javascripts/merge_request_widget_spec.js b/spec/javascripts/merge_request_widget/merge_request_widget_spec.js similarity index 100% rename from spec/javascripts/merge_request_widget_spec.js rename to spec/javascripts/merge_request_widget/merge_request_widget_spec.js diff --git a/spec/javascripts/vue_common_components/link_to_member_avatar_spec.js.es6 b/spec/javascripts/vue_common_components/link_to_member_avatar_spec.js.es6 new file mode 100644 index 0000000000000000000000000000000000000000..55f6f23a0e69bf0d88ead6d443972615dbb5588b --- /dev/null +++ b/spec/javascripts/vue_common_components/link_to_member_avatar_spec.js.es6 @@ -0,0 +1,74 @@ +/* eslint-disable */ +//= require vue +//= require jquery +//= require vue_common_component/link_to_member_avatar + +((gl) => { + function initComponent(propsData = {}) { + fixture.set(` +
+
+
+ `); + + const LinkToMembersComponent = Vue.component('link-to-member-avatar'); + + this.component = new LinkToMembersComponent({ + el: '#mock-container', + propsData, + }).$mount(); + + this.$document = $(document); + } + describe('Link To Members Components', function() { + describe('Initialization', function() { + beforeEach(function() { + const propsData = this.propsData = { + avatarSize: 32, + avatarUrl: 'myavatarurl.com', + displayName: 'mydisplayname', + extraAvatarClass: 'myextraavatarclass', + extraLinkClass: 'myextralinkclass', + showTooltip: true, + }; + initComponent.call(this, { + propsData + }); + }); + + it('should return a defined Vue component', function() { + expect(this.component).toBeDefined(); + expect(this.component.$data).toBeDefined(); + }); + + it('should have and children', function() { + const componentLink = this.component.$el.querySelector('a'); + const componentImg = componentLink.querySelector('img'); + + expect(componentLink).not.toBeNull(); + expect(componentImg).not.toBeNull(); + }); + + it('should correctly compute computed values', function(done) { + const correctVals = { + disabledClass: '', + avatarSizeClass: 's32', + avatarHtmlClass: 's32 avatar avatar-inline', + avatarClass: 'avatar avatar-inline s32 ', + tooltipClass: 'has-tooltip', + linkClass: 'author_link has-tooltip ', + tooltipContainerAttr: 'body', + }; + + Vue.nextTick(() => { + for (var computedKey in correctVals) { + const expectedVal = correctVals[computedKey]; + const actualComputed = this.component[computedKey]; + expect(actualComputed).toBe(expectedVal); + } + done(); + }); + }); + }); + }); +})(window.gl || (window.gl = {})); diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index feeaa987105eb917a959cd1e1579011a211b6c52..76bc2d0685cc4c49405f0373caabc99deafb4b15 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -405,6 +405,44 @@ end end + describe 'that are unapproved' do + let(:last_unapprover) { create(:user) } + subject { Notify.unapproved_merge_request_email(recipient.id, merge_request.id, last_unapprover.id) } + + before do + merge_request.approvals.create(user: merge_request.assignee) + end + + it_behaves_like 'a multiple recipients email' + it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do + let(:model) { merge_request } + end + it_behaves_like 'it should show Gmail Actions View Merge request link' + it_behaves_like 'an unsubscribeable thread' + + it 'is sent as the last unapprover' do + sender = subject.header[:from].addrs[0] + expect(sender.display_name).to eq(last_unapprover.name) + expect(sender.address).to eq(gitlab_sender) + end + + it 'has the correct subject' do + is_expected.to have_subject /#{merge_request.title} \(#{merge_request.to_reference}\)/ + end + + it 'contains the new status' do + is_expected.to have_body_text /unapproved/i + end + + it 'contains a link to the merge request' do + is_expected.to have_body_text /#{namespace_project_merge_request_path project.namespace, project, merge_request}/ + end + + it 'contains the names of all of the approvers' do + is_expected.to have_body_text /#{merge_request.assignee.name}/ + end + end + describe 'that are merged' do subject { Notify.merged_merge_request_email(recipient.id, merge_request.id, merge_author.id) } diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index a65cac088f6524a7d760cb4fde7737daf5d9ef82..352644ab7a53317d0d53c894475219864dd2f17a 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -745,6 +745,8 @@ def create_merge_request(approvals_before_merge) expect(json_response['approvals_required']).to eq 2 expect(json_response['approvals_left']).to eq 1 expect(json_response['approved_by'][0]['user']['username']).to eq(approver.username) + expect(json_response['user_can_approve']).to be false + expect(json_response['user_has_approved']).to be false end end @@ -773,6 +775,36 @@ def create_merge_request(approvals_before_merge) expect(response.status).to eq(201) expect(json_response['approvals_left']).to eq(1) expect(json_response['approved_by'][0]['user']['username']).to eq(approver.username) + expect(json_response['user_has_approved']).to be true + end + end + end + + describe 'DELETE :id/merge_requests/:merge_request_id/unapprove' do + before { project.update_attribute(:approvals_before_merge, 2) } + + context 'as a user who has approved the merge request' do + let(:approver) { create(:user) } + let(:unapprover) { create(:user) } + + before do + project.team << [approver, :developer] + project.team << [unapprover, :developer] + project.team << [create(:user), :developer] + merge_request.approvals.create(user: approver) + merge_request.approvals.create(user: unapprover) + + delete api("/projects/#{project.id}/merge_requests/#{merge_request.id}/unapprove", unapprover) + end + + it 'unapproves the merge request' do + expect(response.status).to eq(200) + expect(json_response['approvals_left']).to eq(1) + usernames = json_response['approved_by'].map { |u| u['user']['username'] } + expect(usernames).not_to include(unapprover.username) + expect(usernames.size).to be 1 + expect(json_response['user_has_approved']).to be false + expect(json_response['user_can_approve']).to be true end end end diff --git a/spec/services/merge_requests/remove_approval_service_spec.rb b/spec/services/merge_requests/remove_approval_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..ebfd1ed603da5cf74f2c661772c73a81032bf4d9 --- /dev/null +++ b/spec/services/merge_requests/remove_approval_service_spec.rb @@ -0,0 +1,54 @@ +require 'rails_helper' + +describe MergeRequests::RemoveApprovalService, services: true do + describe '#execute' do + let(:user) { create(:user) } + let(:merge_request) { create(:merge_request) } + let(:project) { merge_request.project } + + subject(:service) { described_class.new(project, user) } + + def execute! + service.execute(merge_request) + end + + context 'with a user who has approved' do + before do + merge_request.approvals.create(user: user) + end + + it 'removes the approval' do + expect(merge_request.approvals.size).to eq 1 + execute! + expect(merge_request.approvals).to be_empty + end + + it 'creates an unapproval note' do + expect(SystemNoteService).to receive(:unapprove_mr) + + execute! + end + + it 'does not send a notification' do + expect(Notify).not_to receive(:unapprove_mr) + + execute! + end + end + + context 'with an approved merge request' do + let(:notify) { Object.new } + + before do + merge_request.update_attribute :approvals_before_merge, 1 + merge_request.approvals.create(user: user) + allow(service).to receive(:notification_service).and_return(notify) + end + + it 'sends a notification' do + expect(notify).to receive(:unapprove_mr) + execute! + end + end + end +end