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

Users search fix #2847

Merged
merged 7 commits into from
Aug 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Changed HTTP Endpoint to Email for inbound email integrations
([#2816](https://github.com/grafana/oncall/issues/2816))
- Enable inbound email feature flag by default by @vadimkerr ([#2846](https://github.com/grafana/oncall/pull/2846))
- Fixed initial search on Users page ([#2842](https://github.com/grafana/oncall/issues/2842))

## v1.3.25 (2023-08-18)

Expand Down
14 changes: 14 additions & 0 deletions grafana-plugin/e2e-tests/users/usersActions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,20 @@ test.describe('Users screen actions', () => {
await _testButtons(editorRolePage.page, 'button.edit-other-profile-button:not([disabled])');
});

test('Search updates the table view', async ({ adminRolePage }) => {
const { page } = adminRolePage;
await goToOnCallPage(page, 'users');

const searchInput = page.locator(`[data-testid="search-users"]`);

await searchInput.fill('oncall');
await page.waitForTimeout(5000);

const result = page.locator(`[data-testid="users-username"]`);

expect(await result.count()).toBe(1);
});

/*
* Helper methods
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,11 @@ interface UsersFiltersProps {
value: any;
onChange: (filters: any) => void;
className?: string;
isLoading?: boolean;
}

const UsersFilters = (props: UsersFiltersProps) => {
const { value = { searchTerm: '' }, onChange, className } = props;
const { value = { searchTerm: '' }, onChange, className, isLoading } = props;

const onSearchTermChangeCallback = useCallback(
(e: ChangeEvent<HTMLInputElement>) => {
Expand All @@ -31,11 +32,13 @@ const UsersFilters = (props: UsersFiltersProps) => {
return (
<div className={cx('root', className)}>
<Input
loading={isLoading}
prefix={<Icon name="search" />}
className={cx('search', 'control')}
placeholder="Search users..."
value={value.searchTerm}
onChange={onSearchTermChangeCallback}
data-testid="search-users"
/>
</div>
);
Expand Down
20 changes: 3 additions & 17 deletions grafana-plugin/src/pages/schedule/Schedule.tsx
Original file line number Diff line number Diff line change
@@ -1,15 +1,6 @@
import React from 'react';

import {
Button,
HorizontalGroup,
VerticalGroup,
IconButton,
ToolbarButton,
Icon,
Modal,
LoadingPlaceholder,
} from '@grafana/ui';
import { Button, HorizontalGroup, VerticalGroup, IconButton, ToolbarButton, Icon, Modal } from '@grafana/ui';
import cn from 'classnames/bind';
import dayjs from 'dayjs';
import { observer } from 'mobx-react';
Expand Down Expand Up @@ -48,9 +39,7 @@ import styles from './Schedule.module.css';

const cx = cn.bind(styles);

interface SchedulePageProps extends PageProps, WithStoreProps, RouteComponentProps<{ id: string }> {
basicDataLoaded: boolean;
}
interface SchedulePageProps extends PageProps, WithStoreProps, RouteComponentProps<{ id: string }> {}

interface SchedulePageState extends PageBaseState {
startMoment: dayjs.Dayjs;
Expand Down Expand Up @@ -123,7 +112,6 @@ class SchedulePage extends React.Component<SchedulePageProps, SchedulePageState>
match: {
params: { id: scheduleId },
},
basicDataLoaded,
} = this.props;

const {
Expand Down Expand Up @@ -160,9 +148,7 @@ class SchedulePage extends React.Component<SchedulePageProps, SchedulePageState>
!!shiftIdToShowOverridesForm ||
shiftIdToShowRotationForm;

return !basicDataLoaded ? (
<LoadingPlaceholder text="Loading..." />
) : (
return (
<PageErrorHandlingWrapper errorData={errorData} objectName="schedule" pageName="schedules">
{() => (
<>
Expand Down
60 changes: 35 additions & 25 deletions grafana-plugin/src/pages/users/Users.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,28 +47,34 @@ interface UsersState extends PageBaseState {
searchTerm: string;
};
initialUsersLoaded: boolean;
queuedUpdateUsers: boolean;
}

@observer
class Users extends React.Component<UsersProps, UsersState> {
state: UsersState = {
page: 1,
isWrongTeam: false,
userPkToEdit: undefined,
usersFilters: {
searchTerm: '',
},

errorData: initErrorDataState(),
initialUsersLoaded: false,
};
constructor(props: UsersProps) {
super(props);

async componentDidMount() {
const {
query: { p },
} = this.props;
this.setState({ page: p ? Number(p) : 1 }, this.updateUsers);
} = props;

this.state = {
page: p ? Number(p) : 1,
isWrongTeam: false,
userPkToEdit: undefined,
usersFilters: {
searchTerm: '',
},

errorData: initErrorDataState(),
initialUsersLoaded: false,
queuedUpdateUsers: false,
};
}

async componentDidMount() {
this.updateUsers();
this.parseParams();
}

Expand All @@ -84,14 +90,15 @@ class Users extends React.Component<UsersProps, UsersState> {
LocationHelper.update({ p: page }, 'partial');
await userStore.updateItems(usersFilters, page);

this.setState({ initialUsersLoaded: true });
const { queuedUpdateUsers } = this.state;
this.setState({ initialUsersLoaded: true, queuedUpdateUsers: false }, () => {
if (queuedUpdateUsers) {
this.updateUsers();
}
});
};

componentDidUpdate(prevProps: UsersProps) {
if (!this.state.initialUsersLoaded) {
this.updateUsers();
}

if (prevProps.match.params.id !== this.props.match.params.id) {
this.parseParams();
}
Expand Down Expand Up @@ -176,14 +183,15 @@ class Users extends React.Component<UsersProps, UsersState> {
const {
store: { userStore },
} = this.props;
const { usersFilters, page, initialUsersLoaded, userPkToEdit } = this.state;

const { usersFilters, page, initialUsersLoaded, userPkToEdit, queuedUpdateUsers } = this.state;

const { count, results } = userStore.getSearchResult();
const columns = this.getTableColumns();

const handleClear = () =>
this.setState({ usersFilters: { searchTerm: '' } }, () => {
this.debouncedUpdateUsers();
this.updateUsers();
});

return (
Expand All @@ -194,6 +202,7 @@ class Users extends React.Component<UsersProps, UsersState> {
<UsersFilters
className={cx('users-filters')}
value={usersFilters}
isLoading={queuedUpdateUsers}
onChange={this.handleUsersFiltersChange}
/>
<Button variant="secondary" icon="times" onClick={handleClear} className={cx('searchIntegrationClear')}>
Expand Down Expand Up @@ -425,6 +434,11 @@ class Users extends React.Component<UsersProps, UsersState> {

handleUsersFiltersChange = (usersFilters: any) => {
this.setState({ usersFilters, page: 1 }, () => {
if (!this.state.initialUsersLoaded) {
// queue delayed users update
return this.setState({ queuedUpdateUsers: true });
}

this.debouncedUpdateUsers();
});
};
Expand All @@ -435,10 +449,6 @@ class Users extends React.Component<UsersProps, UsersState> {

history.push(`${PLUGIN_ROOT}/users`);
};

handleUserUpdate = () => {
this.updateUsers();
};
}

export default withRouter(withMobXProviderContext(Users));
Loading