From a2b3e7bdd7123e46836c4df94efcfcfe84151e07 Mon Sep 17 00:00:00 2001
From: Sheldon Led <sheldonled@gitlab.com>
Date: Mon, 8 Jul 2024 10:43:40 +0000
Subject: [PATCH] Add `seats_in_use` to GitlabSubscriptionHistory trarcked
 attributes

GitlabSubscriptionHistory now tracks `seats_in_use` changes for
subscriptions attached to a namespace

Changelog: changed
MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/157311
EE: true
---
 .rubocop_todo/layout/array_alignment.yml               |  1 -
 .rubocop_todo/rspec/feature_category.yml               |  1 -
 ..._add_seats_in_use_to_gitlab_subscription_history.rb |  9 +++++++++
 db/schema_migrations/20240625124852                    |  1 +
 db/structure.sql                                       |  1 +
 ee/app/models/gitlab_subscription_history.rb           | 10 ++++++----
 ee/spec/models/gitlab_subscription_history_spec.rb     | 10 ++++++++--
 spec/support/rspec_order_todo.yml                      |  1 -
 8 files changed, 25 insertions(+), 9 deletions(-)
 create mode 100644 db/migrate/20240625124852_add_seats_in_use_to_gitlab_subscription_history.rb
 create mode 100644 db/schema_migrations/20240625124852

diff --git a/.rubocop_todo/layout/array_alignment.yml b/.rubocop_todo/layout/array_alignment.yml
index a94423620a2a..6619ee4138d5 100644
--- a/.rubocop_todo/layout/array_alignment.yml
+++ b/.rubocop_todo/layout/array_alignment.yml
@@ -19,7 +19,6 @@ Layout/ArrayAlignment:
     - 'ee/app/models/ee/epic.rb'
     - 'ee/app/models/ee/user.rb'
     - 'ee/app/models/geo/event_log.rb'
-    - 'ee/app/models/gitlab_subscription_history.rb'
     - 'ee/app/models/ip_restriction.rb'
     - 'ee/app/models/license.rb'
     - 'ee/app/models/protected_environment.rb'
diff --git a/.rubocop_todo/rspec/feature_category.yml b/.rubocop_todo/rspec/feature_category.yml
index 1156e89a0197..11d5abd997b4 100644
--- a/.rubocop_todo/rspec/feature_category.yml
+++ b/.rubocop_todo/rspec/feature_category.yml
@@ -854,7 +854,6 @@ RSpec/FeatureCategory:
     - 'ee/spec/models/elastic/reindexing_task_spec.rb'
     - 'ee/spec/models/epic_user_mention_spec.rb'
     - 'ee/spec/models/gitlab/seat_link_data_spec.rb'
-    - 'ee/spec/models/gitlab_subscription_history_spec.rb'
     - 'ee/spec/models/gitlab_subscriptions/upcoming_reconciliation_spec.rb'
     - 'ee/spec/models/group_deletion_schedule_spec.rb'
     - 'ee/spec/models/group_merge_request_approval_setting_spec.rb'
diff --git a/db/migrate/20240625124852_add_seats_in_use_to_gitlab_subscription_history.rb b/db/migrate/20240625124852_add_seats_in_use_to_gitlab_subscription_history.rb
new file mode 100644
index 000000000000..b59588c63dbf
--- /dev/null
+++ b/db/migrate/20240625124852_add_seats_in_use_to_gitlab_subscription_history.rb
@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+class AddSeatsInUseToGitlabSubscriptionHistory < Gitlab::Database::Migration[2.2]
+  milestone '17.2'
+
+  def change
+    add_column :gitlab_subscription_histories, :seats_in_use, :integer, null: true
+  end
+end
diff --git a/db/schema_migrations/20240625124852 b/db/schema_migrations/20240625124852
new file mode 100644
index 000000000000..253df66ed608
--- /dev/null
+++ b/db/schema_migrations/20240625124852
@@ -0,0 +1 @@
+f7d04a541cef9331788b7192b098d14b640a60f340d44cd2805d9bcd970bb7af
\ No newline at end of file
diff --git a/db/structure.sql b/db/structure.sql
index b6ac1d911d2e..9cd33629ef01 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -10699,6 +10699,7 @@ CREATE TABLE gitlab_subscription_histories (
     trial_starts_on date,
     auto_renew boolean,
     trial_extension_type smallint,
+    seats_in_use integer,
     CONSTRAINT check_6d5f27a106 CHECK ((namespace_id IS NOT NULL))
 );
 
