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

#468 #484 #470 LTI Integration #494

Merged
merged 17 commits into from
Jan 23, 2025

Conversation

pushyamig
Copy link
Contributor

@pushyamig pushyamig commented Jan 20, 2025

Fixes #468 #484 #470

This PR handles the complete LTI integration for CCM, also enabling backend debugging.

  1. LTI setup details are given in README, also LTI registration is handled by Django Admin Console with django-lti library
  2. CommandLine setup option is available for LTI registration. Otherwise create a Root user using the createsuperuser django command and login to http://0.0.0.0/admin/ with login with credential created then add the LTI Registration.
    3. Also You can look at my LTI configuration from Canvas Test and start from there.
    4. DeploymentID is needed. So just follow along the instructions
  3. The new version of Debugpy and Python version has some warning, i supressed the warning since it did not had any impact to using Debugging. Read more on this going here. As far I can tell the issue isn't resolved but they closed the issue. Any suggestion on this is helpful
  4. UI is integrated with Django Admin console link inline with Breadcrumb component.
  5. Frontend globals are now provided from context_processors.py as script tag, currently the globals are fetched using API call.
    6. useGlobals.ts hook will go away, the calls getGlobals and getCSRF is not needed. but I am going to keep that until API integration component via DRF added. But I added logic in such a way that I am injecting global from context_processors.py to useGlobals as intermediate step.
    7. The Globals will reflect LTI launch user. So login with different users and see the results
  6. globals has attribute hasCanvasToken it is hard-coded since that variable is related to Canvas OAuth integration. So Please be aware for the value hasCanvasToken=true in context_processors.py
  7. Google Analytics wired with Django Backend. But I could't see where to add /privacy/ @jaydonkrooss any suggestion. I won't test if events are tracked as of now. I felt that will be derailing to this PR. Once we have major integration then It will make sense to do that.
  8. I added DRF package but it is not fully integration but only used for serializing the ccm_globals
  9. Redis caching is also added

Next Steps:

  1. Canvas OAuth Integration Integrate the Canvas OAuthworkflow #469
  2. I did not added test yet, I will work as separate PR since I felt this PR was already getting bigger and I am giving priority for having the Major components integration. Support Test Driven Development #472

@pushyamig pushyamig changed the title I468 lti support #468 #484 #470 LTI Integration Jan 21, 2025
@pushyamig pushyamig requested a review from jonespm January 22, 2025 13:40
Copy link
Contributor

@jaydonkrooss jaydonkrooss left a comment

Choose a reason for hiding this comment

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

code looks good to me, and locally runs as expected. Google analytics is also behaving normally, the only thing missing relating to the OneTrust integration is the redirect from /privacy to the privacy link in the environment variables. I could add this in a separate issue if you'd like to move forward with this PR.

Copy link
Member

@jonespm jonespm left a comment

Choose a reason for hiding this comment

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

Changes look good. Just create that issue @jaydonkrooss mentioned if you didn't already.

@pushyamig
Copy link
Contributor Author

Google analytics is also behaving normally, the only thing missing relating to the OneTrust integration is the redirect from /privacy to the privacy link in the environment variables. I could add this in a separate issue if you'd like to move forward with this PR.

@jaydonkrooss I have created the issue since felt it will be easier to do without a re-review. This issue can be worked anytime after this PR get merged.
#496

@pushyamig pushyamig merged commit 3d34f90 into tl-its-umich-edu:develop Jan 23, 2025
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants