Merge remote-tracking branch 'ce/master'

CHANGELOG

@@ -2,6 +2,7 @@ Please view this file on the master branch, on stable branches it's out of date.
 
 v 8.10.0 (unreleased)
   - Expose {should,force}_remove_source_branch (Ben Boeckel)
+  - Disable PostgreSQL statement timeout during migrations
   - Fix projects dropdown loading performance with a simplified api cal. !5113 (tiagonbotelho)
   - Fix commit builds API, return all builds for all pipelines for given commit. !4849
   - Replace Haml with Hamlit to make view rendering faster. !3666
@@ -95,6 +96,7 @@ v 8.10.0 (unreleased)
   - Redesign Builds and Pipelines pages
   - Change status color and icon for running builds
   - Fix markdown rendering for: consecutive labels references, label references that begin with a digit or contains `.`
+  - Fix last update timestamp on issues not preserved on gitlab.com and project imports
 
 v 8.9.6
   - Fix importing of events under notes for GitLab projects. !5154

Gemfile

@@ -310,7 +310,7 @@ group :development, :test do
   gem 'spring-commands-spinach',  '~> 1.1.0'
   gem 'spring-commands-teaspoon', '~> 0.0.2'
 
-  gem 'rubocop', '~> 0.40.0', require: false
+  gem 'rubocop', '~> 0.41.2', require: false
   gem 'rubocop-rspec', '~> 1.5.0', require: false
   gem 'scss_lint', '~> 0.47.0', require: false
   gem 'simplecov', '~> 0.11.0', require: false

Gemfile.lock

@@ -58,7 +58,7 @@ GEM
       faraday_middleware-multi_json (~> 0.0)
       oauth2 (~> 1.0)
     asciidoctor (1.5.3)
-    ast (2.2.0)
+    ast (2.3.0)
     attr_encrypted (3.0.1)
       encryptor (~> 3.0.0)
     attr_required (1.0.0)
@@ -497,7 +497,7 @@ GEM
     orm_adapter (0.5.0)
     paranoia (2.1.4)
       activerecord (~> 4.0)
-    parser (2.3.1.0)
+    parser (2.3.1.2)
       ast (~> 2.2)
     pg (0.18.4)
     pkg-config (1.1.7)
@@ -630,8 +630,8 @@ GEM
     rspec-retry (0.4.5)
       rspec-core
     rspec-support (3.5.0)
-    rubocop (0.40.0)
-      parser (>= 2.3.1.0, < 3.0)
+    rubocop (0.41.2)
+      parser (>= 2.3.1.1, < 3.0)
       powerpack (~> 0.1)
       rainbow (>= 1.99.1, < 3.0)
       ruby-progressbar (~> 1.7)
@@ -782,7 +782,7 @@ GEM
     unf (0.1.4)
       unf_ext
     unf_ext (0.0.7.2)
-    unicode-display_width (1.0.5)
+    unicode-display_width (1.1.0)
     unicorn (4.9.0)
       kgio (~> 2.6)
       rack
@@ -970,7 +970,7 @@ DEPENDENCIES
   rqrcode-rails3 (~> 0.1.7)
   rspec-rails (~> 3.5.0)
   rspec-retry (~> 0.4.5)
-  rubocop (~> 0.40.0)
+  rubocop (~> 0.41.2)
   rubocop-rspec (~> 1.5.0)
   ruby-fogbugz (~> 0.2.1)
   sanitize (~> 2.0)

app/controllers/projects/issues_controller.rb

@@ -125,10 +125,6 @@ class Projects::IssuesController < Projects::ApplicationController
         render json: @issue.to_json(include: { milestone: {}, assignee: { methods: :avatar_url }, labels: { methods: :text_color } })
       end
     end
-
-  rescue ActiveRecord::StaleObjectError
-    @conflict = true
-    render :edit
   end
 
   def referenced_merge_requests
@@ -230,7 +226,7 @@ class Projects::IssuesController < Projects::ApplicationController
   def issue_params
     params.require(:issue).permit(
       :title, :assignee_id, :position, :description, :confidential, :weight,
-      :milestone_id, :due_date, :state_event, :task_num, :lock_version, label_ids: []
+      :milestone_id, :due_date, :state_event, :task_num, label_ids: []
     )
   end
 

app/controllers/projects/merge_requests_controller.rb

