feat(Diagnostics): display db feature flags #1229

sareyu · 2024-08-28T12:20:46Z

CI Results

Test Status: ⚠️ FLAKY

Total	Passed	Failed	Flaky	Skipped
124	121	0	3	0

Bundle Size: 🔺

Current: 78.93 MB | Main: 78.92 MB
Diff: +0.01 MB (0.02%)

⚠️ Bundle size increased. Please review.

ℹ️ CI Information

Test recordings for failed tests are available in the full report.
Bundle size is measured for the entire 'dist' directory.
📊 indicates links to detailed reports.
🔺 indicates increase, 🔽 decrease, and ✅ no change in bundle size.

src/containers/Tenant/Diagnostics/Configs/Configs.tsx

artemmufazalov · 2024-09-03T12:47:23Z

src/types/api/cluster.ts

+export interface TClusterConfigFeatureFlag {
+    Name: string;
+    Current?: boolean;
+    Default?: boolean;
+}
+
+export interface TClusterConfigDb {
+    Name: string;
+    FeatureFlags: TClusterConfigFeatureFlag[];
+}
+
+export interface TClusterConfigs {
+    Databases: TClusterConfigDb[];
+}


In types/api types are grouped by endpoints or entites. Please, create a separate file for configs, they don't connected with TClusterInfo - /viewer/cluster response

src/store/reducers/tenant/tenant.ts

src/containers/Tenant/Diagnostics/Configs/Configs.tsx

ValeraS · 2024-09-05T09:09:19Z

src/containers/Tenant/Diagnostics/Configs/Configs.tsx

+    const featureFlagsFilter = useTypedSelector(
+        (state) => state.tenant.featureFlagsFilter,
+    )?.toLocaleLowerCase();


Do we really need this state in the store?

yes. in some other tabs search filter is saved between tab navigation

But current solution is not good, we have two sources of truth. On other tabs behavior is different. For example, on Top queries tab the search is saved only in the store, and on Nodes and Storage tabs the search is saved only in query params.

I suggested to have same UX in such cases - sync btw store and url, others agreed.
Why do you think it's not good?

discussed in pm, fixed

ValeraS · 2024-09-05T09:14:16Z

src/containers/Tenant/Diagnostics/Configs/Configs.tsx

