Skip to content

Commit 15e8f04

Browse files
liuyinglaoYinglao Liu
andauthored
fix/artifact-copy-button-not-working (#171)
* fix: artifact copy button not working - Added position: relative and z-index: 1 to Modal container to fix z-index stacking - Added notification feedback when artifact code is copied to clipboard - Added mock artifacts data for testing - Added NotificationsProvider to test wrapper for notification support - Added unit test for copy button functionality * fix test * fix eslint --------- Co-authored-by: Yinglao Liu <yinglaol@netflix.com>
1 parent ed4c2d3 commit 15e8f04

File tree

5 files changed

+179
-24
lines changed

5 files changed

+179
-24
lines changed

src/components/Modal/index.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,8 @@ const ModalClickHandler = styled.div`
7474

7575
const ModalContainer = styled.div`
7676
${PopoverStyles}
77+
position: relative;
78+
z-index: 1;
7779
padding: 0.75rem 1rem 1rem 1rem;
7880
width: 80%;
7981
max-width: 80rem;

src/mocks/browser.js

Lines changed: 85 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -399,6 +399,88 @@ export const worker = setupWorker(
399399
decorators: [],
400400
extensions: {},
401401
}),
402+
stantardGetEndpoint('api/flows/:flowid/runs/:runid/steps/:stepid/tasks/:taskid/artifacts', [
403+
{
404+
flow_id: 'BasicFlow',
405+
run_number: 1,
406+
run_id: null,
407+
step_name: 'start',
408+
task_id: 520362,
409+
task_name: null,
410+
name: 'aggregated_table',
411+
location: 's3://metaflow-bucket/BasicFlow/1/start/520362/aggregated_table',
412+
ds_type: 's3',
413+
sha: 'abc123def456',
414+
type: 'string',
415+
content_type: 'text/plain',
416+
content: 'user.aggregated_data_20251209_180857_19',
417+
attempt_id: 0,
418+
user_name: 'santeri@outerbounds.co',
419+
ts_epoch: 1739390575000,
420+
tags: ['attempt_id:0'],
421+
system_tags: ['user:santeri@outerbounds.co'],
422+
},
423+
{
424+
flow_id: 'BasicFlow',
425+
run_number: 1,
426+
run_id: null,
427+
step_name: 'start',
428+
task_id: 520362,
429+
task_name: null,
430+
name: 'environment',
431+
location: 's3://metaflow-bucket/BasicFlow/1/start/520362/environment',
432+
ds_type: 's3',
433+
sha: 'def789ghi012',
434+
type: 'string',
435+
content_type: 'text/plain',
436+
content: 'prod',
437+
attempt_id: 0,
438+
user_name: 'santeri@outerbounds.co',
439+
ts_epoch: 1739390575100,
440+
tags: ['attempt_id:0'],
441+
system_tags: ['user:santeri@outerbounds.co'],
442+
},
443+
{
444+
flow_id: 'BasicFlow',
445+
run_number: 1,
446+
run_id: null,
447+
step_name: 'start',
448+
task_id: 520362,
449+
task_name: null,
450+
name: 'sample_rate',
451+
location: 's3://metaflow-bucket/BasicFlow/1/start/520362/sample_rate',
452+
ds_type: 's3',
453+
sha: 'ghi345jkl678',
454+
type: 'float',
455+
content_type: 'application/json',
456+
content: '0.3',
457+
attempt_id: 0,
458+
user_name: 'santeri@outerbounds.co',
459+
ts_epoch: 1739390575200,
460+
tags: ['attempt_id:0'],
461+
system_tags: ['user:santeri@outerbounds.co'],
462+
},
463+
{
464+
flow_id: 'BasicFlow',
465+
run_number: 1,
466+
run_id: null,
467+
step_name: 'start',
468+
task_id: 520362,
469+
task_name: null,
470+
name: 'log_level',
471+
location: 'local://data/BasicFlow/1/start/520362/log_level',
472+
ds_type: 'local',
473+
sha: 'jkl901mno234',
474+
type: 'string',
475+
content_type: 'text/plain',
476+
content: 'INFO',
477+
attempt_id: 0,
478+
user_name: 'santeri@outerbounds.co',
479+
ts_epoch: 1739390575300,
480+
tags: ['attempt_id:0'],
481+
system_tags: ['user:santeri@outerbounds.co'],
482+
},
483+
]),
402484
stantardGetEndpoint('api/flows/:flowid/runs/:runid/steps/:stepid/tasks/:taskid/metadata', [
403485
{
404486
id: 10926826,
@@ -480,7 +562,9 @@ export const worker = setupWorker(
480562
},
481563
]),
482564
rawGetEndpoint('api/plugin', []),
483-
rawGetEndpoint('api/features', {}),
565+
rawGetEndpoint('api/features', {
566+
FEATURE_ARTIFACT_TABLE: true,
567+
}),
484568
stantardGetEndpoint('api/links', []),
485569
stantardGetEndpoint('api/notifications', []),
486570
stantardGetEndpoint('api/flows/autocomplete', ['BasicFlow', 'NewFlow']),

src/pages/Task/__tests__/ArtifactTable.test.cypress.tsx

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,4 +74,40 @@ describe('ArtifactTable', () => {
7474
gid('modal-container');
7575
gid('modal-content').contains("Task('LogTestFlow/968832/loglines/33632798', attempt=0)['FirstArtifact'].data");
7676
});
77+
78+
it('Should copy artifact code when copy button is clicked', () => {
79+
// Stub document.execCommand to verify copy was called (used by copy-to-clipboard library)
80+
cy.document().then((doc) => {
81+
cy.stub(doc, 'execCommand').as('execCommand').returns(true);
82+
});
83+
84+
mount(
85+
<TestWrapper>
86+
<ArtifactTable
87+
artifacts={[createArtifact({ name: 'FirstArtifact', content: 'ImportantStuff' })]}
88+
onOpenContentClick={cy.stub()}
89+
/>
90+
</TestWrapper>,
91+
);
92+
93+
// Open the modal
94+
gid('select-field').eq(0).click();
95+
cy.get('button').contains('Python').click();
96+
gid('modal-container');
97+
98+
// Verify the code snippet is displayed in the modal
99+
gid('modal-content').contains("Task('LogTestFlow/968832/loglines/33632798', attempt=0)['FirstArtifact'].data");
100+
101+
// Click the copy button in the modal header
102+
gid('modal-container').find('button').click();
103+
104+
// Verify execCommand('copy') was called (copy-to-clipboard library uses this)
105+
cy.get('@execCommand').should('have.been.calledWith', 'copy');
106+
107+
// Verify notification appears confirming copy was successful
108+
cy.contains('Artifact content copied to clipboard').should('be.visible');
109+
110+
// Modal should close after clicking copy
111+
gid('modal-container').should('not.exist');
112+
});
77113
});

