-
Notifications
You must be signed in to change notification settings - Fork 1
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
BA-2020: mobile internal testing #172
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
module.exports = require('@baseapp-frontend/test/__mocks__/expo.ts') | ||
|
||
export {} |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
module.exports = require('@baseapp-frontend/test/__mocks__/react-native.ts') | ||
|
||
export {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
import { getExpoConstant } from '@baseapp-frontend/utils' | ||
import type { JWTResponse } from '@baseapp-frontend/utils/types/jwt' | ||
|
||
const preAuthenticateJWT = async (token?: string) => { | ||
|
@@ -6,8 +7,9 @@ const preAuthenticateJWT = async (token?: string) => { | |
throw new Error('No token provided.') | ||
} | ||
|
||
const EXPO_PUBLIC_API_BASE_URL = getExpoConstant('EXPO_PUBLIC_API_BASE_URL') | ||
const response = await fetch( | ||
`${process.env.NEXT_PUBLIC_API_BASE_URL ?? process.env.EXPO_PUBLIC_API_BASE_URL}/auth/pre-auth/jwt`, | ||
`${process.env.NEXT_PUBLIC_API_BASE_URL ?? EXPO_PUBLIC_API_BASE_URL}/auth/pre-auth/jwt`, | ||
Comment on lines
+10
to
+12
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. 🛠️ Refactor suggestion Consider extracting URL resolution to a shared utility. This URL resolution pattern is duplicated across multiple files. Consider extracting it to a shared utility function. +// In packages/utils/functions/url/index.ts
+export const getApiBaseUrl = () => {
+ const baseUrl = process.env.NEXT_PUBLIC_API_BASE_URL ?? getExpoConstant('EXPO_PUBLIC_API_BASE_URL')
+ if (!baseUrl) {
+ throw new Error('API base URL is not configured.')
+ }
+ return baseUrl
+}
- const EXPO_PUBLIC_API_BASE_URL = getExpoConstant('EXPO_PUBLIC_API_BASE_URL')
- const response = await fetch(
- `${process.env.NEXT_PUBLIC_API_BASE_URL ?? EXPO_PUBLIC_API_BASE_URL}/auth/pre-auth/jwt`,
+ const response = await fetch(
+ `${getApiBaseUrl()}/auth/pre-auth/jwt`,
|
||
{ | ||
method: 'POST', | ||
body: JSON.stringify({ token }), | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,24 +1,24 @@ | ||
import type { CookiesGetByNameFn } from '@baseapp-frontend/test' | ||
|
||
import Cookies from 'js-cookie' | ||
import { getToken } from '@baseapp-frontend/utils/functions/token/getToken' | ||
|
||
import getUser from '../index' | ||
import jwt from './fixtures/jwt.json' | ||
|
||
jest.mock('@baseapp-frontend/utils/functions/token/getToken', () => ({ | ||
getToken: jest.fn(), | ||
})) | ||
|
||
describe('getUser', () => { | ||
it('should return the user from the JWT cookie', async () => { | ||
;(Cookies.get as CookiesGetByNameFn) = jest.fn(() => jwt.token) | ||
it('should return the user from the JWT token', () => { | ||
;(getToken as jest.Mock).mockReturnValue(jwt.token) | ||
const user = getUser() | ||
|
||
expect(user?.email).toBe('user@company.com') | ||
expect(user?.firstName).toBe('John') | ||
expect(user?.lastName).toBe('Doe') | ||
}) | ||
|
||
it('should return null if the JWT cookie is not set', async () => { | ||
;(Cookies.get as CookiesGetByNameFn) = jest.fn(() => undefined) | ||
it('should return null if no token is set', () => { | ||
;(getToken as jest.Mock).mockReturnValue(undefined) | ||
const user = getUser() | ||
|
||
expect(user).toBeNull() | ||
}) | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,22 +1,26 @@ | ||
import type { CookiesGetByNameFn } from '@baseapp-frontend/test' | ||
|
||
import Cookies from 'js-cookie' | ||
import { getTokenAsync } from '@baseapp-frontend/utils/functions/token/getTokenAsync' | ||
|
||
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. 💡 Codebase verification Transition to token-based authentication is incomplete The codebase shows mixed authentication methods:
Note: Cookie usage for language preferences is a separate concern and can be retained. 🔗 Analysis chainLGTM! Clean transition from cookies to tokens. The import changes and mock setup correctly reflect the architectural shift from cookie-based to token-based authentication. Let's verify that this transition is consistent across the codebase: Also applies to: 6-8 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Check for any remaining cookie-based authentication
# that might have been missed during the transition
# Search for remaining js-cookie imports
echo "Checking for remaining js-cookie imports..."
rg "from 'js-cookie'" -l
# Search for cookie-related function calls
echo "Checking for remaining cookie-related function calls..."
rg "Cookies\.(get|set|remove)" -l
# Search for cookie-related types/interfaces
echo "Checking for cookie-related types..."
rg "CookiesGetByNameFn" -l
Length of output: 2214 Script: #!/bin/bash
# Check the context of cookie usage in core functionality files
echo "Checking cookie usage in core functionality files..."
echo -e "\nsetTokenAsync implementation:"
rg -A 5 "from 'js-cookie'" packages/utils/functions/token/setTokenAsync/index.ts
echo -e "\naxios instance creation:"
rg -A 5 "from 'js-cookie'" packages/utils/functions/axios/createAxiosInstance/index.ts
echo -e "\nCookie usage in authentication modules:"
rg -A 5 "Cookies\.(get|set|remove)" packages/authentication/modules/user/useUser/index.ts
echo -e "\nLanguage-related cookie usage:"
rg -A 5 "Cookies\.(get|set|remove)" packages/utils/functions/language/getLanguage/__tests__/getLanguage.client.test.ts
Length of output: 1721 Script: #!/bin/bash
# Check for token-based implementations
echo "Checking token-based implementations..."
echo -e "\nToken storage/retrieval implementations:"
rg -A 5 "getTokenAsync|setTokenAsync" packages/utils/functions/token/ -l
echo -e "\nToken usage in authentication:"
rg -A 5 "getTokenAsync|setTokenAsync" packages/authentication/modules/user/ -l
echo -e "\nCheck if there are parallel implementations:"
ast-grep --pattern 'function $_($$TOKEN$$) {
$$$
Cookies.$_($$$)
$$$
}'
Length of output: 1532 |
||
import getUserAsync from '../index' | ||
import jwt from './fixtures/jwt.json' | ||
|
||
jest.mock('@baseapp-frontend/utils/functions/token/getTokenAsync', () => ({ | ||
getTokenAsync: jest.fn(), | ||
})) | ||
|
||
describe('getUserAsync', () => { | ||
it('should return the user from the JWT cookie', async () => { | ||
;(Cookies.get as CookiesGetByNameFn) = jest.fn(() => jwt.token) | ||
it('should return the user from the JWT token', async () => { | ||
;(getTokenAsync as jest.Mock).mockReturnValue(jwt.token) | ||
|
||
const user = await getUserAsync() | ||
|
||
expect(user?.email).toBe('user@company.com') | ||
expect(user?.firstName).toBe('John') | ||
expect(user?.lastName).toBe('Doe') | ||
}) | ||
|
||
it('should return null if the JWT cookie is not set', async () => { | ||
;(Cookies.get as CookiesGetByNameFn) = jest.fn(() => undefined) | ||
it('should return null if no token is set', async () => { | ||
;(getTokenAsync as jest.Mock).mockReturnValue(undefined) | ||
|
||
const user = await getUserAsync() | ||
|
||
expect(user).toBeNull() | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
module.exports = require('@baseapp-frontend/test/__mocks__/expo.ts') | ||
|
||
export {} |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
module.exports = require('@baseapp-frontend/test/__mocks__/react-native.ts') | ||
|
||
export {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
const ExpoSecureStore = { | ||
getItemAsync: jest.fn(async (key: string) => { | ||
return key === 'ACCESS_KEY_NAME' ? 'mocked_value' : null | ||
}), | ||
setItemAsync: jest.fn(async (key: string, value: string) => { | ||
return true | ||
}), | ||
deleteItemAsync: jest.fn(async (key: string) => { | ||
return true | ||
}), | ||
} | ||
|
||
const Constants = { | ||
expoConfig: { | ||
extra: { | ||
EXPO_PUBLIC_API_BASE_URL: undefined, | ||
EXPO_PUBLIC_RELAY_ENDPOINT: undefined, | ||
EXPO_PUBLIC_WS_RELAY_ENDPOINT: undefined, | ||
}, | ||
}, | ||
} | ||
|
||
module.exports = { | ||
...ExpoSecureStore, | ||
...Constants, | ||
} |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
// ignoring react-native imports in tests, we may change this in the future | ||
module.exports = { | ||
Platform: { OS: 'web' }, | ||
} |
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.
💡 Codebase verification
Mobile platform test coverage is incomplete
The mobile-specific changes in
useLogin
lack test coverage:isMobilePlatform
🔗 Analysis chain
Verify mobile platform test coverage
The changes introduce significant mobile-specific adaptations:
useLogin
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
Length of output: 1978
Script:
Length of output: 3269