-
Notifications
You must be signed in to change notification settings - Fork 96
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
feat: upgrade to quince #156
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This change is due to changing quickstart to launch in tutor.
This change does a couple of things and it introduce breaking changes, in summary it does: - It remove the usage of env/(productoin/development) - It's no longer possible to set/override MFE config via MFE_APP_* - It make MFEs rely to config that are defied in LMS settins - Other essetinal config that are needed at build time are are defined in the Dockerfile - It sets a proxy for both Caddy (for production) and Webpack (for development) - For old patches that are used to set/override MFE config, via production/development would need to be rewritten
Persistent grades are now enabled everywhere and cannot be disabled. As a consequence the PERSISTENT_GRADES_ENABLED_FOR_ALL_TESTS setting does not exist anymore.
In the account MFE (for instance), the top-right menu link to the profile MFE was incorrect, as it included an "undefined/" portion.
The CREDENTIALS_BASE_URL MFE config was inadvertently set in a previous PR, thus enabling learner records. For instance, the profile page included a link to the records page: "View my records"
Calls to the MFE config API endpoint are made with two caches: one on the client side, in the "edx-cache" local storage, and one on the server side. The client side cache has a hard-coded timeout of 5 minutes. I see no reason to have such a high cache timeout on the server side.
The only feature enabled by default is the "Pages & Resources" menu item in Studio. Waffle flags for the new React editors are created but disabled by default. (The only editor that seems to be fully featured at the moment is the HTML block editor; Video seems to work without a preview, and Problem is just a placeholder.) The new proctoring interface is also disabled, as it requires the `edx-exams` IDA, which Tutor doesn't currently support.
Adds support for the Discussions MFE, enabling it by default. Optional features also enabled via waffle flag: * Learner's tab in the MFE * Extended moderation reason codes * Reported content email notifications to moderators * Enable learner stats in the activity API
When introducing dynamic config[1], `CSRF_TOKEN_API_PATH` was removed from the MFE build environment and not added to the corresponding Django settings, leading to POST failures across the board. This addresses the problem by adding `CSRF_TOKEN_API_PATH` back in. [1] #69
Adds support for the Authn MFE, enabling it by default. This disable enterprise login by default. Co-authored-by: Adolfo R. Brandes <[email protected]>
Since we no longer need the build context to pass in MFE configuration at runtime, by removing it from the `tutor local` docker-compose we can avoid rebuilding the image whenever a `start` is issued.
regisb
force-pushed
the
quince
branch
3 times, most recently
from
December 4, 2023 12:51
2a7e58a
to
6a2dba1
Compare
This reverts ae60b2d. We depend heavily on Webpack's `publicPath` for everything to work in MFEs that are hosted in sub-paths. This variable affects many things, including URLs that are automatically generated for things like static assets or lazily loaded chunks of code. If the trailing slash is not appended to, say, `/learning`, a React.lazy() will try to fetch this from Caddy, and fail: `/learning210.3f8d0b2a9dbf3b1f1e69.js` While this can (and probably should) be dealt with in frontend-platform, we do it here for now.
MFEs with URLs that get used "naked" (without any suffixes) require a trailing slash, because that's what our current PUBLIC_PATH setting expects. Without it, these MFEs don't render.
This is related to overhangio/tutor#945
ALWAYS CLEAR CACHE ON PLUGIN LOAD!!!
regisb
force-pushed
the
quince
branch
3 times, most recently
from
December 11, 2023 17:12
abc1eb0
to
8a8c540
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.