Skip to content
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!: Add JS-based configuration capabilities to Learner Record #275

Merged
merged 15 commits into from
Feb 12, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ node_modules
npm-debug.log
coverage
module.config.js
env.config.js

dist/
src/i18n/transifex_input.json
Expand Down
48 changes: 48 additions & 0 deletions example.env.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
Learner Record is now able to handle JS-based configuration!

For the time being, the `.env.*` files are still made available when cloning down this repo or pulling from
the master branch. To switch to using `env.config.js`:

1. Make a copy of `example.env.config.js` and configure as needed.
2. To ensure the port number is assigned to the one provided in your env.config,
uncomment the lines in webpack.dev.config.js . This will eventually be resolved in https:/openedx/frontend-build/issues/513

Note: having both .env and env.config.js files will follow a predictable order, in which non-empty values in the
JS-based config will overwrite the .env environment variables.

frontend-platform's getConfig loads configuration in the following sequence:
- .env file config
- optional handlers (commonly used to merge MFE-specific config in via additional process.env variables)
- env.config.js file config
- runtime config
*/

module.exports = {
NODE_ENV: 'development',
PORT: 1990,
ACCESS_TOKEN_COOKIE_NAME: 'edx-jwt-cookie-header-payload',
BASE_URL: 'http://localhost:1990',
CREDENTIALS_BASE_URL: 'http://localhost:18150',
CSRF_TOKEN_API_PATH: '/csrf/api/v1/token',
ECOMMERCE_BASE_URL: 'http://localhost:18130',
LANGUAGE_PREFERENCE_COOKIE_NAME: 'openedx-language-preference',
LMS_BASE_URL: 'http://localhost:18000',
LOGIN_URL: 'http://localhost:18000/login',
LOGOUT_URL: 'http://localhost:18000/logout',
LOGO_URL: 'https://edx-cdn.org/v3/default/logo.svg',
LOGO_TRADEMARK_URL: 'https://edx-cdn.org/v3/default/logo-trademark.svg',
LOGO_WHITE_URL: 'https://edx-cdn.org/v3/default/logo-white.svg',
FAVICON_URL: 'https://edx-cdn.org/v3/default/favicon.ico',
jsnwesson marked this conversation as resolved.
Show resolved Hide resolved
MARKETING_SITE_BASE_URL: 'http://localhost:18000',
ORDER_HISTORY_URL: 'http://localhost:1996/orders',
REFRESH_ACCESS_TOKEN_ENDPOINT: 'http://localhost:18000/login_refresh',
SEGMENT_KEY: '',
SITE_NAME: 'localhost',
USER_INFO_COOKIE_NAME: 'edx-user-info',
SUPPORT_URL_LEARNER_RECORDS: 'https://support.edx.org/hc/en-us/sections/360001216693-Learner-records',
APP_ID: '',
MFE_CONFIG_API_URL: '',
ENABLE_VERIFIABLE_CREDENTIALS: true,
SUPPORT_URL_VERIFIABLE_CREDENTIALS: '',
};
17 changes: 17 additions & 0 deletions jest.config.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,19 @@
const path = require('path');
const fs = require('fs');
const { createConfig } = require('@edx/frontend-build');

/** in case there isn't an env.config.js in development or pushed into the master branch
we want to rely on a fallback config that exists in ./jest that provides an empty object.
This implementation was first done in frontend-build
jsnwesson marked this conversation as resolved.
Show resolved Hide resolved
https:/openedx/frontend-build/blob/master/config/jest/fallback.env.config.js
*/
let envConfigPath = path.resolve(__dirname, './jest/fallback.env.config.js');
const appEnvConfigPath = path.resolve(process.cwd(), './env.config.js');

if (fs.existsSync(appEnvConfigPath)) {
envConfigPath = appEnvConfigPath;
}

module.exports = createConfig('jest', {
// setupFilesAfterEnv is used after the jest environment has been loaded. In general this is what you want.
// If you want to add config BEFORE jest loads, use setupFiles instead.
Expand All @@ -10,4 +24,7 @@ module.exports = createConfig('jest', {
'src/setupTest.js',
'src/i18n',
],
moduleNameMapper: {
'env.config': envConfigPath,
},
});
jsnwesson marked this conversation as resolved.
Show resolved Hide resolved
12 changes: 12 additions & 0 deletions jest/fallback.env.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
module.exports = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[clarification] Similar to the other comment, I have a suspicion the jest/fallback.env.config.js used in the base jest.config.js provided by @(open)edx/frontend-build might be sufficient without having to duplicate into consuming projects?

// This file is used as a fallback to prevent build errors if an env.config.js file has not been
// defined in a consuming application.

I read this as though @(open)edx/frontend-build handles the fallback behind-the-scenes for consuming repositories such that the changes could be released without a breaking change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As noted in your comment in the jest.config.js file, I'll very likely remove this for the PR because I agree that frontend-build should already provide this fallback.

// DO NOT PUT ANY CONFIGURATION IN THIS FILE.

// The existence of this file is a technical detail of the mechanism used to implement
// env.config.js files for jest.

// This file is used as a fallback to prevent build errors if an env.config.js file has not been
// defined in a consuming application. If we could inline an empty object instead of needing a
// file to reference, that'd be clearer, but doesn't seem to be an option.

// This is NOT an appropriate place to put actual configuration values for tests.
};
Loading
Loading