Skip to content

Commit

Permalink
task/TUI-307 -- bug fixes tapis training (#308)
Browse files Browse the repository at this point in the history
* Use undefined fields when adding new app args

* remove undefined fields

* Job Type

* Consistent headings for job steps

* Validate execution options with batch type jobs

* Stringify non string values in description lists, allow useDetails for systems to specify request parameters

* Refactor DescriptionListValue

* Remove console statement

* Refactor JSONDisplay

* Refactor tabs, system detail tabs with json

* Handle object keys that are Javascript Sets

* Added permission checking to file toolbar

* fix unit test

* linting
  • Loading branch information
jchuahtacc authored May 16, 2022
1 parent ac65e20 commit 8060c24
Show file tree
Hide file tree
Showing 28 changed files with 375 additions and 136 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { FileStat, FileOperation } from 'tapis-ui/components/files';
import { useTapisConfig } from 'tapis-hooks';
import { QueryWrapper } from 'tapis-ui/_wrappers';
import { Files } from '@tapis/tapis-typescript';
import { Tabs } from 'tapis-app/_components';
import { Tabs } from 'tapis-ui/_common';
import styles from './PermissionsModal.module.scss';
import React from 'react';

Expand Down
12 changes: 11 additions & 1 deletion src/tapis-app/Files/_components/Toolbar/Toolbar.test.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import { act } from '@testing-library/react';
import renderComponent from 'utils/testing';
import Toolbar from './Toolbar';
import { Files } from '@tapis/tapis-typescript';
import { fileInfo } from 'fixtures/files.fixtures';
import { useFilesSelect } from 'tapis-app/Files/_components/FilesContext';
import { useDownload } from 'tapis-hooks/files';
import { useDownload, usePermissions } from 'tapis-hooks/files';
import RenameModal from 'tapis-app/Files/_components/Toolbar/RenameModal';

jest.mock('tapis-hooks/files');
Expand All @@ -16,6 +17,15 @@ describe('Toolbar', () => {
downloadAsync: jest.fn(),
download: jest.fn(),
});
(usePermissions as jest.Mock).mockReturnValue({
data: {
result: {
permission: Files.FilePermissionPermissionEnum.Modify,
},
},
isLoading: false,
error: null,
});
});
it('enables rename buttons', async () => {
(useFilesSelect as jest.Mock).mockReturnValue({
Expand Down
30 changes: 23 additions & 7 deletions src/tapis-app/Files/_components/Toolbar/Toolbar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,11 @@ import DeleteModal from './DeleteModal';
import TransferModal from './TransferModal';
import { useLocation } from 'react-router-dom';
import { useFilesSelect } from '../FilesContext';
import { useDownload, DownloadStreamParams } from 'tapis-hooks/files';
import {
useDownload,
DownloadStreamParams,
usePermissions,
} from 'tapis-hooks/files';
import { useNotifications } from 'tapis-app/_components/Notifications';

type ToolbarButtonProps = {
Expand Down Expand Up @@ -59,6 +63,9 @@ const Toolbar: React.FC = () => {
const { download } = useDownload();
const { add } = useNotifications();

const { data } = usePermissions({ systemId, path: currentPath });
const permission = data?.result?.permission;

const onDownload = useCallback(() => {
selectedFiles.forEach((file) => {
const params: DownloadStreamParams = {
Expand Down Expand Up @@ -99,14 +106,20 @@ const Toolbar: React.FC = () => {
<ToolbarButton
text="Rename"
icon="rename"
disabled={selectedFiles.length !== 1}
disabled={
selectedFiles.length !== 1 ||
permission !== Files.FilePermissionPermissionEnum.Modify
}
onClick={() => setModal('rename')}
aria-label="Rename"
/>
<ToolbarButton
text="Move"
icon="move"
disabled={selectedFiles.length === 0}
disabled={
selectedFiles.length === 0 ||
permission !== Files.FilePermissionPermissionEnum.Modify
}
onClick={() => setModal('move')}
aria-label="Move"
/>
Expand All @@ -121,7 +134,7 @@ const Toolbar: React.FC = () => {
<ToolbarButton
text="Permissions"
icon="gear"
disabled={selectedFiles.length !== 1}
disabled={selectedFiles.length !== 1 || permission !== Files.FilePermissionPermissionEnum.Modify}
onClick={() => setModal('permissions')}
/>
*/}
Expand All @@ -141,7 +154,7 @@ const Toolbar: React.FC = () => {
<ToolbarButton
text="Upload"
icon="upload"
disabled={false}
disabled={permission !== Files.FilePermissionPermissionEnum.Modify}
onClick={() => {
setModal('upload');
}}
Expand All @@ -150,14 +163,17 @@ const Toolbar: React.FC = () => {
<ToolbarButton
text="Folder"
icon="add"
disabled={false}
disabled={permission !== Files.FilePermissionPermissionEnum.Modify}
onClick={() => setModal('createdir')}
aria-label="Add"
/>
<ToolbarButton
text="Delete"
icon="trash"
disabled={selectedFiles.length === 0}
disabled={
selectedFiles.length === 0 ||
permission !== Files.FilePermissionPermissionEnum.Modify
}
onClick={() => setModal('delete')}
aria-label="Delete"
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { useLocation } from 'react-router';
import { Files } from '@tapis/tapis-typescript';
import styles from './TransferModal.module.scss';
import { useFilesSelect } from '../../FilesContext';
import { Tabs } from 'tapis-app/_components';
import { Tabs } from 'tapis-ui/_common';
import {
TransferListing,
TransferDetails,
Expand Down
1 change: 0 additions & 1 deletion src/tapis-app/_components/index.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
export { default as Sidebar } from './Sidebar';
export { default as Tabs } from './Tabs';
8 changes: 5 additions & 3 deletions src/tapis-hooks/systems/useDetails.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import { useQuery } from 'react-query';
import { useQuery, QueryObserverOptions } from 'react-query';
import { details } from 'tapis-api/systems';
import { Systems } from '@tapis/tapis-typescript';
import { useTapisConfig } from 'tapis-hooks';
import QueryKeys from './queryKeys';

const useDetails = (systemId: string) => {
const useDetails = (
params: Systems.GetSystemRequest,
options: QueryObserverOptions<Systems.RespSystem, Error> = {}
) => {
const { accessToken, basePath } = useTapisConfig();
const params: Systems.GetSystemRequest = { systemId };
const result = useQuery<Systems.RespSystem, Error>(
[QueryKeys.details, params, accessToken],
// Default to no token. This will generate a 403 when calling the list function
Expand Down
72 changes: 55 additions & 17 deletions src/tapis-ui/_common/DescriptionList/DescriptionList.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,36 +18,74 @@ export const DENSITY_CLASS_MAP = {
export const DEFAULT_DENSITY = 'default';
export const DENSITIES = ['', ...Object.keys(DENSITY_CLASS_MAP)];

const DescriptionListArray = ({ value }) => {
if (value.length === 0) {
return (
<dd className={styles.value}>
<i>None</i>
</dd>
);
}
return (
<dl>
{value.map((val, index) => (
<div key={uuidv4()} className={styles['array-entry']}>
<dt className={styles.key}>
<i>{index}</i>
</dt>
<dd className={styles.value} data-testid="value">
<DescriptionListValue value={val} />
</dd>
</div>
))}
</dl>
);
};

const DescriptionListValue = ({ value }) => {
if (value === undefined) {
return <i>Undefined</i>;
}
if (Array.isArray(value)) {
return <DescriptionListArray value={value} />;
}
if (value instanceof Set) {
return <DescriptionListArray value={Array.from(value)} />;
}
if (typeof value === 'object') {
return <DescriptionList data={value} />;
}
if (typeof value === 'string') {
return <>{value}</>;
}
return <>{JSON.stringify(value)}</>;
};

const DescriptionList = ({ className, data, density, direction }) => {
const modifierClasses = [];
modifierClasses.push(DENSITY_CLASS_MAP[density || DEFAULT_DENSITY]);
modifierClasses.push(DIRECTION_CLASS_MAP[direction || DEFAULT_DIRECTION]);
const containerStyleNames = ['container', ...modifierClasses]
.map((name) => styles[name])
.join(' ');

const entries = Object.entries(data);
if (entries.length === 0) {
return (
<div>
<i>Empty object</i>
</div>
);
}
return (
<dl className={`${className} ${containerStyleNames}`} data-testid="list">
{Object.entries(data).map(([key, value]) => (
{entries.map(([key, value]) => (
<React.Fragment key={key}>
<dt className={styles.key} data-testid="key">
{key}
</dt>
{Array.isArray(value) ? (
value.map((val) => (
<dd className={styles.value} data-testid="value" key={uuidv4()}>
{typeof val === 'object' ? <DescriptionList data={val} /> : val}
</dd>
))
) : (
<dd className={styles.value} data-testid="value">
{typeof value === 'object' ? (
<DescriptionList data={value} />
) : (
value
)}
</dd>
)}
<dd className={styles.value} data-testid="value">
<DescriptionListValue value={value} />
</dd>
</React.Fragment>
))}
</dl>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
@import '../../styles/tools/mixins.scss';

.array-entry {
display: flex;
}

.container.is-horz {
margin-bottom: 0; /* overwrite Bootstrap's `_reboot.scss` */
& dd {
Expand Down Expand Up @@ -52,7 +56,7 @@
padding-left: 0;
}
.is-vert.is-wide > .value {
padding-left: 2.5rem;
padding-left: 1rem;
} /* 40px Firefox default margin */

/* Truncate specific edge cases */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,6 @@ describe('Description List', () => {
const keys = await findAllByTestId('key');
const values = await findAllByTestId('value');
expect(keys.length).toEqual(1)
expect(values.length).toEqual(4)
expect(values.length).toEqual(5)
});
});
92 changes: 92 additions & 0 deletions src/tapis-ui/_common/JSONDisplay/JSONDisplay.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
import { useMemo, useState, useCallback } from 'react';
import { Input, FormGroup, Label } from 'reactstrap';
import { CopyButton } from 'tapis-ui/_common';
import styles from './JSONDisplay.module.scss';

const simplifyObject = (obj: any) => {
const result = JSON.parse(JSON.stringify(obj));
Object.entries(result).forEach(([key, value]) => {
if (Array.isArray(value)) {
if ((value as Array<any>).length === 0) {
delete result[key];
}
return;
}
if (typeof value === 'object') {
const simplifiedValue = simplifyObject(value);
if (Object.entries(simplifiedValue).length === 0) {
delete result[key];
} else {
result[key] = simplifiedValue;
}
return;
}
if (value === undefined) {
delete result[key];
}
});
return result;
};

const convertSets = (obj: any): any => {
if (obj === undefined) {
return undefined;
}
if (Array.isArray(obj)) {
return (obj as Array<any>).map((value) => convertSets(value));
}
if (obj instanceof Set) {
return Array.from(obj).map((value) => convertSets(value));
}
if (typeof obj === 'object') {
const result: any = {};
Object.entries(obj).forEach(([key, value]) => {
result[key] = convertSets(value);
});
return result;
}
return JSON.parse(JSON.stringify(obj));
};

type JSONDisplayProps = {
json: any;
className?: string;
};

const JSONDisplay: React.FC<JSONDisplayProps> = ({ json, className }) => {
const [simplified, setSimplified] = useState(false);
const onChange = useCallback(() => {
setSimplified(!simplified);
}, [setSimplified, simplified]);
const jsonString = useMemo(
() =>
JSON.stringify(
simplified ? simplifyObject(convertSets(json)) : convertSets(json),
null,
2
),
[json, simplified]
);
return (
<div className={className}>
<div className={styles.controls}>
<FormGroup check>
<Label check size="sm" className={`form-field__label`}>
<Input type="checkbox" onChange={onChange} />
Simplified
</Label>
</FormGroup>
<CopyButton value={jsonString} />
</div>
<Input
type="textarea"
value={jsonString}
className={styles.json}
rows="20"
disabled={true}
/>
</div>
);
};

export default JSONDisplay;
3 changes: 3 additions & 0 deletions src/tapis-ui/_common/JSONDisplay/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import JSONDisplay from './JSONDisplay';

export default JSONDisplay;
25 changes: 25 additions & 0 deletions src/tapis-ui/_common/Tabs/Tabs.module.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
.tab {
border-top-left-radius: 3px;
border-top-right-radius: 3px;
border-top: 1px dotted var(--global-color-primary--dark);
border-left: 1px dotted var(--global-color-primary--dark);
border-bottom: 1px dotted var(--global-color-primary-dark);
background-color: var(--global-color-primary--normal);
a {
color: var(--global-color-primary--xx-dark);
height: 2.5em;
font-size: 0.8em;
}
:hover {
cursor: pointer;
}
margin-right: 2px;
}

.active {
background-color: var(--global-color-primary--xx-light);
}

.pane {
padding: 0.5em 0.5em 0.5em 0.5em;
}
Loading

0 comments on commit 8060c24

Please sign in to comment.