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

Add filtering by team, is_currently_oncall and search on the user page #4575

Merged
merged 6 commits into from
Jul 8, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 1 addition & 1 deletion Tiltfile
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ docker_build_sub(
"localhost:63628/oncall/engine:dev",
context="./engine",
cache_from=["grafana/oncall:latest", "grafana/oncall:dev"],
ignore=["./test-results/", "./grafana-plugin/dist/", "./grafana-plugin/e2e-tests/"],
ignore=["./test-results/", "./grafana-plugin/dist/", "./grafana-plugin/e2e-tests/", "./grafana-plugin/node_modules/"],
child_context=".",
target="dev",
extra_cmds=["ADD ./grafana-plugin/src/plugin.json /etc/grafana-plugin/src/plugin.json"],
Expand Down
26 changes: 26 additions & 0 deletions engine/apps/api/views/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ class UserView(
"send_test_sms": [RBACPermission.Permissions.USER_SETTINGS_WRITE],
"export_token": [RBACPermission.Permissions.USER_SETTINGS_WRITE],
"upcoming_shifts": [RBACPermission.Permissions.USER_SETTINGS_READ],
"filters": [RBACPermission.Permissions.USER_SETTINGS_READ],
}

rbac_object_permissions = {
Expand Down Expand Up @@ -841,6 +842,31 @@ def export_token(self, request, pk) -> Response:
return Response(status=status.HTTP_204_NO_CONTENT)
return Response(status=status.HTTP_405_METHOD_NOT_ALLOWED)

@action(methods=["get"], detail=False)
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe we have openapi docs for the /users routes (code). do we need to add anything here to document this new endpoint?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point, added

def filters(self, request):
filter_name = request.query_params.get("search", None)
api_root = "/api/internal/v1/"

filter_options = [
{
"name": "team",
"type": "team_select",
"href": api_root + "teams/",
"global": True,
},
{
"name": "is_currently_oncall",
"type": "boolean",
"display_name": "Is currently oncall",
"default": "true",
},
]

if filter_name is not None:
filter_options = list(filter(lambda f: filter_name in f["name"], filter_options))
Comment on lines +885 to +886
Copy link
Member

Choose a reason for hiding this comment

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

does the frontend use this search functionality? if not, I think it might make sense to remove this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, it does

Copy link
Member

Choose a reason for hiding this comment

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

I can't find any usages in the frontend code 🤔 Am I missing something?


return Response(filter_options)


def handle_phone_notificator_failed(exc: BaseFailed) -> Response:
if exc.graceful_msg:
Expand Down
5 changes: 2 additions & 3 deletions grafana-plugin/src/models/user/user.helpers.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,8 @@ export class UserHelper {
* NOTE: if is_currently_oncall=all the backend will not paginate the results, it will send back an array of ALL users
*/
static async search(f: any = { searchTerm: '' }, page = 1) {
const filters = typeof f === 'string' ? { searchTerm: f } : f; // for GSelect compatibility
const { searchTerm: search, ...restFilters } = filters;
return (await onCallApi().GET('/users/', { params: { query: { search, page, ...restFilters } } })).data;
const filters = typeof f === 'string' ? { search: f } : f; // for GSelect compatibility
return (await onCallApi().GET('/users/', { params: { query: { ...filters, page } } })).data;
}

static getSearchResult(userStore: UserStore) {
Expand Down
7 changes: 6 additions & 1 deletion grafana-plugin/src/models/user/user.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import dayjs from 'dayjs';
import { get } from 'lodash-es';
import { action, computed, runInAction, makeAutoObservable } from 'mobx';

import { RemoteFiltersType } from 'containers/RemoteFilters/RemoteFilters.types';
import { ActionKey } from 'models/loader/action-keys';
import { NotificationPolicyType } from 'models/notification_policy/notification_policy';
import { makeRequest } from 'network/network';
Expand Down Expand Up @@ -36,7 +37,11 @@ export class UserStore {
this.rootStore = rootStore;
}

async fetchItems(f: any = { searchTerm: '' }, page = 1, invalidateFn?: () => boolean): Promise<any> {
async fetchItems(
f: RemoteFiltersType | string = { searchTerm: '', type: undefined, used: undefined },
page = 1,
invalidateFn?: () => boolean
): Promise<any> {
const response = await UserHelper.search(f, page);

if (invalidateFn && invalidateFn()) {
Expand Down
4 changes: 4 additions & 0 deletions grafana-plugin/src/pages/users/Users.styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ import { GrafanaTheme2 } from '@grafana/data';

export const getUsersStyles = (theme: GrafanaTheme2) => {
return {
filters: css`
margin-bottom: 20px;
`,

usersTtitle: css`
display: flex;
align-items: center;
Expand Down
79 changes: 40 additions & 39 deletions grafana-plugin/src/pages/users/Users.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ import {
import { PluginLink } from 'components/PluginLink/PluginLink';
import { Text } from 'components/Text/Text';
import { TooltipBadge } from 'components/TooltipBadge/TooltipBadge';
import { UsersFilters } from 'components/UsersFilters/UsersFilters';
import { RemoteFilters } from 'containers/RemoteFilters/RemoteFilters';
import { RemoteFiltersType } from 'containers/RemoteFilters/RemoteFilters.types';
import { UserSettings } from 'containers/UserSettings/UserSettings';
import { WithPermissionControlTooltip } from 'containers/WithPermissionControl/WithPermissionControlTooltip';
import { UserHelper } from 'models/user/user.helpers';
Expand All @@ -44,9 +45,8 @@ const REQUIRED_PERMISSION_TO_VIEW_USERS = UserActions.UserSettingsWrite;
interface UsersState extends PageBaseState {
isWrongTeam: boolean;
userPkToEdit?: ApiSchemas['User']['pk'] | 'new';
usersFilters?: {
searchTerm: string;
};

filters: RemoteFiltersType;
}

@observer
Expand All @@ -62,9 +62,7 @@ class Users extends React.Component<UsersProps, UsersState> {
this.state = {
isWrongTeam: false,
userPkToEdit: undefined,
usersFilters: {
searchTerm: '',
},
filters: { searchTerm: '', type: undefined, used: undefined, mine: undefined },

errorData: initErrorDataState(),
};
Expand All @@ -80,7 +78,7 @@ class Users extends React.Component<UsersProps, UsersState> {

updateUsers = debounce(async (invalidateFn?: () => boolean) => {
const { store } = this.props;
const { usersFilters } = this.state;
const { filters } = this.state;
const { userStore, filtersStore } = store;
const page = filtersStore.currentTablePageNum[PAGE.Users];

Expand All @@ -89,7 +87,7 @@ class Users extends React.Component<UsersProps, UsersState> {
}

LocationHelper.update({ p: page }, 'partial');
await userStore.fetchItems(usersFilters, page, invalidateFn);
await userStore.fetchItems(filters, page, invalidateFn);

this.forceUpdate();
}, DEBOUNCE_MS);
Expand Down Expand Up @@ -184,38 +182,20 @@ class Users extends React.Component<UsersProps, UsersState> {
renderContentIfAuthorized(authorizedToViewUsers: boolean) {
const {
store: { userStore, filtersStore },
theme,
} = this.props;

const { usersFilters, userPkToEdit } = this.state;
const { userPkToEdit } = this.state;

const page = filtersStore.currentTablePageNum[PAGE.Users];

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

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

return (
<>
{authorizedToViewUsers ? (
<>
<div className={styles.userFiltersContainer} data-testid="users-filters">
<UsersFilters
className={styles.usersFilters}
value={usersFilters}
isLoading={results === undefined}
onChange={this.handleUsersFiltersChange}
/>
<Button variant="secondary" icon="times" onClick={handleClear}>
Clear filters
</Button>
</div>

{this.renderFilters()}
<GTable
data-testid="users-table"
emptyText={results ? 'No users found' : 'Loading...'}
Expand Down Expand Up @@ -250,6 +230,37 @@ class Users extends React.Component<UsersProps, UsersState> {
);
}

renderFilters() {
const { query, store, theme } = this.props;
const styles = getUsersStyles(theme);

return (
<div className={styles.filters}>
<RemoteFilters
query={query}
page={PAGE.Users}
grafanaTeamStore={store.grafanaTeamStore}
onChange={this.handleFiltersChange}
/>
</div>
);
}

handleFiltersChange = (filters: RemoteFiltersType, _isOnMount: boolean) => {
const { filtersStore } = this.props.store;
const currentTablePage = filtersStore.currentTablePageNum[PAGE.Users];

LocationHelper.update({ p: currentTablePage }, 'partial');

this.setState({ filters }, () => {
this.updateUsers();
});
};

applyFilters = () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems this method is unused, can we remove it? Not related to your changes but there's another unused method in this component - renderContacts, we could remove it too

this.updateUsers();
};

renderTitle = (user: ApiSchemas['User']) => {
const {
store: { userStore },
Expand Down Expand Up @@ -442,16 +453,6 @@ class Users extends React.Component<UsersProps, UsersState> {
this.updateUsers();
};

handleUsersFiltersChange = (usersFilters: any, invalidateFn: () => boolean) => {
const { filtersStore } = this.props.store;

filtersStore.currentTablePageNum[PAGE.Users] = 1;

this.setState({ usersFilters }, () => {
this.updateUsers(invalidateFn);
});
};

handleHideUserSettings = () => {
const { history } = this.props;
this.setState({ userPkToEdit: undefined });
Expand Down
Loading