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

feat(datatrakWeb): RN-1362: Datatrak Web User question type #5815

Merged
merged 6 commits into from
Jul 31, 2024
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
2 changes: 1 addition & 1 deletion packages/admin-panel/src/routes/surveys/surveys.js
Original file line number Diff line number Diff line change
Expand Up @@ -541,7 +541,7 @@ const QUESTION_COLUMNS = [
fieldName: 'permissionGroup',
optionsEndpoint: 'permissionGroups',
optionLabelKey: 'name',
optionValueKey: 'name',
optionValueKey: 'id',
labelTooltip: 'Select the permission group the user list should be filtered by',
},
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { processConditionConfig } from './processConditionConfig';
import { processAutocompleteConfig } from './processAutocompleteConfig';
import { processEntityConfig } from './processEntityConfig';
import { processTaskConfig } from './processTaskConfig';
import { processUserConfig } from './processUserConfig';

const { CODE_GENERATOR, ARITHMETIC, CONDITION, AUTOCOMPLETE, ENTITY, PRIMARY_ENTITY, TASK, USER } =
ANSWER_TYPES;
Expand Down Expand Up @@ -91,7 +92,8 @@ export class ConfigImporter {
return { task: taskConfig };
}
case USER: {
return { user: config };
const userConfig = await processUserConfig(this.models, config);
return { user: userConfig };
}

default:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/**
* Tupaia
* Copyright (c) 2017 - 2024 Beyond Essential Systems Pty Ltd
*/

const translatePermissionGroup = async (config, models) => {
const { permissionGroup } = config;
const actualPermissionGroup = await models.permissionGroup.findOne({ name: permissionGroup });
if (!actualPermissionGroup) {
throw new Error(`Permission group ${permissionGroup} not found`);
}

return {
...config,
permissionGroup: actualPermissionGroup.id,
};
};

