Commit f6ddac23 authored by Stan Hu's avatar Stan Hu

Merge branch '220246-need-a-way-to-ensure-all-newly-created-paid-groups-are-indexed' into 'master'

Resolve "Need a way to ensure all newly created paid groups are indexed"

See merge request gitlab-org/gitlab!35172
parents caad4c70 de536471
# Elasticsearch for paid tiers on GitLab.com
> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/220246) in GitLab 13.2
> - It's deployed behind a feature flag, disabled by default.
> - It's disabled on GitLab.com.
> - It's not recommended for use in GitLab self-managed instances.
This document describes how to enable Elasticsearch with GitLab for all paid tiers on GitLab.com. Once enabled,
all paid tiers will have access to the [Advanced Global Search feature](../../integration/elasticsearch.md) on GitLab.com.
## Enable or disable Elasticsearch for all paid tiers on GitLab.com
Since we're still in the process of rolling this out and want to control the timing this is behind a feature flag
which defaults to off.
To enable it:
```ruby
# Instance-wide
Feature.enable(:elasticsearch_index_only_paid_groups)
```
To disable it:
```ruby
# Instance-wide
Feature.disable(:elasticsearch_index_only_paid_groups)
```
...@@ -3,6 +3,7 @@ ...@@ -3,6 +3,7 @@
class GitlabSubscription < ApplicationRecord class GitlabSubscription < ApplicationRecord
default_value_for(:start_date) { Date.today } default_value_for(:start_date) { Date.today }
before_update :log_previous_state_for_update before_update :log_previous_state_for_update
after_commit :index_namespace, on: [:create, :update]
after_destroy_commit :log_previous_state_for_destroy after_destroy_commit :log_previous_state_for_destroy
belongs_to :namespace belongs_to :namespace
...@@ -86,4 +87,15 @@ class GitlabSubscription < ApplicationRecord ...@@ -86,4 +87,15 @@ class GitlabSubscription < ApplicationRecord
def hosted? def hosted?
namespace_id.present? namespace_id.present?
end end
# Kick off Elasticsearch indexing for paid groups with new or upgraded paid, hosted subscriptions
# Uses safe_find_or_create_by to avoid ActiveRecord::RecordNotUnique exception when upgrading from
# one paid plan to another paid plan
def index_namespace
return unless ::Feature.enabled?(:elasticsearch_index_only_paid_groups) &&
has_a_paid_hosted_plan? &&
saved_changes.key?('hosted_plan_id')
ElasticsearchIndexedNamespace.safe_find_or_create_by!(namespace_id: namespace_id)
end
end end
---
title: Ensure all newly paid groups are indexed.
merge_request: 35172
author:
type: changed
...@@ -28,5 +28,17 @@ FactoryBot.define do ...@@ -28,5 +28,17 @@ FactoryBot.define do
trait :gold do trait :gold do
association :hosted_plan, factory: :gold_plan association :hosted_plan, factory: :gold_plan
end end
# for testing elasticsearch_indexed_namespace and elastic_namespace_rollout_worker which
# eventually will not be required once all paid groups are indexed
trait :without_index_namespace_callback do
after(:build) do |gitlab_subcription|
GitlabSubscription.skip_callback(:commit, :after, :index_namespace)
end
after(:create) do
GitlabSubscription.set_callback(:commit, :after, :index_namespace)
end
end
end end
end end
...@@ -35,9 +35,9 @@ RSpec.describe ElasticsearchIndexedNamespace do ...@@ -35,9 +35,9 @@ RSpec.describe ElasticsearchIndexedNamespace do
end end
let_it_be(:namespaces) { create_list(:namespace, 3) } let_it_be(:namespaces) { create_list(:namespace, 3) }
let_it_be(:subscription1) { create(:gitlab_subscription, namespace: namespaces[2]) } let_it_be(:subscription1) { create(:gitlab_subscription, :without_index_namespace_callback, namespace: namespaces[2]) }
let_it_be(:subscription2) { create(:gitlab_subscription, namespace: namespaces[0]) } let_it_be(:subscription2) { create(:gitlab_subscription, :without_index_namespace_callback, namespace: namespaces[0]) }
let_it_be(:subscription3) { create(:gitlab_subscription, :silver, namespace: namespaces[1]) } let_it_be(:subscription3) { create(:gitlab_subscription, :silver, :without_index_namespace_callback, namespace: namespaces[1]) }
before do before do
stub_ee_application_setting(elasticsearch_indexing: false) stub_ee_application_setting(elasticsearch_indexing: false)
......
...@@ -3,6 +3,12 @@ ...@@ -3,6 +3,12 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe GitlabSubscription do RSpec.describe GitlabSubscription do
using RSpec::Parameterized::TableSyntax
before do
stub_feature_flags(elasticsearch_index_only_paid_groups: false)
end
%i[free_plan bronze_plan silver_plan gold_plan early_adopter_plan].each do |plan| %i[free_plan bronze_plan silver_plan gold_plan early_adopter_plan].each do |plan|
let_it_be(plan) { create(plan) } let_it_be(plan) { create(plan) }
end end
...@@ -247,6 +253,44 @@ RSpec.describe GitlabSubscription do ...@@ -247,6 +253,44 @@ RSpec.describe GitlabSubscription do
end end
describe 'callbacks' do describe 'callbacks' do
context 'after_commit :index_namespace' do
where(:elasticsearch_index_only_paid_groups_setting, :operation, :initial_plan, :should_update_plan?, :should_index_namespace?) do
false | :create | :bronze | false | false
true | :create | :bronze | false | true
true | :create | :free | false | false
true | :create | { trial: true } | false | false
false | :update | :bronze | false | false
true | :update | :bronze | false | false
true | :update | :bronze | true | true
true | :update | :free | true | true
true | :update | { trial: true } | true | true
end
with_them do
let!(:gitlab_subscription) { operation == :create ? build(:gitlab_subscription, initial_plan) : create(:gitlab_subscription, initial_plan) }
before do
stub_feature_flags(elasticsearch_index_only_paid_groups: elasticsearch_index_only_paid_groups_setting)
end
it 'indexes namespace' do
if should_index_namespace?
expect(ElasticsearchIndexedNamespace).to receive(:safe_find_or_create_by!).with(namespace_id: gitlab_subscription.namespace_id)
else
expect(ElasticsearchIndexedNamespace).not_to receive(:safe_find_or_create_by!)
end
case operation
when :create
gitlab_subscription.save!
when :update
update_attributes = should_update_plan? ? { hosted_plan: create(:silver_plan), trial: false } : { max_seats_used: 32 }
gitlab_subscription.update!(update_attributes)
end
end
end
end
it 'gitlab_subscription columns are contained in gitlab_subscription_history columns' do it 'gitlab_subscription columns are contained in gitlab_subscription_history columns' do
diff_attrs = %w(updated_at) diff_attrs = %w(updated_at)
expect(described_class.attribute_names - GitlabSubscriptionHistory.attribute_names).to eq(diff_attrs) expect(described_class.attribute_names - GitlabSubscriptionHistory.attribute_names).to eq(diff_attrs)
......
...@@ -15,7 +15,7 @@ RSpec.describe ElasticNamespaceRolloutWorker do ...@@ -15,7 +15,7 @@ RSpec.describe ElasticNamespaceRolloutWorker do
before_all do before_all do
Plan::PAID_HOSTED_PLANS.each do |plan| Plan::PAID_HOSTED_PLANS.each do |plan|
create_list(:gitlab_subscription, 4, hosted_plan: public_send("#{plan}_plan")) create_list(:gitlab_subscription, 4, :without_index_namespace_callback, hosted_plan: public_send("#{plan}_plan"))
end end
end end
......
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