Merge branch 'vij-remove-member-created-state' into 'master'

Replace Member created state with active

See merge request gitlab-org/gitlab!76653

app/models/member.rb

@@ -18,9 +18,8 @@ class Member < ApplicationRecord
   AVATAR_SIZE = 40
   ACCESS_REQUEST_APPROVERS_TO_BE_NOTIFIED_LIMIT = 10
 
-  STATE_CREATED = 0
+  STATE_ACTIVE = 0
   STATE_AWAITING = 1
-  STATE_ACTIVE = 2
 
   attr_accessor :raw_invite_token
 

db/post_migrate/20211220123956_update_invalid_member_states.rb

+# frozen_string_literal: true
+
+class UpdateInvalidMemberStates < Gitlab::Database::Migration[1.0]
+  class Member < ActiveRecord::Base
+    include EachBatch
+
+    self.table_name = 'members'
+
+    scope :in_invalid_state, -> { where(state: 2) }
+  end
+
+  def up
+    Member.in_invalid_state.each_batch do |relation|
+      relation.update_all(state: 0)
+    end
+  end
+
+  def down
+    # no-op as we don't need to revert any changed records
+  end
+end

db/schema_migrations/20211220123956

+088d17a1f55522c48e69ec78717b39b8c7538474a9263621bba1fa0280a27ad7
\ No newline at end of file

doc/api/members.md

