Merge branch '21567-fix-sorting-issues-by-last-updated-after-import-from-github' into 'master'

Fix sorting issues by "last updated" after import from GitHub

## What does this MR do?

Don't touch Issue/Merge Request when importing GitHub comments as it will trigger an update on `updated_at` field. It also use `updated_at` as the last updated date doesn't matter the Issue/Pull Request state.

## Why was this MR needed?

After import from GitHub, sorting issues by "last updated" doesn't work as expected.

## What are the relevant issue numbers?

Fixes #21567

See merge request !6110

CHANGELOG

@@ -8,6 +8,8 @@ v 8.11.4
   - Fix "Wiki" link not appearing in navigation for projects with external wiki. !6057
   - Do not enforce using hash with hidden key in CI configuration. !6079
   - Fix hover leading space bug in pipeline graph !5980
+  - Fix sorting issues by "last updated" doesn't work after import from GitHub
+  - Creating an issue through our API now emails label subscribers !5720
   - Block concurrent updates for Pipeline
   - Fix issue boards leak private label names and descriptions
 

lib/gitlab/github_import/importer.rb

@@ -152,12 +152,14 @@ module Gitlab
       end
 
       def create_comments(issuable, comments)
-        comments.each do |raw|
-          begin
-            comment = CommentFormatter.new(project, raw)
-            issuable.notes.create!(comment.attributes)
-          rescue => e
-            errors << { type: :comment, url: Gitlab::UrlSanitizer.sanitize(raw.url), errors: e.message }
+        ActiveRecord::Base.no_touching do
+          comments.each do |raw|
+            begin
+              comment = CommentFormatter.new(project, raw)
+              issuable.notes.create!(comment.attributes)
+            rescue => e
+              errors << { type: :comment, url: Gitlab::UrlSanitizer.sanitize(raw.url), errors: e.message }
+            end
           end
         end
       end

lib/gitlab/github_import/issue_formatter.rb

@@ -12,7 +12,7 @@ module Gitlab
           author_id: author_id,
           assignee_id: assignee_id,
           created_at: raw_data.created_at,
-          updated_at: updated_at
+          updated_at: raw_data.updated_at
         }
       end
 
@@ -69,10 +69,6 @@ module Gitlab
       def state
         raw_data.state == 'closed' ? 'closed' : 'opened'
       end
-
-      def updated_at
-        state == 'closed' ? raw_data.closed_at : raw_data.updated_at
-      end
     end
   end
 end

lib/gitlab/github_import/milestone_formatter.rb

@@ -3,14 +3,14 @@ module Gitlab
     class MilestoneFormatter < BaseFormatter
       def attributes
         {
-          iid: number,
+          iid: raw_data.number,
           project: project,
-          title: title,
-          description: description,
-          due_date: due_date,
+          title: raw_data.title,
+          description: raw_data.description,
+          due_date: raw_data.due_on,
           state: state,
-          created_at: created_at,
-          updated_at: updated_at
+          created_at: raw_data.created_at,
+          updated_at: raw_data.updated_at
         }
       end
 
@@ -20,33 +20,9 @@ module Gitlab
 
       private
 
-      def number
-        raw_data.number
-      end
-
-      def title
-        raw_data.title
-      end
-
-      def description
-        raw_data.description
-      end
-
-      def due_date
-        raw_data.due_on
-      end
-
       def state
         raw_data.state == 'closed' ? 'closed' : 'active'
       end
-
-      def created_at
-        raw_data.created_at
-      end
-
-      def updated_at
-        state == 'closed' ? raw_data.closed_at : raw_data.updated_at
-      end
     end
   end
 end

lib/gitlab/github_import/pull_request_formatter.rb

@@ -20,7 +20,7 @@ module Gitlab
           author_id: author_id,
           assignee_id: assignee_id,
           created_at: raw_data.created_at,
-          updated_at: updated_at
+          updated_at: raw_data.updated_at
         }
       end
 
@@ -103,15 +103,6 @@ module Gitlab
                      'opened'
                    end
       end
-
-      def updated_at
-        case state
-        when 'merged' then raw_data.merged_at
-        when 'closed' then raw_data.closed_at
-        else
-          raw_data.updated_at
-        end
-      end
     end
   end
 end

spec/lib/gitlab/github_import/issue_formatter_spec.rb

@@ -48,8 +48,7 @@ describe Gitlab::GithubImport::IssueFormatter, lib: true do
     end
 
     context 'when issue is closed' do
-      let(:closed_at) { DateTime.strptime('2011-01-28T19:01:12Z') }
-      let(:raw_data) { double(base_data.merge(state: 'closed', closed_at: closed_at)) }
+      let(:raw_data) { double(base_data.merge(state: 'closed')) }
 
       it 'returns formatted attributes' do
         expected = {
@@ -62,7 +61,7 @@ describe Gitlab::GithubImport::IssueFormatter, lib: true do
           author_id: project.creator_id,
           assignee_id: nil,
           created_at: created_at,
-          updated_at: closed_at
+          updated_at: updated_at
         }
 
         expect(issue.attributes).to eq(expected)

spec/lib/gitlab/github_import/milestone_formatter_spec.rb

@@ -40,8 +40,7 @@ describe Gitlab::GithubImport::MilestoneFormatter, lib: true do
     end
 
     context 'when milestone is closed' do
-      let(:closed_at) { DateTime.strptime('2011-01-28T19:01:12Z') }
-      let(:raw_data) { double(base_data.merge(state: 'closed', closed_at: closed_at)) }
+      let(:raw_data) { double(base_data.merge(state: 'closed')) }
 
       it 'returns formatted attributes' do
         expected = {
@@ -52,7 +51,7 @@ describe Gitlab::GithubImport::MilestoneFormatter, lib: true do
           state: 'closed',
           due_date: nil,
           created_at: created_at,
-          updated_at: closed_at
+          updated_at: updated_at
         }
 
         expect(formatter.attributes).to eq(expected)

spec/lib/gitlab/github_import/pull_request_formatter_spec.rb

@@ -62,8 +62,7 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do
     end
 
     context 'when pull request is closed' do
-      let(:closed_at) { DateTime.strptime('2011-01-28T19:01:12Z') }
-      let(:raw_data) { double(base_data.merge(state: 'closed', closed_at: closed_at)) }
+      let(:raw_data) { double(base_data.merge(state: 'closed')) }
 
       it 'returns formatted attributes' do
         expected = {
@@ -81,7 +80,7 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do
           author_id: project.creator_id,
           assignee_id: nil,
           created_at: created_at,
-          updated_at: closed_at
+          updated_at: updated_at
         }
 
         expect(pull_request.attributes).to eq(expected)
@@ -108,7 +107,7 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do
           author_id: project.creator_id,
           assignee_id: nil,
           created_at: created_at,
-          updated_at: merged_at
+          updated_at: updated_at
         }
 
         expect(pull_request.attributes).to eq(expected)