Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: add an opt-in count to early access management #29841

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion frontend/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3140,13 +3140,14 @@ export enum EarlyAccessFeatureTabs {
export interface EarlyAccessFeatureType {
/** UUID */
id: string
feature_flag: FeatureFlagBasicType
feature_flag: FeatureFlagType
name: string
description: string
stage: EarlyAccessFeatureStage
/** Documentation URL. Can be empty. */
documentation_url: string
created_at: string
opt_in_count?: number
}

export interface NewEarlyAccessFeatureType extends Omit<EarlyAccessFeatureType, 'id' | 'created_at' | 'feature_flag'> {
Expand Down Expand Up @@ -5069,3 +5070,4 @@ export interface ProductManifest {
redirects?: Record<string, string | ((params: Params, searchParams: Params, hashParams: Params) => string)>
urls?: Record<string, string | ((...args: any[]) => string)>
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,12 @@ export function EarlyAccessFeatures(): JSX.Element {
},
sorter: (a, b) => STAGES_IN_ORDER[a.stage] - STAGES_IN_ORDER[b.stage],
},
{
title: 'Opted-in users',
key: 'opt_in_count',
render: (_, feature) => feature.opt_in_count ?? 0,
sorter: (a, b) => (a.opt_in_count ?? 0) - (b.opt_in_count ?? 0),
},
]}
dataSource={earlyAccessFeatures}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,26 @@ export const earlyAccessFeaturesLogic = kea<earlyAccessFeaturesLogicType>([
__default: [] as EarlyAccessFeatureType[],
loadEarlyAccessFeatures: async () => {
const response = await api.earlyAccessFeatures.list()
return response.results
const featuresWithCounts = await Promise.all(
response.results.map(async (feature) => {
const key = `$feature_enrollment/${feature.feature_flag.key}`
const optInCount = await api.persons.list({
properties: [
{
key: key,
value: ['true'],
operator: 'exact',
type: 'person',
},
],
})
return {
...feature,
opt_in_count: optInCount.count,
}
})
)
return featuresWithCounts
Comment on lines -17 to +36
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So... this looks like it might work, but there are two main issues with this approach:

  • The list of features will not show up until every single API call to get the count finishes. This should be split into two operations, and the data combined later on (e.g. with a selector).
  • Making a separate API call for each feature is costly. For each call, we need to read from disk all the persons in the team, filter by a specific feature, and return the count. We should instead read all the data just once, count the features, and return a combined count.

So... the first step towards a solution is to write a SQL query that selects from the persons table and returns a count for each feature enrolment property of the user.

The second step is to edit the logic, and in addition to the earlyAccessFeatures loader, create a new earlyAccessFeatureEnrolment loader which makes the SQL query.

Then in the table you can combine the two data sources.

I'm not sure how deep you want to go with this, but feel free to have a crack and I'm happy to help along the way.

},
},
}),
Expand Down
1 change: 1 addition & 0 deletions types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

Loading