Merge branch '9-2-stable' of gitlab.com:gitlab-org/gitlab-ce into 9-2-stable

7 files changed, 229 insertions, 42 deletions

diff --git a/spec/factories/group_members.rb b/spec/factories/group_members.rb
index 080b2e75ea1..32cbfe28a60 100644
--- a/spec/factories/group_members.rb
+++ b/spec/factories/group_members.rb
@@ -10,5 +10,11 @@ FactoryGirl.define do
     trait(:master)    { access_level GroupMember::MASTER }
     trait(:owner)     { access_level GroupMember::OWNER }
     trait(:access_request) { requested_at Time.now }
+
+    trait(:invited) do
+      user_id nil
+      invite_token 'xxx'
+      invite_email 'email@email.com'
+    end
   end
 end
diff --git a/spec/factories/project_members.rb b/spec/factories/project_members.rb
index d62799a5a47..fe4518caadf 100644
--- a/spec/factories/project_members.rb
+++ b/spec/factories/project_members.rb
@@ -9,5 +9,11 @@ FactoryGirl.define do
     trait(:developer) { access_level ProjectMember::DEVELOPER }
     trait(:master)    { access_level ProjectMember::MASTER }
     trait(:access_request) { requested_at Time.now }
+
+    trait(:invited) do
+      user_id nil
+      invite_token 'xxx'
+      invite_email 'email@email.com'
+    end
   end
 end
diff --git a/spec/features/issues/note_polling_spec.rb b/spec/features/issues/note_polling_spec.rb
index 58b3215f14c..da81fa4e367 100644
--- a/spec/features/issues/note_polling_spec.rb
+++ b/spec/features/issues/note_polling_spec.rb
@@ -18,58 +18,93 @@ feature 'Issue notes polling', :feature, :js do
   end
 
   describe 'updates' do
-    let(:user) { create(:user) }
-    let(:note_text) { "Hello World" }
-    let(:updated_text) { "Bye World" }
-    let!(:existing_note) { create(:note, noteable: issue, project: project, author: user, note: note_text) }
+    context 'when from own user' do
+      let(:user) { create(:user) }
+      let(:note_text) { "Hello World" }
+      let(:updated_text) { "Bye World" }
+      let!(:existing_note) { create(:note, noteable: issue, project: project, author: user, note: note_text) }
 
-    before do
-      login_as(user)
-      visit namespace_project_issue_path(project.namespace, project, issue)
-    end
+      before do
+        login_as(user)
+        visit namespace_project_issue_path(project.namespace, project, issue)
+      end
 
-    it 'displays the updated content' do
-      expect(page).to have_selector("#note_#{existing_note.id}", text: note_text)
+      it 'has .original-note-content to compare against' do
+        expect(page).to have_selector("#note_#{existing_note.id}", text: note_text)
+        expect(page).to have_selector("#note_#{existing_note.id} .original-note-content", visible: false)
 
-      update_note(existing_note, updated_text)
+        update_note(existing_note, updated_text)
 
-      expect(page).to have_selector("#note_#{existing_note.id}", text: updated_text)
-    end
+        expect(page).to have_selector("#note_#{existing_note.id}", text: updated_text)
+        expect(page).to have_selector("#note_#{existing_note.id} .original-note-content", visible: false)
+      end
 
-    it 'when editing but have not changed anything, and an update comes in, show the updated content in the textarea' do
-      find("#note_#{existing_note.id} .js-note-edit").click
+      it 'displays the updated content' do
+        expect(page).to have_selector("#note_#{existing_note.id}", text: note_text)
 
-      expect(page).to have_field("note[note]", with: note_text)
+        update_note(existing_note, updated_text)
 
-      update_note(existing_note, updated_text)
+        expect(page).to have_selector("#note_#{existing_note.id}", text: updated_text)
+      end
 
-      expect(page).to have_field("note[note]", with: updated_text)
-    end
+      it 'when editing but have not changed anything, and an update comes in, show the updated content in the textarea' do
+        find("#note_#{existing_note.id} .js-note-edit").click
 
-    it 'when editing but you changed some things, and an update comes in, show a warning' do
-      find("#note_#{existing_note.id} .js-note-edit").click
+        expect(page).to have_field("note[note]", with: note_text)
 
