Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis pull request introduces a comprehensive feature flags system with flag management (creation, retrieval, update, deletion), flag evaluation with targeting rules and rollout percentages, and evaluation tracking. It refactors analytics session recording by renaming Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant API as FeatureFlagController
participant Service as FeatureFlagService
participant DB as Database<br/>(MySQL/TypeORM)
participant Cache as ClickHouse<br/>(Evaluations)
rect rgb(200, 150, 255)
Note over Client,Cache: Flag Evaluation Flow
Client->>API: POST /evaluate { pid, profileId? }
activate API
API->>API: Derive visitor attributes<br/>(IP, geo, device, browser, OS)
API->>API: Generate profileId if missing
API->>Service: findEnabledByProject(pid)
activate Service
Service->>DB: Query enabled flags
DB-->>Service: [FeatureFlag...]
deactivate Service
API->>Service: evaluateFlags(flags[], profileId, attributes)
activate Service
loop For each flag
Service->>Service: evaluateFlag(flag, profileId, attributes)
alt BOOLEAN type
Service->>Service: matchesTargetingRules()?
alt Matches
Service-->>Service: return enabled
else No match
Service-->>Service: return false
end
else ROLLOUT type
Service->>Service: matchesTargetingRules()?
alt Matches
Service->>Service: isInRolloutPercentage<br/>(MD5 hash)?
alt In rollout
Service-->>Service: return true
else Not in rollout
Service-->>Service: return false
end
else No match
Service-->>Service: return false
end
end
end
Service-->>API: { flagKey: boolean, ... }
deactivate Service
API->>API: Prepare evaluation records
par Async Tracking (non-blocking)
API->>Cache: INSERT evaluations<br/>(pid, flagId, result, profileId)
Cache-->>API: ✓ OK / × Error logged
end
API-->>Client: { flagKey: boolean, ... }
deactivate API
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Areas requiring extra attention:
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Blaumaus
force-pushed
the
feature-flags
branch
from
04b869b to
81a21ee
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/migrations/clickhouse/2025_12_02_profile_id.js (1)
21-35: ReplaceNOT INwithNOT EXISTSto avoid NULL-handling issues and improve performance.The
NOT INclause on line 35 has two significant problems:
- NULL semantics bug: If any
pidorpsidin the sessions table isNULL, theNOT INpredicate may return no rows due to SQL's three-valued logic, silently skipping the entire migration.- Performance:
NOT INwith subqueries is typically slower thanNOT EXISTSorLEFT JOIN ... IS NULLpatterns on large datasets.Apply this diff to use a more reliable pattern:
- WHERE (sd.pid, sd.psid) NOT IN (SELECT pid, psid FROM ${dbName}.sessions);`, + WHERE NOT EXISTS ( + SELECT 1 FROM ${dbName}.sessions s + WHERE s.pid = sd.pid AND s.psid = sd.psid + );`,Additionally, consider validating the
durationfield to prevent invalidfirstSeencalculations ifdurationcould beNULLor negative.
♻️ Duplicate comments (2)
web/app/pages/Project/View/components/Sessions.tsx (1)
25-45: Duplicate of online status logic in Users.tsx.This is the same code flagged in
Users.tsx. Once extracted to a shared utility, import it here.backend/apps/community/src/feature-flag/dto/feature-flag.dto.ts (1)
1-245: Same DTO design as cloud module; keep them in sync with shared helpers if possibleThis file mirrors the cloud feature‑flag DTOs, which is good for consistency. The same minor notes apply here (e.g., possible
@IsIntforrolloutPercentage, and potential to centralize these DTOs in a shared module to avoid duplication).Functionally this is fine as‑is.
🧹 Nitpick comments (25)
backend/migrations/clickhouse/2025_12_02_profile_id.js (1)
37-38: Good practice: Commented DROP for manual verification.Leaving the
DROP TABLEstatement commented is appropriate for a destructive operation. Ensure operational documentation specifies when and how to execute this cleanup step after verifying the migration succeeded.web/app/ui/Switch.tsx (1)
15-15: Usesubstringinstead of deprecatedsubstr.
String.prototype.substris deprecated. Usesubstringfor the same result.- const switchId = id || `switch-${Math.random().toString(36).substr(2, 9)}` + const switchId = id || `switch-${Math.random().toString(36).substring(2, 11)}`web/app/pages/Project/View/components/Users.tsx (1)
22-42: Extract shared online status logic to a utility.The
getOnlineStatusfunction,OnlineStatustype, and threshold constants are duplicated inSessions.tsx(lines 25-45). Consider extracting to a shared utility file (e.g.,~/utils/onlineStatus.ts) to ensure consistency and reduce maintenance burden.// ~/utils/onlineStatus.ts export const ONLINE_THRESHOLD_MINUTES = 5 export const RECENTLY_ACTIVE_THRESHOLD_MINUTES = 30 export type OnlineStatus = 'online' | 'recently_active' | 'offline' export const getOnlineStatus = (lastSeen: string): OnlineStatus => { const now = dayjs() const lastSeenTime = dayjs(lastSeen) const minutesAgo = now.diff(lastSeenTime, 'minute') if (minutesAgo < ONLINE_THRESHOLD_MINUTES) return 'online' if (minutesAgo < RECENTLY_ACTIVE_THRESHOLD_MINUTES) return 'recently_active' return 'offline' }web/app/pages/Project/FeatureFlags/View/FeatureFlagSettingsModal.tsx (3)
94-126: Consider stabilizingfetchFilterValuescallback dependencies.The
useCallbackincludesfilterValuesCacheandloadingColumnsin its dependency array. Since these change on every fetch, the callback reference changes frequently. While the guard conditions prevent redundant fetches, this could cause unnecessary effect re-runs.Consider using refs to access the latest cache/loading state without adding them to dependencies:
+ const filterValuesCacheRef = useRef(filterValuesCache) + filterValuesCacheRef.current = filterValuesCache + const loadingColumnsRef = useRef(loadingColumns) + loadingColumnsRef.current = loadingColumns const fetchFilterValues = useCallback( async (column: string) => { - if (filterValuesCache[column] || loadingColumns.has(column)) return + if (filterValuesCacheRef.current[column] || loadingColumnsRef.current.has(column)) return // ... }, - [projectId, filterValuesCache, loadingColumns], + [projectId], )
199-213: Consider type-safe approach for dynamic field assignment.The
@ts-expect-errorcomment can be avoided with a more type-safe approach:const updateTargetingRule = (index: number, field: keyof TargetingRule, value: string | boolean) => { const updated = [...targetingRules] if (field === 'column') { updated[index] = { ...updated[index], column: value as string, filter: '' } if (value && !filterValuesCache[value as string] && !loadingColumns.has(value as string)) { fetchFilterValues(value as string) } + } else if (field === 'filter') { + updated[index] = { ...updated[index], filter: value as string } + } else if (field === 'isExclusive') { + updated[index] = { ...updated[index], isExclusive: value as boolean } } else { - // @ts-expect-error - TypeScript doesn't like dynamic field assignment - updated[index][field] = value + updated[index] = { ...updated[index], [field]: value } } setTargetingRules(updated) }
296-302: Add aria-label to close button for accessibility.The close button has an icon but no accessible label:
<button type='button' onClick={onClose} + aria-label={t('common.close')} className='rounded-md p-1.5 text-gray-400 hover:bg-gray-100 hover:text-gray-500 dark:hover:bg-slate-800 dark:hover:text-gray-300' > <XMarkIcon className='h-5 w-5' /> </button>web/app/pages/Project/FeatureFlags/View/FeatureFlagsView.tsx (2)
661-667: Consider batching stats requests for performance.When
flagschanges,loadFlagStatsis called for each flag individually (line 663-665), which could result in many parallel API requests if there are many flags. Consider implementing a batch endpoint or adding request throttling:// Option 1: Batch endpoint (preferred, requires backend change) // Option 2: Sequential loading with concurrency limit const loadAllFlagStats = async () => { const BATCH_SIZE = 5 for (let i = 0; i < flags.length; i += BATCH_SIZE) { const batch = flags.slice(i, i + BATCH_SIZE) await Promise.all(batch.map(flag => loadFlagStats(flag.id))) } }
706-716: Consider static import forupdateFeatureFlag.The dynamic import of
updateFeatureFlagon line 708 adds unnecessary complexity. Since this function is likely to be used (toggle is a common action), a static import would be simpler and more consistent with howdeleteFeatureFlagApiis imported:import { deleteFeatureFlag as deleteFeatureFlagApi, getProjectFeatureFlags, getFeatureFlagStats, getFeatureFlagProfiles, + updateFeatureFlag, // ... } from '~/api' const handleToggleFlag = async (flagId: string, enabled: boolean) => { try { - const { updateFeatureFlag } = await import('~/api') await updateFeatureFlag(flagId, { enabled })web/public/locales/en.json (1)
1175-1176: Online status strings look good but watch pluralisation
project.onlineandproject.lastSeenAgoare clear and match intended usage for presence indicators. If you later expose “X users online” or similar, you may want separate pluralisable keys, but for now these are fine.web/app/pages/Project/View/ViewProject.tsx (2)
1691-1694: Refresh flow for feature flags correctly uses a dedicated triggerIn
refreshStats, the new branch forPROJECT_TABS.featureFlagsincrementsfeatureFlagsRefreshTriggerinstead of reloading analytics. This mirrors the existing pattern for captcha/goals and letsFeatureFlagsViewdecide how to react to refreshes.Just ensure
FeatureFlagsViewactually subscribes tofeatureFlagsRefreshTriggerviauseViewProjectContext, otherwise the trigger will be a no‑op.
4031-4033: SessionDetails now receives pages; consider minor type‑safety tighteningPassing
pages={activeSession?.pages}intoSessionDetailsenables its new duration fallback logic based on page timestamps and is functionally correct, given it already guards onactiveSession?.details.If
SessionDetailsexpects non‑nullabledetails, you could tighten this slightly:- {activeSession?.details ? ( - <SessionDetails details={activeSession?.details} pages={activeSession?.pages} /> - ) : null} + {activeSession ? ( + <SessionDetails details={activeSession.details} pages={activeSession.pages} /> + ) : null}to keep TypeScript happy and avoid passing potentially‑undefined
details.backend/migrations/clickhouse/selfhosted_2025_12_10_feature_flags.js (1)
6-20: Consider usingUInt8for theenabledcolumn.The
enabledcolumn is defined asInt8 DEFAULT 1, which allows negative values. Since this represents a boolean state,UInt8would be more semantically appropriate and consistent with theresultcolumn infeature_flag_evaluations.This is a minor consistency suggestion and not blocking.
- enabled Int8 DEFAULT 1, + enabled UInt8 DEFAULT 1,backend/apps/cloud/src/analytics/analytics.service.ts (2)
1221-1266: Session recording logic is sound but may accumulate multiple rows per session over timeThe
recordSessionActivityimplementation correctly preserves the originalfirstSeenand updateslastSeen, but every call inserts a new row instead of updating/merging an existing one. Depending on thesessionstable engine, this can:
- Produce multiple physical rows per
(pid, psid)and makeLIMIT 1consumers likegetSessionDurationFromClickHousesensitive to merge order.- Inflate storage and make aggregates slightly harder to reason about if the engine doesn’t collapse rows by primary key.
If the intention is “one logical row per session,” consider either:
- Using a Replacing/Versioned engine keyed by
(pid, psid)withFINALeverywhere you read, or- Changing this to an upsert pattern (e.g., aggregating in queries with
argMin(firstSeen), argMax(lastSeen)or similar) so downstream callers don’t depend on implicit engine behavior.Given it’s analytics-only, this is not a blocker but worth confirming against the actual table schema.
4145-4165: NewprofileId/isIdentifiedfields in sessions list look good; handle missing session rows explicitlyJoining
session_duration_aggintogetSessionsListto exposesdur,profileId, andisIdentifiedis a nice addition. Two minor points to consider:
- For sessions that predate the
sessionstable or whererecordSessionActivityfailed,sda.avg_durationandsda.profileIdwill beNULL. If the UI expects numbers/booleans, you may want toCOALESCEavg_durationto0and explicitly treatNULLprofileIdas anonymous instead of relying on ClickHouse’sstartsWithbehavior on nullables.- You already reference
AnalyticsService.PROFILE_PREFIX_USERin the query; this keeps the “identified” definition consistent with the TS helper, which is good. Just ensure theprofileIdcolumn insessionsisNullable(String)orStringand always set for new data soisIdentifiedis reliable going forward.Behavior is otherwise coherent with the rest of the analytics API.
backend/apps/community/src/analytics/analytics.controller.ts (1)
926-930: Consistent use ofrecordSessionActivity; consider moving it after uniqueness rejection inlogUsing
recordSessionActivity(psid, pid, profileId)across custom events, heartbeat, pageviews, noscript, and errors is consistent with the new sessions model and keepsprofileIdin sync.One small refinement: in
logthe session activity is recorded before theunique/ForbiddenExceptioncheck. That means requests you ultimately reject as “not unique” will still refreshlastSeeninsessions. If you want session tracking to reflect only accepted events, you could move therecordSessionActivitycall to just after the uniqueness check.Functionality is otherwise correct.
Also applies to: 1013-1015, 1048-1052, 1194-1198, 1501-1505
backend/apps/cloud/src/feature-flag/dto/feature-flag.dto.ts (1)
1-245: DTO shapes look solid; minor validation/DRY opportunitiesThe DTOs and validation rules look consistent with the feature‑flag entity and are generally good to go. A couple of small improvements to consider:
- If
rolloutPercentageis intended to be an integer 0–100, you could tighten validation with@IsInt()and (optionally)@Type(() => Number)rather than a generic@IsNumber().- Cloud and Community DTOs are effectively identical; if you expect them to stay in lockstep, a shared package/module for these types would reduce duplication and the risk of drift.
Nothing here blocks merging.
backend/apps/community/src/feature-flag/feature-flag.controller.ts (3)
194-278: Public/evaluateendpoint is coherent; confirm suspension/blacklist behavior and evaluation logging assumptionsThe evaluation flow is clean: you load enabled flags, derive visitor attributes, generate a
profileIdviaAnalyticsService.generateProfileId, evaluate flags, and then track evaluations asynchronously into ClickHouse.Two behavioral points to double‑check:
- Unlike analytics ingestion, this endpoint doesn’t apply
checkIfAccountSuspended, origin/IP/country blacklist, or bot detection – it only checksproject.active. If billing suspension or blocking rules should also prevent feature‑flag evaluation, it’s worth reusing the same checks here; otherwise, clarifying that flags remain active when analytics is blocked would help future maintainers.trackEvaluationswrites one row perflagsentry and treatsevaluatedFlags[flag.key]as falsey →0. IffeatureFlagService.evaluateFlagsever omits a key (e.g., in case of internal filtering), those flags will be logged asresult = 0. If that’s not desired, consider skipping flags not present in the result map.Functionally fine, but aligning these behaviors explicitly will avoid subtle surprises later.
Also applies to: 280-309
529-537: Profiles listing endpoint is solid; reuse profile prefix constant for identified detectionThe
/feature-flag/:id/profilesendpoint correctly:
- Reuses analytics time‑bucket helpers for the date window.
- Caps
taketo<= 50and defaults sensibly.- Filters by last evaluation result via a
HAVINGclause, which is safe given the controlledresultvalues.One small consistency tweak: when mapping
profiles, you currently do:isIdentified: row.profileId.startsWith('usr_'),Elsewhere (
AnalyticsServiceand sessions queries) you centralize this viaPROFILE_PREFIX_USER. Reusing that constant here:isIdentified: row.profileId.startsWith(AnalyticsService.PROFILE_PREFIX_USER),would keep the “identified profile” definition consistent across analytics and feature flags.
Also applies to: 549-601, 619-631
139-191: Create/update flows handle quotas and duplicate keys correctlyThe create and update handlers correctly:
- Enforce a per‑project flag limit via
FEATURE_FLAGS_MAXIMUM.- Check for duplicate
keyon both create and key changes during update.- Enforce project‑level
allowedToManagebefore mutating flags.The error mapping to
BadRequestExceptionfor unknown persistence errors is a policy choice; if you ever need clearer 5xx vs 4xx semantics, you could switch those catch‑blocks toInternalServerErrorExceptionwhile retaining the same generic message.No blocking issues here.
Also applies to: 323-372
backend/apps/cloud/src/feature-flag/feature-flag.service.ts (1)
73-83: Return type forupdateanddeletemethods isany.Using
Promise<any>obscures the actual return type. TypeORM'supdatereturnsUpdateResultanddeletereturnsDeleteResult. Typing these explicitly improves maintainability.- async update(id: string, flagData: Partial<FeatureFlag>): Promise<any> { + async update(id: string, flagData: Partial<FeatureFlag>): Promise<UpdateResult> { return this.featureFlagRepository.update(id, flagData) } - async delete(id: string): Promise<any> { + async delete(id: string): Promise<DeleteResult> { return this.featureFlagRepository.delete(id) }Add import:
- import { FindManyOptions, FindOneOptions, Repository } from 'typeorm' + import { FindManyOptions, FindOneOptions, Repository, UpdateResult, DeleteResult } from 'typeorm'backend/apps/cloud/src/feature-flag/feature-flag.controller.ts (4)
110-117: Consider extracting the flag-to-DTO transformation.The pattern of mapping flags with
_omit(flag, ['project'])and addingpidis repeated across multiple endpoints (getProjectFeatureFlags, getFeatureFlag, createFeatureFlag, updateFeatureFlag). Consider extracting to a private helper for consistency.private toFlagDto(flag: FeatureFlag & { project?: { id: string } }): FeatureFlagDto { return { ..._omit(flag, ['project']), pid: flag.project?.id, } }
204-209: HTTP status code may be semantically incorrect for quota limit.
HttpStatus.PAYMENT_REQUIRED(402) is used when hitting the 50 flags per project limit. However, this is a resource quota limit, not a billing issue. Consider usingHttpStatus.FORBIDDEN(403) orHttpStatus.UNPROCESSABLE_ENTITY(422) with a clear message instead.if (flagsCount >= FEATURE_FLAGS_MAXIMUM) { throw new HttpException( `You cannot create more than ${FEATURE_FLAGS_MAXIMUM} feature flags per project.`, - HttpStatus.PAYMENT_REQUIRED, + HttpStatus.FORBIDDEN, ) }
322-332: Floating promise with.catch()may silently fail.The
trackEvaluations()call uses.catch()without awaiting, which is intentional for fire-and-forget. However, the error is only logged. Consider adding monitoring/metrics for tracking failures to detect systematic issues.
541-551: Silently returning empty stats on error may mask issues.While graceful degradation is good, returning zero stats on any error (including schema issues, connection problems) makes debugging harder. Consider differentiating between "no data" vs "error fetching data" scenarios.
backend/apps/community/src/feature-flag/feature-flag.service.ts (1)
63-79: Unsafe type assertion withas any.Line 66 uses
as anyto bypass type checking, which can hide type errors. Consider building the result object more explicitly.formatFlagToClickhouse( flag: Partial<FeatureFlag>, ): Partial<ClickhouseFeatureFlag> { - const result: Partial<ClickhouseFeatureFlag> = { ...flag } as any + const result: Partial<ClickhouseFeatureFlag> = {} + + if (flag.id !== undefined) result.id = flag.id + if (flag.key !== undefined) result.key = flag.key + if (flag.description !== undefined) result.description = flag.description + if (flag.flagType !== undefined) result.flagType = flag.flagType + if (flag.rolloutPercentage !== undefined) result.rolloutPercentage = flag.rolloutPercentage + if (flag.projectId !== undefined) result.projectId = flag.projectId + if (flag.created !== undefined) result.created = flag.created if (flag.targetingRules !== undefined) { result.targetingRules = flag.targetingRules ? JSON.stringify(flag.targetingRules) : null } if (flag.enabled !== undefined) { result.enabled = flag.enabled ? 1 : 0 } return result }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (38)
backend/apps/cloud/src/analytics/analytics.controller.ts(5 hunks)backend/apps/cloud/src/analytics/analytics.service.ts(4 hunks)backend/apps/cloud/src/app.module.ts(2 hunks)backend/apps/cloud/src/feature-flag/dto/feature-flag.dto.ts(1 hunks)backend/apps/cloud/src/feature-flag/entity/feature-flag.entity.ts(1 hunks)backend/apps/cloud/src/feature-flag/feature-flag.controller.ts(1 hunks)backend/apps/cloud/src/feature-flag/feature-flag.module.ts(1 hunks)backend/apps/cloud/src/feature-flag/feature-flag.service.ts(1 hunks)backend/apps/cloud/src/project/entity/project.entity.ts(2 hunks)backend/apps/community/src/analytics/analytics.controller.ts(5 hunks)backend/apps/community/src/analytics/analytics.service.ts(4 hunks)backend/apps/community/src/app.module.ts(2 hunks)backend/apps/community/src/feature-flag/dto/feature-flag.dto.ts(1 hunks)backend/apps/community/src/feature-flag/entity/feature-flag.entity.ts(1 hunks)backend/apps/community/src/feature-flag/feature-flag.controller.ts(1 hunks)backend/apps/community/src/feature-flag/feature-flag.module.ts(1 hunks)backend/apps/community/src/feature-flag/feature-flag.service.ts(1 hunks)backend/migrations/clickhouse/2025_12_02_profile_id.js(1 hunks)backend/migrations/clickhouse/2025_12_08_feature_flags.js(1 hunks)backend/migrations/clickhouse/selfhosted_2025_12_10_feature_flags.js(1 hunks)backend/migrations/mysql/2025_12_08_feature_flags.sql(1 hunks)web/app/api/index.ts(1 hunks)web/app/lib/constants/index.ts(2 hunks)web/app/lib/models/Project.ts(1 hunks)web/app/pages/Project/FeatureFlags/View/FeatureFlagSettingsModal.tsx(1 hunks)web/app/pages/Project/FeatureFlags/View/FeatureFlagsView.tsx(1 hunks)web/app/pages/Project/FeatureFlags/View/index.tsx(1 hunks)web/app/pages/Project/View/ViewProject.tsx(11 hunks)web/app/pages/Project/View/components/Pageflow.tsx(4 hunks)web/app/pages/Project/View/components/ProjectSidebar.tsx(10 hunks)web/app/pages/Project/View/components/SearchFilters.tsx(2 hunks)web/app/pages/Project/View/components/SessionDetails.tsx(3 hunks)web/app/pages/Project/View/components/Sessions.tsx(5 hunks)web/app/pages/Project/View/components/Users.tsx(2 hunks)web/app/ui/FilterValueInput.tsx(1 hunks)web/app/ui/Switch.tsx(1 hunks)web/app/ui/Tooltip.tsx(1 hunks)web/public/locales/en.json(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (19)
backend/apps/cloud/src/feature-flag/feature-flag.module.ts (2)
backend/apps/cloud/src/app.module.ts (1)
Module(93-107)backend/apps/community/src/feature-flag/entity/feature-flag.entity.ts (1)
FeatureFlag(12-22)
backend/migrations/clickhouse/2025_12_08_feature_flags.js (1)
backend/migrations/clickhouse/setup.js (2)
dbName(107-107)queriesRunner(76-105)
web/app/pages/Project/View/components/Sessions.tsx (2)
web/app/utils/profileAvatars.tsx (2)
getProfileDisplayName(190-199)ProfileAvatar(210-216)web/app/ui/Badge.tsx (1)
Badge(11-34)
web/app/ui/Switch.tsx (1)
web/app/ui/Text.tsx (1)
Text(51-75)
web/app/pages/Project/FeatureFlags/View/FeatureFlagsView.tsx (7)
web/app/api/index.ts (9)
FeatureFlagProfile(1351-1357)ProjectFeatureFlag(1256-1266)FeatureFlagStats(1268-1274)DEFAULT_FEATURE_FLAGS_TAKE(1286-1286)getProjectFeatureFlags(1288-1301)getFeatureFlagStats(1335-1349)getFeatureFlagProfiles(1363-1388)DEFAULT_FEATURE_FLAG_PROFILES_TAKE(1359-1359)updateFeatureFlag(1319-1325)web/app/utils/profileAvatars.tsx (2)
getProfileDisplayName(190-199)ProfileAvatar(210-216)web/app/ui/Badge.tsx (1)
Badge(11-34)web/app/ui/Text.tsx (1)
Text(51-75)web/app/ui/Switch.tsx (1)
Switch(14-68)web/app/providers/CurrentProjectProvider.tsx (1)
useCurrentProject(244-252)web/app/pages/Project/View/ViewProject.tsx (1)
useViewProjectContext(447-450)
web/app/ui/FilterValueInput.tsx (1)
web/app/lib/constants/index.ts (3)
BROWSER_LOGO_MAP(865-912)OS_LOGO_MAP(914-940)OS_LOGO_MAP_DARK(942-947)
web/app/pages/Project/View/components/Pageflow.tsx (2)
web/fly_import_env.cjs (1)
value(14-14)web/app/utils/generic.ts (2)
getStringFromTime(81-91)getTimeFromSeconds(64-79)
web/app/pages/Project/FeatureFlags/View/FeatureFlagSettingsModal.tsx (5)
web/app/providers/ThemeProvider.tsx (1)
useTheme(29-37)web/app/api/index.ts (2)
TargetingRule(1250-1254)getFilters(1725-1735)backend/apps/cloud/src/feature-flag/entity/feature-flag.entity.ts (1)
TargetingRule(18-22)web/app/ui/FilterValueInput.tsx (1)
filterCategoryIcons(48-68)web/app/ui/Text.tsx (1)
Text(51-75)
backend/apps/community/src/feature-flag/feature-flag.module.ts (1)
backend/apps/community/src/app.module.ts (1)
Module(74-78)
backend/apps/cloud/src/feature-flag/dto/feature-flag.dto.ts (2)
web/app/api/index.ts (1)
TargetingRule(1250-1254)backend/apps/cloud/src/feature-flag/entity/feature-flag.entity.ts (1)
TargetingRule(18-22)
web/app/pages/Project/View/components/Users.tsx (1)
web/app/utils/profileAvatars.tsx (1)
ProfileAvatar(210-216)
backend/apps/community/src/feature-flag/dto/feature-flag.dto.ts (1)
backend/apps/community/src/feature-flag/entity/feature-flag.entity.ts (1)
TargetingRule(6-10)
backend/apps/community/src/feature-flag/feature-flag.controller.ts (2)
backend/apps/community/src/feature-flag/dto/feature-flag.dto.ts (7)
FeatureFlagDto(166-193)CreateFeatureFlagDto(36-98)EvaluatedFlagsResponseDto(212-218)EvaluateFeatureFlagsDto(152-164)UpdateFeatureFlagDto(100-150)FeatureFlagStatsDto(195-210)FeatureFlagProfilesResponseDto(239-245)backend/apps/community/src/feature-flag/entity/feature-flag.entity.ts (1)
FeatureFlag(12-22)
web/app/pages/Project/View/components/ProjectSidebar.tsx (2)
web/app/lib/constants/index.ts (1)
PROJECT_TABS(425-427)web/app/ui/Text.tsx (1)
Text(51-75)
backend/apps/cloud/src/feature-flag/feature-flag.controller.ts (2)
backend/apps/cloud/src/feature-flag/dto/feature-flag.dto.ts (7)
FeatureFlagDto(166-193)CreateFeatureFlagDto(36-98)EvaluatedFlagsResponseDto(212-218)EvaluateFeatureFlagsDto(152-164)UpdateFeatureFlagDto(100-150)FeatureFlagStatsDto(195-210)FeatureFlagProfilesResponseDto(239-245)backend/apps/community/src/feature-flag/entity/feature-flag.entity.ts (1)
FeatureFlag(12-22)
web/app/pages/Project/View/ViewProject.tsx (4)
web/app/lib/constants/index.ts (1)
PROJECT_TABS(425-427)web/app/pages/Project/View/components/SessionDetails.tsx (1)
SessionDetails(27-230)web/app/lib/models/Project.ts (1)
SessionDetails(106-125)web/app/pages/Project/View/ViewProject.helpers.tsx (1)
getFormatDate(1944-1944)
backend/apps/community/src/feature-flag/feature-flag.service.ts (2)
backend/apps/community/src/feature-flag/entity/feature-flag.entity.ts (3)
ClickhouseFeatureFlag(24-34)FeatureFlag(12-22)TargetingRule(6-10)backend/apps/community/src/common/pagination/pagination.results.interface.ts (1)
PaginationOptionsInterface(8-11)
backend/apps/cloud/src/project/entity/project.entity.ts (1)
backend/apps/community/src/feature-flag/entity/feature-flag.entity.ts (1)
FeatureFlag(12-22)
backend/migrations/clickhouse/2025_12_02_profile_id.js (1)
backend/migrations/clickhouse/setup.js (1)
dbName(107-107)
🔇 Additional comments (80)
backend/migrations/clickhouse/2025_12_02_profile_id.js (2)
4-6: LGTM: Idempotent column additions.The
profileIdcolumn additions useIF NOT EXISTSandNullable(String)appropriately, allowing for safe re-execution and gradual feature rollout.
9-19: No action required. This migration safely creates a newsessionstable.The
IF NOT EXISTSclause is appropriate here because this is the initial creation of thesessionstable, not a modification of an existing one. The INSERT statement includes aWHERE (sd.pid, psid) NOT IN (SELECT pid, psid FROM sessions)clause to prevent duplicate data on re-runs. The ReplacingMergeTree engine withlastSeenas the version column properly handles row versioning for subsequent updates. The commented DROP statement forsession_durationsshows appropriate caution for manual verification before cleanup.Likely an incorrect or invalid review comment.
web/app/ui/Tooltip.tsx (1)
39-39: LGTM - Faster tooltip delay improves UX for collapsed sidebar.The reduced delay from 200ms to 50ms makes tooltips more responsive, which is particularly useful for the new collapsed sidebar state where icon-only tabs rely on tooltips for labels.
web/app/ui/Switch.tsx (1)
19-65: Well-implemented accessible switch component.Good use of ARIA attributes (
role="switch",aria-checked), proper label association viahtmlFor, and appropriate disabled state handling for both the button and label.web/app/pages/Project/View/components/ProjectSidebar.tsx (4)
219-232: LGTM - SSR-safe localStorage initialization with memoized toggle.The
typeof window !== 'undefined'check properly handles SSR, anduseCallbackprevents unnecessary re-renders of child components that receivetoggleCollapsed.
81-134: Collapsed view implementation is correct.The duplicated rendering logic between collapsed and expanded modes is acceptable given the significantly different layouts (icon-only with tooltips vs. full labels with collapsible groups).
399-441: Bottom section properly implements collapse toggle with accessibility.Good use of
aria-labelwith internationalized strings for screen reader support, and the conditional text rendering based onisCollapsedstate maintains usability in both modes.
30-30: LGTM - Feature flags icon color follows existing pattern.web/app/api/index.ts (3)
1249-1266: LGTM - Well-typed interfaces for feature flags.The interfaces follow the existing patterns in the codebase and properly type the feature flag entities with appropriate nullable fields (
description,targetingRules).
1288-1333: LGTM - CRUD operations follow existing API patterns.The feature flag API functions consistently use the same error handling pattern and return type annotations as existing endpoints in this file.
1363-1388: LGTM - Profile fetching with proper filter handling.The
resultfilter correctly converts'all'toundefinedto avoid sending unnecessary query parameters, keeping the URL clean.web/app/pages/Project/FeatureFlags/View/index.tsx (1)
1-1: LGTM - Standard barrel export.Clean re-export pattern that allows importing the view component via the directory path.
web/app/lib/models/Project.ts (1)
101-104: LGTM! Type additions align with backend data shape.The
profileIdandisIdentifiedfields extend the session model appropriately. Note thatSession.isIdentifieduses1 | 0(consistent withisLiveabove) whileProfile.isIdentifiedat line 129 usesboolean. This is likely intentional if Session data comes raw from ClickHouse (UInt8) while Profile data is transformed, but worth verifying this is the intended pattern.web/app/lib/constants/index.ts (2)
408-408: LGTM! Feature flags tab added to selfhosted configuration.
419-419: LGTM! Feature flags tab added to production configuration.web/app/pages/Project/View/components/Users.tsx (2)
84-86: LGTM! Memoization is appropriate here.Memoizing
onlineStatusandlastSeenAgobased onprofile.lastSeenprevents unnecessary recalculations during re-renders.
95-113: LGTM! Clean implementation of the online status indicator.The conditional rendering for offline status (no indicator) and the tooltip for online/recently_active statuses provides good UX feedback.
web/app/pages/Project/View/components/Sessions.tsx (4)
85-90: LGTM! Handles missing profileId gracefully.The fallback to
t('project.unknownUser')whenprofileIdis null provides clear UX for sessions without an associated profile.
92-121: LGTM! Comprehensive date/time formatting logic.The
dateLineStringcomputation handles live sessions (start time only) and completed sessions (start-end with duration) appropriately. The memoization dependencies correctly include all variables that affect the output.
134-158: LGTM! Avatar with fallback for unknown users.The placeholder avatar with "?" for sessions without a
profileIdprovides clear visual differentiation from identified users.
162-163: LGTM! Consistent identified badge implementation.Matches the pattern used in
Users.tsxfor displaying the identified badge.web/app/pages/Project/View/components/SearchFilters.tsx (3)
4-5: LGTM! Import cleanup aligns with centralized FilterValueInput.Removing the broad icon imports and unused hooks in favor of the centralized
FilterValueInputcomponent reduces bundle size and code duplication.
13-13: LGTM! Clean import of centralized filter input utilities.Importing
filterCategoryIcons,createVersionValue, andparseVersionValuefromFilterValueInputconsolidates the filter-related logic appropriately.
456-466: LGTM! Clean integration of FilterValueInput component.The component receives all necessary props for proper functionality: cached filter values, language for localization, loading state, and theme for styling. This is a good refactor that centralizes the filter value input logic.
web/app/pages/Project/View/components/SessionDetails.tsx (3)
16-25: LGTM! Clean interface definition and prop extension.The
PageflowIteminterface and updatedSessionDetailsPropsare well-defined. Consider exportingPageflowItemif it needs to be shared with other components (e.g.,Pageflow.tsxwhich has a similar structure).
33-53: Well-implemented fallback duration calculation.The
useMemocorrectly prioritizesdetails.sdurand falls back to calculating duration from pageview timestamps. The dependency array is appropriate.
209-227: LGTM! Rendering correctly uses computed session duration.The conditional rendering properly displays the live indicator, formatted duration, or 'N/A' fallback based on the computed
sessionDuration.web/app/pages/Project/FeatureFlags/View/FeatureFlagSettingsModal.tsx (5)
37-49: LGTM! Well-organized constants.The
FLAG_TYPESandTARGETING_COLUMNSarrays are clearly defined and provide good labels for the UI.
147-156: LGTM! Modal initialization logic is correct.The effect properly handles both create (reset form) and edit (load flag data) modes. The eslint-disable comment is acceptable here since
loadFlagshould only run whenflagIdchanges, not on every render.
158-193: LGTM! Form submission logic is well-structured.The
handleSubmitcorrectly differentiates between create and update operations, properly handles the rollout percentage based on flag type, and includes appropriate error handling with user feedback via toasts.
219-226: LGTM! Good key normalization.The key transformation enforces a consistent kebab-case format with allowed characters, which is appropriate for feature flag identifiers that may be used in code.
364-553: LGTM! Well-implemented targeting rules UI.The targeting rules section provides a clean interface with column selection (using icons from
filterCategoryIcons), include/exclude operator toggle, and autocomplete value input with loading states. The delete functionality is clear and accessible.web/app/pages/Project/FeatureFlags/View/FeatureFlagsView.tsx (6)
63-130: LGTM! Clean profile row component.The
FeatureFlagProfileRowcomponent is well-structured with memoized computed values and proper accessibility (link navigation, visual indicators).
150-418: LGTM! Comprehensive flag row component.The
FeatureFlagRowcomponent handles:
- Flag display with status/type badges
- Responsive stats (mobile vs desktop)
- Action buttons with proper event propagation handling
- Expandable profiles section with filtering
- Delete confirmation modal
Well-structured with good UX patterns.
432-460: LGTM! Proper state management with unmount safety.The use of
isMountedRefandisLoadingRefcorrectly prevents state updates after unmount and concurrent data loading, which is a good pattern for async operations in React.
473-496: LGTM! Robust data loading with guards.The
loadFlagsfunction properly prevents concurrent loads and checks mount status before state updates. The error handling gracefully extracts the error message with fallbacks.
555-625: Well-designed auto-filter selection on expand.The
handleToggleExpandfunction intelligently selects the result filter that has data (preferring 'true', falling back to 'false'), providing a better initial UX. The error handling is comprehensive.
718-762: LGTM! Good error and loading state handling.The error state provides clear feedback with recovery options (reload page, contact support). The loading logic correctly distinguishes between initial load (full loader) and subsequent loads (loading bar).
web/app/pages/Project/View/components/Pageflow.tsx (5)
41-99: LGTM! Clean metadata panel with progressive disclosure.The
MetadataPanelcomponent provides a good UX with:
- Limited initial display (5 items) to avoid overwhelming users
- Show more/less toggle for full content
- Truncation with tooltips for long values
- Proper keyboard accessibility for the toggle button
101-107: LGTM! Clean duration formatting helper.The
formatDurationfunction correctly handles edge cases and delegates to existing utility functions for consistent formatting.
109-234: LGTM! Well-implemented pageflow item with accessibility.The
PageflowItemcomponent provides:
- Accessible expand/collapse for metadata (keyboard support)
- Visual differentiation by event type (pageview, event, error)
- Timeline visualization (solid line to next event, dashed to session end)
- Time duration display between events
257-285: LGTM! Correct time calculation logic.The
timeBetweenEventsandtimeAfterLastEventmemoized calculations are well-implemented:
timeBetweenEventscorrectly computes inter-event durationstimeAfterLastEventproperly calculates remaining time based on session duration minus the span of recorded events- Both handle edge cases (empty pages, single event, sdur <= 0)
326-366: LGTM! Clear end-of-session visual indicator.The end-of-session marker provides a clear visual termination point for the timeline, with optional display of remaining session time. The hollow circle icon distinguishes it from event icons.
web/app/ui/FilterValueInput.tsx (5)
47-92: LGTM! Well-organized exports for filter UI.The exported icon mappings (
filterCategoryIcons,deviceIconMapping) and version value helpers provide a clean API for consumers. TheVERSION_SEPARATORpattern handles the edge case of browser/OS versions that need to display parent name alongside version.
154-225: LGTM! Comprehensive icon rendering logic.The
getIconForItemcallback handles multiple column types with appropriate icons:
- Country flags for
cc- Browser logos for
br/brvwith version parsing- OS logos for
os/osvwith dark mode support- Device icons for
dv- Fallback globe/question icons for unknowns
266-292: LGTM! Good keyboard handling with country code conversion.The Enter key handling is well-implemented:
- For country codes, it converts country names to alpha-2 codes using
countries.getAlpha2Code- Falls back to matching existing items or using the raw input
- Escape properly closes the dropdown and clears input
356-399: LGTM! Efficient virtualized list implementation.The virtualized list using
@tanstack/react-virtualensures good performance even with large item lists. The overscan of 15 items provides smooth scrolling, and the absolute positioning with transforms is the correct pattern for virtualization.
333-355: LGTM! Good empty state with "use value" action.The empty state provides a clear path forward when no matches are found:
- Allows users to use their custom input value
- Maintains country code conversion for consistency
- Falls back to a simple "nothing found" message when input is empty
web/public/locales/en.json (3)
120-121: New generic “collapse/expand” labels look fineThe added
common.collapse/common.expandkeys are clear, generic, and consistent with existing common actions.
889-890: Dashboard feature flags entry strings are consistent with new tab
dashboard.featureFlags/dashboard.featureFlagsDescalign with the new project tab and describe the feature succinctly; no issues here.
1095-1095:project.unknownUseris a useful complement tounknownCountryThe new
project.unknownUserkey matches existing patterns (e.g.unknownCountry) and should integrate cleanly with user/profile views.web/app/pages/Project/View/ViewProject.tsx (9)
19-45: LucideFlagIconimport is consistent with existing icon usageAdding
FlagIconalongside other lucide icons keeps icon usage uniform across tabs; no issues here.
170-175: FeatureFlagsView import mirrors existing Goals/Alerts structureImporting
FeatureFlagsViewfrom../FeatureFlags/Viewmatches the existing pattern used for Goals and Alerts, keeping the project‑level feature modules organised.
396-413: Context extended cleanly withfeatureFlagsRefreshTriggerAdding
featureFlagsRefreshTriggertoViewProjectContextTypealongside the existing captcha/goals triggers is coherent and keeps the refresh plumbing centralised.
423-440: Default context value includes new trigger correctlyThe
defaultViewProjectContextinitialisesfeatureFlagsRefreshTriggerto0, in line with other trigger defaults, so consumers can safely read it before any refresh occurs.
550-553: State hook forfeatureFlagsRefreshTriggeris correctly wiredLocal state for
featureFlagsRefreshTriggerparallelscaptchaRefreshTriggerandgoalsRefreshTrigger, making it straightforward forFeatureFlagsViewto subscribe via context.
3438-3438: Context provider now exposesfeatureFlagsRefreshTriggerPassing
featureFlagsRefreshTriggerthroughViewProjectContext.Providercompletes the wiring so any nested component (notablyFeatureFlagsView) can react to refresh events.
4063-4064: Passingsdurinto Pageflow is straightforward and non‑breaking
sdur={activeSession?.details?.sdur}cleanly forwards the (optional) session duration toPageflow. As the prop is optional at the call site, it should remain backward‑compatible with existing Pageflow behaviour.
4339-4345: FeatureFlagsView render block is aligned with GoalsView patternRendering
FeatureFlagsViewwhenactiveTab === PROJECT_TABS.featureFlagsand passingperiod,from,to, andtimezonematches the existingGoalsViewintegration, so it will respect the project’s current time context.Once
FeatureFlagsViewis in place, this should behave as expected.
1473-1520: Feature Flags tab definition integrates well into the tab arrayThe new tab entry uses
PROJECT_TABS.featureFlagsas ID with proper type safety, and pairs it witht('dashboard.featureFlags')andFlagIconconsistent with other tabs. The constant is properly defined in web/app/lib/constants/index.ts for both selfhosted and production configurations, ensuring the integration is sound.backend/apps/cloud/src/project/entity/project.entity.ts (1)
12-12: LGTM!The
FeatureFlagrelation follows the established pattern used by other entity relations in this file (e.g.,goals,alerts,funnels). The@ApiPropertydecorator with lazy loading syntax and the@OneToManyrelation are correctly configured.Also applies to: 132-134
backend/migrations/clickhouse/2025_12_08_feature_flags.js (1)
1-20: LGTM!The migration follows ClickHouse best practices:
MergeTreeengine is appropriate for append-only evaluation logs- Monthly partitioning with 1-year TTL is sensible for analytics data retention
ORDER BY (pid, flagId, created)supports efficient queries by project and flagFixedString(12)forpidaligns with the project ID format used elsewhereThe denormalization of
flagKeyalongsideflagIdis acceptable for query efficiency.backend/migrations/clickhouse/selfhosted_2025_12_10_feature_flags.js (1)
22-36: LGTM!The
feature_flag_evaluationstable schema is consistent with the cloud migration, ensuring parity between cloud and self-hosted deployments.backend/apps/community/src/app.module.ts (1)
14-14: LGTM!The
FeatureFlagModuleis correctly imported and registered in the modules array, making feature flags available to all Community Edition instances as stated in the PR objectives.Also applies to: 70-70
backend/apps/community/src/feature-flag/feature-flag.module.ts (1)
1-15: LGTM!The module is well-structured and follows NestJS conventions. The dependencies are appropriate for the community edition's architecture:
ProjectModulefor project validationAnalyticsModulefor ClickHouse accessAppLoggerModulefor loggingThe absence of
TypeOrmModuleis correct since the community edition stores feature flags directly in ClickHouse rather than MySQL.backend/apps/cloud/src/feature-flag/feature-flag.module.ts (1)
1-24: LGTM!The module is correctly configured for the cloud architecture:
TypeOrmModule.forFeature([FeatureFlag])properly registers the entity for repository injectionUserModuleinclusion enables user-based authorization for feature flag managementAnalyticsModuleprovides ClickHouse access for evaluation trackingThe additional dependencies compared to the community module (
TypeOrmModule,UserModule) reflect the cloud edition's use of MySQL for persistent entities and more sophisticated access control.backend/apps/cloud/src/app.module.ts (1)
24-24: LGTM! Clean module integration.The FeatureFlagModule is properly imported and registered following NestJS conventions, consistent with other modules in the application.
Also applies to: 84-84
backend/apps/community/src/feature-flag/entity/feature-flag.entity.ts (1)
1-34: LGTM! Well-structured type definitions.The interfaces are cleanly defined with clear separation between standard TypeScript types and ClickHouse-optimized types. The enum values align with the MySQL migration, and the TargetingRule interface provides good documentation of supported columns.
backend/apps/cloud/src/analytics/analytics.controller.ts (1)
1119-1123: LGTM! Consistent refactor of session recording.All call sites have been updated to use the renamed
recordSessionActivity()method with the simplified signature (removing theisNewparameter). The changes are consistent across error logging, custom events, heartbeat, pageview, and noscript endpoints.Also applies to: 1258-1262, 1346-1346, 1381-1385, 1526-1530
backend/apps/cloud/src/feature-flag/entity/feature-flag.entity.ts (1)
1-70: LGTM! Entity definition aligns with migration schema.The TypeORM entity is well-structured with proper decorators, matches the MySQL migration schema, and includes Swagger documentation via
ApiProperty. The@Unique(['project', 'key'])constraint correctly mirrors the database constraint.Note: The
projectManyToOne relation is nullable by default in TypeORM, which aligns with the nullableprojectIdin the migration. If feature flags should always belong to a project, consider adding{ nullable: false }to the relation decorator and updating the migration accordingly.backend/apps/community/src/analytics/analytics.service.ts (2)
1234-1281: LGTM! Simplified session tracking with cleaner API.The refactor improves the codebase by:
- Renaming
recordSessiontorecordSessionActivityfor clarity- Removing the
isNewparameter to simplify the API- Streamlining the logic to focus on recording firstSeen/lastSeen timestamps
- Maintaining proper error handling
The trade-off of querying for existing sessions on every call instead of using the
isNewflag is acceptable given ClickHouse's performance characteristics and the simplicity gained.
3906-3907: LGTM! Profile tracking enhancements.The addition of
profileIdand computedisIdentifiedflag enables user identification and profile-based analytics. TheisIdentifiedlogic correctly distinguishes user-supplied profiles (prefixed withusr_) from anonymous ones.Also applies to: 3924-3925
backend/migrations/mysql/2025_12_08_feature_flags.sql (1)
11-11: MakeprojectIdNOT NULL or clarify if project-agnostic flags are intentional.The database schema allows nullable
projectId, but the API enforces it as required (pidinCreateFeatureFlagDtohas no@IsOptional()) and the controller explicitly assigns the project before saving. The entity relationship also declaresprojectwithout optional typing. This inconsistency means:
- The API guarantees every flag belongs to a project
- The schema permits orphaned flags with
projectId = NULL- With the unique constraint on
(projectId, key), multiple flags can share the same key whenprojectId IS NULL(standard SQL NULL behavior)If all flags must belong to a project, add
NOT NULLto the migration. If project-agnostic flags are supported, update the entity, service, and API to explicitly allow it.backend/apps/cloud/src/feature-flag/feature-flag.service.ts (4)
1-17: LGTM!The service is properly structured with dependency injection via
@InjectRepository. Imports are appropriate for the functionality.
19-38: LGTM!Pagination implementation with sensible defaults (take: 100, skip: 0) and consistent ordering by key.
147-184: Permissive targeting behavior may be unexpected.When
attributesisundefinedor when a specific attribute is missing, the method returnstrue(permissive). This means:
- If no attributes are provided, all targeting rules are bypassed
- If a specific attribute is missing, that rule is skipped
This could lead to users seeing flags they shouldn't if attributes fail to be derived. Consider whether this is the intended behavior or if a more restrictive default (return
falsewhen required attributes are missing) would be safer.
199-224: Rollout percentage calculation is correct.The deterministic hash using MD5 with flag key and profile ID ensures consistent bucketing. The normalization logic using
0xffffffff(2^32-1) is correct for converting 8 hex chars to a 0-100 range.Note: Using
<instead of<=for the comparison means a 50% rollout will include profiles with normalized values 0-49.99..., which is mathematically correct.backend/apps/cloud/src/feature-flag/feature-flag.controller.ts (2)
56-75: LGTM!Controller setup with proper dependency injection of required services. Route versioning with both
/feature-flagand/v1/feature-flagpaths is appropriate.
490-501: ClickHouse query uses parameterized queries correctly.Good use of
{pid:FixedString(12)},{flagId:String}, etc. for SQL injection prevention.backend/apps/community/src/feature-flag/feature-flag.service.ts (1)
6-11: These lodash imports are actually used in the file at lines 251–265 (_reduce,_filter, and_keysappear in the update logic), so they are not unused. No changes needed.Likely an incorrect or invalid review comment.
backend/apps/community/src/feature-flag/feature-flag.service.ts
Outdated
Show resolved
Hide resolved
1 participant
Changes
Community Edition support
Database migrations
Documentation
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.