Merge branch 'approval-integration' into 'master'

MR Approvals Frontend-Backend Integration

Closes #1262, #894, and #1287

See merge request !954

.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:

app/assets/javascripts/generic_bundles/vue_resource.js.es6

+//= require vue
+//= require vue-resource

app/assets/javascripts/merge_request_widget/approvals/approvals_api.js.es6

+/* 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;
+})();

app/assets/javascripts/merge_request_widget/approvals/approvals_bundle.js.es6

+//= require ./approvals_store
+//= require ./approvals_api
+//= require_directory ./components

app/assets/javascripts/merge_request_widget/approvals/approvals_store.js.es6

+/* 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 = {}));
+

app/assets/javascripts/merge_request_widget/approvals/components/approvals_body.js.es6

+/* 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: `
+      <div class='approvals-body mr-widget-footer'>
+        <h4> Requires {{ approvalsRequiredStringified }}
+          <span v-if='showSuggestedApprovers'> (from {{ approverNamesStringified }}) </span>
+        </h4>
+        <div v-if='showApproveButton' class='append-bottom-10'>
+          <button
+            :disabled='approving'
+            @click='approveMergeRequest'
+            class='btn btn-primary approve-btn'>
+            Approve Merge Request
+          </button>
+        </div>
+      </div>
+    `,
+  });
+})();

app/assets/javascripts/merge_request_widget/approvals/components/approvals_footer.js.es6

+/* 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: `
+      <div class='mr-widget-footer approved-by-users approvals-footer clearfix'>
+        <span class='approvers-prefix'> Approved by </span>
+        <span v-for='approver in approvedBy'>
+          <link-to-member-avatar
+            extra-link-class='approver-avatar'
+            :avatar-url='approver.user.avatar_url'
+            :display-name='approver.user.name'
+            :profile-url='approver.user.web_url'
+            :avatar-html='checkmarkSvg'
+            :show-tooltip='true'>
+          </link-to-member-avatar>
+        </span>
+        <span v-for='n in approvalsLeft'>
+          <link-to-member-avatar
+            :clickable='false'
+            :avatar-html='pendingAvatarSvg'
+            :show-tooltip='false'
+            extra-link-class='hide-asset'>
+          </link-to-member-avatar>
+        </span>
+        <span class='unapprove-btn-wrap' v-if='showUnapproveButton'>
+          <button
+            :disabled='unapproving'
+            @click='unapproveMergeRequest'
+            class='btn btn-link unapprove-btn'>
+            <i class='fa fa-close'></i>
+            Remove your approval</span>
+          </button>
+        </span>
+      </div>
+    `,
+  });
+})();
+

app/assets/javascripts/merge_request_widget/widget_bundle.js.es6

+/* 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 = {}));

app/assets/javascripts/merge_request_widget/widget_store.js.es6

+//= 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 = {}));

app/assets/javascripts/vue_common_component/link_to_member_avatar.js.es6

+/* 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: `
+      <div class='link-to-member-avatar'>
+        <a :href='profileUrl' :class='linkClass' :data-original-title='displayName' :data-container='tooltipContainerAttr'>
+          <svg v-if='avatarHtml' v-html='avatarHtml' :class='avatarHtmlClass' :width='avatarSize' :height='avatarSize' :alt='displayName'></svg>
+          <img :class='avatarClass' :src='avatarUrl' :width='avatarSize' :height='avatarSize' :alt='displayName'/>
+        </a>
+      </div>
+    `,
+  });
+})();

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;
+  }
+}

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 @@ class Projects::MergeRequestsController < Projects::ApplicationController
       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

app/helpers/merge_requests_helper.rb

@@ -90,6 +90,7 @@ module MergeRequestsHelper
     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"

app/mailers/emails/merge_requests.rb

@@ -54,11 +54,18 @@ module Emails
       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)

app/models/concerns/approvable.rb

@@ -123,6 +123,12 @@ module Approvable
       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.
     #

app/services/merge_requests/remove_approval_service.rb

+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

app/services/notification_service.rb

@@ -162,6 +162,10 @@ class NotificationService
     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 @@ class NotificationService
     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

app/services/system_note_service.rb

@@ -474,6 +474,11 @@ module SystemNoteService
     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)

app/views/notify/approved_merge_request_email.text.haml

-= "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)}
 

app/views/notify/unapproved_merge_request_email.text.haml

+= "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}

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}"
+    });
   });

app/views/projects/merge_requests/widget/_open.html.haml

-.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'
+
+

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

app/views/projects/merge_requests/widget/open/_approvals_body.html.haml

+%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' }

app/views/projects/merge_requests/widget/open/_approvals_footer.html.haml

+%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' }

app/views/projects/merge_requests/widget/open/_approve.html.haml

-%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"

app/views/projects/merge_requests/widget/open/_rebase.html.haml

+- 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("<i class='fa fa-spinner fa-spin'></i> Rebase in progress");
-    });

app/views/shared/icons/_icon_checkmark.svg

+<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 10 10" xmlns:xlink="http://www.w3.org/1999/xlink"><defs><path id="0" d="m5.743 5.743h-2c-.556 0-1 .448-1 1 0 .556.448 1 1 1h3c.556 0 1-.448 1-1v-5.01c0-.54-.448-.991-1-.991-.556 0-1 .444-1 .991v4.01"/><mask id="1" width="6" height="8" x="-.5" y="-.5"><path fill="#fff" d="m2.243.243h6v8h-6z"/><use xlink:href="#0"/></mask></defs><g fill="none" fill-rule="evenodd" transform="matrix(.70711.70711-.70711.70711 4.536-2.465)"><use fill="#31af64" xlink:href="#0"/><use stroke="#fff" mask="url(#1)" xlink:href="#0"/></g></svg>
\ No newline at end of file

app/views/shared/icons/_icon_dotted_circle.svg

+<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 27 27"><path fill="#bfbfbf" fill-rule="evenodd" d="m13.5 26.5c1.412 0 2.794-.225 4.107-.662l-.316-.949c-1.212.403-2.487.611-3.792.611v1m6.06-1.495c1.234-.651 2.355-1.498 3.321-2.504l-.721-.692c-.892.929-1.928 1.711-3.067 2.312l.467.884m4.66-4.147c.79-1.149 1.391-2.418 1.777-3.762l-.961-.276c-.356 1.24-.911 2.411-1.64 3.471l.824.567m2.184-5.761c.063-.518.096-1.041.097-1.568 0-.896-.085-1.758-.255-2.603l-.98.197c.157.78.236 1.576.236 2.405-.001.486-.031.97-.09 1.448l.993.122m-.738-6.189c-.493-1.307-1.195-2.523-2.075-3.605l-.776.631c.812.999 1.46 2.122 1.916 3.327l.935-.353m-3.539-5.133c-1.043-.926-2.229-1.68-3.512-2.229l-.394.919c1.184.507 2.279 1.203 3.242 2.058l.664-.748m-5.463-2.886c-1.012-.253-2.058-.384-3.119-.388-.378 0-.717.013-1.059.039l.077.997c.316-.024.629-.036.98-.036.979.003 1.944.124 2.879.358l.243-.97m-6.238-.022c-1.361.33-2.653.878-3.832 1.619l.532.847c1.089-.684 2.281-1.189 3.536-1.494l-.236-.972m-5.517 2.878c-1.047.922-1.94 2.01-2.643 3.212l.864.504c.649-1.112 1.474-2.114 2.441-2.966l-.661-.75m-3.54 5.076c-.499 1.293-.789 2.664-.854 4.072l.999.046c.06-1.3.328-2.564.788-3.758l-.933-.36m-.78 6.202c.163 1.396.549 2.744 1.14 4l.905-.425c-.545-1.16-.902-2.404-1.052-3.692l-.993.116m2.177 5.814c.788 1.151 1.756 2.169 2.866 3.01l.606-.796c-1.025-.78-1.919-1.721-2.646-2.783l-.825.565m4.665 4.164c1.23.65 2.559 1.1 3.943 1.328l.162-.987c-1.278-.21-2.503-.625-3.638-1.225l-.468.884m6.02 1.501c.024 0 .024 0 .048 0v-1c-.022 0-.022 0-.044 0l-.004 1"/></svg>
\ No newline at end of file

config/application.rb

@@ -101,6 +101,9 @@ module Gitlab
     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"

config/routes/project.rb

@@ -106,7 +106,10 @@ constraints(ProjectUrlConstrainer.new) do
           post :toggle_subscription
 
           ## EE-specific
-          post :approve
+          get :approvals
+          post :approvals, action: :approve
+          delete :approvals, action: :unapprove
+
           post :rebase
           ## EE-specific
 

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
 

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
 

lib/api/entities.rb

@@ -340,6 +340,15 @@ module API
       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

lib/api/merge_requests.rb

@@ -313,6 +313,18 @@ module API
 
           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

spec/controllers/projects/merge_requests_controller_spec.rb

@@ -125,6 +125,83 @@ describe Projects::MergeRequestsController do
     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,

spec/features/merge_requests/approvals_spec.rb

@@ -75,7 +75,9 @@ feature 'Merge request approvals', js: true, feature: true do
 
         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 @@ feature 'Merge request approvals', js: true, feature: true do
         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 @@ feature 'Merge request approvals', js: true, feature: true do
         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 @@ feature 'Merge request approvals', js: true, feature: true do
         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 @@ feature 'Merge request approvals', js: true, feature: true do
       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

spec/features/merge_requests/merge_when_pipeline_succeeds_spec.rb

@@ -32,7 +32,7 @@ feature 'Merge When Pipeline Succeeds', :feature, :js do
       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

spec/javascripts/approvals/approvals_body_spec.js.es6

+/* 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(`
+      <div>
+        <div id="mock-container"></div>
+      </div>
+    `);
+
+    this.initialData = {
+      suggestedApprovers: [{ name: 'Approver 1' }],
+      userCanApprove: false,
+      userHasApproved: true,
+      approvedBy: [],
+      approvalsLeft: 1,
+      pendingAvatarSvg: '<svg></svg>',
+      checkmarkSvg: '<svg></svg>',
+    };
+
+    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 = {}));
+

spec/javascripts/approvals/approvals_footer_spec.js.es6

+/* 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(`
+      <div>
+        <div id="mock-container"></div>
+      </div>
+    `);
+
+    this.initialData = {
+      userCanApprove: false,
+      userHasApproved: true,
+      approvedBy: [],
+      approvalsLeft: 1,
+      pendingAvatarSvg: '<svg></svg>',
+      checkmarkSvg: '<svg></svg>',
+    };
+
+    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 = {}));
+

spec/javascripts/approvals/approvals_store_spec.js.es6

+//= 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 = {}));

spec/javascripts/labels_issue_sidebar_spec.js.es6

-/* 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 */
 

spec/javascripts/vue_common_components/link_to_member_avatar_spec.js.es6

+/* eslint-disable */
+//= require vue
+//= require jquery
+//= require vue_common_component/link_to_member_avatar
+
+((gl) => {
+  function initComponent(propsData = {}) {
+    fixture.set(`
+      <div>
+        <div id="mock-container"></div>
+      </div>
+    `);
+
+    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 <a> and <img> 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 = {}));

spec/mailers/notify_spec.rb

@@ -405,6 +405,44 @@ describe Notify do
           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) }
 

spec/requests/api/merge_requests_spec.rb

@@ -745,6 +745,8 @@ describe API::MergeRequests, api: true  do
       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 @@ describe API::MergeRequests, api: true  do
         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

spec/services/merge_requests/remove_approval_service_spec.rb

+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