Skip to content

Regression: Data in the "New Users" section is delayed in 1 day #22751

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.

Sign up for GitHub

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

Merged
matheusbsilva137 merged 9 commits into from
Jul 27, 2021
Merged

Regression: Data in the "New Users" section is delayed in 1 day #22751

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
8fd5ac1
Fix 1 day delay and update nivo version
matheusbsilva137 Jul 23, 2021
9edfef2
Merge branch 'develop' into regression/new-users-ed
matheusbsilva137 Jul 23, 2021
a073bbc
Merge branch 'develop' into regression/new-users-ed
tassoevan Jul 26, 2021
519bd22
Revert tooltip style changes
matheusbsilva137 Jul 26, 2021
ba28375
Add tooltip style
matheusbsilva137 Jul 26, 2021
25a2f4b
Merge branch 'develop' into regression/new-users-ed
tassoevan Jul 26, 2021
ffc8102
Add type definitions for GenericTable
tassoevan Jul 26, 2021
de99fd8
Merge branch 'develop' into regression/new-users-ed
matheusbsilva137 Jul 27, 2021
2f937db
Merge branch 'develop' into regression/new-users-ed
matheusbsilva137 Jul 27, 2021
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
65 changes: 52 additions & 13 deletions client/components/GenericTable/index.js → .../components/GenericTable/GenericTable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,34 +1,67 @@
import { Box, Pagination, Table, Tile } from '@rocket.chat/fuselage';
import { useDebouncedValue } from '@rocket.chat/fuselage-hooks';
import React, { useState, useEffect, useCallback, forwardRef } from 'react';
import React, {
useState,
useEffect,
useCallback,
forwardRef,
ReactNode,
ReactElement,
Key,
RefAttributes,
} from 'react';
import flattenChildren from 'react-keyed-flatten-children';

import { useTranslation } from '../../contexts/TranslationContext';
import ScrollableContentWrapper from '../ScrollableContentWrapper';
import HeaderCell from './HeaderCell';
import LoadingRow from './LoadingRow';

