Skip to content

Commit

Permalink
Remove some unused references to the codes context.
Browse files Browse the repository at this point in the history
Update migration to combine queries into one pieces of sql.
  • Loading branch information
NickPhura committed Jul 22, 2024
1 parent 27737be commit a646855
Show file tree
Hide file tree
Showing 10 changed files with 196 additions and 361 deletions.
6 changes: 3 additions & 3 deletions api/src/paths/user/add.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { SystemUser } from '../../repositories/user-repository';
import { UserService } from '../../services/user-service';
import * as keycloakUtils from '../../utils/keycloak-utils';
import { getMockDBConnection, getRequestHandlerMocks } from '../../__mocks__/db';
import * as user from './add';
import { addSystemRoleUser } from './add';

chai.use(sinonChai);

Expand Down Expand Up @@ -56,7 +56,7 @@ describe('user', () => {

const adduserSystemRolesStub = sinon.stub(UserService.prototype, 'addUserSystemRoles');

const requestHandler = user.addSystemRoleUser();
const requestHandler = addSystemRoleUser();

await requestHandler(mockReq, mockRes, mockNext);

Expand Down Expand Up @@ -102,7 +102,7 @@ describe('user', () => {

const adduserSystemRolesStub = sinon.stub(UserService.prototype, 'addUserSystemRoles');

const requestHandler = user.addSystemRoleUser();
const requestHandler = addSystemRoleUser();

await requestHandler(mockReq, mockRes, mockNext);
expect(getUserByIdentifierStub).to.have.been.calledOnce;
Expand Down
10 changes: 7 additions & 3 deletions api/src/paths/user/add.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { Operation } from 'express-openapi';
import { SOURCE_SYSTEM, SYSTEM_IDENTITY_SOURCE } from '../../constants/database';
import { SYSTEM_ROLE } from '../../constants/roles';
import { getDBConnection, getServiceClientDBConnection } from '../../database/db';
import { HTTP409 } from '../../errors/http-error';
import { authorizeRequestHandler } from '../../request-handlers/security/authorization';
import { UserService } from '../../services/user-service';
import { getKeycloakSource } from '../../utils/keycloak-utils';
Expand Down Expand Up @@ -104,6 +105,9 @@ POST.apiDoc = {
403: {
$ref: '#/components/responses/403'
},
409: {
$ref: '#/components/responses/409'
},
500: {
$ref: '#/components/responses/500'
},
Expand Down Expand Up @@ -145,9 +149,9 @@ export function addSystemRoleUser(): RequestHandler {

// If user already exists, do nothing and return early
const user = await userService.getUserByIdentifier(userIdentifier, identitySource);

if (user) {
await connection.commit();
return res.status(400).send('That user has already been added.');
throw new HTTP409('Failed to add user. User with matching identifier already exists.');
}

// This function also verifies that the user exists, but we explicitly check above to return a useful error message.
Expand All @@ -173,7 +177,7 @@ export function addSystemRoleUser(): RequestHandler {

return res.status(200).send();
} catch (error) {
defaultLog.error({ label: 'getUser', message: 'error', error });
defaultLog.error({ label: 'addSystemRoleUser', message: 'error', error });
await connection.rollback();
throw error;
} finally {
Expand Down
2 changes: 1 addition & 1 deletion app/src/features/admin/users/AccessRequestList.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -235,8 +235,8 @@ describe('AccessRequestList', () => {
expect(mockUseApi.admin.approveAccessRequest).toHaveBeenCalledWith(1, {
displayName: 'test user',
email: 'email@email.com',
roleIds: [],
identitySource: 'IDIR',
roleIds: [],
userGuid: 'aaaa',
userIdentifier: 'testusername'
});
Expand Down
5 changes: 1 addition & 4 deletions app/src/features/admin/users/AccessRequestList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -241,10 +241,7 @@ const AccessRequestList = (props: IAccessRequestListProps) => {
onDeny={handleReviewDialogDeny}
onApprove={handleReviewDialogApprove}
component={{
initialValues: {
...ReviewAccessRequestFormInitialValues,
system_role: ''
},
initialValues: ReviewAccessRequestFormInitialValues,
validationSchema: ReviewAccessRequestFormYupSchema,
element: activeReview ? (
<ReviewAccessRequestForm
Expand Down
4 changes: 2 additions & 2 deletions app/src/features/admin/users/ActiveUsersList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -304,10 +304,10 @@ const ActiveUsersList = (props: IActiveUsersListProps) => {
} catch (error) {
const apiError = error as APIError;

if (apiError.status === 400) {
if (apiError.status === 409) {
dialogContext.setErrorDialog({
open: true,
dialogTitle: 'User already exists',
dialogTitle: 'Failed to create users',
dialogText: 'One of the users you added already exists.',
onClose: () => {
dialogContext.setErrorDialog({ open: false });
Expand Down
24 changes: 0 additions & 24 deletions app/src/pages/access/AccessRequestPage.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@ jest.mock('../../hooks/useBioHubApi');
const mockBiohubApi = useBiohubApi as jest.Mock;

const mockUseApi = {
codes: {
getAllCodeSets: jest.fn<Promise<object>, []>()
},
admin: {
createAdministrativeActivity: jest.fn()
}
Expand All @@ -42,7 +39,6 @@ const renderContainer = () => {
describe('AccessRequestPage', () => {
beforeEach(() => {
mockBiohubApi.mockImplementation(() => mockUseApi);
mockUseApi.codes.getAllCodeSets.mockClear();
mockUseApi.admin.createAdministrativeActivity.mockClear();
});

Expand All @@ -54,10 +50,6 @@ describe('AccessRequestPage', () => {
const history = createMemoryHistory();

it('should call the auth signoutRedirect function', async () => {
mockUseApi.codes.getAllCodeSets.mockResolvedValue({
system_roles: [{ id: 1, name: 'Creator' }]
});

const signoutRedirectStub = jest.fn();

const authState = getMockAuthState({
Expand Down Expand Up @@ -90,10 +82,6 @@ describe('AccessRequestPage', () => {
});

it.skip('processes a successful request submission', async () => {
mockUseApi.codes.getAllCodeSets.mockResolvedValue({
system_roles: [{ id: 1, name: 'Creator' }]
});

mockUseApi.admin.createAdministrativeActivity.mockResolvedValue({
id: 1
});
Expand All @@ -118,10 +106,6 @@ describe('AccessRequestPage', () => {
});

it.skip('takes the user to the request-submitted page immediately if they already have an access request', async () => {
mockUseApi.codes.getAllCodeSets.mockResolvedValue({
system_roles: [{ id: 1, name: 'Creator' }]
});

const authState = getMockAuthState({ base: SystemAdminAuthState });

render(
Expand All @@ -140,10 +124,6 @@ describe('AccessRequestPage', () => {
});

it.skip('shows error dialog with api error message when submission fails', async () => {
mockUseApi.codes.getAllCodeSets.mockResolvedValue({
system_roles: [{ id: 1, name: 'Creator' }]
});

mockUseApi.admin.createAdministrativeActivity = jest.fn(() => Promise.reject(new Error('API Error is Here')));

const { getByText, getAllByRole, getByRole, queryByText } = renderContainer();
Expand Down Expand Up @@ -172,10 +152,6 @@ describe('AccessRequestPage', () => {
});

it.skip('shows error dialog with default error message when response from createAdministrativeActivity is invalid', async () => {
mockUseApi.codes.getAllCodeSets.mockResolvedValue({
system_roles: [{ id: 1, name: 'Creator' }]
});

mockUseApi.admin.createAdministrativeActivity.mockResolvedValue({
id: null
});
Expand Down
4 changes: 0 additions & 4 deletions app/src/pages/access/AccessRequestPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import { Formik } from 'formik';
import { APIError } from 'hooks/api/useAxios';
import { useAuthStateContext } from 'hooks/useAuthStateContext';
import { useBiohubApi } from 'hooks/useBioHubApi';
import useDataLoader from 'hooks/useDataLoader';
import {
IBCeIDBasicAccessRequestDataObject,
IBCeIDBusinessAccessRequestDataObject,
Expand Down Expand Up @@ -67,9 +66,6 @@ export const AccessRequestPage: React.FC = () => {

const [isSubmittingRequest, setIsSubmittingRequest] = useState(false);

const codesDataLoader = useDataLoader(() => biohubApi.codes.getAllCodeSets());
codesDataLoader.load();

const showAccessRequestErrorDialog = (textDialogProps?: Partial<IErrorDialogProps>) => {
dialogContext.setErrorDialog({
...defaultErrorDialogProps,
Expand Down
Loading

0 comments on commit a646855

Please sign in to comment.