-      expect(page).to have_field("note[note]", with: note_text)
+        update_note(existing_note, updated_text)
 
-      find("#note_#{existing_note.id} .js-note-text").set('something random')
+        expect(page).to have_field("note[note]", with: updated_text)
+      end
 
-      update_note(existing_note, updated_text)
+      it 'when editing but you changed some things, and an update comes in, show a warning' do
+        find("#note_#{existing_note.id} .js-note-edit").click
 
-      expect(page).to have_selector(".alert")
-    end
+        expect(page).to have_field("note[note]", with: note_text)
+
+        find("#note_#{existing_note.id} .js-note-text").set('something random')
+
+        update_note(existing_note, updated_text)
 
-    it 'when editing but you changed some things, an update comes in, and you press cancel, show the updated content' do
-      find("#note_#{existing_note.id} .js-note-edit").click
+        expect(page).to have_selector(".alert")
+      end
+
+      it 'when editing but you changed some things, an update comes in, and you press cancel, show the updated content' do
+        find("#note_#{existing_note.id} .js-note-edit").click
+
+        expect(page).to have_field("note[note]", with: note_text)
+
+        find("#note_#{existing_note.id} .js-note-text").set('something random')
+
+        update_note(existing_note, updated_text)
+
+        find("#note_#{existing_note.id} .note-edit-cancel").click
+
+        expect(page).to have_selector("#note_#{existing_note.id}", text: updated_text)
+      end
+    end
 
-      expect(page).to have_field("note[note]", with: note_text)
+    context 'when from another user' do
+      let(:user1) { create(:user) }
+      let(:user2) { create(:user) }
+      let(:note_text) { "Hello World" }
+      let(:updated_text) { "Bye World" }
+      let!(:existing_note) { create(:note, noteable: issue, project: project, author: user1, note: note_text) }
 
-      find("#note_#{existing_note.id} .js-note-text").set('something random')
+      before do
+        login_as(user2)
+        visit namespace_project_issue_path(project.namespace, project, issue)
+      end
 
-      update_note(existing_note, updated_text)
+      it 'has .original-note-content to compare against' do
+        expect(page).to have_selector("#note_#{existing_note.id}", text: note_text)
+        expect(page).to have_selector("#note_#{existing_note.id} .original-note-content", visible: false)
 
-      find("#note_#{existing_note.id} .note-edit-cancel").click
+        update_note(existing_note, updated_text)
 
-      expect(page).to have_selector("#note_#{existing_note.id}", text: updated_text)
+        expect(page).to have_selector("#note_#{existing_note.id}", text: updated_text)
+        expect(page).to have_selector("#note_#{existing_note.id} .original-note-content", visible: false)
+      end
     end
   end
 
diff --git a/spec/javascripts/notes_spec.js b/spec/javascripts/notes_spec.js
index 8243a9c991a..83a151f908f 100644
--- a/spec/javascripts/notes_spec.js
+++ b/spec/javascripts/notes_spec.js
@@ -78,6 +78,47 @@ import '~/notes';
       });
     });
 
