Commit 9c6777ea authored by Robert Speicher's avatar Robert Speicher

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
parents 44a16ebf 9ef743d1
...@@ -58,6 +58,7 @@ v 8.12.0 (unreleased) ...@@ -58,6 +58,7 @@ v 8.12.0 (unreleased)
v 8.11.4 (unreleased) v 8.11.4 (unreleased)
- Fix broken gitlab:backup:restore because of bad permissions on repo storage !6098 (Dirk Hörner) - Fix broken gitlab:backup:restore because of bad permissions on repo storage !6098 (Dirk Hörner)
- Fix sorting issues by "last updated" doesn't work after import from GitHub
- Creating an issue through our API now emails label subscribers !5720 - Creating an issue through our API now emails label subscribers !5720
- Block concurrent updates for Pipeline - Block concurrent updates for Pipeline
- Fix resolving conflicts on forks - Fix resolving conflicts on forks
......
...@@ -152,12 +152,14 @@ module Gitlab ...@@ -152,12 +152,14 @@ module Gitlab
end end
def create_comments(issuable, comments) def create_comments(issuable, comments)
comments.each do |raw| ActiveRecord::Base.no_touching do
begin comments.each do |raw|
comment = CommentFormatter.new(project, raw) begin
issuable.notes.create!(comment.attributes) comment = CommentFormatter.new(project, raw)
rescue => e issuable.notes.create!(comment.attributes)
errors << { type: :comment, url: Gitlab::UrlSanitizer.sanitize(raw.url), errors: e.message } rescue => e
errors << { type: :comment, url: Gitlab::UrlSanitizer.sanitize(raw.url), errors: e.message }
end
end end
end end
end end
......
...@@ -12,7 +12,7 @@ module Gitlab ...@@ -12,7 +12,7 @@ module Gitlab
author_id: author_id, author_id: author_id,
assignee_id: assignee_id, assignee_id: assignee_id,
created_at: raw_data.created_at, created_at: raw_data.created_at,
updated_at: updated_at updated_at: raw_data.updated_at
} }
end end
...@@ -69,10 +69,6 @@ module Gitlab ...@@ -69,10 +69,6 @@ module Gitlab
def state def state
raw_data.state == 'closed' ? 'closed' : 'opened' raw_data.state == 'closed' ? 'closed' : 'opened'
end end
def updated_at
state == 'closed' ? raw_data.closed_at : raw_data.updated_at
end
end end
end end
end end
...@@ -3,14 +3,14 @@ module Gitlab ...@@ -3,14 +3,14 @@ module Gitlab
class MilestoneFormatter < BaseFormatter class MilestoneFormatter < BaseFormatter
def attributes def attributes
{ {
iid: number, iid: raw_data.number,
project: project, project: project,
title: title, title: raw_data.title,
description: description, description: raw_data.description,
due_date: due_date, due_date: raw_data.due_on,
state: state, state: state,
created_at: created_at, created_at: raw_data.created_at,
updated_at: updated_at updated_at: raw_data.updated_at
} }
end end
...@@ -20,33 +20,9 @@ module Gitlab ...@@ -20,33 +20,9 @@ module Gitlab
private 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 def state
raw_data.state == 'closed' ? 'closed' : 'active' raw_data.state == 'closed' ? 'closed' : 'active'
end 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 end
end end
...@@ -20,7 +20,7 @@ module Gitlab ...@@ -20,7 +20,7 @@ module Gitlab
author_id: author_id, author_id: author_id,
assignee_id: assignee_id, assignee_id: assignee_id,
created_at: raw_data.created_at, created_at: raw_data.created_at,
updated_at: updated_at updated_at: raw_data.updated_at
} }
end end
...@@ -103,15 +103,6 @@ module Gitlab ...@@ -103,15 +103,6 @@ module Gitlab
'opened' 'opened'
end end
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 end
end end
...@@ -48,8 +48,7 @@ describe Gitlab::GithubImport::IssueFormatter, lib: true do ...@@ -48,8 +48,7 @@ describe Gitlab::GithubImport::IssueFormatter, lib: true do
end end
context 'when issue is closed' do 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')) }
let(:raw_data) { double(base_data.merge(state: 'closed', closed_at: closed_at)) }
it 'returns formatted attributes' do it 'returns formatted attributes' do
expected = { expected = {
...@@ -62,7 +61,7 @@ describe Gitlab::GithubImport::IssueFormatter, lib: true do ...@@ -62,7 +61,7 @@ describe Gitlab::GithubImport::IssueFormatter, lib: true do
author_id: project.creator_id, author_id: project.creator_id,
assignee_id: nil, assignee_id: nil,
created_at: created_at, created_at: created_at,
updated_at: closed_at updated_at: updated_at
} }
expect(issue.attributes).to eq(expected) expect(issue.attributes).to eq(expected)
......
...@@ -40,8 +40,7 @@ describe Gitlab::GithubImport::MilestoneFormatter, lib: true do ...@@ -40,8 +40,7 @@ describe Gitlab::GithubImport::MilestoneFormatter, lib: true do
end end
context 'when milestone is closed' do 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')) }
let(:raw_data) { double(base_data.merge(state: 'closed', closed_at: closed_at)) }
it 'returns formatted attributes' do it 'returns formatted attributes' do
expected = { expected = {
...@@ -52,7 +51,7 @@ describe Gitlab::GithubImport::MilestoneFormatter, lib: true do ...@@ -52,7 +51,7 @@ describe Gitlab::GithubImport::MilestoneFormatter, lib: true do
state: 'closed', state: 'closed',
due_date: nil, due_date: nil,
created_at: created_at, created_at: created_at,
updated_at: closed_at updated_at: updated_at
} }
expect(formatter.attributes).to eq(expected) expect(formatter.attributes).to eq(expected)
......
...@@ -62,8 +62,7 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do ...@@ -62,8 +62,7 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do
end end
context 'when pull request is closed' do 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')) }
let(:raw_data) { double(base_data.merge(state: 'closed', closed_at: closed_at)) }
it 'returns formatted attributes' do it 'returns formatted attributes' do
expected = { expected = {
...@@ -81,7 +80,7 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do ...@@ -81,7 +80,7 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do
author_id: project.creator_id, author_id: project.creator_id,
assignee_id: nil, assignee_id: nil,
created_at: created_at, created_at: created_at,
updated_at: closed_at updated_at: updated_at
} }
expect(pull_request.attributes).to eq(expected) expect(pull_request.attributes).to eq(expected)
...@@ -108,7 +107,7 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do ...@@ -108,7 +107,7 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do
author_id: project.creator_id, author_id: project.creator_id,
assignee_id: nil, assignee_id: nil,
created_at: created_at, created_at: created_at,
updated_at: merged_at updated_at: updated_at
} }
expect(pull_request.attributes).to eq(expected) expect(pull_request.attributes).to eq(expected)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment