Merge branch 'sh-fix-bitbucket-importer-pr-state' into 'master'

Fix Bitbucket Cloud importer pull request state

Closes #35746

See merge request gitlab-org/gitlab!19734

app/models/concerns/issuable.rb

@@ -137,6 +137,26 @@ module Issuable
 
     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
     # http://api.rubyonrails.org/classes/ActiveRecord/Locking/Optimistic.html
     def locking_enabled?

changelogs/unreleased/sh-fix-bitbucket-importer-pr-state.yml

+---
+title: Fix Bitbucket Cloud importer pull request state
+merge_request: 19734
+author:
+type: fixed

spec/lib/gitlab/bitbucket_import/importer_spec.rb

@@ -158,6 +158,7 @@ describe Gitlab::BitbucketImport::Importer do
       expect { subject.execute }.to change { MergeRequest.count }.by(1)
 
       merge_request = MergeRequest.first
+      expect(merge_request.state).to eq('merged')
       expect(merge_request.notes.count).to eq(2)
       expect(merge_request.notes.map(&:discussion_id).uniq.count).to eq(1)
 

spec/models/concerns/issuable_spec.rb

@@ -111,6 +111,34 @@ describe Issuable do
     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
     let(:group) { create(:group) }
     let(:project) { create(:project, group: group) }