@@ -114,7 +114,7 @@ GET /projects/:id/members/all
 | `id`      | integer/string | yes | The ID or [URL-encoded path of the project or group](index.md#namespaced-path-encoding) owned by the authenticated user |
 | `query`   | string | no     | A query string to search for members |
 | `user_ids`   | array of integers | no     | Filter the results on the given user IDs |
-| `state`   | string | no | Filter results by member state, one of `awaiting`, `active` or `created` **(PREMIUM)** |
+| `state`   | string | no | Filter results by member state, one of `awaiting` or `active` **(PREMIUM)** |
 
 ```shell
 curl --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/groups/:id/members/all"

ee/app/models/ee/member.rb

@@ -6,18 +6,15 @@ module EE
     extend ::Gitlab::Utils::Override
 
     prepended do
-      state_machine :state, initial: :created do
+      state_machine :state, initial: :active do
         event :wait do
-          transition created: :awaiting
           transition active: :awaiting
         end
 
         event :activate do
-          transition created: :active
           transition awaiting: :active
         end
 
-        state :created, value: ::Member::STATE_CREATED
         state :awaiting, value: ::Member::STATE_AWAITING
         state :active, value: ::Member::STATE_ACTIVE
       end
@@ -150,9 +147,9 @@ module EE
     end
 
     def set_membership_activation
-      return unless group && ::Feature.enabled?(:saas_user_caps, group.root_ancestor, default_enabled: :yaml)
+      return unless group
 
-      self.state = group.user_cap_reached? ? ::Member::STATE_AWAITING : ::Member::STATE_ACTIVE
+      self.state = ::Member::STATE_AWAITING if group.user_cap_reached?
     end
   end
 end

ee/lib/ee/api/helpers/members_helpers.rb

@@ -22,7 +22,7 @@ module EE
           end
 
           params :optional_state_filter_ee do
-            optional :state, type: String, desc: 'Filter results by member state', values: %w(awaiting active created)
+            optional :state, type: String, desc: 'Filter results by member state', values: %w(awaiting active)
           end
         end
 

ee/spec/factories/group_members.rb

@@ -7,17 +7,5 @@ FactoryBot.modify do
         member.wait
       end
     end
-
-    trait :active do
-      after(:create) do |member|
-        member.activate
-      end
-    end
-
-    trait :created do
-      after(:create) do |member|
-        member.update!(state: Member::STATE_CREATED)
-      end
-    end
   end
 end

ee/spec/factories/project_members.rb

@@ -7,17 +7,5 @@ FactoryBot.modify do
         member.wait
       end
     end
-
-    trait :active do
-      after(:create) do |member|
-        member.activate
-      end
-    end
-
-    trait :created do
-      after(:create) do |member|
-        member.update!(state: Member::STATE_CREATED)
-      end
-    end
   end
 end

ee/spec/models/member_spec.rb

@@ -176,16 +176,16 @@ RSpec.describe Member, type: :model do
         stub_feature_flags(saas_user_caps: false)
       end
 
-      it 'sets the group member state to created' do
+      it 'sets the group member state to active' do
         group.add_developer(user)
 
-        expect(user.group_members.last).to be_created
+        expect(user.group_members.last).to be_active
       end
 
-      it 'sets the project member state to created' do
+      it 'sets the project member state to active' do
         project.add_developer(user)
 
-        expect(user.project_members.last).to be_created
+        expect(user.project_members.last).to be_active
       end
     end
 
@@ -238,10 +238,10 @@ RSpec.describe Member, type: :model do
     context 'when user is added to a group-less project' do
       let(:project) { create(:project) }
 
-      it 'adds project member and leaves the state to created' do
+      it 'adds project member and leaves the state to active' do
         project.add_developer(user)
 
-        expect(user.project_members.last).to be_created
+        expect(user.project_members.last).to be_active
       end
     end
   end

ee/spec/requests/api/members_spec.rb

@@ -1021,7 +1021,7 @@ RSpec.describe API::Members do
         end
 
         context 'when the activation fails due to no pending members to activate' do
-          let(:member) { create(:group_member, :active, group: group) }
+          let(:member) { create(:group_member, group: group) }
 
           it 'returns a bad request response' do
             put api(url, owner)
@@ -1232,14 +1232,6 @@ RSpec.describe API::Members do
           expect(subject.map { |u| u['id'] }).to match_array [awaiting_member.user_id]
         end
       end
-
-      context 'for created members' do
-        let(:state) { 'created' }
-
-        it 'returns only created members' do
-          expect(subject.map { |u| u['id'] }).to match_array [created_member.user_id]
-        end
-      end
     end
 
     context 'for group sources' do
@@ -1248,8 +1240,7 @@ RSpec.describe API::Members do
 
       it_behaves_like 'filtered results' do
         let_it_be(:awaiting_member) { create(:group_member, :awaiting, group: group) }
-        let_it_be(:active_member)   { create(:group_member, :active, group: group) }
-        let_it_be(:created_member)  { create(:group_member, :created, group: group) }
+        let_it_be(:active_member)   { create(:group_member, group: group) }
       end
     end
 
@@ -1259,8 +1250,7 @@ RSpec.describe API::Members do
 
       it_behaves_like 'filtered results' do
         let_it_be(:awaiting_member) { create(:project_member, :awaiting, project: project) }
-        let_it_be(:active_member)   { create(:project_member, :active, project: project) }
-        let_it_be(:created_member)  { create(:project_member, :created, project: project) }
+        let_it_be(:active_member)   { create(:project_member, project: project) }
       end
     end
   end

ee/spec/services/members/activate_service_spec.rb

@@ -94,7 +94,7 @@ RSpec.describe Members::ActivateService do
         end
 
         context 'when member is not an awaiting member' do
-          let(:member) { create(:group_member, :active, group: root_group, user: user) }
+          let(:member) { create(:group_member, group: root_group, user: user) }
 
           it 'returns an error' do
             result = execute

spec/migrations/update_invalid_member_states_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+require_migration!
+
+RSpec.describe UpdateInvalidMemberStates do
+  let(:members) { table(:members) }
+  let(:groups) { table(:namespaces) }
+  let(:projects) { table(:projects) }
+  let(:users) { table(:users) }
+
+  before do
+    user = users.create!(first_name: 'Test', last_name: 'User', email: 'test@user.com', projects_limit: 1)
+    group = groups.create!(name: 'gitlab', path: 'gitlab-org')
+    project = projects.create!(namespace_id: group.id)
+
+    members.create!(state: 2, source_id: group.id, source_type: 'Group', type: 'GroupMember', user_id: user.id, access_level: 50, notification_level: 0)
+    members.create!(state: 2, source_id: project.id, source_type: 'Project', type: 'ProjectMember', user_id: user.id, access_level: 50, notification_level: 0)
+    members.create!(state: 1, source_id: group.id, source_type: 'Group', type: 'GroupMember', user_id: user.id, access_level: 50, notification_level: 0)
+    members.create!(state: 0, source_id: group.id, source_type: 'Group', type: 'GroupMember', user_id: user.id, access_level: 50, notification_level: 0)
+  end
+
+  it 'updates matching member record states' do
+    expect { migrate! }
+      .to change { members.where(state: 0).count }.from(1).to(3)
+      .and change { members.where(state: 2).count }.from(2).to(0)
+      .and change { members.where(state: 1).count }.by(0)
+  end
+end