export const processUserConfig = async (models, config) => {
const translatedConfig = await translatePermissionGroup(config, models);
return translatedConfig;
};
3 changes: 3 additions & 0 deletions packages/datatrak-web-server/src/app/createApp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ import {
GenerateLoginTokenRoute,
LeaderboardRequest,
LeaderboardRoute,
PermissionGroupUsersRequest,
PermissionGroupUsersRoute,
ProjectRequest,
ProjectRoute,
ProjectsRequest,
Expand Down Expand Up @@ -88,6 +90,7 @@ export async function createApp() {
.get<TaskRequest>('tasks/:taskId', handleWith(TaskRoute))
.get<SingleSurveyResponseRequest>('surveyResponse/:id', handleWith(SingleSurveyResponseRoute))
.get<SurveyUsersRequest>('users/:surveyCode/:countryCode', handleWith(SurveyUsersRoute))
.get<PermissionGroupUsersRequest>('users/:countryCode', handleWith(PermissionGroupUsersRoute))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tossed up amending the SurveyUsersRoute to take a permissionGroupName parameter, or making this one more generic so that it could handle both cases, but I decided to create separate routes and move common logic to a util, because there would have been too much split logic to contend with and it would be hard to read later.

I also decided not to have permissionGroupName in the url params, but in the query instead, because permission group names have spaces

Copy link
Contributor

Choose a reason for hiding this comment

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

The reasoning sounds good to me. One thing though is that I think that it's not great using the permissionGroupName in the api request. It's a shame there is no code so I guess we really should use the permissionGroupId. I guess the reason that you used the name is that it is more convenient as it is in the User question config? Perhaps we could transform the User question config to save the permissionGroupId?

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, have made the name transform to be the id

// Post Routes
.post<CreateTaskRequest>('tasks', handleWith(CreateTaskRoute))
.put<EditTaskRequest>('tasks/:taskId', handleWith(EditTaskRoute))
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/**
* Tupaia
* Copyright (c) 2017 - 2024 Beyond Essential Systems Pty Ltd
*/

/**
* Tupaia
* Copyright (c) 2017 - 2024 Beyond Essential Systems Pty Ltd
*/

import { Request } from 'express';
import { Route } from '@tupaia/server-boilerplate';
import { DatatrakWebUsersRequest } from '@tupaia/types';
import { getFilteredUsers } from '../utils';

export type PermissionGroupUsersRequest = Request<
DatatrakWebUsersRequest.Params,
DatatrakWebUsersRequest.ResBody,
DatatrakWebUsersRequest.ReqBody,
DatatrakWebUsersRequest.ReqQuery
>;

export class PermissionGroupUsersRoute extends Route<PermissionGroupUsersRequest> {
public async buildResponse() {
const { models, params, query } = this.req;
const { countryCode } = params;

const { searchTerm, permissionGroupId } = query;

if (!permissionGroupId) {
throw new Error('Permission group id is required');
}

// get the permission group
const permissionGroup = await models.permissionGroup.findById(permissionGroupId);

if (!permissionGroup) {
throw new Error(`Permission group with id '${permissionGroupId}' not found`);
}

return getFilteredUsers(models, countryCode, permissionGroup, searchTerm);
}
}
70 changes: 7 additions & 63 deletions packages/datatrak-web-server/src/routes/SurveyUsersRoute.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,33 +5,16 @@

import { Request } from 'express';
import { Route } from '@tupaia/server-boilerplate';
import { DatatrakWebSurveyUsersRequest, EntityType } from '@tupaia/types';
import { QUERY_CONJUNCTIONS } from '@tupaia/database';

const USERS_EXCLUDED_FROM_LIST = [
'edmofro@gmail.com', // Edwin
'kahlinda.mahoney@gmail.com', // Kahlinda
'lparish1980@gmail.com', // Lewis
'sus.lake@gmail.com', // Susie
'michaelnunan@hotmail.com', // Michael
'vanbeekandrew@gmail.com', // Andrew
'gerardckelly@gmail.com', // Gerry K
'geoffreyfisher@hotmail.com', // Geoff F
'josh@sussol.net', // mSupply API Client
'unicef.laos.edu@gmail.com', // Laos Schools Data Collector
'tamanu-server@tupaia.org', // Tamanu Server
'public@tupaia.org', // Public User
];
import { DatatrakWebUsersRequest } from '@tupaia/types';
import { getFilteredUsers } from '../utils';

export type SurveyUsersRequest = Request<
DatatrakWebSurveyUsersRequest.Params,
DatatrakWebSurveyUsersRequest.ResBody,
DatatrakWebSurveyUsersRequest.ReqBody,
DatatrakWebSurveyUsersRequest.ReqQuery
DatatrakWebUsersRequest.Params,
DatatrakWebUsersRequest.ResBody,
DatatrakWebUsersRequest.ReqBody,
DatatrakWebUsersRequest.ReqQuery
>;

const DEFAULT_PAGE_SIZE = 100;

export class SurveyUsersRoute extends Route<SurveyUsersRequest> {
public async buildResponse() {
const { models, params, query } = this.req;
Expand All @@ -58,45 +41,6 @@ export class SurveyUsersRoute extends Route<SurveyUsersRequest> {
throw new Error(`Permission group with id ${permissionGroupId} not found`);
}

// get the ancestors of the permission group
const permissionGroupWithAncestors = await permissionGroup.getAncestors();

const entity = await models.entity.findOne({
country_code: countryCode,
type: EntityType.country,
});

// get the user entity permissions for the permission group and its ancestors
const userEntityPermissions = await models.userEntityPermission.find({
permission_group_id: permissionGroupWithAncestors.map(p => p.id),
entity_id: entity.id,
});

const userIds = userEntityPermissions.map(uep => uep.user_id);

const usersFilter = {
id: userIds,
email: { comparator: 'not in', comparisonValue: USERS_EXCLUDED_FROM_LIST },
[QUERY_CONJUNCTIONS.RAW]: {
// exclude E2E users and any internal users
sql: `(email NOT LIKE '%tupaia.org' AND email NOT LIKE '%beyondessential.com.au' AND email NOT LIKE '%@bes.au')`,
},
} as Record<string, any>;

if (searchTerm) {
usersFilter.full_name = { comparator: 'ilike', comparisonValue: `${searchTerm}%` };
}

const users = await models.user.find(usersFilter, {
sort: ['full_name ASC'],
limit: DEFAULT_PAGE_SIZE,
});
const userData = users.map(user => ({
id: user.id,
name: user.full_name,
}));

// only return the id and name of the users
return userData;
return getFilteredUsers(models, countryCode, permissionGroup, searchTerm);
}
}
4 changes: 4 additions & 0 deletions packages/datatrak-web-server/src/routes/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,7 @@ export { TaskRequest, TaskRoute } from './TaskRoute';
export { SurveyUsersRequest, SurveyUsersRoute } from './SurveyUsersRoute';
export { CreateTaskRequest, CreateTaskRoute } from './CreateTaskRoute';
export { EditTaskRequest, EditTaskRoute } from './EditTaskRoute';
export {
PermissionGroupUsersRequest,
PermissionGroupUsersRoute,
} from './PermissionGroupUsersRoute';
71 changes: 71 additions & 0 deletions packages/datatrak-web-server/src/utils/getFilteredUsers.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/**
* Tupaia
* Copyright (c) 2017 - 2024 Beyond Essential Systems Pty Ltd
*/

import { QUERY_CONJUNCTIONS } from '@tupaia/database';
import { Country, EntityType } from '@tupaia/types';
import { DatatrakWebServerModelRegistry } from '../types';
import { PermissionGroupRecord } from '@tupaia/server-boilerplate';

const USERS_EXCLUDED_FROM_LIST = [
'edmofro@gmail.com', // Edwin
'kahlinda.mahoney@gmail.com', // Kahlinda
'lparish1980@gmail.com', // Lewis
'sus.lake@gmail.com', // Susie
'michaelnunan@hotmail.com', // Michael
'vanbeekandrew@gmail.com', // Andrew
'gerardckelly@gmail.com', // Gerry K
'geoffreyfisher@hotmail.com', // Geoff F
'josh@sussol.net', // mSupply API Client
'unicef.laos.edu@gmail.com', // Laos Schools Data Collector
'tamanu-server@tupaia.org', // Tamanu Server
'public@tupaia.org', // Public User
];

const DEFAULT_PAGE_SIZE = 100;

export const getFilteredUsers = async (
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice idea splitting this out and composing two endpoints

models: DatatrakWebServerModelRegistry,
countryCode: Country['code'],
permissionGroup: PermissionGroupRecord,
searchTerm?: string,
) => {
// get the ancestors of the permission group
const permissionGroupWithAncestors = await permissionGroup.getAncestors();
const entity = await models.entity.findOne({
country_code: countryCode,
type: EntityType.country,
});

// get the user entity permissions for the permission group and its ancestors
const userEntityPermissions = await models.userEntityPermission.find({
permission_group_id: permissionGroupWithAncestors.map(p => p.id),
entity_id: entity.id,
});

const userIds = userEntityPermissions.map(uep => uep.user_id);

const usersFilter = {
id: userIds,
email: { comparator: 'not in', comparisonValue: USERS_EXCLUDED_FROM_LIST },
[QUERY_CONJUNCTIONS.RAW]: {
// exclude E2E users and any internal users
sql: `(email NOT LIKE '%tupaia.org' AND email NOT LIKE '%beyondessential.com.au' AND email NOT LIKE '%@bes.au')`,
},
} as Record<string, any>;

if (searchTerm) {
usersFilter.full_name = { comparator: 'ilike', comparisonValue: `${searchTerm}%` };
}

const users = await models.user.find(usersFilter, {
sort: ['full_name ASC'],
limit: DEFAULT_PAGE_SIZE,
});

return users.map(user => ({
id: user.id,
name: user.full_name,
}));
};
1 change: 1 addition & 0 deletions packages/datatrak-web-server/src/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ export { sortSearchResults } from './sortSearchResults';
export { addRecentEntities } from './addRecentEntities';
export * from './formatTaskResponse';
export * from './formatTaskChanges';
export { getFilteredUsers } from './getFilteredUsers';
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
/*
* Tupaia
* Copyright (c) 2017 - 2024 Beyond Essential Systems Pty Ltd
*/
import React, { Ref } from 'react';
import { screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { rest } from 'msw';
import { setupServer } from 'msw/node';
import { renderComponent } from '../../helpers/render';
import { UserQuestion } from '../../../features/Questions';

jest.mock('../../../features/Survey/SurveyContext/SurveyContext.tsx', () => ({
useSurveyForm: () => ({
countryCode: 'DL',
}),
}));

jest.mock('../../../api/queries/useUser', () => {
return {
useUser: jest.fn().mockReturnValue({}),
};
});

const users = [
{
name: 'Teddy Bear',
id: '1',
},
{
name: 'Grizzly Bear',
id: '2',
},
{
name: 'Panda Bear',
id: '3',
},
{
name: 'Koala Bear',
id: '4',
},
{
name: 'Polar Bear',
id: '5',
},
];
const server = setupServer(
rest.get('*/v1/users/DL', (_, res, ctx) => {
return res(ctx.status(200), ctx.json(users));
}),
);

beforeAll(() => server.listen());
afterEach(() => server.resetHandlers());
afterAll(() => server.close());

describe('User Question', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

horay for front end tests

const onChange = jest.fn();
const props = {
id: 'theId',
label: 'Who should the task be assigned to?',
name: 'assignee',
config: {
user: {
permissionGroup: 'Public',
},
},
controllerProps: {
value: null,
onChange,
ref: {
current: {},
} as Ref<HTMLInputElement>,
},
};

it('renders the user question component without crashing', () => {
renderComponent(<UserQuestion {...props} />);
});

it('renders all the options', async () => {
renderComponent(<UserQuestion {...props} />);

const openButton = screen.getByTitle('Open');
openButton.click();

const displayOptions = await screen.findAllByRole('option');
expect(displayOptions.length).toBe(users.length);
displayOptions.forEach((option, index) => {
const text = users[index].name;
expect(option).toHaveTextContent(text);
});
});

it('Calls the onChange method with the option value', async () => {
renderComponent(<UserQuestion {...props} />);

const openButton = screen.getByTitle('Open');
openButton.click();

const displayOption = await screen.findByRole('option', { name: users[0].name });
userEvent.click(displayOption);

expect(onChange).toHaveBeenCalledWith(users[0].id);
});
});
1 change: 1 addition & 0 deletions packages/datatrak-web/src/api/queries/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,4 @@ export { useEntities } from './useEntities';
export { useTasks } from './useTasks';
export { useTask } from './useTask';
export { useSurveyUsers } from './useSurveyUsers';
export { usePermissionGroupUsers } from './usePermissionGroupUsers';
Loading