@@ -209,9 +209,6 @@ class Projects::MergeRequestsController < Projects::ApplicationController
 
       render "edit"
     end
-  rescue ActiveRecord::StaleObjectError
-    @conflict = true
-    render :edit
   end
 
   def remove_wip
@@ -476,7 +473,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
       :title, :assignee_id, :source_project_id, :source_branch,
       :target_project_id, :target_branch, :milestone_id, :approver_ids,
       :state_event, :description, :task_num, :force_remove_source_branch,
-      :lock_version, :approvals_before_merge, label_ids: []
+      :approvals_before_merge, label_ids: []
     )
   end
 

app/models/concerns/issuable.rb

@@ -90,12 +90,6 @@ module Issuable
       User.find(assignee_id_was).update_cache_counts if assignee_id_was
       assignee.update_cache_counts if assignee
     end
-
-    # We want to use optimistic lock for cases when only title or description are involved
-    # http://api.rubyonrails.org/classes/ActiveRecord/Locking/Optimistic.html
-    def locking_enabled?
-      title_changed? || description_changed?
-    end
   end
 
   module ClassMethods

app/views/shared/issuable/_form.html.haml

 = form_errors(issuable)
 
-- if @conflict
-  .alert.alert-danger
-    Someone edited the #{issuable.class.model_name.human.downcase} the same time you did.
-    Please check out
-    = link_to "the #{issuable.class.model_name.human.downcase}", polymorphic_path([@project.namespace.becomes(Namespace), @project, issuable]), target: "_blank"
-    and make sure your changes will not unintentionally remove theirs
-
 .form-group
   = f.label :title, class: 'control-label'
   .col-sm-10
@@ -199,12 +192,3 @@
         = link_to 'Delete', polymorphic_path([@project.namespace.becomes(Namespace), @project, issuable]), data: { confirm: "#{issuable.class.name.titleize} will be removed! Are you sure?" },
                                                                                                   method: :delete, class: 'btn btn-danger btn-grouped'
       = link_to 'Cancel', polymorphic_path([@project.namespace.becomes(Namespace), @project, issuable]), class: 'btn btn-grouped btn-cancel'
-
-= f.hidden_field :lock_version
-
-%li.project-approvers.hide.approver-template{id: "user_{user_id}"}
-  = link_to "{approver_name}", "#"
-  .pull-right
-    = link_to "#", data: { confirm: "Are you sure you want to remove approver {approver_name}"}, class: "btn-xs btn btn-remove", title: 'Remove approver' do
-      = icon("sign-out")
-      Remove

db/migrate/20160707104333_add_lock_to_issuables.rb

-# See http://doc.gitlab.com/ce/development/migration_style_guide.html
-# for more information on how to write migrations for GitLab.
-
-class AddLockToIssuables < ActiveRecord::Migration
-  include Gitlab::Database::MigrationHelpers
-  disable_ddl_transaction!
-
-  def up
-    add_column_with_default :issues, :lock_version, :integer, default: 0
-    add_column_with_default :merge_requests, :lock_version, :integer, default: 0
-  end
-
-  def down
-    remove_column :issues, :lock_version
-    remove_column :merge_requests, :lock_version
-  end
-end

db/schema.rb

@@ -73,11 +73,11 @@ ActiveRecord::Schema.define(version: 20160712171823) do
     t.string   "recaptcha_site_key"
     t.string   "recaptcha_private_key"
     t.integer  "metrics_port",                          default: 8089
+    t.boolean  "akismet_enabled",                       default: false
+    t.string   "akismet_api_key"
     t.integer  "metrics_sample_interval",               default: 15
     t.boolean  "sentry_enabled",                        default: false
     t.string   "sentry_dsn"
-    t.boolean  "akismet_enabled",                       default: false
-    t.string   "akismet_api_key"
     t.boolean  "email_author_in_body",                  default: false
     t.integer  "default_group_visibility"
     t.boolean  "repository_checks_enabled",             default: false
@@ -87,6 +87,7 @@ ActiveRecord::Schema.define(version: 20160712171823) do
     t.string   "health_check_access_token"
     t.boolean  "send_user_confirmation_email",          default: false
     t.integer  "container_registry_token_expire_delay", default: 5
+    t.boolean  "user_default_external",                 default: false,        null: false
     t.text     "after_sign_up_text"
     t.boolean  "elasticsearch_indexing",                default: false,       null: false
     t.boolean  "elasticsearch_search",                  default: false,       null: false