+    describe('updateNote', () => {
+      let sampleComment;
+      let noteEntity;
+      let $form;
+      let $notesContainer;
+
+      beforeEach(() => {
+        this.notes = new Notes('', []);
+        window.gon.current_username = 'root';
+        window.gon.current_user_fullname = 'Administrator';
+        sampleComment = 'foo';
+        noteEntity = {
+          id: 1234,
+          html: `<li class="note note-row-1234 timeline-entry" id="note_1234">
+                  <div class="note-text">${sampleComment}</div>
+                 </li>`,
+          note: sampleComment,
+          valid: true
+        };
+        $form = $('form.js-main-target-form');
+        $notesContainer = $('ul.main-notes-list');
+        $form.find('textarea.js-note-text').val(sampleComment);
+      });
+
+      it('updates note and resets edit form', () => {
+        const deferred = $.Deferred();
+        spyOn($, 'ajax').and.returnValue(deferred.promise());
+        spyOn(this.notes, 'revertNoteEditForm');
+
+        $('.js-comment-button').click();
+        deferred.resolve(noteEntity);
+
+        const $targetNote = $notesContainer.find(`#note_${noteEntity.id}`);
+        const updatedNote = Object.assign({}, noteEntity);
+        updatedNote.note = 'bar';
+        this.notes.updateNote(updatedNote, $targetNote);
+
+        expect(this.notes.revertNoteEditForm).toHaveBeenCalledWith($targetNote);
+      });
+    });
+
     describe('renderNote', () => {
       let notes;
       let note;
@@ -98,8 +139,6 @@ import '~/notes';
 
         notes = jasmine.createSpyObj('notes', [
           'refresh',
-          'isNewNote',
-          'isUpdatedNote',
           'collapseLongCommitList',
           'updateNotesCount',
           'putConflictEditWarningInPlace'
@@ -109,13 +148,15 @@ import '~/notes';
         notes.updatedNotesTrackingMap = {};
 
         spyOn(gl.utils, 'localTimeAgo');
+        spyOn(Notes, 'isNewNote').and.callThrough();
+        spyOn(Notes, 'isUpdatedNote').and.callThrough();
         spyOn(Notes, 'animateAppendNote').and.callThrough();
         spyOn(Notes, 'animateUpdateNote').and.callThrough();
       });
 
       describe('when adding note', () => {
         it('should call .animateAppendNote', () => {
-          notes.isNewNote.and.returnValue(true);
+          Notes.isNewNote.and.returnValue(true);
           Notes.prototype.renderNote.call(notes, note, null, $notesList);
 
           expect(Notes.animateAppendNote).toHaveBeenCalledWith(note.html, $notesList);
@@ -124,7 +165,8 @@ import '~/notes';
 
       describe('when note was edited', () => {
         it('should call .animateUpdateNote', () => {
-          notes.isUpdatedNote.and.returnValue(true);
+          Notes.isNewNote.and.returnValue(false);
+          Notes.isUpdatedNote.and.returnValue(true);
           const $note = $('<div>');
           $notesList.find.and.returnValue($note);
           Notes.prototype.renderNote.call(notes, note, null, $notesList);
@@ -134,7 +176,8 @@ import '~/notes';
 
         describe('while editing', () => {
           it('should update textarea if nothing has been touched', () => {
-            notes.isUpdatedNote.and.returnValue(true);
+            Notes.isNewNote.and.returnValue(false);
+            Notes.isUpdatedNote.and.returnValue(true);
             const $note = $(`<div class="is-editing">
               <div class="original-note-content">initial</div>
               <textarea class="js-note-text">initial</textarea>
@@ -146,7 +189,8 @@ import '~/notes';
           });
 
           it('should call .putConflictEditWarningInPlace', () => {
-            notes.isUpdatedNote.and.returnValue(true);
+            Notes.isNewNote.and.returnValue(false);
+            Notes.isUpdatedNote.and.returnValue(true);
             const $note = $(`<div class="is-editing">
               <div class="original-note-content">initial</div>
               <textarea class="js-note-text">different</textarea>
@@ -160,6 +204,47 @@ import '~/notes';
       });
     });
 
+    describe('isUpdatedNote', () => {
+      it('should consider same note text as the same', () => {
+        const result = Notes.isUpdatedNote(
+          {
+            note: 'initial'
+          },
+          $(`<div>
+            <div class="original-note-content">initial</div>
+          </div>`)
+        );
+
+        expect(result).toEqual(false);
+      });
+
+      it('should consider same note with trailing newline as the same', () => {
+        const result = Notes.isUpdatedNote(
+          {
+            note: 'initial\n'
+          },
+          $(`<div>
+            <div class="original-note-content">initial\n</div>
+          </div>`)
+        );
+
+        expect(result).toEqual(false);
+      });
+
+      it('should consider different notes as different', () => {
+        const result = Notes.isUpdatedNote(
+          {
+            note: 'foo'
+          },
+          $(`<div>
+            <div class="original-note-content">bar</div>
+          </div>`)
+        );
+
+        expect(result).toEqual(true);
+      });
+    });
+
     describe('renderDiscussionNote', () => {
       let discussionContainer;
       let note;
@@ -179,15 +264,15 @@ import '~/notes';
         row = jasmine.createSpyObj('row', ['prevAll', 'first', 'find']);
 
         notes = jasmine.createSpyObj('notes', [
-          'isNewNote',
           'isParallelView',
           'updateNotesCount',
         ]);
         notes.note_ids = [];
 
         spyOn(gl.utils, 'localTimeAgo');
+        spyOn(Notes, 'isNewNote');
         spyOn(Notes, 'animateAppendNote');
-        notes.isNewNote.and.returnValue(true);
+        Notes.isNewNote.and.returnValue(true);
         notes.isParallelView.and.returnValue(false);
         row.prevAll.and.returnValue(row);
         row.first.and.returnValue(row);
diff --git a/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_base_spec.rb b/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_base_spec.rb
index 64bc5fc0429..a3ab4e3dd9e 100644
--- a/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_base_spec.rb
+++ b/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_base_spec.rb
@@ -107,6 +107,15 @@ describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameBase do
         expect(new_path).to eq('the-path0')
       end
 
+      it "doesn't rename routes that start with a similar name" do
+        other_namespace = create(:namespace, path: 'the-path-but-not-really')
+        project = create(:empty_project, path: 'the-project', namespace: other_namespace)
+
+        subject.rename_path_for_routable(migration_namespace(namespace))
+
+        expect(project.route.reload.path).to eq('the-path-but-not-really/the-project')
+      end
+
       context "the-path namespace -> subgroup -> the-path0 project" do
         it "updates the route of the project correctly" do
           subgroup = create(:group, path: "subgroup", parent: namespace)
diff --git a/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_namespaces_spec.rb b/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_namespaces_spec.rb
index ec444942804..c56fded7516 100644
--- a/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_namespaces_spec.rb
+++ b/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_namespaces_spec.rb
@@ -137,7 +137,7 @@ describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameNamespaces do
   end
 
   describe "#rename_namespace" do
-    let(:namespace) { create(:namespace, path: 'the-path') }
+    let(:namespace) { create(:group, name: 'the-path') }
 
     it 'renames paths & routes for the namespace' do
       expect(subject).to receive(:rename_path_for_routable).
@@ -177,6 +177,31 @@ describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameNamespaces do
 
       subject.rename_namespace(namespace)
     end
+
+    it "doesn't rename users for other namespaces" do
+      expect(subject).not_to receive(:rename_user)
+
+      subject.rename_namespace(namespace)
+    end
+
+    it 'renames the username of a namespace for a user' do
+      user = create(:user, username: 'the-path')
+
+      expect(subject).to receive(:rename_user).with('the-path', 'the-path0')
+
+      subject.rename_namespace(user.namespace)
+    end
+  end
+
+  describe '#rename_user' do
+    it 'renames a username' do
+      subject = described_class.new([], migration)
+      user = create(:user, username: 'broken')
+
+      subject.rename_user('broken', 'broken0')
+
+      expect(user.reload.username).to eq('broken0')
+    end
   end
 
   describe '#rename_namespaces' do
diff --git a/spec/services/members/authorized_destroy_service_spec.rb b/spec/services/members/authorized_destroy_service_spec.rb
index ab440d18e9f..8a6732faa19 100644
--- a/spec/services/members/authorized_destroy_service_spec.rb
+++ b/spec/services/members/authorized_destroy_service_spec.rb
@@ -10,6 +10,27 @@ describe Members::AuthorizedDestroyService, services: true do
     Issue.assigned_to(user).count + MergeRequest.assigned_to(user).count
   end
 
+  context 'Invited users' do
+    # Regression spec for issue: https://gitlab.com/gitlab-org/gitlab-ce/issues/32504
+    it 'destroys invited project member' do
+      project.team << [member_user, :developer]
+
+      member = create :project_member, :invited, project: project
+
+      expect { described_class.new(member, member_user).execute }
+        .to change { Member.count }.from(2).to(1)
+    end
+
+    it 'destroys invited group member' do
+      group.add_developer(member_user)
+
+      member = create :group_member, :invited, group: group
+
+      expect { described_class.new(member, member_user).execute }
+        .to change { Member.count }.from(2).to(1)
+    end
+  end
+
   context 'Group member' do
     it "unassigns issues and merge requests" do
       group.add_developer(member_user)