fix: tests fixed after design tokens support change by jesusbalderramawgu · Pull Request #1 · arbrandes/frontend-app-learner-dashboard

jesusbalderramawgu · 2026-01-16T17:12:17Z

Description

this is a PR to fix the tests after the changes in this PR

now the tests are passing successfully.

jesusbalderramawgu · 2026-01-16T17:14:41Z

src/containers/Dashboard/hooks.test.js

    });
  });
-  describe('useInitializeDashboard', () => {
-    it('dispatches initialize thunk action on component load', () => {


this was deleted because in the Dashboard/hooks.js file the useInitializeDashboard() function was deleted in a previous commit. Is that correct or should I bring that back?

diana-villalvazo-wgu · 2026-01-16T22:20:45Z

src/containers/CourseCard/components/CourseCardBanners/CreditBanner/views/ApprovedContent.jsx

@@ -1,6 +1,6 @@
 import React from 'react';


Suggested Change, add here useContext:

Suggested change

import React from 'react';

import React, { useContext } from 'react';

if React import is not needed then remove it pls

diana-villalvazo-wgu · 2026-01-16T22:21:32Z

...ntainers/CourseCard/components/CourseCardBanners/CreditBanner/views/ApprovedContent.test.jsx

        const actionButton = screen.getByRole('link', { name: messages.viewCredit.defaultMessage });
-        expect(actionButton).toHaveAttribute('aria-disabled', 'true');
-        expect(actionButton).toHaveClass('disabled');
+        // Paragon is not injecting this arribute that's why for now is commented, not it add this classes as well instead of class disabled


but is disabling the button or not? 👀

well, I couldn't see that attribute, I just saw that it returns those classes instead of the disabled class.
is there anything that I'm missing and that's why I don't see the attribute.
Not sure if is because of the new paragon version

it should have the attribute disabled, if not, then the button is not on that state, so your component is not getting that prop/attribute, you need to check how to mock the needed value to get that, btn btn-outline-primary these ones are not related to that state they are related to the variant and border-gray-400 is added directly to the component

thank you so much Diana for your comment, I was able to find the issue, now I'm mocking correctly and I got the correct attribute and class. Pr has been updated

diana-villalvazo-wgu · 2026-01-16T22:21:49Z

...containers/CourseCard/components/CourseCardBanners/CreditBanner/views/MustRequestContent.jsx

@@ -1,6 +1,6 @@
 import React from 'react';


Suggested change

import React from 'react';

import React, { useContext } from 'react';

diana-villalvazo-wgu · 2026-01-16T22:22:07Z

src/containers/CourseCard/components/CourseCardBanners/CreditBanner/views/PendingContent.jsx

@@ -1,6 +1,6 @@
 import React from 'react';


Suggested change

import React from 'react';

import React, { useContext } from 'react';

diana-villalvazo-wgu · 2026-01-16T22:23:05Z

src/containers/CourseCard/components/CourseCardBanners/RelatedProgramsBanner/index.test.jsx

-import { IntlProvider } from '@edx/frontend-platform/i18n';
+import { IntlProvider } from '@openedx/frontend-base';

-import { reduxHooks } from '../../../../../hooks';


why changing this one? 🤔

it wasn't necessary to have the path in that way, that's why I simplified

diana-villalvazo-wgu · 2026-01-16T22:26:07Z

src/containers/CourseCard/components/CourseCardMenu/SocialShareMenu.test.jsx

 import { IntlProvider } from '@openedx/frontend-base';
-import { render, screen } from '@testing-library/react';
-
+import { fireEvent, render, screen } from '@testing-library/react';


why you are importing fireEvent if you are not using it? 🤔

sorry, my mistake I was checking something and I forgot to remove it.
but I've updated the PR removing this

diana-villalvazo-wgu · 2026-01-16T22:27:04Z

src/containers/CourseCard/components/RelatedProgramsBadge/hooks.test.js


 const state = new MockUseState(hooks);
-const numPrograms = 27;
+let numPrograms = 3;


i know this new number makes the test to pass, but why the huge difference?

it wasn't un purpose I got the original value back

…aragon button

diana-villalvazo-wgu · 2026-01-19T16:29:38Z

src/containers/CourseCard/components/CourseCardMenu/index.test.jsx

              const unenrollOption = screen.getByRole('button', { name: messages.unenroll.defaultMessage });
              expect(unenrollOption).toBeInTheDocument();
-              expect(unenrollOption).toHaveAttribute('aria-disabled', 'true');
+              // the component is not returning the attribute aria disabled


this still true or need to change the mock here too?

src/containers/CourseCard/components/CourseCardMenu/SocialShareMenu.test.jsx

…tons

arbrandes

The tests themselves pass, which is great sign: thanks! There are a few problems, though:

Linting errors need to be addressed (npm run lint)
A few small changes requested
I'd like the hooks/api tests to be refactored: the masquerade stuff seems overly complicated and potentially buggy

Thanks for taking this on!

arbrandes · 2026-01-21T19:21:04Z

src/utils/StrictDict.js

+  if (typeof window !== 'undefined') {
+    window.console?.error({ target, name });
+    window.console?.error(`invalid property "${name}"`);
+  }


We should not be adding console logging upstream. It's good for debugging during development, but should not be committed.

this was fixed

arbrandes · 2026-01-21T19:22:11Z

src/containers/CourseCard/components/CourseCardMenu/SocialShareMenu.test.jsx

    });
  });
 });