@@ -94,7 +95,6 @@ ActiveRecord::Schema.define(version: 20160712171823) do
     t.string   "elasticsearch_port",                    default: "9200"
     t.string   "repository_storage",                    default: "default"
     t.string   "enabled_git_access_protocol"
-    t.boolean  "user_default_external",                 default: false,       null: false
   end
 
   create_table "approvals", force: :cascade do |t|
@@ -190,8 +190,8 @@ ActiveRecord::Schema.define(version: 20160712171823) do
     t.text     "artifacts_metadata"
     t.integer  "erased_by_id"
     t.datetime "erased_at"
-    t.datetime "artifacts_expire_at"
     t.string   "environment"
+    t.datetime "artifacts_expire_at"
     t.integer  "artifacts_size"
   end
 
@@ -545,7 +545,6 @@ ActiveRecord::Schema.define(version: 20160712171823) do
     t.boolean  "confidential",  default: false
     t.datetime "deleted_at"
     t.date     "due_date"
-    t.integer  "lock_version",  default: 0,     null: false
   end
 
   add_index "issues", ["assignee_id"], name: "index_issues_on_assignee_id", using: :btree
@@ -702,7 +701,6 @@ ActiveRecord::Schema.define(version: 20160712171823) do
     t.datetime "deleted_at"
     t.integer  "approvals_before_merge"
     t.string   "rebase_commit_sha"
-    t.integer  "lock_version",              default: 0,     null: false
   end
 
   add_index "merge_requests", ["assignee_id"], name: "index_merge_requests_on_assignee_id", using: :btree
@@ -878,10 +876,10 @@ ActiveRecord::Schema.define(version: 20160712171823) do
     t.integer  "user_id",                    null: false
     t.string   "token",                      null: false
     t.string   "name",                       null: false
-    t.boolean  "revoked",    default: false
-    t.datetime "expires_at"
     t.datetime "created_at",                 null: false
     t.datetime "updated_at",                 null: false
+    t.boolean  "revoked",    default: false
+    t.datetime "expires_at"
   end
 
   add_index "personal_access_tokens", ["token"], name: "index_personal_access_tokens_on_token", unique: true, using: :btree
@@ -1048,9 +1046,9 @@ ActiveRecord::Schema.define(version: 20160712171823) do
     t.string   "type"
     t.string   "title"
     t.integer  "project_id"
-    t.datetime "created_at",                               null: false
-    t.datetime "updated_at",                               null: false
-    t.boolean  "active",                                   null: false
+    t.datetime "created_at"
+    t.datetime "updated_at"
+    t.boolean  "active",                default: false,    null: false
     t.text     "properties"
     t.boolean  "template",              default: false
     t.boolean  "push_events",           default: true

features/project/merge_requests.feature

@@ -89,7 +89,7 @@ Feature: Project Merge Requests
     Then The list should be sorted by "Oldest updated"
 
   @javascript
-  Scenario: Visiting Merge Requests from a different Project after sorting
+  Scenario: Visiting Merge Requests from a differente Project after sorting
     Given I visit project "Shop" merge requests page
     And I sort the list by "Oldest updated"
     And I visit dashboard merge requests page

lib/gitlab/database/migration_helpers.rb

@@ -20,11 +20,19 @@ module Gitlab
 
         if Database.postgresql?
           options = options.merge({ algorithm: :concurrently })
+          disable_statement_timeout
         end
 
         add_index(table_name, column_name, options)
       end
 
+      # Long-running migrations may take more than the timeout allowed by
+      # the database. Disable the session's statement timeout to ensure
+      # migrations don't get killed prematurely. (PostgreSQL only)
+      def disable_statement_timeout
+        ActiveRecord::Base.connection.execute('SET statement_timeout TO 0') if Database.postgresql?
+      end
+
       # Updates the value of a column in batches.
       #
       # This method updates the table in batches of 5% of the total row count.
@@ -133,6 +141,8 @@ module Gitlab
             'in the body of your migration class'
         end
 
+        disable_statement_timeout
+
         transaction do
           add_column(table, column, type, default: nil)
 

lib/gitlab/gitlab_import/importer.rb

@@ -15,32 +15,35 @@ module Gitlab
       end
 
       def execute