const GenericTable = (
const defaultParamsValue = { text: '', current: 0, itemsPerPage: 25 } as const;
const defaultSetParamsValue = (): void => undefined;

type Params = { text?: string; current?: number; itemsPerPage?: 25 | 50 | 100 };

type GenericTableProps<
FilterProps extends { onChange?: (params: Params) => void },
ResultProps extends { _id?: Key },
> = {
fixed?: boolean;
header?: ReactNode;
params?: Params;
setParams?: (params: Params) => void;
children?: (props: ResultProps, key: number) => ReactElement;
renderFilter?: (props: FilterProps) => ReactElement;
renderRow?: (props: ResultProps) => ReactElement;
results?: ResultProps[];
total?: number;
pagination?: boolean;
} & FilterProps;

const GenericTable: {
<FilterProps extends { onChange?: (params: Params) => void }, ResultProps extends { _id?: Key }>(
props: GenericTableProps<FilterProps, ResultProps> & RefAttributes<HTMLElement>,
): ReactElement | null;
} = forwardRef(function GenericTable(
{
children,
fixed = true,
header,
params: paramsDefault = '',
params: paramsDefault = defaultParamsValue,
setParams = defaultSetParamsValue,
renderFilter,
renderRow: RenderRow,
results,
setParams = () => {},
total,
pagination = true,
...props
},
ref,
) => {
) {
const t = useTranslation();

const [filter, setFilter] = useState(paramsDefault);

const [itemsPerPage, setItemsPerPage] = useState(25);
const [itemsPerPage, setItemsPerPage] = useState<25 | 50 | 100>(25);

const [current, setCurrent] = useState(0);

Expand All @@ -40,7 +73,13 @@ const GenericTable = (

const Loading = useCallback(() => {
const headerCells = flattenChildren(header);
return Array.from({ length: 10 }, (_, i) => <LoadingRow key={i} cols={headerCells.length} />);
return (
<>
{Array.from({ length: 10 }, (_, i) => (
<LoadingRow key={i} cols={headerCells.length} />
))}
</>
);
}, [header]);

const showingResultsLabel = useCallback(
Expand All @@ -53,7 +92,9 @@ const GenericTable = (

return (
<>
{typeof renderFilter === 'function' ? renderFilter({ onChange: setFilter, ...props }) : null}
{typeof renderFilter === 'function'
? renderFilter({ ...props, onChange: setFilter } as any) // TODO: ugh
: null}
{results && !results.length ? (
<Tile fontScale='p1' elevation='0' color='info' textAlign='center'>
{t('No_data_found')}
Expand Down Expand Up @@ -98,8 +139,6 @@ const GenericTable = (
)}
</>
);
};

export default Object.assign(forwardRef(GenericTable), {
HeaderCell,
});

export default GenericTable;
6 changes: 6 additions & 0 deletions client/components/GenericTable/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import GenericTable from './GenericTable';
import HeaderCell from './HeaderCell';

export default Object.assign(GenericTable, {

@d-gubert d-gubert Jul 27, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious as to why you used Object.assign here 🤓 Is it just a shortcut to writing GenericTable.HeaderCell = HeaderCell; in the line above? Or is there another reason?

@tassoevan tassoevan Jul 27, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh boy, you are the right guy to ask me this one. GenericTable has a function type (kind of (props: any) => ReactElement) defined at './GenericTable'. It doesn't provide any mechanism to extend it while changing the concrete value: GenericTable.HeaderCell = HeaderCell ; would trigger an error for me because GenericTable's type is not an extension of { HeaderCell: (props: any) => ReactElement; }. I've noticed that Object.assign() intersects the types of its arguments, so we get { (props: any): ReactElement; HeaderCell: (props: any) => ReactElement; } at the return type.

@d-gubert d-gubert Jul 27, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it!

Does this generate a side-effect? I mean, if someone imported from the ./GenericTable file directly, the GenericTable exported there would also have the HeaderCell object due to Object.assign?

Of course, that wouldn't be a problem; TS wouldn't consider the original object to contain HeaderCell and I guess even the usual console.log would not print it there, it could only be confusing if you tried to see GenericTable's keys with Object.keys or other similar functions. Just asking, again, out of curiosity 😛

HeaderCell,
});
4 changes: 2 additions & 2 deletions client/views/teams/ChannelDesertionTable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,11 @@ const ChannelDesertionTable: FC<ChannelDesertionTableProps> = ({
fixed={false}
pagination={false}
>
{(room: IRoom, key: string): ReactElement => (
{(room, key): ReactElement => (
<ChannelRow
key={key}
formatDate={formatDate}
room={room}
key={key}
onChange={onChangeRoomSelection}
selected={!!selectedRooms[room._id]}
lastOwnerWarning={lastOwnerWarning}
Expand Down
13 changes: 6 additions & 7 deletions ee/app/engagement-dashboard/client/components/UsersTab/NewUsersSection.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ const NewUsersSection = ({ timezone }) => {
? moment().set({ hour: 0, minute: 0, second: 0, millisecond: 0 }).subtract(7, 'days').utc()
: moment().set({ hour: 0, minute: 0, second: 0, millisecond: 0 }).subtract(7, 'days'),
end: utc
? moment.utc().set({ hour: 0, minute: 0, second: 0, millisecond: 0 }).subtract(1, 'days').utc()
? moment().set({ hour: 0, minute: 0, second: 0, millisecond: 0 }).subtract(1, 'days').utc()
: moment().set({ hour: 0, minute: 0, second: 0, millisecond: 0 }).subtract(1, 'days'),
};

Expand All @@ -45,7 +45,7 @@ const NewUsersSection = ({ timezone }) => {
? moment().set({ hour: 0, minute: 0, second: 0, millisecond: 0 }).subtract(30, 'days').utc()
: moment().set({ hour: 0, minute: 0, second: 0, millisecond: 0 }).subtract(30, 'days'),
end: utc
? moment.utc().set({ hour: 0, minute: 0, second: 0, millisecond: 0 }).subtract(1, 'days').utc()
? moment().set({ hour: 0, minute: 0, second: 0, millisecond: 0 }).subtract(1, 'days').utc()
: moment().set({ hour: 0, minute: 0, second: 0, millisecond: 0 }).subtract(1, 'days'),
};

Expand Down Expand Up @@ -209,11 +209,10 @@ const NewUsersSection = ({ timezone }) => {
},
},
tooltip: {
container: {
backgroundColor: '#1F2329',
boxShadow: '0px 0px 12px rgba(47, 52, 61, 0.12), 0px 0px 2px rgba(47, 52, 61, 0.08)',
borderRadius: 2,
},
backgroundColor: '#1F2329',
boxShadow: '0px 0px 12px rgba(47, 52, 61, 0.12), 0px 0px 2px rgba(47, 52, 61, 0.08)',
borderRadius: 2,
padding: 4,
},
}}
tooltip={({ value, indexValue }) => <Box fontScale='p2' color='alternative'>
Expand Down
10 changes: 5 additions & 5 deletions ee/client/omnichannel/cannedResponses/CannedResponsesPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Button, Icon, ButtonGroup } from '@rocket.chat/fuselage';
import { useMutableCallback } from '@rocket.chat/fuselage-hooks';
import React, { FC, ReactNode, ReactElement, Dispatch, SetStateAction } from 'react';
import React, { FC, ReactElement, Dispatch, SetStateAction } from 'react';

import GenericTable from '../../../../client/components/GenericTable';
import NoResults from '../../../../client/components/GenericTable/NoResults';
Expand All @@ -11,11 +11,11 @@ import { useTranslation } from '../../../../client/contexts/TranslationContext';
export type CannedResponsesPageProps = {
data: any;
header: ReactElement[];
setParams: Dispatch<SetStateAction<{ current: number; itemsPerPage: number }>>;
params: { current: number; itemsPerPage: number };
setParams: Dispatch<SetStateAction<{ current?: number; itemsPerPage?: 25 | 50 | 100 }>>;
params: { current?: number; itemsPerPage?: 25 | 50 | 100 };
title: string;
renderRow: ReactNode | null;
renderFilter: FC;
renderFilter?: (props: any) => ReactElement;
renderRow?: (props: any) => ReactElement;
};

const CannedResponsesPage: FC<CannedResponsesPageProps> = ({
Expand Down
9 changes: 6 additions & 3 deletions ee/client/omnichannel/cannedResponses/CannedResponsesRoute.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Table, Box } from '@rocket.chat/fuselage';
import { useDebouncedValue, useMutableCallback } from '@rocket.chat/fuselage-hooks';
import React, { useMemo, useCallback, useState, FC, ReactNode, ReactElement } from 'react';
import React, { useMemo, useCallback, useState, FC, ReactElement } from 'react';

import GenericTable from '../../../../client/components/GenericTable';
import NotAuthorizedPage from '../../../../client/components/NotAuthorizedPage';
Expand Down Expand Up @@ -39,7 +39,10 @@ const CannedResponsesRoute: FC = () => {
const { sharing, createdBy, tags, text } = values as CannedResponseFilterValues;
const { handleSharing, handleCreatedBy, handleTags, handleText } = handlers;

const [params, setParams] = useState({ current: 0, itemsPerPage: 25 });
const [params, setParams] = useState<{ current?: number; itemsPerPage?: 25 | 50 | 100 }>({
current: 0,
itemsPerPage: 25,
});
const [sort, setSort] = useState<[string, 'asc' | 'desc' | undefined]>(['shortcut', 'asc']);

const debouncedParams = useDebouncedValue(params, 500);
Expand Down Expand Up @@ -141,7 +144,7 @@ const CannedResponsesRoute: FC = () => {
);

const renderRow = useCallback(
({ _id, shortcut, scope, createdBy, createdAt, tags = [] }): ReactNode => (
({ _id, shortcut, scope, createdBy, createdAt, tags = [] }): ReactElement => (
<Table.Row
key={_id}
tabIndex={0}
Expand Down
Loading