Skip to content

Commit

Permalink
chore: harmonize and clean up list views (apache#25961)
Browse files Browse the repository at this point in the history
  • Loading branch information
villebro authored and sfirke committed Mar 22, 2024
1 parent 5599ec3 commit 76a6143
Show file tree
Hide file tree
Showing 47 changed files with 783 additions and 665 deletions.
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
42 changes: 42 additions & 0 deletions superset-frontend/src/components/AuditInfo/ModifiedInfo.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import React from 'react';
import { render, screen, waitFor } from 'spec/helpers/testing-library';
import '@testing-library/jest-dom';
import userEvent from '@testing-library/user-event';

import { ModifiedInfo } from '.';

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

test('should render a tooltip when user is provided', async () => {
render(<ModifiedInfo 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(<ModifiedInfo 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 },
);
});
30 changes: 30 additions & 0 deletions superset-frontend/src/components/AuditInfo/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
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 type ModifiedInfoProps = {
user?: Owner;
date: string;
};

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

if (user) {
const userName = getOwnerName(user);
const title = 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
74 changes: 37 additions & 37 deletions superset-frontend/src/pages/AlertReportList/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ import { isUserAdmin } from 'src/dashboard/util/permissionUtils';
import Owner from 'src/types/Owner';
import AlertReportModal from 'src/features/alerts/AlertReportModal';
import { AlertObject, AlertState } from 'src/features/alerts/types';
import { ModifiedInfo } from 'src/components/AuditInfo';
import { QueryObjectColumns } from 'src/views/CRUD/types';

const extensionsRegistry = getExtensionsRegistry();

Expand Down Expand Up @@ -303,18 +305,6 @@ function AlertList({
disableSortBy: true,
size: 'xl',
},
{
Cell: ({
row: {
original: { created_by },
},
}: any) =>
created_by ? `${created_by.first_name} ${created_by.last_name}` : '',
Header: t('Created by'),
id: 'created_by',
disableSortBy: true,
size: 'xl',
},
{
Cell: ({
row: {
Expand All @@ -329,10 +319,13 @@ function AlertList({
{
Cell: ({
row: {
original: { changed_on_delta_humanized: changedOn },
original: {
changed_on_delta_humanized: changedOn,
changed_by: changedBy,
},
},
}: any) => <span className="no-wrap">{changedOn}</span>,
Header: t('Modified'),
}: any) => <ModifiedInfo date={changedOn} user={changedBy} />,
Header: t('Last modified'),
accessor: 'changed_on_delta_humanized',
size: 'xl',
},
Expand Down Expand Up @@ -407,6 +400,10 @@ function AlertList({
disableSortBy: true,
size: 'xl',
},
{
accessor: QueryObjectColumns.changed_by,
hidden: true,
},
],
[canDelete, canEdit, isReportEnabled, toggleActive],
);
Expand Down Expand Up @@ -448,6 +445,13 @@ function AlertList({

const filters: Filters = useMemo(
() => [
{
Header: t('Name'),
key: 'search',
id: 'name',
input: 'search',
operator: FilterOperator.contains,
},
{
Header: t('Owner'),
key: 'owner',
Expand All @@ -465,23 +469,6 @@ function AlertList({
),
paginate: true,
},
{
Header: t('Created by'),
key: 'created_by',
id: 'created_by',
input: 'select',
operator: FilterOperator.relationOneMany,
unfilteredLabel: 'All',
fetchSelects: createFetchRelated(
'report',
'created_by',
createErrorHandler(errMsg =>
t('An error occurred while fetching created by values: %s', errMsg),
),
user,
),
paginate: true,
},
{
Header: t('Status'),
key: 'status',
Expand All @@ -504,11 +491,24 @@ function AlertList({
],
},
{
Header: t('Search'),
key: 'search',
id: 'name',
input: 'search',
operator: FilterOperator.contains,
Header: t('Modified by'),
key: 'changed_by',
id: 'changed_by',
input: 'select',
operator: FilterOperator.relationOneMany,
unfilteredLabel: t('All'),
fetchSelects: createFetchRelated(
'report',
'changed_by',
createErrorHandler(errMsg =>
t(
'An error occurred while fetching dataset datasource values: %s',
errMsg,
),
),
user,
),
paginate: true,
},
],
[],
Expand Down
Loading

0 comments on commit 76a6143

Please sign in to comment.