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.

Sign up for GitHub

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!: Remove unused feature flag #209

Merged
merged 1 commit into from
Oct 24, 2023
Merged

feat!: Remove unused feature flag #209

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 0 additions & 1 deletion .env.development
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ 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'
USE_LR_MFE='true'
APP_ID=''
MFE_CONFIG_API_URL=''
ENABLE_VERIFIABLE_CREDENTIALS='true'
Expand Down
13 changes: 6 additions & 7 deletions README.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,16 +77,15 @@ Environment Variables/Setup Notes

Currently, this MFE is not intergrated into the devstack, and must be run locally. This MFE requires credentials to be running, and will use a REST API from the Credentials IDA located at `credentials/apps/records/rest_api`.

Credentials uses 2 enviroment variables to link to this MFE:
Credentials requires configuring a Django setting to support directing traffic to the Learner Record MFE:

* ``USE_LEARNER_RECORD_MFE`` -- Toggles the navigation in credentials to redirect to this MFE
* ``LEARNER_RECORD_MFE_RECORDS_PAGE_URL`` -- The URL for the base URL of this MFE
* ``LEARNER_RECORD_MFE_RECORDS_PAGE_URL`` -- The base URL of the Learne Record MFE

More details for these flags can be found in the base configuration of credentials: ``credentials/settings/base``
This MFE has 2 flags of its own:
For more info, see the Learner Records documentation on ReadTheDocs: https://edx-credentials.readthedocs.io/en/latest/learner_records.html.

This MFE has a setting of its own:

* ``SUPPORT_URL_LEARNER_RECORDS`` -- A link to a help/support center for learners who run into problems whilst trying to share their records
* ``USE_LR_MFE`` -- A toggle that when on, uses the MFE to host shared records instead of the the old UI inside of credentials

Verifiable Credentials
......................
Expand Down Expand Up @@ -188,4 +187,4 @@ Please do not report security issues in public. Please email [email protected]
.. |Codecov| image:: https://codecov.io/gh/edx/frontend-app-learner-record/branch/master/graph/badge.svg
:target: https://codecov.io/gh/edx/frontend-app-learner-record
.. |license| image:: https://img.shields.io/npm/l/@edx/frontend-app-learner-record.svg
:target: https://github.com/openedx/frontend-app-learner-record/blob/master/LICENSE
:target: https://github.com/openedx/frontend-app-learner-record/blob/master/LICENSE
2 changes: 1 addition & 1 deletion src/components/ProgramRecordsList/ProgramRecordsList.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ function ProgramRecordsList() {
<div className="d-flex align-items-center pt-3 pt-lg-0">
<Hyperlink
variant="muted"
destination={getConfig().USE_LR_MFE ? `/${record.uuid}` : `${getConfig().CREDENTIALS_BASE_URL}/records/programs/${record.uuid}/`}
destination={`/${record.uuid}`}
>
<Button variant="outline-primary">
<FormattedMessage
Expand Down
41 changes: 18 additions & 23 deletions src/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,30 +25,26 @@ subscribe(APP_READY, () => {
<HelmetProvider>
<Head />
<Header />
{getConfig().USE_LR_MFE ? (
<Routes>
<Routes>
<Route
path={ROUTES.PROGRAM_RECORDS}
element={<AuthenticatedPageRoute><ProgramRecordsList /></AuthenticatedPageRoute>}
/>
<Route
path={ROUTES.PROGRAM_RECORD_SHARED}
element={<ProgramRecord isPublic />}
/>
<Route
path={ROUTES.PROGRAM_RECORD_ITEM}
element={<AuthenticatedPageRoute><ProgramRecord isPublic={false} /></AuthenticatedPageRoute>}
/>
{getConfig().ENABLE_VERIFIABLE_CREDENTIALS && (
Copy link
Member

Choose a reason for hiding this comment

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

I assume it's fine that you moved this? The motivation for it isn't clear.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was originally conditionally rendered inside of the the other conditional (the route to verifiable credentials would only be enabled iff both flags were enabled). I just moved it up a layer (and at the bottom of all the other routes)

<Route
path={ROUTES.PROGRAM_RECORDS}
element={<AuthenticatedPageRoute><ProgramRecordsList /></AuthenticatedPageRoute>}
path={ROUTES.VERIFIABLE_CREDENTIALS}
element={<AuthenticatedPageRoute><ProgramCertificatesList /></AuthenticatedPageRoute>}
/>
{getConfig().ENABLE_VERIFIABLE_CREDENTIALS && (
<Route
path={ROUTES.VERIFIABLE_CREDENTIALS}
element={<AuthenticatedPageRoute><ProgramCertificatesList /></AuthenticatedPageRoute>}
/>
)}
<Route
path={ROUTES.PROGRAM_RECORD_SHARED}
element={<ProgramRecord isPublic />}
/>
<Route
path={ROUTES.PROGRAM_RECORD_ITEM}
element={<AuthenticatedPageRoute><ProgramRecord isPublic={false} /></AuthenticatedPageRoute>}
/>
</Routes>
) : (
<ProgramRecordsList />
)}
)}
</Routes>
<Footer />
</HelmetProvider>
</AppProvider>,
Expand All @@ -65,7 +61,6 @@ initialize({
config: () => {
mergeConfig({
SUPPORT_URL_LEARNER_RECORDS: process.env.SUPPORT_URL_LEARNER_RECORDS || '',
USE_LR_MFE: process.env.USE_LR_MFE || false,
ENABLE_VERIFIABLE_CREDENTIALS: process.env.ENABLE_VERIFIABLE_CREDENTIALS || false,
SUPPORT_URL_VERIFIABLE_CREDENTIALS: process.env.SUPPORT_URL_VERIFIABLE_CREDENTIALS || '',
}, 'LearnerRecordConfig');
Expand Down