Skip to content

Commit

Permalink
ExternalTool:
Browse files Browse the repository at this point in the history
- changed to launch url with open() so uses new tab
- add alert() if opening URL fails (probably won't happen)
- added Test Tool to config
- fixed showing multiple <ExternalTool>
  • Loading branch information
pappde committed Jul 4, 2024
1 parent 88daf06 commit 68b7318
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 7 deletions.
11 changes: 10 additions & 1 deletion src/shared/components/ExternalTools/ExternalTool.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,12 @@ describe('ExternalTool Component', () => {
'http://example.com?study=${studyName}&-DataLength=${dataLength}';
const testStudyName = 'Test Study';
const navigatorClipboardOriginal = navigator.clipboard;

// we used to use window.location to navigate, then changed to window.open
const windowLocationOriginal = window.location;
const windowOpenOriginal = window.open;
const windowOpenMock = jest.fn();

const mockProps: IExternalToolProps = {
toolConfig: {
name: 'Test',
Expand Down Expand Up @@ -58,12 +63,16 @@ describe('ExternalTool Component', () => {
(window as any).location.href = url;
}),
};

// Mock window.open
(window as any).open = windowOpenMock;
});

afterEach(() => {
delete (window as any).groupComparisonPage;
Object.assign(navigator, navigatorClipboardOriginal);
window.location = windowLocationOriginal;
window.open = windowOpenOriginal;
});

it('renders correctly', () => {
Expand Down Expand Up @@ -119,6 +128,6 @@ describe('ExternalTool Component', () => {

component.handleLaunchReady(urlParametersLaunch);

expect(window.location.href).toBe(expectedUrl);
expect(windowOpenMock).toHaveBeenCalledWith(expectedUrl, '_blank');
});
});
7 changes: 6 additions & 1 deletion src/shared/components/ExternalTools/ExternalTool.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,12 @@ export class ExternalTool extends React.Component<IExternalToolProps, {}> {
);
});

window.location.href = url;
try {
window.open(url, '_blank');
} catch (e) {
// TECH: in practice, this never gets hit. If the URL protocol is not supported, then a blank window appears.
alert('Launching ' + this.config.name + ' failed: ' + e);
}
}

// pass data using Clipboard to the external tool
Expand Down
12 changes: 12 additions & 0 deletions src/shared/components/ExternalTools/ExternalToolConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,16 @@ export const ExternalToolConfigDefaults: ExternalToolConfig[] = [
url_format:
'avm://?importclipboard&-AutoMode=true&-ProjectNameHint=${studyName}&-ImportDataLength=${dataLength}',
},

/* TEST: uncomment to test
* ASNEEDED: we could add a localStorage prop to enable
{
id: 'test',
name: 'Test Tool',
tooltip: 'This button shows that the Test Tool is working',
iconImageSrc: require('../../../globalStyles/images/cbioportal_logo.png'),
url_format:
'https://eu.httpbin.org/anything?-StudyName=${studyName}&-ImportDataLength=${dataLength}',
},
*/
];
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export function isExternalToolAvailable(
toolConfig.id
);
if (resultCached !== undefined) {
// console.log('isExternalToolAvailable.Cache:' + resultCached);
// console.log(toolConfig.id + '.isExternalToolAvailable.Cache:' + resultCached);
return resultCached;
}
}
Expand All @@ -64,7 +64,7 @@ export function isExternalToolAvailable(

// compute and store the value
var resultComputed = computeIsExternalToolAvaialble(toolConfig);
// console.log('isExternalToolAvailable.Computed:' + resultComputed);
// console.log(toolConfig.id + '.isExternalToolAvailable.Computed:' + resultComputed);
try {
if (groupComparisonPage) {
groupComparisonPage.store.setIsExternalToolAvailable(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { getServerConfig } from '../../../config/config';
import { ExternalToolConfig } from '../ExternalTools/ExternalToolConfig';
import { ExternalTool } from '../ExternalTools/ExternalTool';
import { isExternalToolAvailable } from '../ExternalTools/ExternalToolConfigUtils';
import { isJSDocNonNullableType } from 'typescript';

export interface ICopyDownloadButtonsProps extends ICopyDownloadInputsProps {
copyButtonRef?: (el: HTMLButtonElement | null) => void;
Expand Down Expand Up @@ -83,6 +84,11 @@ export class CopyDownloadButtons extends React.Component<
}

buttonsExternalTools() {
// TECH: <If condition={this.props.showDownload}> was not working with returning multiple items in JSX.Element[], so moved the conditional here.
if (!this.props.showDownload) {
return null;
}

const config = getServerConfig().external_tools;
if (!config) {
return null;
Expand All @@ -91,8 +97,10 @@ export class CopyDownloadButtons extends React.Component<
return config
.filter((tool: ExternalToolConfig) => isExternalToolAvailable(tool))
.map((tool: ExternalToolConfig, index: number) => {
console.log('render:' + tool.id);
return (
<ExternalTool
key={tool.id}
toolConfig={tool}
baseTooltipProps={this.baseTooltipProps}
downloadData={this.props.downloadData}
Expand All @@ -110,9 +118,7 @@ export class CopyDownloadButtons extends React.Component<
<If condition={this.props.showDownload}>
{this.downloadButton()}
</If>
<If condition={this.props.showDownload}>
{this.buttonsExternalTools()}
</If>
{this.buttonsExternalTools()}
</ButtonGroup>
</span>
);
Expand Down

0 comments on commit 68b7318

Please sign in to comment.