src/pages/Task/components/ArtifactTable.tsx

Lines changed: 32 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import GenericError from '@components/GenericError';
99
import Icon from '@components/Icon';
1010
import InformationRow from '@components/InformationRow';
1111
import Modal from '@components/Modal';
12+
import { NotificationType, useNotifications } from '@components/Notifications';
1213
import PropertyTable, { PropertyTableColumns } from '@components/PropertyTable';
1314
import { ItemRow } from '@components/Structure';
1415
import { valueToRenderableType } from '@components/TitledRow';
@@ -156,28 +157,37 @@ const LocationRenderTitle = styled.div`
156157
// Highlight link renderer
157158
//
158159

159-
const LinkPopup: React.FC<{ link: PopupLink | null; onClose: () => void }> = ({ link, onClose }) => (
160-
<Modal
161-
show={!!link}
162-
onClose={onClose}
163-
title={`${link?.artifact.name} - ${link?.variant}`}
164-
actionbar={
165-
<Button
166-
iconOnly
167-
onClick={() => {
168-
if (link) {
169-
copy(makeArtifactCode(link));
170-
}
171-
onClose();
172-
}}
173-
>
174-
<Icon name="copy" />
175-
</Button>
176-
}
177-
>
178-
<CodeBlock>{link && makeArtifactCode(link)}</CodeBlock>
179-
</Modal>
180-
);
160+
const LinkPopup: React.FC<{ link: PopupLink | null; onClose: () => void }> = ({ link, onClose }) => {
161+
const { addNotification } = useNotifications();
162+
const { t } = useTranslation();
163+
164+
return (
165+
<Modal
166+
show={!!link}
167+
onClose={onClose}
168+
title={`${link?.artifact.name} - ${link?.variant}`}
169+
actionbar={
170+
<Button
171+
iconOnly
172+
onClick={() => {
173+
if (link) {
174+
copy(makeArtifactCode(link));
175+
addNotification({
176+
type: NotificationType.Info,
177+
message: t('task.artifact-copied'),
178+
});
179+
}
180+
onClose();
181+
}}
182+
>
183+
<Icon name="copy" />
184+
</Button>
185+
}
186+
>
187+
<CodeBlock>{link && makeArtifactCode(link)}</CodeBlock>
188+
</Modal>
189+
);
190+
};
181191

182192
// Keeping code snippet in object for now since we might to add separate snippets for different situations.
183193
const codeByType = {

src/utils/testing.tsx

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,26 @@
11
import React, { ReactNode } from 'react';
22
import { MemoryRouter, Route } from 'react-router-dom';
33
import { QueryParamProvider } from 'use-query-params';
4+
import { Notifications, NotificationsProvider } from '@components/Notifications';
5+
import { PluginsContext } from '@components/Plugins/PluginManager';
46
import GlobalStyle from '../GlobalStyle';
57
import '../theme/font/roboto.css';
68
import './i18n';
79

10+
// Mock PluginsContext for testing - provides stub functions for plugin communication
11+
// eslint-disable-next-line @typescript-eslint/no-empty-function
12+
const noop = () => {};
13+
const mockPluginsContextValue = {
14+
plugins: [],
15+
getPluginsBySlot: () => [],
16+
register: noop,
17+
subscribeToDatastore: noop,
18+
unsubscribeFromDatastore: noop,
19+
subscribeToEvent: noop,
20+
unsubscribeFromEvent: noop,
21+
callEvent: noop,
22+
};
23+
824
/**
925
* Wrapper for testing component that depends on theming and routing. Also accepts route as param if
1026
* component depends on certain routes.
@@ -14,7 +30,14 @@ const TestWrapper: React.FC<{ children: ReactNode; route?: string }> = ({ childr
1430
<>
1531
<GlobalStyle />
1632
<MemoryRouter initialEntries={[route]}>
17-
<QueryParamProvider ReactRouterRoute={Route}>{children}</QueryParamProvider>
33+
<QueryParamProvider ReactRouterRoute={Route}>
34+
<PluginsContext.Provider value={mockPluginsContextValue as never}>
35+
<NotificationsProvider>
36+
{children}
37+
<Notifications />
38+
</NotificationsProvider>
39+
</PluginsContext.Provider>
40+
</QueryParamProvider>
1841
</MemoryRouter>
1942
</>
2043
);

0 commit comments

Comments
 (0)