+


Please remove the extraneous whitespace.

this was fixed

arbrandes · 2026-01-21T19:37:54Z

src/hooks/api.js

+export const useClearMasquerade = () => {
+  const clearRequest = reduxHooks.useClearRequest();
+  const initializeApp = module.useInitializeApp();
+  return () => {
+    clearRequest(RequestKeys.masquerade);
+    initializeApp();
+  };
+};
+
+export const useMasqueradeAs = (cardId) => {
+  reduxHooks.useCardEntitlementData(cardId);
+  const onSuccess = ({ data }) => {
+    reduxHooks.useLoadData()(data);
+  };
+
+  return module.useNetworkRequest(
+    (user) => api.initializeList({ user }),
+    { onSuccess, requestKey: RequestKeys.masquerade },
+  );
+};


Both of these functions are only used by tests. They should be moved to the test file, for starters.

But that's not all. They feel overly complicated, and I suspect there are bugs hidden in them. There must be a simpler way to test these hooks, no?

I took a look into this and in fact those functions were removed in a previous commit.
for example this function
useClearMasquerade was used in ‎src/containers/MasqueradeBar/hooks.js
but that doesn't exist anymore in our current branch, that component changed, now it is here
src/widgets/LearnerDashboardHeader/MasqueradeBar/hooks.js but is not the same, is not using the useClearMasquerade function anymore, nothing uses that. I couldn't find a reference in our current branch code.
So I removed the tests for this cases because are not in use anymore.

please let me know if you agree with this or if is something that I'm missing.
thanks for your feedback!

jesusbalderramawgu · 2026-01-21T20:43:45Z

The tests themselves pass, which is great sign: thanks! There are a few problems, though:

Linting errors need to be addressed (npm run lint)

A few small changes requested

I'd like the hooks/api tests to be refactored: the masquerade stuff seems overly complicated and potentially buggy

Thanks for taking this on!

Thank you so much for your feedback, ah yeah I forgot to run the lint in my local.
I will take a look into this.

jesusbalderramawgu · 2026-01-21T21:49:08Z

The tests themselves pass, which is great sign: thanks! There are a few problems, though:

Linting errors need to be addressed (npm run lint)

A few small changes requested

I'd like the hooks/api tests to be refactored: the masquerade stuff seems overly complicated and potentially buggy

Thanks for taking this on!

I made the changes, linting errors were fixed

arbrandes

Thanks for addressing my previous comments. Since all tests are now passing, I took a closer look and found a few more things to change:

The masquerade code was removed from the Redux state on purpose: it shouldn't be restored. The corresponding tests should be removed instead.
The multiple aliases workaround should be removed, and the '@src' prefix used instead (as introduced by Jacobo in frontend-base@alpha.9).

I apologize for the last one, as it means a potentially large refactor. But it's for a good cause.

arbrandes · 2026-01-22T11:02:24Z

src/data/constants/app.js

 import { StrictDict } from '../../utils';
-
-export const routePath = `${getSiteConfig().publicPath}:courseId`;
+export const routePath = `${getSiteConfig().PUBLIC_PATH}:courseId`;


