Skip to content

Conversation

@jonkafton
Copy link
Contributor

@jonkafton jonkafton commented Oct 23, 2024

What are the relevant tickets?

https://github.com/mitodl/hq/issues/5846

Description (What does it do?)

Removes the ./frontends/mit-learn workspace and any references. This was the entrypoint for the previous static frontend, now replaced by the Next.js entrypoint at ./frontends/main.

Also removes the E2E/browser tests as these have been inactive and have not been migrated to work with Next.js.

How can this be tested?

Generally, all tests should pass on frontend and backend and services should build and run with Docker without error.

@gitguardian
Copy link

gitguardian bot commented Oct 23, 2024

⚠️ GitGuardian has uncovered 2 secrets following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secrets in your pull request
GitGuardian id GitGuardian status Secret Commit Filename
9430286 Triggered Generic Password 2b15331 docker-compose-e2e-tests.yml View secret
9430286 Triggered Generic Password 2b15331 docker-compose-e2e-tests.yml View secret
🛠 Guidelines to remediate hardcoded secrets
  1. Understand the implications of revoking this secret by investigating where it is used in your code.
  2. Replace and store your secrets safely. Learn here the best practices.
  3. Revoke and rotate these secrets.
  4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

@jonkafton jonkafton marked this pull request as draft October 23, 2024 16:03
"BACKEND": "django.template.backends.django.DjangoTemplates",
"DIRS": [
BASE_DIR + "/templates/",
BASE_DIR + "/frontends/mit-learn/build",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this needed for anything?

Copy link
Contributor

Choose a reason for hiding this comment

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

It was necessary when django served the JS


# Placeholder build for use in django tests.
mkdir ./frontends/mit-learn/build
touch ./frontends/mit-learn/build/index.html
Copy link
Contributor Author

@jonkafton jonkafton Oct 23, 2024

Choose a reason for hiding this comment

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

Is this needed for anything?

Copy link
Contributor

Choose a reason for hiding this comment

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

Only back when django served the JS

"main",
"public",
"images",
"blank.png",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added a blank.png at this path, but do we actually need it for this test? It's not used in the frontend.

Copy link
Contributor

@ChristopherChudzicki ChristopherChudzicki Oct 23, 2024

Choose a reason for hiding this comment

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

It's "supposed" to be the fallback for embedly. See frontends/ol-utilities/src/learning-resources/learning-resources.ts

That said:

  • we don't need the /static version
  • The version we do have set isn't working right now (https://rc.learn.mit.edu/static/images/blank.png) ... unsure why
    • EDIT: I forgot I was on your branch. The link I posted does not work, but should after this release, I think.

@jonkafton jonkafton added the Needs Review An open Pull Request that is ready for review label Oct 23, 2024
@jonkafton jonkafton marked this pull request as ready for review October 23, 2024 16:12
Copy link
Contributor

@ChristopherChudzicki ChristopherChudzicki left a comment

Choose a reason for hiding this comment

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

Approved, but could you change the url in

const BLANK_IMAGE = "https://rc.learn.mit.edu/static/images/blank.png"

to use NEXT_PUBLIC_ORIGIN

@jonkafton jonkafton merged commit c0ac29b into main Oct 23, 2024
11 checks passed
@rhysyngsun rhysyngsun deleted the jk/5846-remove-mit-learn branch February 7, 2025 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs Review An open Pull Request that is ready for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants