Commit 8011b9c9 authored by Stan Hu's avatar Stan Hu

Fix Bitbucket Cloud importer pull request state

Due to e404c311, all pull requests are
created in the `opened` state because assigning the `state` parameter
when creating a `MergeRequest` no longer works. To fix this, define the
`state_id` in the `initalize` step to ensure state_machine has the
right value to use.

Closes https://gitlab.com/gitlab-org/gitlab/issues/35746
parent ccb00f35
...@@ -137,6 +137,26 @@ module Issuable ...@@ -137,6 +137,26 @@ module Issuable
strip_attributes :title strip_attributes :title
# The state_machine gem will reset the value of state_id unless it
# is a raw attribute passed in here:
# https://gitlab.com/gitlab-org/gitlab/issues/35746#note_241148787
#
# This assumes another initialize isn't defined. Otherwise this
# method may need to be prepended.
def initialize(attributes = nil)
if attributes.is_a?(Hash)
attr = attributes.symbolize_keys
if attr.key?(:state) && !attr.key?(:state_id)
value = attr.delete(:state)
state_id = self.class.available_states[value]
attributes[:state_id] = state_id if state_id
end
end
super(attributes)
end
# We want to use optimistic lock for cases when only title or description are involved # We want to use optimistic lock for cases when only title or description are involved
# http://api.rubyonrails.org/classes/ActiveRecord/Locking/Optimistic.html # http://api.rubyonrails.org/classes/ActiveRecord/Locking/Optimistic.html
def locking_enabled? def locking_enabled?
......
---
title: Fix Bitbucket Cloud importer pull request state
merge_request: 19734
author:
type: fixed
...@@ -158,6 +158,7 @@ describe Gitlab::BitbucketImport::Importer do ...@@ -158,6 +158,7 @@ describe Gitlab::BitbucketImport::Importer do
expect { subject.execute }.to change { MergeRequest.count }.by(1) expect { subject.execute }.to change { MergeRequest.count }.by(1)
merge_request = MergeRequest.first merge_request = MergeRequest.first
expect(merge_request.state).to eq('merged')
expect(merge_request.notes.count).to eq(2) expect(merge_request.notes.count).to eq(2)
expect(merge_request.notes.map(&:discussion_id).uniq.count).to eq(1) expect(merge_request.notes.map(&:discussion_id).uniq.count).to eq(1)
......
...@@ -111,6 +111,34 @@ describe Issuable do ...@@ -111,6 +111,34 @@ describe Issuable do
end end
end end
describe '.initialize' do
it 'maps the state to the right state_id' do
described_class::STATE_ID_MAP.each do |key, value|
issuable = MergeRequest.new(state: key)
expect(issuable.state).to eq(key)
expect(issuable.state_id).to eq(value)
end
end
it 'maps a string version of the state to the right state_id' do
described_class::STATE_ID_MAP.each do |key, value|
issuable = MergeRequest.new('state' => key)
expect(issuable.state).to eq(key)
expect(issuable.state_id).to eq(value)
end
end
it 'gives preference to state_id if present' do
issuable = MergeRequest.new('state' => 'opened',
'state_id' => described_class::STATE_ID_MAP['merged'])
expect(issuable.state).to eq('merged')
expect(issuable.state_id).to eq(described_class::STATE_ID_MAP['merged'])
end
end
describe '#milestone_available?' do describe '#milestone_available?' do
let(:group) { create(:group) } let(:group) { create(:group) }
let(:project) { create(:project, group: group) } let(:project) { create(:project, group: group) }
......
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