getSiteConfig() does not support PUBLIC_PATH anymore, but publicPath is also wrong. It should be basename: https://github.com/openedx/frontend-base/blob/main/types.ts#L58 as of openedx/frontend-base@f9600ee84.

That said, this constant is not used anywhere. I think we can get rid of id.

arbrandes · 2026-01-22T11:06:21Z

src/data/redux/app/selectors/courseCard.test.js

 import { keyStore } from 'utils';
 import { baseAppUrl } from 'data/services/lms/urls';
-import { EXECUTIVE_EDUCATION_COURSE_MODES } from 'data/constants/course';
+import { EXECUTIVE_EDUCATION_COURSE_MODES } from '../../../../data/constants/course';


Wouldn't we also have to use relative imports for the two lines above?

I just ran across the aliases you added. We should be able to use the @src prefix now, as I commented below.

arbrandes · 2026-01-22T11:37:20Z

jest.config.js

+    // App aliases
+    '^hooks$': '<rootDir>/src/hooks',
+    '^testUtils$': '<rootDir>/src/testUtils.js',
+    '^data/constants/app$': '<rootDir>/src/data/constants/app.js',
+    '^data/constants/requests$': '<rootDir>/src/data/constants/requests.js',
+    '^data/services/lms/api$': '<rootDir>/src/data/services/lms/api.js',
+    '^data/services/lms/utils$': '<rootDir>/src/data/services/lms/utils.js',
+    '^data/services/lms/urls$': '<rootDir>/src/data/services/lms/urls.js',
+    '^data/redux/hooks$': '<rootDir>/src/data/redux/hooks/index.js',
+    '^data/redux/app/reducer$': '<rootDir>/src/data/redux/app/reducer.js',
+    '^data/redux$': '<rootDir>/src/data/redux/index.js',
+    '^utils$': '<rootDir>/src/utils/index.js',
+    '^utils/hooks$': '<rootDir>/src/utils/hooks.js',
+    '^components/(.*)$': '<rootDir>/src/components/$1',
+    '^containers/(.*)$': '<rootDir>/src/containers/$1',
+    '^tracking$': '<rootDir>/src/tracking/index.js',


I know using relative imports is painful, but an alias per import isn't the best way to solve it. I just published frontend-base@alpha.9 with should allow you to use a '@src' prefix in the imports.

arbrandes · 2026-01-22T11:45:10Z

src/data/redux/requests/selectors.js

+export const masquerade = (state) => {
+  const request = state.requests[RequestKeys.masquerade] || {};
+  return {
+    isMasquerading: request.status === RequestStates.completed,
+    isMasqueradingFailed: request.status === RequestStates.failed,
+    isMasqueradingPending: request.status === RequestStates.pending,
+    masqueradeErrorStatus: request.error?.response?.status,
+  };
+};
+