+];
+
+interface Props {
+    path: string;


path is not used

ValeraS · 2024-09-05T09:28:50Z

src/containers/Tenant/Diagnostics/Configs/Configs.tsx

+
+    return (
+        <TableWithControlsLayout>
+            <TableWithControlsLayout.Controls>{renderControls()}</TableWithControlsLayout.Controls>


Why not inline controls here?

ValeraS · 2024-09-05T09:32:32Z

src/containers/Tenant/Diagnostics/Configs/Configs.tsx

+        <TableWithControlsLayout>
+            <TableWithControlsLayout.Controls>{renderControls()}</TableWithControlsLayout.Controls>
+            <TableWithControlsLayout.Table loading={isFetching}>
+                {renderContent()}


Let's extract this to another component

artemmufazalov · 2024-09-05T13:44:28Z

src/types/api/featureFlags.ts

+export interface TConfigFeatureFlag {
+    Name: string;
+    Current?: boolean;
+    Default?: boolean;
+}
+
+export interface TConfigDb {
+    Name: string;
+    FeatureFlags: TConfigFeatureFlag[];
+}
+
+export interface TConfigs {
+    Databases: TConfigDb[];
+}


Not a request, but for you information: types are called TSomething and ESomething to match with names from Swagger or backend types, you may not follow the same pattern for your own types

Co-authored-by: Valerii Sidorenko <[email protected]>

ValeraS · 2024-09-06T08:52:51Z

src/containers/Tenant/Diagnostics/Configs/Configs.tsx

+                    placeholder={i18n('search-placeholder')}
+                />
+            </TableWithControlsLayout.Controls>
+            <TableWithControlsLayout.Table loading={isFetching}>


Let's move this and fetching and filtering data to the FeatureFlagsList.

Why? Now Configs component includes logic and FeatureFlagsList is stupid (and is not very necessary imho). After moving logic Config will be almost redundant

discussed in pm, fixed

ValeraS · 2024-09-06T08:52:56Z

src/containers/Tenant/Diagnostics/Configs/Configs.tsx

+    const featureFlagsFilter = useTypedSelector(
+        (state) => state.tenant.featureFlagsFilter,
+    )?.toLocaleLowerCase();


But current solution is not good, we have two sources of truth. On other tabs behavior is different. For example, on Top queries tab the search is saved only in the store, and on Nodes and Storage tabs the search is saved only in query params.

ValeraS · 2024-09-06T09:16:23Z

src/containers/Tenant/Diagnostics/Configs/Configs.tsx

+        <ResizeableDataTable
+            columnsWidthLSKey={FEATURE_FLAGS_COLUMNS_WIDTH_LS_KEY}
+            columns={columns}
+            data={featureFlags}
+            settings={DEFAULT_TABLE_SETTINGS}
+        />


Suggested change

<ResizeableDataTable

columnsWidthLSKey={FEATURE_FLAGS_COLUMNS_WIDTH_LS_KEY}

columns={columns}

data={featureFlags}

settings={DEFAULT_TABLE_SETTINGS}

/>

<ResizeableDataTable

columnsWidthLSKey={FEATURE_FLAGS_COLUMNS_WIDTH_LS_KEY}

columns={columns}

data={featureFlags}

settings={DEFAULT_TABLE_SETTINGS}

emptyDataMessage={i18n(featureFlagsFilter ? 'search-empty' : 'no-data')}

/>

ValeraS · 2024-09-06T09:18:15Z

src/containers/Tenant/Diagnostics/Configs/Configs.tsx

+
+const FeatureFlagsList = ({isError, featureFlags, featureFlagsFilter}: FeatureFlagsListsProps) => {
+    if (isError) {
+        return i18n('error-msg');


Why not a real error from query?

Suggested change

return i18n('error-msg');

return <ResponseError error={error} />;

ValeraS · 2024-09-09T08:07:56Z

src/store/reducers/tenant/tenant.ts

@@ -62,6 +62,19 @@ export const tenantApi = api.injectEndpoints({
            },
            providesTags: ['All'],
        }),
+        getClusterConfig: builder.query({
+            queryFn: async ({database}: {database?: string}, {signal}) => {


Suggested change

queryFn: async ({database}: {database?: string}, {signal}) => {

queryFn: async ({database}: {database: string}, {signal}) => {

feat(Diagnostics): display db feature flags

8cf14db

sareyu linked an issue Aug 28, 2024 that may be closed by this pull request

display feature flags status #1035

Closed

fix: api changed

e749043

sareyu requested a review from artemmufazalov September 2, 2024 10:30

sareyu marked this pull request as ready for review September 2, 2024 10:30

sareyu added 3 commits September 3, 2024 11:48

fix: fixup

ccfaf17

fix: add search

f8dae05

fix: sync state

ff7b87f

sareyu requested a review from Raubzeug September 3, 2024 11:58

artemmufazalov requested changes Sep 3, 2024

View reviewed changes

sareyu added 2 commits September 4, 2024 13:35

fix: review fixes

7940dbf

fix: small fixes

f2b6ca4

ValeraS requested changes Sep 5, 2024

View reviewed changes

artemmufazalov reviewed Sep 5, 2024

View reviewed changes

sareyu and others added 2 commits September 6, 2024 09:47

Apply suggestions from code review

7ae6965

Co-authored-by: Valerii Sidorenko <[email protected]>

fix: review fixes

1e62780

ValeraS reviewed Sep 6, 2024

View reviewed changes

sareyu added 3 commits September 9, 2024 09:40

fix: fixes

18afcc7

fix: cleanup

9ae8f23

fix: rm filter from store

ca745dd

ValeraS requested changes Sep 9, 2024

View reviewed changes

sareyu added 2 commits September 9, 2024 11:17

fix: fix todo

ff42d75

fix: fix type

4a6fbc7

ValeraS approved these changes Sep 9, 2024

View reviewed changes

artemmufazalov approved these changes Sep 9, 2024

View reviewed changes

Merge branch 'main' into issue1035

be03914

sareyu merged commit 182d803 into main Sep 9, 2024
6 checks passed

sareyu deleted the issue1035 branch September 9, 2024 10:05

ydb-platform-bot mentioned this pull request Sep 10, 2024

chore(main): release 6.19.0 #1261

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(Diagnostics): display db feature flags #1229

feat(Diagnostics): display db feature flags #1229

sareyu commented Aug 28, 2024 •

edited by github-actions bot

Loading

artemmufazalov Sep 3, 2024

ValeraS Sep 5, 2024

sareyu Sep 5, 2024

ValeraS Sep 6, 2024

sareyu Sep 6, 2024

sareyu Sep 9, 2024

ValeraS Sep 5, 2024

ValeraS Sep 5, 2024

ValeraS Sep 5, 2024

artemmufazalov Sep 5, 2024

ValeraS Sep 6, 2024

sareyu Sep 6, 2024

sareyu Sep 9, 2024

ValeraS Sep 6, 2024

ValeraS Sep 6, 2024

ValeraS Sep 6, 2024

ValeraS Sep 9, 2024

	return i18n('error-msg');
	return <ResponseError error={error} />;

	queryFn: async ({database}: {database?: string}, {signal}) => {
	queryFn: async ({database}: {database: string}, {signal}) => {

feat(Diagnostics): display db feature flags #1229

feat(Diagnostics): display db feature flags #1229

Conversation

sareyu commented Aug 28, 2024 • edited by github-actions bot Loading

CI Results

Test Status: ⚠️ FLAKY

Bundle Size: 🔺

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sareyu commented Aug 28, 2024 •

edited by github-actions bot

Loading