Upgrade frontend-build to v14 #1052

pomegranited · 2024-05-28T07:29:17Z

Description

Upgrades dependencies for frontend-build, frontend-package.

Package	Change	Type
@edx/frontend-platform	7.0.1 -> 8.0.3	major
@edx/frontend-component-footer	13.0.2 -> 14.0.3	major
@edx/react-unit-test-utils	2.0.0 -> 2.1.1	minor
@edx/frontend-component-header	5.1.0 -> 5.3.3	minor
@openedx/frontend-plugin-framework	1.1.0 -> 1.2.1	minor
@openedx/paragon	22.2.1 -> 22.5.1	minor
classnames	2.2.6 -> 2.5.1	minor
core-js	3.8.1 -> 3.37.1	minor
formik	2.2.6 -> 2.4.6	minor
moment	2.29.4 -> 2.30.1	minor
react-router	6.16.0 -> 6.23.1	minor
react-router-dom	6.16.0 -> 6.23.1	minor
react-textarea-autosize	8.4.1 > 8.5.3	minor
@edx/brand	1.2.2 -> 1.2.3	patch
@edx/frontend-component-ai-translations	2.0.0 -> TBD, PR	patch
@edx/frontend-lib-content-components	2.1.9 -> TBD, PR	patch

Supporting information

#1034

Private-ref: FAL-3750

Dependencies

Blocked by:

Based on these PRs, which are likely to merge quickly:

Testing instructions

CI should cover most of the functionality, but manual testing is needed to ensure essential features are not broken.

Enable all the new_studio toggles.
Visit as many pages in the Course Authoring MFE as you can, and check for errors.

openedx-webhooks · 2024-05-28T07:29:22Z

Thanks for the pull request, @pomegranited! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

supporting documentation
Open edX discussion forum threads
timeline information ("this must be merged by XX date", and why that is)
partner information ("this is a course on edx.org")
any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

bradenmacdonald · 2024-05-28T18:59:20Z

src/studio-home/StudioHome.test.jsx

@@ -49,7 +49,7 @@ const RootWrapper = () => (
  </AppProvider>
 );

