Skip to content

Commit d217e1d

Browse files
committed
apply coderabbit's review comments && cleanup
Signed-off-by: Atif Ali <atali@redhat.com>
1 parent 969427f commit d217e1d

10 files changed

Lines changed: 46 additions & 41 deletions

‎src/gitops/components/shared/ApplicationList.tsx‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,10 @@ import {
4141
ShowOperandsInAllNamespacesRadioGroup,
4242
useShowOperandsInAllNamespaces,
4343
} from './AllNamespaces';
44+
import {
45+
APPLICATION_LIST_COLUMN_MANAGEMENT_ID,
46+
getApplicationManagedColumns,
47+
} from './applicationListColumns';
4448
import {
4549
APPLICATION_HEALTH_FILTER_PARAM,
4650
APPLICATION_SYNC_FILTER_PARAM,
@@ -49,10 +53,6 @@ import {
4953
parseRowFilterParam,
5054
} from './applicationListFilters';
5155
import ApplicationSetApplicationsView from './ApplicationSetApplicationsView';
52-
import {
53-
APPLICATION_LIST_COLUMN_MANAGEMENT_ID,
54-
getApplicationManagedColumns,
55-
} from './applicationListColumns';
5656
import {
5757
type GitOpsManagedColumn,
5858
NAMESPACE_COLUMN_ID,
@@ -126,10 +126,7 @@ const ApplicationList: React.FC<ApplicationProps> = ({
126126

127127
const { t } = useTranslation('plugin__gitops-plugin');
128128
const includeNamespaceColumn = !namespace;
129-
const managedColumns = React.useMemo(
130-
() => getApplicationManagedColumns(includeNamespaceColumn, t),
131-
[includeNamespaceColumn, t],
132-
);
129+
const managedColumns = React.useMemo(() => getApplicationManagedColumns(true, t), [t]);
133130

134131
const { activeColumnIds, columnManagement, filterDataView } = useGitOpsColumnManagement({
135132
columnManagementID: APPLICATION_LIST_COLUMN_MANAGEMENT_ID,

‎src/gitops/components/shared/ColumnManagement/GitOpsColumnManagementModal.tsx‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,6 @@ const ColumnRow: React.FC<{
6363
);
6464
};
6565

66-
/**
67-
* Manage columns modal with Default / Additional sections (console list-page layout).
68-
*/
6966
export const GitOpsColumnManagementModal: React.FC<GitOpsColumnManagementModalProps> = ({
7067
isOpen,
7168
onClose,

‎src/gitops/components/shared/ColumnManagement/columnManagementUtils.test.ts‎

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -169,24 +169,39 @@ describe('getSavableColumnIds', () => {
169169
});
170170

171171
it('keeps previously saved ids that the current modal does not manage', () => {
172-
const projectScopedColumns = baseColumns.filter(
173-
(column) => column.id !== NAMESPACE_COLUMN_ID,
174-
);
175172
const modalColumns = toColumnManagementModalColumns(
176-
projectScopedColumns,
173+
baseColumns,
177174
new Set(['name', 'status', 'actions']),
178175
false,
179176
);
180177

181178
expect(
182-
getSavableColumnIds(projectScopedColumns, modalColumns, [
183-
'name',
184-
'namespace',
185-
'labels',
186-
'extra',
187-
]),
179+
getSavableColumnIds(baseColumns, modalColumns, ['name', 'namespace', 'labels', 'extra']),
188180
).toEqual(['namespace', 'name', 'status']);
189181
});
182+
183+
it('preserves Namespace after a first save in a project-scoped view', () => {
184+
const defaultIds = getDefaultActiveColumnIds(baseColumns);
185+
expect(defaultIds).toContain('namespace');
186+
187+
const projectModalColumns = toColumnManagementModalColumns(
188+
baseColumns,
189+
new Set(['name', 'status', 'labels', 'actions']),
190+
false,
191+
);
192+
expect(projectModalColumns.map((column) => column.id)).not.toContain('namespace');
193+
194+
const savedAfterProjectEdit = getSavableColumnIds(baseColumns, projectModalColumns, defaultIds);
195+
expect(savedAfterProjectEdit).toContain('namespace');
196+
197+
expect(
198+
resolveActiveColumnIds({
199+
columns: baseColumns,
200+
savedColumnIds: savedAfterProjectEdit,
201+
includeNamespaceColumn: true,
202+
}),
203+
).toContain('namespace');
204+
});
190205
});
191206

192207
describe('filterDataViewColumnsAndRows', () => {

‎src/gitops/components/shared/ColumnManagement/columnManagementUtils.ts‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,9 +83,7 @@ export const getSavableColumnIds = (
8383
modalColumns.filter((column) => column.isShown).map((column) => column.id),
8484
);
8585

86-
// Keep prefs for columns this modal did not offer (e.g. namespace while project-scoped).
87-
const preserved =
88-
previouslySavedIds?.filter((id) => !managedByModal.has(id)) ?? [];
86+
const preserved = previouslySavedIds?.filter((id) => !managedByModal.has(id)) ?? [];
8987

9088
const fromModal = getManageableColumns(columns)
9189
.map((column) => column.id)

‎src/gitops/components/shared/ColumnManagement/index.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,12 @@ export type {
1515
GitOpsColumnManagementModalColumn,
1616
GitOpsManagedColumn,
1717
ResolveActiveColumnIdsOptions,
18+
UseGitOpsColumnManagementOptions,
1819
} from './types';
1920
export {
2021
getGitOpsColumnManagementSettingKey,
2122
GITOPS_COLUMN_MANAGEMENT_SETTING_PREFIX,
2223
NAMESPACE_COLUMN_ID,
2324
} from './types';
24-
export type {
25-
UseGitOpsColumnManagementOptions,
26-
UseGitOpsColumnManagementResult,
27-
} from './useGitOpsColumnManagement';
25+
export type { UseGitOpsColumnManagementResult } from './useGitOpsColumnManagement';
2826
export { useGitOpsColumnManagement } from './useGitOpsColumnManagement';

‎src/gitops/components/shared/ColumnManagement/types.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,3 +31,12 @@ export type ResolveActiveColumnIdsOptions = {
3131
/** When false, omit the namespace column (project-scoped views). */
3232
includeNamespaceColumn?: boolean;
3333
};
34+
35+
export type UseGitOpsColumnManagementOptions = {
36+
columnManagementID: string;
37+
/** Full catalog for prefs; use includeNamespaceColumn to hide Namespace in project views. */
38+
columns: GitOpsManagedColumn[];
39+
resourceType: string;
40+
includeNamespaceColumn?: boolean;
41+
showNamespaceHelp?: boolean;
42+
};

‎src/gitops/components/shared/ColumnManagement/useGitOpsColumnManagement.tsx‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,16 +13,11 @@ import { GitOpsColumnManagementControl } from './GitOpsColumnManagementControl';
1313
import {
1414
type GitOpsColumnManagementModalColumn,
1515
type GitOpsManagedColumn,
16+
type UseGitOpsColumnManagementOptions,
1617
getGitOpsColumnManagementSettingKey,
1718
} from './types';
1819

19-
export type UseGitOpsColumnManagementOptions = {
20-
columnManagementID: string;
21-
columns: GitOpsManagedColumn[];
22-
resourceType: string;
23-
includeNamespaceColumn?: boolean;
24-
showNamespaceHelp?: boolean;
25-
};
20+
export type { UseGitOpsColumnManagementOptions };
2621

2722
export type UseGitOpsColumnManagementResult = {
2823
loaded: boolean;

‎src/gitops/components/shared/GitOpsListPageToolbar.tsx‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ export type GitOpsListPageToolbarProps = {
88
actions?: React.ReactNode;
99
};
1010

11-
/** Filters + manage-columns control on one row; optional trailing actions. */
1211
export const GitOpsListPageToolbar: React.FC<GitOpsListPageToolbarProps> = ({
1312
filters,
1413
columnManagement,

‎src/gitops/components/shared/applicationListColumns.test.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
1-
import { getDefaultActiveColumnIds } from './ColumnManagement';
2-
31
import {
42
APPLICATION_LIST_COLUMN_MANAGEMENT_ID,
53
getApplicationManagedColumns,
64
} from './applicationListColumns';
5+
import { getDefaultActiveColumnIds } from './ColumnManagement';
76

87
const t = (key: string) => key;
98

‎src/gitops/components/shared/applicationListColumns.ts‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,7 @@
1-
import type { GitOpsManagedColumn } from './ColumnManagement';
2-
import { NAMESPACE_COLUMN_ID } from './ColumnManagement';
1+
import { type GitOpsManagedColumn, NAMESPACE_COLUMN_ID } from './ColumnManagement/types';
32

43
export const APPLICATION_LIST_COLUMN_MANAGEMENT_ID = 'gitops.applications';
54

6-
/** Column management metadata for the Applications list (and shared AppSet/Project app tables). */
75
export const getApplicationManagedColumns = (
86
includeNamespaceColumn: boolean,
97
t: (key: string) => string,

0 commit comments

Comments
 (0)