bito-code-review[bot] commented on code in PR #38033:
URL: https://github.com/apache/superset/pull/38033#discussion_r3627546725
##########
superset-frontend/src/pages/Login/Login.test.tsx:
##########
@@ -31,27 +31,26 @@ const defaultBootstrapData = {
},
};
-const mockApplicationRoot = jest.fn<string, []>(() => '');
+jest.mock('src/utils/getBootstrapData', () => ({
+ __esModule: true,
+ default: jest.fn(() => defaultBootstrapData),
+}));
-jest.mock('src/utils/getBootstrapData', () => {
- const actual = jest.requireActual<
- typeof import('src/utils/getBootstrapData')
- >('src/utils/getBootstrapData');
- return {
- __esModule: true,
- ...actual,
- default: jest.fn(() => defaultBootstrapData),
- applicationRoot: () => mockApplicationRoot(),
- };
-});
+const mockEnsureAppRoot = jest.fn((path: string) => path);
+jest.mock('src/utils/pathUtils', () => ({
Review Comment:
<!-- Bito Reply -->
The suggestion is valid. Mocking the module that the component directly
imports is necessary for the mock to intercept the function call, as re-exports
do not automatically redirect imports to the original module's mock.
**superset-frontend/src/pages/Login/Login.test.tsx**
```
const mockEnsureAppRoot = jest.fn((path: string) => path);
jest.mock('src/utils/pathUtils', () => ({
```
##########
superset-frontend/src/pages/Login/index.tsx:
##########
@@ -96,15 +96,19 @@ export default function Login() {
}, []);
const loginEndpoint = useMemo(
- () => (nextUrl ? `/login/?next=${encodeURIComponent(nextUrl)}` :
'/login/'),
+ () =>
+ ensureAppRoot(
+ nextUrl ? `/login/?next=${encodeURIComponent(nextUrl)}` : '/login/',
+ ),
[nextUrl],
);
const buildProviderLoginUrl = (providerName: string) => {
- const base = `/login/${encodeURIComponent(providerName)}`;
- return ensureAppRoot(
- nextUrl ? `${base}?next=${encodeURIComponent(nextUrl)}` : base,
- );
+ const base = `/login/${providerName}`;
Review Comment:
<!-- Bito Reply -->
The restoration of `encodeURIComponent` on the provider name is appropriate.
This change ensures that provider names containing special characters or spaces
are correctly handled, preventing the generation of malformed URLs and
maintaining consistency with the existing encoding pattern used for the
`nextUrl` parameter.
**superset-frontend/src/pages/Login/index.tsx**
```
const buildProviderLoginUrl = (providerName: string) => {
const base = `/login/${encodeURIComponent(providerName)}`;
return ensureAppRoot(
nextUrl ? `${base}?next=${encodeURIComponent(nextUrl)}` : base,
);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
