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

feat: use relative entity path in schema actions #1366

Merged
merged 2 commits into from
Sep 30, 2024
Merged
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
30 changes: 19 additions & 11 deletions src/containers/Tenant/ObjectSummary/ObjectSummary.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ import {ObjectTree} from './ObjectTree';
import {SchemaActions} from './SchemaActions';
import i18n from './i18n';
import {b} from './shared';
import {transformPath} from './transformPath';
import {isDomain, transformPath} from './transformPath';

import './ObjectSummary.scss';

Expand Down Expand Up @@ -158,7 +158,11 @@ export function ObjectSummary({

const overview: InfoViewerItem[] = [];

overview.push({label: i18n('field_type'), value: PathType?.replace(/^EPathType/, '')});
const normalizedType = isDomain(path, PathType)
? 'Domain'
: PathType?.replace(/^EPathType/, '');

overview.push({label: i18n('field_type'), value: normalizedType});

if (PathSubType !== EPathSubType.EPathSubTypeEmpty) {
overview.push({
Expand Down Expand Up @@ -353,6 +357,8 @@ export function ObjectSummary({
dispatchCommonInfoVisibilityState(PaneVisibilityActionTypes.clear);
};

const relativePath = transformPath(path, tenantName);

const renderCommonInfoControls = () => {
const showPreview = isTableType(type) && !isIndexTableType(subType);
return (
Expand All @@ -364,7 +370,7 @@ export function ObjectSummary({
'm',
)(path, 'preview')}
<ClipboardButton
text={path}
text={relativePath}
view="flat-secondary"
title={i18n('action_copySchemaPath')}
/>
Expand All @@ -380,15 +386,19 @@ export function ObjectSummary({

const renderEntityTypeBadge = () => {
const {Status, Reason} = currentObjectData ?? {};
if (type) {
let label = type.replace('EPathType', '');
if (isDomain(path, type)) {
label = 'domain';
}
return <div className={b('entity-type')}>{label}</div>;
}

let message;
if (!type && Status && Reason) {
if (Status && Reason) {
message = `${Status}: ${Reason}`;
}

return type ? (
<div className={b('entity-type')}>{type.replace('EPathType', '')}</div>
) : (
return (
<div className={b('entity-type', {error: true})}>
<HelpPopover content={message} offset={{left: 0}} />
</div>
Expand All @@ -414,9 +424,7 @@ export function ObjectSummary({
<div className={b('info-header')}>
<div className={b('info-title')}>
{renderEntityTypeBadge()}
<div className={b('path-name')}>
{transformPath(path, tenantName)}
</div>
<div className={b('path-name')}>{relativePath}</div>
</div>
<div className={b('info-controls')}>
{renderCommonInfoControls()}
Expand Down
36 changes: 33 additions & 3 deletions src/containers/Tenant/ObjectSummary/__test__/transformPath.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {transformPath} from '../transformPath';
import {EPathType} from '../../../../types/api/schema';
import {isDomain, transformPath} from '../transformPath';

describe('transformPath', () => {
test.each([
Expand All @@ -9,8 +10,8 @@ describe('transformPath', () => {
['/prod/v1/sth', 'prod/v1', 'sth'],
['/dev/v1/sth', '/dev', 'v1/sth'],
['/dev/v1/sth', 'dev', 'v1/sth'],
['/dev', '/dev', '/'],
['/dev', 'dev', '/'],
['/dev', '/dev', '/dev'],
['/dev', 'dev', '/dev'],
['/', '/dev', '/'],
['/', 'dev', '/'],
['', '/dev', '/'],
Expand Down Expand Up @@ -41,3 +42,32 @@ describe('transformPath', () => {
expect(transformPath('///dev/v1/sth', '/dev')).toBe('v1/sth');
});
});

describe('isDomain', () => {
it('should return true for valid domain paths', () => {
expect(isDomain('/domain', EPathType.EPathTypeDir)).toBe(true);
expect(isDomain('/another-domain', EPathType.EPathTypeDir)).toBe(true);
});

it('should return false for non-directory paths', () => {
expect(isDomain('/domain', EPathType.EPathTypeColumnStore)).toBe(false);
expect(isDomain('/domain', undefined)).toBe(false);
});

it('should return false for paths without slash', () => {
expect(isDomain('domain', EPathType.EPathTypeDir)).toBe(false);
});

it('should return false for paths with multiple slashes', () => {
expect(isDomain('/domain/subdomain', EPathType.EPathTypeDir)).toBe(false);
expect(isDomain('/domain/', EPathType.EPathTypeDir)).toBe(false);
});

it('should return false for empty paths', () => {
expect(isDomain('', EPathType.EPathTypeDir)).toBe(false);
});

it('should return true for root path', () => {
expect(isDomain('/', EPathType.EPathTypeDir)).toBe(true);
});
});
12 changes: 12 additions & 0 deletions src/containers/Tenant/ObjectSummary/transformPath.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import {EPathType} from '../../../types/api/schema';

export function transformPath(path: string, dbName: string): string {
// Normalize the path and dbName by removing leading/trailing slashes
const normalizedPath = path.replace(/^\/+|\/+$/g, '');
Expand All @@ -6,6 +8,9 @@ export function transformPath(path: string, dbName: string): string {
if (!normalizedPath.startsWith(normalizedDbName)) {
return normalizedPath || '/';
}
if (normalizedPath === normalizedDbName) {
return `/${normalizedPath}`;
}

let result = normalizedPath.slice(normalizedDbName.length);

Expand All @@ -14,3 +19,10 @@ export function transformPath(path: string, dbName: string): string {

return result;
}

export function isDomain(path: string, type?: EPathType) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Some tests would be very great

Copy link
Contributor

@astandrik astandrik Sep 27, 2024

Choose a reason for hiding this comment

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

export function isDomain(path: string, type?: EPathType): boolean {
    if (type !== EPathType.EPathTypeDir) {
        return false;
    }

    return path.split('/').length === 2 && path.startsWith('/');
}

Copy link
Contributor

@astandrik astandrik Sep 27, 2024

Choose a reason for hiding this comment

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

Not sure if these tests return what expected
Double check them please

import { isDomain } from './path-utils';
import { EPathType } from './types';

describe('isDomain', () => {
    it('should return true for valid domain paths', () => {
        expect(isDomain('/domain', EPathType.EPathTypeDir)).toBe(true);
        expect(isDomain('/another-domain', EPathType.EPathTypeDir)).toBe(true);
    });

    it('should return false for non-directory paths', () => {
        expect(isDomain('/domain', EPathType.EPathTypeFile)).toBe(false);
        expect(isDomain('/domain', undefined)).toBe(false);
    });

    it('should return false for paths without slash', () => {
        expect(isDomain('domain', EPathType.EPathTypeDir)).toBe(false);
    });

    it('should return false for paths with multiple slashes', () => {
        expect(isDomain('/domain/subdomain', EPathType.EPathTypeDir)).toBe(false);
        expect(isDomain('/domain/', EPathType.EPathTypeDir)).toBe(false);
    });

    it('should return false for empty paths', () => {
        expect(isDomain('', EPathType.EPathTypeDir)).toBe(false);
    });

    it('should return true for root path', () => {
        expect(isDomain('/', EPathType.EPathTypeDir)).toBe(true);
    });
});

if (type !== EPathType.EPathTypeDir) {
return false;
}
return path.split('/').length === 2 && path.startsWith('/');
}
20 changes: 12 additions & 8 deletions src/containers/Tenant/Schema/SchemaTree/SchemaTree.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -110,14 +110,18 @@ export function SchemaTree(props: SchemaTreeProps) {
collapsed: false,
}}
fetchPath={fetchPath}
getActions={getActions(dispatch, {
setActivePath: onActivePathUpdate,
updateQueryExecutionSettings: (settings) =>
setQueryExecutionSettings({...querySettings, ...settings}),
showCreateDirectoryDialog: createDirectoryFeatureAvailable
? handleOpenCreateDirectoryDialog
: undefined,
})}
getActions={getActions(
dispatch,
{
setActivePath: onActivePathUpdate,
updateQueryExecutionSettings: (settings) =>
setQueryExecutionSettings({...querySettings, ...settings}),
showCreateDirectoryDialog: createDirectoryFeatureAvailable
? handleOpenCreateDirectoryDialog
: undefined,
},
rootPath,
)}
renderAdditionalNodeElements={getSchemaControls(dispatch, {
setActivePath: onActivePathUpdate,
})}
Expand Down
12 changes: 7 additions & 5 deletions src/containers/Tenant/utils/schemaActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {TENANT_PAGES_IDS, TENANT_QUERY_TABS_ID} from '../../../store/reducers/te
import {setQueryTab, setTenantPage} from '../../../store/reducers/tenant/tenant';
import type {QueryMode, QuerySettings} from '../../../types/store/query';
import createToast from '../../../utils/createToast';
import {transformPath} from '../ObjectSummary/transformPath';
import i18n from '../i18n';

import {
Expand Down Expand Up @@ -33,7 +34,7 @@ interface ActionsAdditionalEffects {
}

const bindActions = (
path: string,
{path, relativePath}: {path: string; relativePath: string},
dispatch: React.Dispatch<any>,
additionalEffects: ActionsAdditionalEffects,
) => {
Expand All @@ -45,7 +46,7 @@ const bindActions = (
updateQueryExecutionSettings({queryMode: mode});
}

dispatch(changeUserInput({input: tmpl(path)}));
dispatch(changeUserInput({input: tmpl(relativePath)}));
dispatch(setTenantPage(TENANT_PAGES_IDS.query));
dispatch(setQueryTab(TENANT_QUERY_TABS_ID.newQuery));
setActivePath(path);
Expand Down Expand Up @@ -75,7 +76,7 @@ const bindActions = (
dropView: inputQuery(dropViewTemplate, 'script'),
copyPath: () => {
try {
copy(path);
copy(relativePath);
createToast({
name: 'Copied',
title: i18n('actions.copied'),
Expand All @@ -95,9 +96,10 @@ const bindActions = (
type ActionsSet = ReturnType<Required<NavigationTreeProps>['getActions']>;

export const getActions =
(dispatch: React.Dispatch<any>, additionalEffects: ActionsAdditionalEffects) =>
(dispatch: React.Dispatch<any>, additionalEffects: ActionsAdditionalEffects, rootPath = '') =>
(path: string, type: NavigationTreeNodeType) => {
const actions = bindActions(path, dispatch, additionalEffects);
const relativePath = transformPath(path, rootPath);
const actions = bindActions({path, relativePath}, dispatch, additionalEffects);
const copyItem = {text: i18n('actions.copyPath'), action: actions.copyPath};

const DIR_SET: ActionsSet = [
Expand Down
Loading