diff --git a/ee/app/models/gitlab_subscription_history.rb b/ee/app/models/gitlab_subscription_history.rb
index 744c3542b36a..63413d6163e7 100644
--- a/ee/app/models/gitlab_subscription_history.rb
+++ b/ee/app/models/gitlab_subscription_history.rb
@@ -3,9 +3,11 @@
 # GitlabSubscriptionHistory records the previous value before change.
 # `gitlab_subscription_created` is not used. Because there is no previous value before creation.
 class GitlabSubscriptionHistory < ApplicationRecord
-  enum change_type: [:gitlab_subscription_created,
-                     :gitlab_subscription_updated,
-                     :gitlab_subscription_destroyed]
+  enum change_type: [
+    :gitlab_subscription_created,
+    :gitlab_subscription_updated,
+    :gitlab_subscription_destroyed
+  ]
 
   validates :gitlab_subscription_id, presence: true
   validates :namespace_id, presence: true
@@ -22,6 +24,7 @@ class GitlabSubscriptionHistory < ApplicationRecord
     trial_ends_on
     namespace_id
     hosted_plan_id
+    seats_in_use
     max_seats_used
     seats
     trial
@@ -35,7 +38,6 @@ class GitlabSubscriptionHistory < ApplicationRecord
   # good reason not to.
   # We don't use this list other than to raise awareness of which attributes we should not track.
   OMITTED_ATTRIBUTES = %w[
-    seats_in_use
     seats_owed
     max_seats_used_changed_at
     last_seat_refresh_at
diff --git a/ee/spec/models/gitlab_subscription_history_spec.rb b/ee/spec/models/gitlab_subscription_history_spec.rb
index 2905b5bacf19..39a433c963a3 100644
--- a/ee/spec/models/gitlab_subscription_history_spec.rb
+++ b/ee/spec/models/gitlab_subscription_history_spec.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe GitlabSubscriptionHistory do
+RSpec.describe GitlabSubscriptionHistory, feature_category: :seat_cost_management do
   describe '.create_from_change' do
     context 'when supplied an invalid change type' do
       it 'raises an error' do
@@ -61,11 +61,17 @@
           'namespace_id' => 2,
           'gitlab_subscription_created_at' => current_time,
           'gitlab_subscription_updated_at' => current_time,
+          'seats_in_use' => 10,
           'trial' => true,
           'seats' => 15
         )
 
-        expect(record.attributes.keys).not_to include('non_existent_attribute', 'seats_in_use')
+        expect(record.attributes.keys).to include(*GitlabSubscriptionHistory::TRACKED_ATTRIBUTES)
+
+        expect(record.attributes.keys).not_to include(
+          'non_existent_attribute',
+          *GitlabSubscriptionHistory::OMITTED_ATTRIBUTES
+        )
       end
     end
   end
diff --git a/spec/support/rspec_order_todo.yml b/spec/support/rspec_order_todo.yml
index 5dae8a460d52..2ccbb0c75a81 100644
--- a/spec/support/rspec_order_todo.yml
+++ b/spec/support/rspec_order_todo.yml
@@ -1641,7 +1641,6 @@
 - './ee/spec/models/geo/upload_registry_spec.rb'
 - './ee/spec/models/geo/upload_state_spec.rb'
 - './ee/spec/models/gitlab/seat_link_data_spec.rb'
-- './ee/spec/models/gitlab_subscription_history_spec.rb'
 - './ee/spec/models/gitlab_subscriptions/features_spec.rb'
 - './ee/spec/models/gitlab_subscription_spec.rb'
 - './ee/spec/models/gitlab_subscriptions/upcoming_reconciliation_spec.rb'
-- 
GitLab