-
Notifications
You must be signed in to change notification settings - Fork 81
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
test: fix contentContainer test #1516
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1516 +/- ##
==========================================
- Coverage 93.04% 93.04% -0.01%
==========================================
Files 1048 1049 +1
Lines 20544 20582 +38
Branches 4363 4445 +82
==========================================
+ Hits 19116 19151 +35
- Misses 1358 1359 +1
- Partials 70 72 +2 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
Merging manually since there is something wrong with the |
Thanks for the pull request, @dcoa! What's next?Please work through the following steps to get your changes ready for engineering review: 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. 🔘 Let us know that your PR is ready for review:Who will review my changes?This repository is currently maintained by Where can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:
When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
@dcoa perhaps the error in the |
I don't have exactly the same error message but I can tell the error comes from this change
Previously the line was
And Schedule and Details pass the prop (isLibrary as true) so the ImageUploadModal is skip and the error is not displayed I think there should be a different prop, I mean, Schedule and Details is not a library is part of courses. How to test
I would like to ask you @PKulkoRaccoonGang if you can try in your environment and check if the assumption described is right. Solutions A possible solution could be pass a new prop different to isLibrary to skip the modal (Rename isLibrary for enableImageUpload). As well as you thoughts about a new prop. Thanks |
@dcoa That makes sense to me. If this is broken on master and affecting edx.org please make sure we fix it ASAP. It's probably better to ping people on Slack or make a fix proactively. |
This PR fixes the testing after the merge of #1458