From 32780082abf3111d57287c5c81e97aae4a64402e Mon Sep 17 00:00:00 2001
From: Linjie Zhang <ljzhang@gitlab.cn>
Date: Wed, 30 Aug 2023 14:05:10 +0000
Subject: [PATCH] Add user_list to feature_flag api
Changelog: added
---
app/models/operations/feature_flag.rb | 2 +-
doc/api/feature_flags.md | 39 ++++-
.../entities/feature_flag/basic_user_list.rb | 14 ++
lib/api/entities/feature_flag/strategy.rb | 1 +
lib/api/entities/feature_flag/user_list.rb | 6 +-
lib/api/feature_flags.rb | 4 +-
.../public_api/v4/operations/strategy.json | 3 +-
.../public_api/v4/operations/user_list.json | 16 ++
spec/requests/api/feature_flags_spec.rb | 162 +++++++++++++++++-
9 files changed, 234 insertions(+), 13 deletions(-)
create mode 100644 lib/api/entities/feature_flag/basic_user_list.rb
create mode 100644 spec/fixtures/api/schemas/public_api/v4/operations/user_list.json
diff --git a/app/models/operations/feature_flag.rb b/app/models/operations/feature_flag.rb
index 01db0a5cf8b23..b93537e0d1edf 100644
--- a/app/models/operations/feature_flag.rb
+++ b/app/models/operations/feature_flag.rb
@@ -52,7 +52,7 @@ class FeatureFlag < ApplicationRecord
class << self
def preload_relations
- preload(strategies: :scopes)
+ preload(strategies: [:scopes, :user_list])
end
def for_unleash_client(project, environment)
diff --git a/doc/api/feature_flags.md b/doc/api/feature_flags.md
index d7bf4fe826e53..06661207f8eaf 100644
--- a/doc/api/feature_flags.md
+++ b/doc/api/feature_flags.md
@@ -59,7 +59,8 @@ Example response:
"id": 1,
"environment_scope": "production"
}
- ]
+ ],
+ "user_list": null
}
]
},
@@ -81,7 +82,36 @@ Example response:
"id": 2,
"environment_scope": "staging"
}
- ]
+ ],
+ "user_list": null
+ }
+ ]
+ },
+ {
+ "name":"user_list",
+ "description":"This feature is about user list",
+ "active": true,
+ "version": "new_version_flag",
+ "created_at":"2019-11-04T08:13:10.507Z",
+ "updated_at":"2019-11-04T08:13:10.507Z",
+ "scopes":[],
+ "strategies": [
+ {
+ "id": 2,
+ "name": "gitlabUserList",
+ "parameters": {},
+ "scopes": [
+ {
+ "id": 2,
+ "environment_scope": "staging"
+ }
+ ],
+ "user_list": {
+ "id": 1,
+ "iid": 1,
+ "name": "My user list",
+ "user_xids": "user1,user2,user3"
+ }
}
]
}
@@ -126,7 +156,8 @@ Example response:
"id": 37,
"environment_scope": "production"
}
- ]
+ ],
+ "user_list": null
}
]
}
@@ -152,6 +183,7 @@ POST /projects/:id/feature_flags
| `strategies:parameters` | JSON | no | The strategy parameters. |
| `strategies:scopes` | JSON | no | The scopes for the strategy. |
| `strategies:scopes:environment_scope` | string | no | The environment scope of the scope. |
+| `strategies:user_list_id` | integer/string | no | The ID of the feature flag user list. If strategy is `gitlabUserList`. |
```shell
curl "https://gitlab.example.com/api/v4/projects/1/feature_flags" \
@@ -217,6 +249,7 @@ PUT /projects/:id/feature_flags/:feature_flag_name
| `strategies:scopes:id` | JSON | no | The environment scope ID. |
| `strategies:scopes:environment_scope` | string | no | The environment scope of the scope. |
| `strategies:scopes:_destroy` | boolean | no | Delete the scope when true. |
+| `strategies:user_list_id` | integer/string | no | The ID of the feature flag user list. If strategy is `gitlabUserList`. |
```shell
curl "https://gitlab.example.com/api/v4/projects/1/feature_flags/awesome_feature" \
diff --git a/lib/api/entities/feature_flag/basic_user_list.rb b/lib/api/entities/feature_flag/basic_user_list.rb
new file mode 100644
index 0000000000000..df577e9f1a4f8
--- /dev/null
+++ b/lib/api/entities/feature_flag/basic_user_list.rb
@@ -0,0 +1,14 @@
+# frozen_string_literal: true
+
+module API
+ module Entities
+ class FeatureFlag < Grape::Entity
+ class BasicUserList < Grape::Entity
+ expose :id, documentation: { type: 'integer', example: 1 }
+ expose :iid, documentation: { type: 'integer', example: 1 }
+ expose :name, documentation: { type: 'string', example: 'user_list' }
+ expose :user_xids, documentation: { type: 'string', example: 'user1,user2' }
+ end
+ end
+ end
+end
diff --git a/lib/api/entities/feature_flag/strategy.rb b/lib/api/entities/feature_flag/strategy.rb
index 6217842037025..aea38f0e24a61 100644
--- a/lib/api/entities/feature_flag/strategy.rb
+++ b/lib/api/entities/feature_flag/strategy.rb
@@ -8,6 +8,7 @@ class Strategy < Grape::Entity
expose :name, documentation: { type: 'string', example: 'userWithId' }
expose :parameters, documentation: { type: 'string', example: '{"userIds": "user1"}' }
expose :scopes, using: FeatureFlag::Scope
+ expose :user_list, using: FeatureFlag::BasicUserList
end
end
end
diff --git a/lib/api/entities/feature_flag/user_list.rb b/lib/api/entities/feature_flag/user_list.rb
index efb3261658ac7..47f89cea4d20b 100644
--- a/lib/api/entities/feature_flag/user_list.rb
+++ b/lib/api/entities/feature_flag/user_list.rb
@@ -3,16 +3,12 @@
module API
module Entities
class FeatureFlag < Grape::Entity
- class UserList < Grape::Entity
+ class UserList < BasicUserList
include RequestAwareEntity
- expose :id, documentation: { type: 'integer', example: 1 }
- expose :iid, documentation: { type: 'integer', example: 1 }
expose :project_id, documentation: { type: 'integer', example: 2 }
expose :created_at, documentation: { type: 'dateTime', example: '2020-02-04T08:13:10.507Z' }
expose :updated_at, documentation: { type: 'dateTime', example: '2020-02-04T08:13:10.507Z' }
- expose :name, documentation: { type: 'string', example: 'user_list' }
- expose :user_xids, documentation: { type: 'string', example: 'user1,user2' }
expose :path do |list|
project_feature_flags_user_list_path(list.project, list)
diff --git a/lib/api/feature_flags.rb b/lib/api/feature_flags.rb
index 1846ddf683322..4ed288ee997bd 100644
--- a/lib/api/feature_flags.rb
+++ b/lib/api/feature_flags.rb
@@ -63,7 +63,8 @@ class FeatureFlags < ::API::Base
optional :version, type: String, desc: 'The version of the feature flag. Must be `new_version_flag`. Omit to create a Legacy feature flag.'
optional :strategies, type: Array do
requires :name, type: String, desc: 'The strategy name. Can be `default`, `gradualRolloutUserId`, `userWithId`, or `gitlabUserList`. In GitLab 13.5 and later, can be `flexibleRollout`'
- requires :parameters, type: JSON, desc: 'The strategy parameters as a JSON-formatted string e.g. `{"userIds":"user1"}`', documentation: { type: 'String' }
+ optional :parameters, type: JSON, desc: 'The strategy parameters as a JSON-formatted string e.g. `{"userIds":"user1"}`', documentation: { type: 'String' }
+ optional :user_list_id, type: Integer, desc: "The ID of the feature flag user list. If strategy is `gitlabUserList`."
optional :scopes, type: Array do
requires :environment_scope, type: String, desc: 'The environment scope of the scope'
end
@@ -131,6 +132,7 @@ class FeatureFlags < ::API::Base
optional :id, type: Integer, desc: 'The feature flag strategy ID'
optional :name, type: String, desc: 'The strategy name'
optional :parameters, type: JSON, desc: 'The strategy parameters as a JSON-formatted string e.g. `{"userIds":"user1"}`', documentation: { type: 'String' }
+ optional :user_list_id, type: Integer, desc: "The ID of the feature flag user list"
optional :_destroy, type: Boolean, desc: 'Delete the strategy when true'
optional :scopes, type: Array do
optional :id, type: Integer, desc: 'The scope id'
diff --git a/spec/fixtures/api/schemas/public_api/v4/operations/strategy.json b/spec/fixtures/api/schemas/public_api/v4/operations/strategy.json
index f572b1a4f9bbb..a72260af14539 100644
--- a/spec/fixtures/api/schemas/public_api/v4/operations/strategy.json
+++ b/spec/fixtures/api/schemas/public_api/v4/operations/strategy.json
@@ -8,7 +8,8 @@
"id": { "type": "integer" },
"name": { "type": "string" },
"parameters": { "type": "object" },
- "scopes": { "type": "array", "items": { "$ref": "scope.json" } }
+ "scopes": { "type": "array", "items": { "$ref": "scope.json" } },
+ "user_list": { "type": ["object", "null"], "$ref": "user_list.json" }
},
"additionalProperties": false
}
diff --git a/spec/fixtures/api/schemas/public_api/v4/operations/user_list.json b/spec/fixtures/api/schemas/public_api/v4/operations/user_list.json
new file mode 100644
index 0000000000000..6a9f977e37de6
--- /dev/null
+++ b/spec/fixtures/api/schemas/public_api/v4/operations/user_list.json
@@ -0,0 +1,16 @@
+{
+ "type": ["object", "null"],
+ "required": [
+ "id",
+ "iid",
+ "name",
+ "user_xids"
+ ],
+ "properties": {
+ "id": { "type": "integer" },
+ "iid": { "type": "integer" },
+ "name": { "type": "string" },
+ "user_xids": { "type": "string" }
+ },
+ "additionalProperties": false
+}
diff --git a/spec/requests/api/feature_flags_spec.rb b/spec/requests/api/feature_flags_spec.rb
index 69e3633de577e..4fb0dfbb0704e 100644
--- a/spec/requests/api/feature_flags_spec.rb
+++ b/spec/requests/api/feature_flags_spec.rb
@@ -111,7 +111,57 @@
'scopes' => [{
'id' => scope.id,
'environment_scope' => 'production'
- }]
+ }],
+ 'user_list' => nil
+ }]
+ }])
+ end
+ end
+
+ context 'with user_list strategy feature flags' do
+ let!(:feature_flag) do
+ create(:operations_feature_flag, :new_version_flag, project: project, name: 'feature1')
+ end
+
+ let!(:user_list) do
+ create(:operations_feature_flag_user_list, project: project)
+ end
+
+ let!(:strategy) do
+ create(:operations_strategy, :gitlab_userlist, user_list: user_list, feature_flag: feature_flag, name: 'gitlabUserList', parameters: {})
+ end
+
+ let!(:scope) do
+ create(:operations_scope, strategy: strategy, environment_scope: 'production')
+ end
+
+ it 'returns the feature flags', :aggregate_failures do
+ subject
+
+ expect(response).to have_gitlab_http_status(:ok)
+ expect(response).to match_response_schema('public_api/v4/feature_flags')
+ expect(json_response).to eq([{
+ 'name' => 'feature1',
+ 'description' => nil,
+ 'active' => true,
+ 'version' => 'new_version_flag',
+ 'updated_at' => feature_flag.updated_at.as_json,
+ 'created_at' => feature_flag.created_at.as_json,
+ 'scopes' => [],
+ 'strategies' => [{
+ 'id' => strategy.id,
+ 'name' => 'gitlabUserList',
+ 'parameters' => {},
+ 'scopes' => [{
+ 'id' => scope.id,
+ 'environment_scope' => 'production'
+ }],
+ 'user_list' => {
+ 'id' => user_list.id,
+ 'iid' => user_list.iid,
+ 'name' => user_list.name,
+ 'user_xids' => user_list.user_xids
+ }
}]
}])
end
@@ -162,7 +212,57 @@
'scopes' => [{
'id' => scope.id,
'environment_scope' => 'production'
- }]
+ }],
+ 'user_list' => nil
+ }]
+ })
+ end
+ end
+
+ context 'with user_list strategy feature flag' do
+ let!(:feature_flag) do
+ create(:operations_feature_flag, :new_version_flag, project: project, name: 'feature1')
+ end
+
+ let(:user_list) do
+ create(:operations_feature_flag_user_list, project: project)
+ end
+
+ let!(:strategy) do
+ create(:operations_strategy, :gitlab_userlist, user_list: user_list, feature_flag: feature_flag, name: 'gitlabUserList', parameters: {})
+ end
+
+ let!(:scope) do
+ create(:operations_scope, strategy: strategy, environment_scope: 'production')
+ end
+
+ it 'returns the feature flag', :aggregate_failures do
+ subject
+
+ expect(response).to have_gitlab_http_status(:ok)
+ expect(response).to match_response_schema('public_api/v4/feature_flag')
+ expect(json_response).to eq({
+ 'name' => 'feature1',
+ 'description' => nil,
+ 'active' => true,
+ 'version' => 'new_version_flag',
+ 'updated_at' => feature_flag.updated_at.as_json,
+ 'created_at' => feature_flag.created_at.as_json,
+ 'scopes' => [],
+ 'strategies' => [{
+ 'id' => strategy.id,
+ 'name' => 'gitlabUserList',
+ 'parameters' => {},
+ 'scopes' => [{
+ 'id' => scope.id,
+ 'environment_scope' => 'production'
+ }],
+ 'user_list' => {
+ 'id' => user_list.id,
+ 'iid' => user_list.iid,
+ 'name' => user_list.name,
+ 'user_xids' => user_list.user_xids
+ }
}]
})
end
@@ -224,6 +324,10 @@
end
context 'when creating a version 2 feature flag' do
+ let(:user_list) do
+ create(:operations_feature_flag_user_list, project: project)
+ end
+
it 'creates a new feature flag' do
params = {
name: 'new-feature',
@@ -348,6 +452,32 @@
environment_scope: 'staging'
}])
end
+
+ it 'creates a new feature flag with user list strategy', :aggregate_failures do
+ params = {
+ name: 'new-feature',
+ version: 'new_version_flag',
+ strategies: [{
+ name: 'gitlabUserList',
+ parameters: {},
+ user_list_id: user_list.id
+ }]
+ }
+
+ post api("/projects/#{project.id}/feature_flags", user), params: params
+
+ expect(response).to have_gitlab_http_status(:created)
+ expect(response).to match_response_schema('public_api/v4/feature_flag')
+
+ feature_flag = project.operations_feature_flags.last
+ expect(feature_flag.name).to eq(params[:name])
+ expect(feature_flag.version).to eq('new_version_flag')
+ expect(feature_flag.strategies.map { |s| s.slice(:name, :parameters).deep_symbolize_keys }).to eq([{
+ name: 'gitlabUserList',
+ parameters: {}
+ }])
+ expect(feature_flag.strategies.first.user_list).to eq(user_list)
+ end
end
context 'when given invalid parameters' do
@@ -369,6 +499,10 @@
project: project, active: true, name: 'feature1', description: 'old description')
end
+ let(:user_list) do
+ create(:operations_feature_flag_user_list, project: project)
+ end
+
it 'returns a 404 if the feature flag does not exist' do
params = { description: 'new description' }
@@ -537,6 +671,30 @@
}])
end
+ it 'updates an existing feature flag strategy to be gitlab user list strategy', :aggregate_failures do
+ strategy = create(:operations_strategy, feature_flag: feature_flag, name: 'default', parameters: {})
+ params = {
+ strategies: [{
+ id: strategy.id,
+ name: 'gitlabUserList',
+ user_list_id: user_list.id,
+ parameters: {}
+ }]
+ }
+
+ put api("/projects/#{project.id}/feature_flags/feature1", user), params: params
+
+ expect(response).to have_gitlab_http_status(:ok)
+ expect(response).to match_response_schema('public_api/v4/feature_flag')
+ result = feature_flag.reload.strategies.map { |s| s.slice(:id, :name, :parameters).deep_symbolize_keys }
+ expect(result).to eq([{
+ id: strategy.id,
+ name: 'gitlabUserList',
+ parameters: {}
+ }])
+ expect(feature_flag.strategies.first.user_list).to eq(user_list)
+ end
+
it 'adds a new gradual rollout strategy to a feature flag' do
strategy = create(:operations_strategy, feature_flag: feature_flag, name: 'default', parameters: {})
params = {
--
GitLab