I think I remember, now (it's been a while). I believe I moved the masquerade stuff out of Redux into a React context so that it could be moved up into the header. There's no need to restore the masquerade stuff in Redux so the tests work: for now you can just remove the corresponding tests.

arbrandes · 2026-01-22T11:49:25Z

src/data/constants/requests.js

  enrollEntitlementSession: 'enrollEntitlementSession',
  leaveEntitlementSession: 'leaveEntitlementSession',
  recommendedCourses: 'recommendedCourses',
+  masquerade: 'masquerade',


No need to restore this - the corresponding test should be removed, instead. See my other comment about masquerade.

jesusbalderramawgu · 2026-01-22T14:46:14Z

Thanks for addressing my previous comments. Since all tests are now passing, I took a closer look and found a few more things to change:

The masquerade code was removed from the Redux state on purpose: it shouldn't be restored. The corresponding tests should be removed instead.

The multiple aliases workaround should be removed, and the '@src' prefix used instead (as introduced by Jacobo in frontend-base@alpha.9).

I apologize for the last one, as it means a potentially large refactor. But it's for a good cause.

Thanks Adolfo, I will work on the prefixes

jesusbalderramawgu · 2026-01-23T22:07:50Z

Thanks for addressing my previous comments. Since all tests are now passing, I took a closer look and found a few more things to change:

The masquerade code was removed from the Redux state on purpose: it shouldn't be restored. The corresponding tests should be removed instead.

The multiple aliases workaround should be removed, and the '@src' prefix used instead (as introduced by Jacobo in frontend-base@alpha.9).

I apologize for the last one, as it means a potentially large refactor. But it's for a good cause.

I have updated the frontend-base into frontend-base@alpha.11 and changed the prefixes, also I addressed the other comments. Thanks for your feedback, anything please let me now

arbrandes

Good work, it seems like we're almost there. I asked for a few minor changes, but in addition, can you also please remove the npm run build lines from the CI workflow so that tests pass? (We don't have a way to build-test until we figure out a solution for openedx/frontend-base#124).

src/test/app.test.jsx

src/containers/CourseCard/components/CourseCardMenu/SocialShareMenu.test.jsx

src/test/inspector.js

src/containers/CourseCard/components/CourseCardMenu/index.test.jsx

src/containers/CourseCard/components/CourseCardMenu/SocialShareMenu.test.jsx

src/test/app.test.jsx

jesusbalderramawgu · 2026-01-27T15:50:57Z

Good work, it seems like we're almost there. I asked for a few minor changes, but in addition, can you also please remove the npm run build lines from the CI workflow so that tests pass? (We don't have a way to build-test until we figure out a solution for openedx/frontend-base#124).

Thank you so much, for your feedback. I've updated the PR with your latest comments.
anything please let me know

codecov-commenter · 2026-01-27T16:00:00Z

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

arbrandes

Excellent! Approved! Thanks for bearing with me. 👍🏼

fix: tests fixed after design tokens support change

bd73154

jesusbalderramawgu commented Jan 16, 2026

View reviewed changes

diana-villalvazo-wgu reviewed Jan 16, 2026

View reviewed changes

fix: unnecessary imports removed and fix value in numPrograms

55d1903

arbrandes linked an issue Jan 19, 2026 that may be closed by this pull request

[frontend-base] Fix tests openedx/frontend-app-learner-dashboard#779

Closed

arbrandes mentioned this pull request Jan 19, 2026

[frontend-base] Fix tests openedx/frontend-app-learner-dashboard#779

Closed

fix: correctly mock for views inside CreditBanner to test correctly p…

fdd9318

…aragon button

diana-villalvazo-wgu reviewed Jan 19, 2026

View reviewed changes

src/containers/CourseCard/components/CourseCardMenu/SocialShareMenu.test.jsx Show resolved Hide resolved

fix: correctly mock added to SocialShareMenu.test to validate the but…

3b2eb18

…tons

arbrandes requested changes Jan 21, 2026

View reviewed changes

fix: lint fixed and removing unnecessary code

ae118d2

arbrandes requested changes Jan 22, 2026

View reviewed changes

fix: frontend-base updated to use @src prefix, unnecessary tests removed

d0d90fb

jesusbalderramawgu requested a review from arbrandes January 23, 2026 22:07

arbrandes requested changes Jan 27, 2026

View reviewed changes

fix: unnecessary code removed

90e0269

arbrandes approved these changes Jan 27, 2026

View reviewed changes

arbrandes merged commit 57a5c40 into arbrandes:frontend-base-design-tokens Jan 27, 2026
3 checks passed

brian-smith-tcril mentioned this pull request Jan 28, 2026

feat!: add design tokens support openedx/frontend-app-learner-dashboard#759

Merged

arbrandes pushed a commit that referenced this pull request Feb 6, 2026

fix: tests fixed after design tokens support change (#1)

21cb518

	import React from 'react';
	import React, { useContext } from 'react';

                   });
                 });
               });
+                    No newline at end of file

Conversation

jesusbalderramawgu commented Jan 16, 2026

Description

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

arbrandes left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arbrandes Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jesusbalderramawgu Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jesusbalderramawgu commented Jan 21, 2026

Uh oh!

jesusbalderramawgu commented Jan 21, 2026

Uh oh!

arbrandes left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jesusbalderramawgu commented Jan 22, 2026

Uh oh!

jesusbalderramawgu commented Jan 23, 2026

Uh oh!

arbrandes left a comment

arbrandes Jan 21, 2026 •

edited

Loading

jesusbalderramawgu Jan 21, 2026 •

edited

Loading