Skip to content

fix(feature-flags): show all projects' flags in comparison grid #63156

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.

Sign up for GitHub

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

Draft
phillram wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

fix(feature-flags): show all projects' flags in comparison grid #63156

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
45 changes: 21 additions & 24 deletions frontend/src/scenes/feature-flags/projects-grid/ProjectsGrid.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,17 @@ import { LemonTable, LemonTableColumns } from 'lib/lemon-ui/LemonTable'
import { LemonTableLink } from 'lib/lemon-ui/LemonTable/LemonTableLink'
import { organizationLogic } from 'scenes/organizationLogic'
import { teamLogic } from 'scenes/teamLogic'
import { urls } from 'scenes/urls'

import { SceneSection } from '~/layout/scenes/components/SceneSection'
import { FeatureFlagType, OrganizationFeatureFlag } from '~/types'
import { OrganizationFeatureFlag } from '~/types'

import { CellState, ProjectsGridCell } from './ProjectsGridCell'
import { projectsGridLogic } from './projectsGridLogic'
import { ProjectsGridRow, projectsGridLogic } from './projectsGridLogic'
import { ProjectsGridToolbar } from './ProjectsGridToolbar'

function cellStateFor(
flag: FeatureFlagType,
row: ProjectsGridRow,
teamId: number,
currentTeamId: number,
accessibleTeamIds: Set<number>,
siblings: OrganizationFeatureFlag[] | undefined,
siblingsLoading: boolean
Expand All @@ -29,18 +27,18 @@ function cellStateFor(
return { kind: 'present', sibling: siblingForTeam }
}

// Before siblings load, render the current team's cell from the flag directly
// (eval count unavailable until siblings arrive).
if (teamId === currentTeamId) {
// Before siblings load, render the representative project's cell directly so it doesn't flash
// a skeleton (eval count is unavailable until siblings arrive).
if (teamId === row.team_id) {
return {
kind: 'present',
sibling: {
flag_id: flag.id,
flag_id: row.flag_id,
team_id: teamId,
created_by: flag.created_by ?? null,
filters: flag.filters,
created_at: flag.created_at ?? '',
active: flag.active,
created_by: null,
filters: row.filters,
created_at: '',
active: row.active,
},
}
}
Expand Down Expand Up @@ -98,16 +96,16 @@ export function ProjectsGrid(): JSX.Element {

const columnWidth = `${100 / (visibleColumns.length + 1)}%`

const columns: LemonTableColumns<FeatureFlagType> = [
const columns: LemonTableColumns<ProjectsGridRow> = [
{
title: 'Flag',
key: 'flag',
width: columnWidth,
render: (_, flag) => (
render: (_, row) => (
<LemonTableLink
to={urls.featureFlag(flag.id as number)}
title={flag.name || flag.key}
description={flag.key}
to={`/project/${row.team_id}/feature_flags/${row.flag_id}`}
title={row.name || row.key}
description={row.key}
/>
),
},
Expand All @@ -122,15 +120,14 @@ export function ProjectsGrid(): JSX.Element {
),
key: `project-${teamId}`,
width: columnWidth,
render: (_: unknown, flag: FeatureFlagType) => (
render: (_: unknown, row: ProjectsGridRow) => (
<ProjectsGridCell
state={cellStateFor(
flag,
row,
teamId,
currentTeamId,
accessibleTeamIds,
siblingsByFlagKey[flag.key],
siblingsLoadingKeys.includes(flag.key)
siblingsByFlagKey[row.key],
siblingsLoadingKeys.includes(row.key)
)}
/>
),
Expand All @@ -146,7 +143,7 @@ export function ProjectsGrid(): JSX.Element {
<LemonTable
columns={columns}
dataSource={flags}
rowKey="id"
rowKey="key"
loading={flagsPageLoading && flags.length === 0}
emptyState="No flags match your search."
data-attr="projects-grid-table"
Expand Down
49 changes: 40 additions & 9 deletions frontend/src/scenes/feature-flags/projects-grid/projectsGridLogic.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,21 @@ import { initKeaTests } from '~/test/init'

import { projectsGridLogic } from './projectsGridLogic'

interface MockFlag {
id: number
interface MockRow {
key: string
name: string
flag_id: number
team_id: number
filters: { groups: [] }
active: boolean
}

function buildFlag(i: number): MockFlag {
function buildRow(i: number): MockRow {
return {
id: i,
key: `flag_${i}`,
name: `Flag ${i}`,
flag_id: i,
team_id: 1,
filters: { groups: [] },
active: true,
}
Expand All @@ -27,10 +29,16 @@ describe('projectsGridLogic', () => {
let logic: ReturnType<typeof projectsGridLogic.build>

describe('rows and pagination', () => {
let lastRequestedTeamIds: string | null

beforeEach(() => {
lastRequestedTeamIds = null
useMocks({
get: {
'/api/projects/:team/feature_flags/': (req) => {
// `keys/` must be registered before the `:key/` sibling route so it isn't
// swallowed by the single-segment path parameter.
'/api/organizations/:org/feature_flags/keys/': (req) => {
lastRequestedTeamIds = req.url.searchParams.get('team_ids')
const offset = Number(req.url.searchParams.get('offset') ?? 0)
const count = 40
const remaining = Math.max(0, count - offset)
Expand All @@ -40,7 +48,8 @@ describe('projectsGridLogic', () => {
{
count,
next: offset + pageSize < count ? 'next' : null,
results: Array.from({ length: pageSize }, (_, i) => buildFlag(offset + i + 1)),
previous: offset > 0 ? 'prev' : null,
results: Array.from({ length: pageSize }, (_, i) => buildRow(offset + i + 1)),
},
]
},
Expand Down Expand Up @@ -74,6 +83,27 @@ describe('projectsGridLogic', () => {
expect(logic.values.flags.length).toBeGreaterThan(0)
expect(logic.values.flagsOffset).toBeLessThanOrEqual(25)
})

it('reloads rows from the top when the compared projects change', async () => {
await expectLogic(logic).toFinishAllListeners()
logic.actions.loadMoreFlags()
await expectLogic(logic).toFinishAllListeners()
expect(logic.values.flagsOffset).toBe(40)

logic.actions.setPickedTeamIds([2])
await expectLogic(logic).toFinishAllListeners()
// The row set changed, so pagination restarts from the first page.
expect(logic.values.flags).toHaveLength(25)
expect(logic.values.flagsOffset).toBe(25)
})

it('requests keys for the visible columns', async () => {
await expectLogic(logic).toFinishAllListeners()
logic.actions.setPickedTeamIds([7, 9])
await expectLogic(logic).toFinishAllListeners()
// Current team is always first, followed by the picked teams.
expect(lastRequestedTeamIds).toBe(`${logic.values.currentTeamId},7,9`)
})
})

describe('sibling queue (serial)', () => {
Expand All @@ -88,10 +118,11 @@ describe('projectsGridLogic', () => {

useMocks({
get: {
'/api/projects/:team/feature_flags/': {
'/api/organizations/:org/feature_flags/keys/': {
count: 3,
next: null,
results: [buildFlag(1), buildFlag(2), buildFlag(3)],
previous: null,
results: [buildRow(1), buildRow(2), buildRow(3)],
},
'/api/organizations/:org/feature_flags/:key/': async (req) => {
const key = req.params.key as string
Expand Down Expand Up @@ -133,7 +164,7 @@ describe('projectsGridLogic', () => {
localStorage.clear()
useMocks({
get: {
'/api/projects/:team/feature_flags/': { count: 0, next: null, results: [] },
'/api/organizations/:org/feature_flags/keys/': { count: 0, next: null, previous: null, results: [] },
'/api/organizations/:org/feature_flags/:key/': [],
},
})
Expand Down
57 changes: 42 additions & 15 deletions frontend/src/scenes/feature-flags/projects-grid/projectsGridLogic.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,36 +5,41 @@ import api from 'lib/api'
import { toParams } from 'lib/utils'
import { getCurrentTeamId } from 'lib/utils/getAppContext'
import { organizationLogic } from 'scenes/organizationLogic'
import { projectLogic } from 'scenes/projectLogic'
import { teamLogic } from 'scenes/teamLogic'

import { FeatureFlagType, OrganizationFeatureFlag, OrganizationType } from '~/types'
import { FeatureFlagFilters, OrganizationFeatureFlag, OrganizationType } from '~/types'

import type { projectsGridLogicType } from './projectsGridLogicType'

export const PAGE_SIZE = 25

/**
* One row in the comparison grid: a distinct flag key found in at least one of the compared
* projects, plus a representative flag (used for the row's name and link).
*/
export interface ProjectsGridRow {
key: string
name: string
flag_id: number
team_id: number
filters: FeatureFlagFilters
active: boolean
}

export interface LoadFlagsResult {
offset: number
search: string
count: number
next: string | null
results: FeatureFlagType[]
results: ProjectsGridRow[]
}

const storageKey = (teamId: number): string => `ff-projects-grid.picked-teams.${teamId}`

export const projectsGridLogic = kea<projectsGridLogicType>([
path(['scenes', 'feature-flags', 'projects-grid', 'projectsGridLogic']),
connect(() => ({
values: [
teamLogic,
['currentTeamId'],
organizationLogic,
['currentOrganization'],
projectLogic,
['currentProjectId'],
],
values: [teamLogic, ['currentTeamId'], organizationLogic, ['currentOrganization']],
})),
actions({
setSearch: (search: string) => ({ search }),
Expand All @@ -52,11 +57,13 @@ export const projectsGridLogic = kea<projectsGridLogicType>([
reducers({
search: ['', { setSearch: (_, { search }) => search }],
flags: [
[] as FeatureFlagType[],
[] as ProjectsGridRow[],
{
loadFlagsPageSuccess: (state, { flagsPage }: { flagsPage: LoadFlagsResult }) =>
flagsPage.offset === 0 ? flagsPage.results : [...state, ...flagsPage.results],
setSearch: () => [],
setPickedTeamIds: () => [],
resetPickedTeamIds: () => [],
},
],
flagsOffset: [
Expand All @@ -65,13 +72,17 @@ export const projectsGridLogic = kea<projectsGridLogicType>([
loadFlagsPageSuccess: (_, { flagsPage }: { flagsPage: LoadFlagsResult }) =>
flagsPage.offset + flagsPage.results.length,
setSearch: () => 0,
setPickedTeamIds: () => 0,
resetPickedTeamIds: () => 0,
},
],
flagsHasMore: [
true,
{
loadFlagsPageSuccess: (_, { flagsPage }: { flagsPage: LoadFlagsResult }) => flagsPage.next !== null,
setSearch: () => true,
setPickedTeamIds: () => true,
resetPickedTeamIds: () => true,
},
],
siblingsByFlagKey: [
Expand All @@ -87,6 +98,8 @@ export const projectsGridLogic = kea<projectsGridLogicType>([
siblingsLoaded: (state, { flagKey }) => state.filter((k) => k !== flagKey),
siblingsFailed: (state, { flagKey }) => state.filter((k) => k !== flagKey),
setSearch: () => [],
setPickedTeamIds: () => [],
resetPickedTeamIds: () => [],
},
],
siblingQueue: [
Expand All @@ -98,6 +111,8 @@ export const projectsGridLogic = kea<projectsGridLogicType>([
},
startSiblingFetch: (state, { flagKey }) => state.filter((k) => k !== flagKey),
setSearch: () => [],
setPickedTeamIds: () => [],
resetPickedTeamIds: () => [],
},
],
pickedTeamIds: [
Expand All @@ -113,9 +128,16 @@ export const projectsGridLogic = kea<projectsGridLogicType>([
null as LoadFlagsResult | null,
{
loadFlagsPage: async ({ offset, search }: { offset: number; search: string }) => {
const params = toParams({ limit: PAGE_SIZE, offset, search })
const orgId = values.currentOrganization?.id
const params = toParams({
limit: PAGE_SIZE,
offset,
search,
team_ids: values.visibleColumns.join(','),
...(values.currentTeamId ? { current_team_id: values.currentTeamId } : {}),
})
const response = await api.get<Omit<LoadFlagsResult, 'offset' | 'search'>>(
`api/projects/${values.currentProjectId}/feature_flags/?${params}`
`api/organizations/${orgId}/feature_flags/keys/?${params}`
)
return { offset, search, ...response }
},
Expand Down Expand Up @@ -160,18 +182,23 @@ export const projectsGridLogic = kea<projectsGridLogicType>([
},
setPickedTeamIds: ({ teamIds }) => {
localStorage.setItem(storageKey(getCurrentTeamId()), JSON.stringify(teamIds))
// The compared projects changed, so the set of rows does too — reload from the top.
actions.loadFlagsPage({ offset: 0, search: values.search })
},
resetPickedTeamIds: () => {
localStorage.removeItem(storageKey(getCurrentTeamId()))
actions.loadFlagsPage({ offset: 0, search: values.search })
},
})),
afterMount(({ actions }) => {
const raw = localStorage.getItem(storageKey(getCurrentTeamId()))
if (raw) {
try {
const parsed = JSON.parse(raw)
if (Array.isArray(parsed) && parsed.every((x) => typeof x === 'number')) {
if (Array.isArray(parsed) && parsed.length > 0 && parsed.every((x) => typeof x === 'number')) {
// Hydrating picks triggers the initial load through the setPickedTeamIds listener.
actions.setPickedTeamIds(parsed)
return
}
} catch {
// ignore malformed entry
Expand Down
Loading
Loading