-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Pr05 Typescript Migration #20: Refine server/types & client/reducer types #3704
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
base: develop
Are you sure you want to change the base?
Changes from 13 commits
9fc95a9
fda6632
9623321
3533425
6bcdebf
2c0342d
70880de
0ddb69d
aae6687
90335b3
276356f
ad4df7a
997180a
64db3d2
789447a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -54,7 +54,11 @@ const initialTestState: RootState = { | |
| user: { | ||
| email: '[email protected]', | ||
| username: 'happydog', | ||
| preferences: {}, | ||
| preferences: { | ||
| ...initialPrefState, | ||
| indentationAmount: 2, | ||
| isTabIndent: true | ||
| }, | ||
| apiKeys: [], | ||
| verified: 'sent', | ||
| id: '123456789', | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,7 +29,7 @@ export const mockBaseUserSanitised: PublicUser = { | |
| email: '[email protected]', | ||
| username: 'tester', | ||
| preferences: mockUserPreferences, | ||
| apiKeys: ([] as unknown) as Types.DocumentArray<ApiKeyDocument>, | ||
| apiKeys: [], | ||
| verified: 'verified', | ||
| id: 'abc123', | ||
| totalSize: 42, | ||
|
|
@@ -42,6 +42,7 @@ export const mockBaseUserSanitised: PublicUser = { | |
| export const mockBaseUserFull: Omit<User, 'createdAt'> = { | ||
| ...mockBaseUserSanitised, | ||
| name: 'test user', | ||
| apiKeys: ([] as unknown) as Types.DocumentArray<ApiKeyDocument>, | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. for a |
||
| tokens: [], | ||
| password: 'abweorij', | ||
| resetPasswordToken: '1i14ij23', | ||
|
|
@@ -58,9 +59,15 @@ export const mockBaseUserFull: Omit<User, 'createdAt'> = { | |
| export function createMockUser( | ||
| overrides: Partial<UserDocument> = {}, | ||
| unSanitised: boolean = false | ||
| ): PublicUser & Record<string, any> { | ||
| ): PublicUser | UserDocument { | ||
| if (unSanitised) { | ||
| return { | ||
| ...mockBaseUserFull, | ||
| ...overrides | ||
| } as UserDocument; | ||
| } | ||
| return { | ||
| ...(unSanitised ? mockBaseUserFull : mockBaseUserSanitised), | ||
| ...mockBaseUserSanitised, | ||
| ...overrides | ||
| }; | ||
| } as PublicUser; | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried to make the return type of each case super explicit, but we seem to still need to cast the return value of calling this function anyway to resolve type-errors for when the usecase requires |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,7 +29,7 @@ describe('user.controller > auth management > password management', () => { | |
| let response: MockResponse; | ||
| let next: MockNext; | ||
| let mockToken: string; | ||
| let mockUser: Partial<UserDocument>; | ||
| let mockUser: UserDocument; | ||
| const fixedTime = 100000000; | ||
|
|
||
| beforeEach(() => { | ||
|
|
@@ -68,10 +68,13 @@ describe('user.controller > auth management > password management', () => { | |
| describe('if the user is found', () => { | ||
| beforeEach(async () => { | ||
| mockToken = 'mock-token'; | ||
| mockUser = createMockUser({ | ||
| email: '[email protected]', | ||
| save: jest.fn().mockResolvedValue(null) | ||
| }); | ||
| mockUser = createMockUser( | ||
| { | ||
| email: '[email protected]', | ||
| save: jest.fn().mockResolvedValue(null) | ||
| }, | ||
| false | ||
| ) as UserDocument; | ||
|
|
||
| (generateToken as jest.Mock).mockResolvedValue(mockToken); | ||
| User.findByEmail = jest.fn().mockResolvedValue(mockUser); | ||
|
|
@@ -143,10 +146,13 @@ describe('user.controller > auth management > password management', () => { | |
| }); | ||
| it('returns unsuccessful for all other errors', async () => { | ||
| mockToken = 'mock-token'; | ||
| mockUser = createMockUser({ | ||
| email: '[email protected]', | ||
| save: jest.fn().mockResolvedValue(null) | ||
| }); | ||
| mockUser = createMockUser( | ||
| { | ||
| email: '[email protected]', | ||
| save: jest.fn().mockResolvedValue(null) | ||
| }, | ||
| false | ||
| ) as UserDocument; | ||
|
|
||
| (generateToken as jest.Mock).mockRejectedValue( | ||
| new Error('network error') | ||
|
|
@@ -298,7 +304,7 @@ describe('user.controller > auth management > password management', () => { | |
| resetPasswordToken: 'valid-token', | ||
| resetPasswordExpires: fixedTime + 10000, // still valid | ||
| save: jest.fn() | ||
| }; | ||
| } as UserDocument; | ||
|
|
||
| beforeEach(async () => { | ||
| User.findOne = jest.fn().mockReturnValue({ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,19 +1,33 @@ | ||
| import crypto from 'crypto'; | ||
| import type { Response } from 'express'; | ||
| import { User } from '../../models/user'; | ||
| import { PublicUser, UserDocument } from '../../types'; | ||
| import { | ||
| ApiKeyDocument, | ||
| PublicUser, | ||
| SanitisedApiKey, | ||
| UserDocument | ||
| } from '../../types'; | ||
|
|
||
| export function sanitiseApiKey(key: ApiKeyDocument): SanitisedApiKey { | ||
| return { | ||
| id: key.id, | ||
| label: key.label, | ||
| lastUsedAt: key.lastUsedAt, | ||
| createdAt: key.createdAt | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Sanitise user objects to remove sensitive fields | ||
| * @param user | ||
| * @returns Sanitised user | ||
| */ | ||
| export function userResponse(user: PublicUser | UserDocument): PublicUser { | ||
| export function userResponse(user: UserDocument): PublicUser { | ||
| return { | ||
| email: user.email, | ||
| username: user.username, | ||
| preferences: user.preferences, | ||
| apiKeys: user.apiKeys, | ||
| apiKeys: user.apiKeys.map((el) => sanitiseApiKey(el)), | ||
|
Comment on lines
+11
to
+30
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is one of the bigger changes on this PR: |
||
| verified: user.verified, | ||
| id: user.id, | ||
| totalSize: user.totalSize, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,22 +8,26 @@ export interface IApiKey extends VirtualId, MongooseTimestamps { | |
| label: string; | ||
| lastUsedAt?: Date; | ||
| hashedKey: string; | ||
| token?: string; | ||
| } | ||
|
|
||
| /** Mongoose document object for API Key */ | ||
| export interface ApiKeyDocument | ||
| extends IApiKey, | ||
| Omit<Document<Types.ObjectId>, 'id'> { | ||
| toJSON(options?: any): SanitisedApiKey; | ||
| toObject(options?: any): SanitisedApiKey; | ||
| toJSON(options?: any): IApiKey; | ||
| toObject(options?: any): IApiKey; | ||
|
Comment on lines
+18
to
+19
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Previously typed this as |
||
| } | ||
|
|
||
| /** | ||
| * Sanitised API key object which hides the `hashedKey` field | ||
| * and can be exposed to the client | ||
| */ | ||
| export interface SanitisedApiKey | ||
| extends Pick<ApiKeyDocument, 'id' | 'label' | 'lastUsedAt' | 'createdAt'> {} | ||
| extends Pick< | ||
| ApiKeyDocument, | ||
| 'id' | 'label' | 'lastUsedAt' | 'createdAt' | 'token' | ||
| > {} | ||
|
|
||
| /** Mongoose model for API Key */ | ||
| export interface ApiKeyModel extends Model<ApiKeyDocument> {} | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for a
PublicUser, apiKeys isSanitisedApiKey[]