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

chore: harmonize and clean up list views #25961

Merged
merged 34 commits into from
Dec 4, 2023
Merged
Show file tree
Hide file tree
Changes from 32 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
4d98415
chore(listview): harmonize column order and add owners
villebro Mar 1, 2023
65888f2
Merge branch 'master' into villebro/listview-reorder
villebro Nov 12, 2023
d384614
more cleanup
villebro Nov 12, 2023
9f60f26
delint and fix api test
villebro Nov 12, 2023
ef571e9
fix listview tests
villebro Nov 12, 2023
89d6bc7
Merge branch 'master' into villebro/listview-reorder
villebro Nov 17, 2023
35a3657
more fixing
villebro Nov 17, 2023
5eb4bfa
add test
villebro Nov 20, 2023
5230e39
use new component
villebro Nov 20, 2023
2c7ed41
lint + improve test
villebro Nov 20, 2023
f2ed0e7
fix cypress tests
villebro Nov 21, 2023
ec9b711
Merge branch 'master' into villebro/listview-reorder
villebro Nov 21, 2023
1c9a9cf
more cypress fixes
villebro Nov 21, 2023
59da29e
remove webpack-resolver
villebro Nov 21, 2023
4d21068
fix css test
villebro Nov 26, 2023
4e5ae02
change test
villebro Nov 27, 2023
668ea85
Merge branch 'master' into villebro/listview-reorder
villebro Nov 27, 2023
8fcbf26
fix assertion
villebro Nov 27, 2023
2c90bd8
Merge branch 'master' into villebro/listview-reorder
villebro Nov 28, 2023
5f5cbad
fix case
villebro Nov 28, 2023
8a56ffa
unify name title in modals
villebro Nov 29, 2023
76da075
renam dataset name
villebro Nov 30, 2023
e7ee40d
Merge branch 'master' into villebro/listview-reorder
villebro Nov 30, 2023
307e588
fix broken saved queries list
villebro Nov 30, 2023
9b0e240
fix chart changed on regression
villebro Dec 1, 2023
0df6975
change AuditInfo TS smell
villebro Dec 1, 2023
6160103
delint
villebro Dec 1, 2023
c890dcb
Merge branch 'master' into villebro/listview-reorder
villebro Dec 1, 2023
ca14ffe
add modified by filter
villebro Dec 1, 2023
ca5dc9d
fix missing database related columns
villebro Dec 1, 2023
1c989c7
fix cypress test
villebro Dec 2, 2023
8b055b4
fix RLS list RTL test
villebro Dec 2, 2023
9ffb6d2
address review comments
villebro Dec 4, 2023
0b51363
clean up remaining Search headers to Name
villebro Dec 4, 2023
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
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,9 @@ describe('Alert list view', () => {
cy.getBySel('sort-header').eq(2).contains('Name');
cy.getBySel('sort-header').eq(3).contains('Schedule');
cy.getBySel('sort-header').eq(4).contains('Notification method');
cy.getBySel('sort-header').eq(5).contains('Created by');
cy.getBySel('sort-header').eq(6).contains('Owners');
cy.getBySel('sort-header').eq(7).contains('Modified');
cy.getBySel('sort-header').eq(8).contains('Active');
cy.getBySel('sort-header').eq(5).contains('Owners');
cy.getBySel('sort-header').eq(6).contains('Last modified');
cy.getBySel('sort-header').eq(7).contains('Active');
// TODO Cypress won't recognize the Actions column
// cy.getBySel('sort-header').eq(9).contains('Actions');
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,9 @@ describe('Report list view', () => {
cy.getBySel('sort-header').eq(2).contains('Name');
cy.getBySel('sort-header').eq(3).contains('Schedule');
cy.getBySel('sort-header').eq(4).contains('Notification method');
cy.getBySel('sort-header').eq(5).contains('Created by');
cy.getBySel('sort-header').eq(6).contains('Owners');
cy.getBySel('sort-header').eq(7).contains('Modified');
cy.getBySel('sort-header').eq(8).contains('Active');
cy.getBySel('sort-header').eq(5).contains('Owners');
cy.getBySel('sort-header').eq(6).contains('Last modified');
cy.getBySel('sort-header').eq(7).contains('Active');
// TODO Cypress won't recognize the Actions column
// cy.getBySel('sort-header').eq(9).contains('Actions');
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,14 @@ describe('Charts filters', () => {
setFilter('Owner', 'admin user');
});

it('should allow filtering by "Created by" correctly', () => {
setFilter('Created by', 'alpha user');
setFilter('Created by', 'admin user');
it('should allow filtering by "Modified by" correctly', () => {
setFilter('Modified by', 'alpha user');
setFilter('Modified by', 'admin user');
});

it('should allow filtering by "Chart type" correctly', () => {
setFilter('Chart type', 'Area Chart (legacy)');
setFilter('Chart type', 'Bubble Chart');
it('should allow filtering by "Type" correctly', () => {
setFilter('Type', 'Area Chart (legacy)');
setFilter('Type', 'Bubble Chart');
});

it('should allow filtering by "Dataset" correctly', () => {
Expand All @@ -51,7 +51,7 @@ describe('Charts filters', () => {
});

it('should allow filtering by "Dashboards" correctly', () => {
setFilter('Dashboards', 'Unicode Test');
setFilter('Dashboards', 'Tabbed Dashboard');
setFilter('Dashboard', 'Unicode Test');
setFilter('Dashboard', 'Tabbed Dashboard');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -109,14 +109,12 @@ describe('Charts list', () => {

it('should load rows in list mode', () => {
cy.getBySel('listview-table').should('be.visible');
cy.getBySel('sort-header').eq(1).contains('Chart');
cy.getBySel('sort-header').eq(2).contains('Visualization type');
cy.getBySel('sort-header').eq(1).contains('Name');
cy.getBySel('sort-header').eq(2).contains('Type');
cy.getBySel('sort-header').eq(3).contains('Dataset');
// cy.getBySel('sort-header').eq(4).contains('Dashboards added to');
cy.getBySel('sort-header').eq(4).contains('Modified by');
cy.getBySel('sort-header').eq(4).contains('Owners');
cy.getBySel('sort-header').eq(5).contains('Last modified');
cy.getBySel('sort-header').eq(6).contains('Created by');
cy.getBySel('sort-header').eq(7).contains('Actions');
cy.getBySel('sort-header').eq(6).contains('Actions');
});

it('should sort correctly in list mode', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ describe('Dashboards filters', () => {
setFilter('Owner', 'admin user');
});

it('should allow filtering by "Created by" correctly', () => {
setFilter('Created by', 'alpha user');
setFilter('Created by', 'admin user');
it('should allow filtering by "Modified by" correctly', () => {
setFilter('Modified by', 'alpha user');
setFilter('Modified by', 'admin user');
});

it('should allow filtering by "Status" correctly', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,11 @@ describe('Dashboards list', () => {

it('should load rows in list mode', () => {
cy.getBySel('listview-table').should('be.visible');
cy.getBySel('sort-header').eq(1).contains('Title');
cy.getBySel('sort-header').eq(2).contains('Modified by');
cy.getBySel('sort-header').eq(3).contains('Status');
cy.getBySel('sort-header').eq(4).contains('Modified');
cy.getBySel('sort-header').eq(5).contains('Created by');
cy.getBySel('sort-header').eq(6).contains('Owners');
cy.getBySel('sort-header').eq(7).contains('Actions');
cy.getBySel('sort-header').eq(1).contains('Name');
cy.getBySel('sort-header').eq(2).contains('Status');
cy.getBySel('sort-header').eq(3).contains('Owners');
cy.getBySel('sort-header').eq(4).contains('Last modified');
cy.getBySel('sort-header').eq(5).contains('Actions');
});

it('should sort correctly in list mode', () => {
Expand Down
64 changes: 64 additions & 0 deletions superset-frontend/src/components/AuditInfo/AuditInfo.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
import React from 'react';
import { render, screen, waitFor } from '@testing-library/react';
import '@testing-library/jest-dom';
import userEvent from '@testing-library/user-event';

import { supersetTheme, ThemeProvider } from '@superset-ui/core';
import { AuditInfo, AuditInfoProps, AuditInfoType } from '.';

const TEST_DATE = '2023-11-20';
const USER = {
id: 1,
first_name: 'Foo',
last_name: 'Bar',
};

const Wrapper: React.FC<AuditInfoProps> = ({ type, user, date }) => (
villebro marked this conversation as resolved.
Show resolved Hide resolved
<ThemeProvider theme={supersetTheme}>
<AuditInfo type={type} user={user} date={date} />
</ThemeProvider>
);

test('should render a created tooltip when user is provided', async () => {
render(<Wrapper type={AuditInfoType.Created} user={USER} date={TEST_DATE} />);

const dateElement = screen.getByTestId('audit-info-date');
expect(dateElement).toBeInTheDocument();
expect(screen.getByText(TEST_DATE)).toBeInTheDocument();
userEvent.hover(dateElement);
expect(screen.queryByText('Created by: Foo Bar')).not.toBeInTheDocument();
const tooltip = await screen.findByRole('tooltip');
expect(tooltip).toBeInTheDocument();
expect(screen.getByText('Created by: Foo Bar')).toBeInTheDocument();
});

test('should render a modified tooltip when user is provided', async () => {
render(
<Wrapper type={AuditInfoType.Modified} user={USER} date={TEST_DATE} />,
);

const dateElement = screen.getByTestId('audit-info-date');
expect(dateElement).toBeInTheDocument();
expect(screen.getByText(TEST_DATE)).toBeInTheDocument();
expect(screen.queryByText('Modified by: Foo Bar')).not.toBeInTheDocument();
userEvent.hover(dateElement);
const tooltip = await screen.findByRole('tooltip');
expect(tooltip).toBeInTheDocument();
expect(screen.getByText('Modified by: Foo Bar')).toBeInTheDocument();
});

test('should render only the date if username is not provided', async () => {
render(<Wrapper type={AuditInfoType.Modified} date={TEST_DATE} />);

const dateElement = screen.getByTestId('audit-info-date');
expect(dateElement).toBeInTheDocument();
expect(screen.getByText(TEST_DATE)).toBeInTheDocument();
userEvent.hover(dateElement);
await waitFor(
() => {
const tooltip = screen.queryByRole('tooltip');
expect(tooltip).not.toBeInTheDocument();
},
{ timeout: 1000 },
);
});
39 changes: 39 additions & 0 deletions superset-frontend/src/components/AuditInfo/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import React from 'react';

import Owner from 'src/types/Owner';
import { Tooltip } from 'src/components/Tooltip';
import getOwnerName from 'src/utils/getOwnerName';
import { t } from '@superset-ui/core';

export const enum AuditInfoType {
Modified = 'Modified',
Created = 'Created',
}
villebro marked this conversation as resolved.
Show resolved Hide resolved

export type AuditInfoProps = {
type: AuditInfoType;
user?: Owner;
date: string;
};

export const AuditInfo = ({ type, user, date }: AuditInfoProps) => {
const dateSpan = (
<span className="no-wrap" data-test="audit-info-date">
{date}
</span>
);

if (user) {
const userName = getOwnerName(user);
const title =
type === AuditInfoType.Created
? t('Created by: %s', userName)
: t('Modified by: %s', userName);
return (
<Tooltip title={title} placement="bottom">
{dateSpan}
</Tooltip>
);
}
return dateSpan;
};
Original file line number Diff line number Diff line change
Expand Up @@ -1114,7 +1114,7 @@ class DatasourceEditor extends React.PureComponent {
<div css={{ width: 'calc(100% - 34px)', marginTop: -16 }}>
<Field
fieldKey="table_name"
label={t('Dataset name')}
label={t('Name')}
control={
<TextControl
controlId="table_name"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -681,7 +681,7 @@ const PropertiesModal = ({
</Row>
<Row gutter={16}>
<Col xs={24} md={12}>
<FormItem label={t('Title')} name="title">
<FormItem label={t('Name')} name="title">
<Input
data-test="dashboard-title-input"
type="text"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ const AnnotationModal: FunctionComponent<AnnotationModalProps> = ({
</StyledAnnotationTitle>
<AnnotationContainer>
<div className="control-label">
{t('Annotation name')}
{t('Name')}
<span className="required">*</span>
</div>
<input
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,9 @@ const CssTemplateModal: FunctionComponent<CssTemplateModalProps> = ({
const update_id = currentCssTemplate.id;
delete currentCssTemplate.id;
delete currentCssTemplate.created_by;
delete currentCssTemplate.changed_by;
delete currentCssTemplate.changed_on_delta_humanized;

updateResource(update_id, currentCssTemplate).then(response => {
if (!response) {
return;
Expand Down Expand Up @@ -235,7 +238,7 @@ const CssTemplateModal: FunctionComponent<CssTemplateModalProps> = ({
</StyledCssTemplateTitle>
<TemplateContainer>
<div className="control-label">
{t('CSS template name')}
{t('Name')}
<span className="required">*</span>
</div>
<input
Expand Down
11 changes: 4 additions & 7 deletions superset-frontend/src/features/cssTemplates/types.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import Owner from 'src/types/Owner';

/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
Expand All @@ -16,17 +18,12 @@
* specific language governing permissions and limitations
* under the License.
*/
type CreatedByUser = {
id: number;
first_name: string;
last_name: string;
};

export type TemplateObject = {
id?: number;
changed_on_delta_humanized?: string;
created_on?: string;
created_by?: CreatedByUser;
changed_by?: Owner;
created_by?: Owner;
css?: string;
template_name: string;
};
2 changes: 2 additions & 0 deletions superset-frontend/src/features/tags/TagModal.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,12 @@ test('renders correctly in edit mode', () => {
changed_on_delta_humanized: '',
created_on_delta_humanized: '',
created_by: {
id: 1,
first_name: 'joe',
last_name: 'smith',
},
changed_by: {
id: 2,
first_name: 'tom',
last_name: 'brown',
},
Expand Down
Loading
Loading