Skip to content

Conversation

@ethanstrominger
Copy link
Member

@ethanstrominger ethanstrominger commented Mar 18, 2025

Fixes #484

What changes did you make?

  • update-table.md : formatting changes
  • .env.docker-example
    • Add CORS_ALLOWED_ORIGINS
    • Remove all settings related to local development
  • Dockerfile - updated graphiz version
  • views.py: add patch method for user profile to be used. by React app
  • settings.py
    • Added corsheaders to apps (django-cors-headers package)
    • Added CORS_ALLOWED_ORIGINS, CORS_ALLOW_CREDENTIALS, and CORS_ALLOW_HEADERS
    • Added CORS to middleware
    • Changed core.utils.jwt to core.utils.jwt_handler to disambiguate from the cors package
  • requirements.txt: added django-cors-headers

Why did you make the changes?

  • make changes so you can call from REACT

Testing

ethanstrominger and others added 30 commits October 16, 2024 08:10
commit 0d70d8f
Author: Ethan Strominger <ethanstrominger2@gmail.com>
Date:   Wed Dec 18 11:42:21 2024 -0500

    Update graphviz

commit 775d753
Author: Ethan Strominger <ethanstrominger2@gmail.com>
Date:   Sun Dec 1 17:25:56 2024 -0500

    Modify to tricker pre-commit

commit c75db16
Author: Ethan Strominger <ethanstrominger2@gmail.com>
Date:   Sun Dec 1 17:23:16 2024 -0500

    Fix trailing whitespace
commit 0d70d8f
Author: Ethan Strominger <ethanstrominger2@gmail.com>
Date:   Wed Dec 18 11:42:21 2024 -0500

    Update graphviz

commit 775d753
Author: Ethan Strominger <ethanstrominger2@gmail.com>
Date:   Sun Dec 1 17:25:56 2024 -0500

    Modify to tricker pre-commit

commit c75db16
Author: Ethan Strominger <ethanstrominger2@gmail.com>
Date:   Sun Dec 1 17:23:16 2024 -0500

    Fix trailing whitespace
@github-project-automation github-project-automation bot moved this to PR Needs review (automated column, do not place items here manually) in P: PD: Project Board Mar 18, 2025
@github-project-automation github-project-automation bot moved this from PR Needs review (automated column, do not place items here manually) to ✅Done in P: PD: Project Board Mar 18, 2025
@github-project-automation github-project-automation bot moved this from ✅Done to PR Needs review (automated column, do not place items here manually) in P: PD: Project Board Mar 18, 2025
Copy link
Member

@fyliu fyliu left a comment

Choose a reason for hiding this comment

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

I see you're adding cors to allow a frontend to connect from a different domain, which is good.

I see a potential problem as well:

  1. rest_framework.authtoken adds a separate token-based login method in addition to the one provided by cognito. I don't think it's something we want to do.

@github-project-automation github-project-automation bot moved this from PR Needs review (automated column, do not place items here manually) to PR changes requested in P: PD: Project Board Jun 1, 2025
@fyliu
Copy link
Member

fyliu commented Jun 1, 2025

@ethanstrominger is there a good way to test/validate this PR?

@fyliu
Copy link
Member

fyliu commented Sep 5, 2025

This PR is not testable until the following issue is done:

Converting it to draft for now.

@fyliu fyliu marked this pull request as draft September 5, 2025 01:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: PR changes requested

Development

Successfully merging this pull request may close these issues.

Enable single sign on for React app through People Depot

2 participants