-describe('<StudioHome />', async () => {
+describe('<StudioHome />', () => {


Wow, were these tests even running? Probably not...

bradenmacdonald · 2024-05-30T16:44:56Z

@pomegranited If you'd like, it might make sense to cherry-pick some of these commits into separate PRs that can be merged ASAP and independently, like "warnings about Duplicate message id" or "ensure all act() calls are async"

[WARN] [FormatJS CLI] Duplicate message id: "course-authoring.content-tags-drawer.content-tags-collapsible.custom-menu.placeholder-text", but the `description` and/or `defaultMessage` are different. [WARN] [FormatJS CLI] Duplicate message id: "authoring.alert.error.connection", but the `description` and/or `defaultMessage` are different. [WARN] [FormatJS CLI] Duplicate message id: "course-authoring.import.page.title", but the `description` and/or `defaultMessage` are different. [WARN] [FormatJS CLI] Duplicate message id: "course-authoring.course-outline.card.menu.delete", but the `description` and/or `defaultMessage` are different. [WARN] [FormatJS CLI] Duplicate message id: "course-authoring.course-outline.page-alerts.discussionNotificationLearnMore", but the `description` and/or `defaultMessage` are different. [WARN] [FormatJS CLI] Duplicate message id: "course-authoring.course-outline.status-bar.video-sharing.title", but the `description` and/or `defaultMessage` are different. [WARN] [FormatJS CLI] Duplicate message id: "course-authoring.course-outline.subsection.button.new-unit", but the `description` and/or `defaultMessage` are different. [WARN] [FormatJS CLI] Duplicate message id: "course-authoring.course-team.member.button.remove", but the `description` and/or `defaultMessage` are different. [WARN] [FormatJS CLI] Duplicate message id: "course-authoring.files-and-uploads..deleteConfirmation.message", but the `description` and/or `defaultMessage` are different. [WARN] [FormatJS CLI] Duplicate message id: "course-authoring.files-and-videos.sort-and-filter.modal.filter.activeCheckbox.label", but the `description` and/or `defaultMessage` are different. [WARN] [FormatJS CLI] Duplicate message id: "course-authoring.files-and-videos.sort-and-filter.modal.filter.inactiveCheckbox.label", but the `description` and/or `defaultMessage` are different. [WARN] [FormatJS CLI] Duplicate message id: "course-authoring.course-outline.configure-modal.advanced-tab.timed-description", but the `description` and/or `defaultMessage` are different. [WARN] [FormatJS CLI] Duplicate message id: "course-authoring.course-outline.configure-modal.advanced-tab.timed-description", but the `description` and/or `defaultMessage` are different. [WARN] [FormatJS CLI] Duplicate message id: "course-authoring.course-outline.configure-modal.advanced-tab.timed-description", but the `description` and/or `defaultMessage` are different. [WARN] [FormatJS CLI] Duplicate message id: "course-authoring.export.stepper.header.title", but the `description` and/or `defaultMessage` are different. [WARN] [FormatJS CLI] Duplicate message id: "authoring.loading", but the `description` and/or `defaultMessage` are different.

Returning a Promise from "describe" is not supported.

Previous commit revealed several issues with these tests: * Don't nest userAction.click in act() -- nested act() statements have indeterminent behaviour. * Use getBy* instead of findBy* with userAction to avoid nested act() statements * Always await userEvent.click * Use fireEvent.click when the onClick handlers need to be called * Use queryBy* instead of getBy* when using .toBeInTheDocument or waitForElementToBeRemoved queryBy* return null when the element is not found.

Warning: React does not recognize the `data-testId` prop on a DOM element. If you intentionally want it to appear in the DOM as a custom attribute, spell it as lowercase `data-testid` instead.

* Don't nest userAction.click in act() -- nested act() statements have indeterminent behaviour.

test: Use useLocation to test route changes

…jill/frontend-build-14

…ai-translations to support frontend-build v14 Depends on: openedx/frontend-lib-content-components#475 openedx-unsupported/frontend-component-ai-translations#16 Commands used: npm install "git+https://github.com/open-craft/frontend-lib-content-components.git#jill/bilalqamar95/jest-v29-upgrade" npm install "git+https://github.com/open-craft/frontend-component-ai-translations.git#jill/frontend-platform-v8"

* remove "indent" setting from .eslintrc.js Using default config for indent instead of our custom setting. * add @typescript-eslint/ prefix to eslint-disable-line statements where flagged by linter * changed stylelint setting import-notation to "string" The alternative "url" setting doesn't work for all of our imports.

codecov · 2024-06-04T05:37:50Z

Codecov Report

Attention: Patch coverage is 84.23773% with 61 lines in your changes missing coverage. Please review.

Project coverage is 92.60%. Comparing base (3936737) to head (0197c1d).

Files	Patch %	Lines
src/course-outline/utils.jsx	75.00%	13 Missing ⚠️
...ng-settings/assignment-section/utils/validation.js	64.00%	7 Missing and 2 partials ⚠️
src/course-updates/hooks.jsx	50.00%	8 Missing ⚠️
src/course-outline/utils/getChecklistValues.js	75.00%	6 Missing ⚠️
src/course-outline/hooks.jsx	81.81%	4 Missing ⚠️
src/generic/configure-modal/ConfigureModal.jsx	85.71%	4 Missing ⚠️
src/generic/drag-helper/DraggableList.jsx	86.66%	4 Missing ⚠️
src/course-team/utils.js	75.00%	2 Missing ⚠️
src/course-unit/add-component/AddComponent.jsx	94.44%	2 Missing ⚠️
src/course-updates/update-form/utils.js	75.00%	2 Missing ⚠️
... and 4 more

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1052   +/-   ##
=======================================
  Coverage   92.59%   92.60%           
=======================================
  Files         710      710           
  Lines       12683    12683           
  Branches     2764     2764           
=======================================
+ Hits        11744    11745    +1     
+ Misses        903      902    -1     
  Partials       36       36

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

yusuf-musleh

👍 @pomegranited Great work on this! Things seems to be working smoothly. Just had a small non-blocker question about some test.

I tested this: Ran locally and npm installed everything, enabled all new_studio_mfe flags, then navigated through all the pages to confirm nothing is broken.
I read through the code
~~I checked for accessibility issues~~
~~Includes documentation~~

src/files-and-videos/files-page/FilesPage.test.jsx

yusuf-musleh · 2024-06-04T16:15:06Z

@pomegranited Though I'm not sure how to go about the coverage issue in the CI. My hunch is that this is probably because many files were changed, and some code might not have had tests previously, hence the in the patch coverage it falls under the required limit.

pomegranited · 2024-06-05T00:02:38Z

Hi @yusuf-musleh ,

I'm not sure how to go about the coverage issue in the CI. My hunch is that this is probably because many files were changed,

I think if we merge the smaller PRs and then rebase this one, the coverage will fix itself.

better than we did in the previous commit.

xitij2000

I see a couple of await useEvent instances. I assume they were from the other PR in which case this will eb fixed with a rebase, otherwise some of the points from the test warnign fix PR will apply here as well.

xitij2000 · 2024-06-13T05:16:01Z

src/group-configurations/GroupConfigurations.scss

@@ -23,8 +23,7 @@
  .configuration-card-header {
    display: flex;
    align-items: center;
-    align-content: center;
-    justify-content: space-between;
+    place-content: center space-between;

    .configuration-card-header__button {
      display: flex;


Nope, I meat you can use those classes instead of using a custom class to apply the same styles.

i.e. do a search-replace with:
search className="configuration-card-header"
replace className="configuration-card-header d-flex align-items-start p-0 h-auto text-black "

Then these can be removed from the custom css

src/generic/delete-modal/DeleteModal.test.jsx

rayzhou-bit · 2024-06-13T17:52:35Z

@pomegranited Thank you for working on this PR! TnL is looking to get this moved in asap so please let me know if there's anything I can do to help.

bradenmacdonald · 2024-06-13T22:08:03Z

@rayzhou-bit Could you help us get openedx-unsupported/frontend-component-ai-translations#16 merged? That's blocking this PR so that would definitely be a big help in getting this over the line sooner. Thanks :)

pomegranited · 2024-06-13T22:32:24Z

@bradenmacdonald @xitij2000

I thought that the codecov/patch would fix itself when we merged those other PRs, but it hasn't. But now I'm worried that it will take too much more time to fix.

Is there any way we can ignore this test failure for this particular case, and merge anyway?

bradenmacdonald · 2024-06-13T22:33:59Z

@pomegranited Yes I believe it will be fine just to ignore that and merge anyways. You're not responsible for increasing the test coverage of unrelated components that the diff happens to touch, which already weren't meeting the requirement. IMHO.

It would be a different story if you added a bunch of new code without tests.

As you can see, the overall coverage % of the project is not affected at all.

pomegranited · 2024-06-14T06:57:51Z

Hi @rayzhou-bit , thank you for your help! Could we also get a release created for https://github.com/openedx/frontend-lib-content-components ? I need to update the dependency here now that openedx/frontend-lib-content-components#475 has been merged.

rayzhou-bit · 2024-06-14T16:13:35Z

@pomegranited 2.2.0 should work for frontend-lib-content-component version! I had a change merged in with 2.2.0 set, but we reverted it because we thought it was blocking the pipeline without the upgrades in this PR.

bradenmacdonald · 2024-06-18T16:19:46Z

@pomegranited @xitij2000 What's left before we can merge this? Merge the @edx/frontend-component-ai-translations PR ?

pomegranited · 2024-06-18T22:58:29Z

@bradenmacdonald Yep, we need openedx-unsupported/frontend-component-ai-translations#16 merged & tagged, that's all that's blocking us now.

pomegranited · 2024-06-21T00:12:18Z

@xitij2000 There's an issue with the codecov token:

Error: Codecov token not found. Please provide Codecov token with -t flag.

The validate workflow looks like it's configured correctly, so maybe that secret got removed? Or maybe these tokens expire? I don't know..

CC @bradenmacdonald @arbrandes in case you can help.

xitij2000 · 2024-06-21T10:38:55Z

@xitij2000 There's an issue with the codecov token:

Error: Codecov token not found. Please provide Codecov token with -t flag.

The validate workflow looks like it's configured correctly, so maybe that secret got removed? Or maybe these tokens expire? I don't know..

CC @bradenmacdonald @arbrandes in case you can help.

Since the tests are passing and this PR doesn't have a coverage impact, I think it's safe to merge. However, I don't see any option to merge. As you said, it's probably an expired or missing token. Rerunning didn't help and I don't have the ability or access to fix the token issue.

yusuf-musleh · 2024-06-21T15:42:28Z

Error: Codecov token not found. Please provide Codecov token with -t flag.

@pomegranited @xitij2000 I've seen this error happen sometimes, generally repushing/rerunning the tests after some time fixes the issue and the coverage gets uploaded.

bradenmacdonald · 2024-06-21T17:44:47Z

@pomegranited @xitij2000 I believe the token is only available for PRs that come from the openedx organization. Since this PR is hosted on an OpenCraft branch, it only uses the shared GitHub tokenless approach which is subject to rate limits. I force-pushed it now and we'll see what that does.

xitij2000 · 2024-06-21T18:43:03Z

@pomegranited @xitij2000 I believe the token is only available for PRs that come from the openedx organization. Since this PR is hosted on an OpenCraft branch, it only uses the shared GitHub tokenless approach which is subject to rate limits. I force-pushed it now and we'll see what that does.

I had already rerun the tests. I also pushed this branch to this repo, but didn't want to create another PR from it, I remember sometimes having the same commits in a branch on the same repo helps.

openedx-webhooks · 2024-06-21T18:44:49Z

@pomegranited 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label May 28, 2024

bradenmacdonald reviewed May 28, 2024

View reviewed changes

pomegranited added 5 commits May 31, 2024 09:32

fix: paragon's Hyperlink no longer accepts a 'content' attribute

2bf0d6f

test: ensure all act() calls are async

961a8a2

test: Removed "async" from "describe"

af50999

Returning a Promise from "describe" is not supported.

pomegranited mentioned this pull request May 31, 2024

Fix some test warnings #1062

Merged

pomegranited and others added 8 commits May 31, 2024 15:45

fix: typo in data-testid

3c48d81

Warning: React does not recognize the `data-testId` prop on a DOM element. If you intentionally want it to appear in the DOM as a custom attribute, spell it as lowercase `data-testid` instead.

test: Use useLocation to test route changes

79def95

test: cleanup

7434cd0

* Don't nest userAction.click in act() -- nested act() statements have indeterminent behaviour.

Merge pull request #36 from open-craft/yusuf-musleh/fix-routes-in-tests

3bbb63c

test: Use useLocation to test route changes

Merge remote-tracking branch 'opencraft/jill/fix-test-warnings' into …

b75584d

…jill/frontend-build-14

test: fix failing tests after upgrade

48cda53

pomegranited force-pushed the jill/frontend-build-14 branch from c68e81a to 48cda53 Compare May 31, 2024 12:21

pomegranited added 2 commits June 4, 2024 14:45

test: fix failing tests after upgrade

0a801c1

Merge branch 'master' into jill/frontend-build-14

65a6807

pomegranited marked this pull request as ready for review June 4, 2024 05:28

pomegranited requested a review from a team as a code owner June 4, 2024 05:28

pomegranited requested a review from yusuf-musleh June 4, 2024 05:28

yusuf-musleh approved these changes Jun 4, 2024

View reviewed changes

src/files-and-videos/files-page/FilesPage.test.jsx Outdated Show resolved Hide resolved

test: fix tests failing after upgrade

3f53e96

better than we did in the previous commit.

pomegranited force-pushed the jill/frontend-build-14 branch from fb86d8a to 3f53e96 Compare June 5, 2024 00:45

pomegranited mentioned this pull request Jun 13, 2024

Bumps frontend-build@^14, frontend-platform@^8 openedx-unsupported/frontend-component-ai-translations#16

Merged

xitij2000 reviewed Jun 13, 2024

View reviewed changes

pomegranited added 2 commits June 13, 2024 15:12

Merge branch 'master' into jill/frontend-build-14

2541806

fix: css error "target selector was not found"

4738cbc

pomegranited added 2 commits June 14, 2024 06:01

Merge branch 'master' into jill/frontend-build-14

0e5c2fb

chore: upgrades dependency [email protected]

21924cb

xitij2000 approved these changes Jun 14, 2024

View reviewed changes

KristinAoki mentioned this pull request Jun 14, 2024

Revert "feat: flcc to 2.2.0" #1111

Merged

pomegranited force-pushed the jill/frontend-build-14 branch from df7b0fe to 21924cb Compare June 18, 2024 03:24

chore: bumps @edx/frontend-component-ai-translations to ^2.1.0

9c16156

Merge branch 'master' into jill/frontend-build-14

0197c1d

bradenmacdonald force-pushed the jill/frontend-build-14 branch from a63bfa0 to 0197c1d Compare June 21, 2024 17:41

xitij2000 merged commit 6b6d3aa into openedx:master Jun 21, 2024
5 of 6 checks passed

xitij2000 deleted the jill/frontend-build-14 branch June 21, 2024 18:44

yusuf-musleh mentioned this pull request Jun 23, 2024

[FC-0059] refactor: Change lib v2 code to TS #1124

Closed

BilalQamar95 mentioned this pull request Jul 1, 2024

feat: frontend-build & frontend-platform major version upgrade, with respective packages #1139

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade frontend-build to v14 #1052

Upgrade frontend-build to v14 #1052

pomegranited commented May 28, 2024 •

edited

Loading

openedx-webhooks commented May 28, 2024 •

edited

Loading

bradenmacdonald May 28, 2024

bradenmacdonald commented May 30, 2024

codecov bot commented Jun 4, 2024 •

edited

Loading

yusuf-musleh left a comment

yusuf-musleh commented Jun 4, 2024

pomegranited commented Jun 5, 2024

xitij2000 left a comment

xitij2000 Jun 13, 2024

rayzhou-bit commented Jun 13, 2024

bradenmacdonald commented Jun 13, 2024

pomegranited commented Jun 13, 2024

bradenmacdonald commented Jun 13, 2024 •

edited

Loading

pomegranited commented Jun 14, 2024

rayzhou-bit commented Jun 14, 2024

bradenmacdonald commented Jun 18, 2024

pomegranited commented Jun 18, 2024

pomegranited commented Jun 21, 2024

xitij2000 commented Jun 21, 2024

yusuf-musleh commented Jun 21, 2024

bradenmacdonald commented Jun 21, 2024

xitij2000 commented Jun 21, 2024

openedx-webhooks commented Jun 21, 2024

Upgrade frontend-build to v14 #1052

Upgrade frontend-build to v14 #1052

Conversation

pomegranited commented May 28, 2024 • edited Loading

Description

Supporting information

Dependencies

Testing instructions

openedx-webhooks commented May 28, 2024 • edited Loading

bradenmacdonald May 28, 2024

Choose a reason for hiding this comment

bradenmacdonald commented May 30, 2024

codecov bot commented Jun 4, 2024 • edited Loading

Codecov Report

yusuf-musleh left a comment

Choose a reason for hiding this comment

yusuf-musleh commented Jun 4, 2024

pomegranited commented Jun 5, 2024

xitij2000 left a comment

Choose a reason for hiding this comment

xitij2000 Jun 13, 2024

Choose a reason for hiding this comment

rayzhou-bit commented Jun 13, 2024

bradenmacdonald commented Jun 13, 2024

pomegranited commented Jun 13, 2024

bradenmacdonald commented Jun 13, 2024 • edited Loading

pomegranited commented Jun 14, 2024

rayzhou-bit commented Jun 14, 2024

bradenmacdonald commented Jun 18, 2024

pomegranited commented Jun 18, 2024

pomegranited commented Jun 21, 2024

xitij2000 commented Jun 21, 2024

yusuf-musleh commented Jun 21, 2024

bradenmacdonald commented Jun 21, 2024

xitij2000 commented Jun 21, 2024

openedx-webhooks commented Jun 21, 2024

pomegranited commented May 28, 2024 •

edited

Loading

openedx-webhooks commented May 28, 2024 •

edited

Loading

codecov bot commented Jun 4, 2024 •

edited

Loading

bradenmacdonald commented Jun 13, 2024 •

edited

Loading