-        project_identifier = CGI.escape(project.import_source)
-
-        # Issues && Comments
-        issues = client.issues(project_identifier)
-
-        issues.each do |issue|
-          body = @formatter.author_line(issue["author"]["name"])
-          body += issue["description"]
-
-          comments = client.issue_comments(project_identifier, issue["id"])
-
-          if comments.any?
-            body += @formatter.comments_header
+        ActiveRecord::Base.no_touching do
+          project_identifier = CGI.escape(project.import_source)
+
+          # Issues && Comments
+          issues = client.issues(project_identifier)
+
+          issues.each do |issue|
+            body = @formatter.author_line(issue["author"]["name"])
+            body += issue["description"]
+
+            comments = client.issue_comments(project_identifier, issue["id"])
+
+            if comments.any?
+              body += @formatter.comments_header
+            end
+
+            comments.each do |comment|
+              body += @formatter.comment(comment["author"]["name"], comment["created_at"], comment["body"])
+            end
+
+            project.issues.create!(
+              iid: issue["iid"],
+              description: body,
+              title: issue["title"],
+              state: issue["state"],
+              updated_at: issue["updated_at"],
+              author_id: gl_user_id(project, issue["author"]["id"])
+            )
           end
-
-          comments.each do |comment|
-            body += @formatter.comment(comment["author"]["name"], comment["created_at"], comment["body"])
-          end
-
-          project.issues.create!(
-            iid: issue["iid"],
-            description: body,
-            title: issue["title"],
-            state: issue["state"],
-            author_id: gl_user_id(project, issue["author"]["id"])
-          )
         end
 
         true

lib/gitlab/import_export/project_tree_restorer.rb

@@ -12,7 +12,10 @@ module Gitlab
         json = IO.read(@path)
         @tree_hash = ActiveSupport::JSON.decode(json)
         @project_members = @tree_hash.delete('project_members')
-        create_relations
+
+        ActiveRecord::Base.no_touching do
+          create_relations
+        end
       rescue => e
         @shared.error(e)
         false

spec/features/issues_spec.rb

@@ -120,17 +120,6 @@ describe 'Issues', feature: true do
           expect(page).to have_content date.to_s(:medium)
         end
       end
-
-      it 'warns about version conflict' do
-        issue.update(title: "New title")
-
-        fill_in 'issue_title', with: 'bug 345'
-        fill_in 'issue_description', with: 'bug description'
-
-        click_button 'Save changes'
-
-        expect(page).to have_content 'Someone edited the issue the same time you did'
-      end
     end
   end
 

spec/features/merge_requests/edit_mr_spec.rb

@@ -17,17 +17,6 @@ feature 'Edit Merge Request', feature: true do
     it 'form should have class js-quick-submit' do
       expect(page).to have_selector('.js-quick-submit')
     end
-
-    it 'warns about version conflict' do
-      merge_request.update(title: "New title")
-
-      fill_in 'merge_request_title', with: 'bug 345'
-      fill_in 'merge_request_description', with: 'bug description'
-
-      click_button 'Save changes'
-
-      expect(page).to have_content 'Someone edited the merge request the same time you did'
-    end
   end
 
   context 'saving the MR that needs approvals' do

spec/lib/gitlab/database/migration_helpers_spec.rb

@@ -13,6 +13,10 @@ describe Gitlab::Database::MigrationHelpers, lib: true do
     context 'outside a transaction' do
       before do
         expect(model).to receive(:transaction_open?).and_return(false)
+
+        unless Gitlab::Database.postgresql?
+          allow_any_instance_of(Gitlab::Database::MigrationHelpers).to receive(:disable_statement_timeout)
+        end
       end
 
       context 'using PostgreSQL' do

spec/lib/gitlab/import_export/project_tree_restorer_spec.rb

@@ -30,6 +30,14 @@ describe Gitlab::ImportExport::ProjectTreeRestorer, services: true do
         expect(Event.where.not(data: nil).first.data[:ref]).not_to be_empty
       end
 
+      it 'preserves updated_at on issues' do
+        restored_project_json
+
+        issue = Issue.where(description: 'Aliquam enim illo et possimus.').first
+
+        expect(issue.reload.updated_at.to_s).to eq('2016-06-14 15:02:47 UTC')
+      end
+
       context 'event at forth level of the tree' do
         let(:event) { Event.where(title: 'test levels').first }