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

SAK-50682 LTI Add support for 1EdTech PNP URL to launch data #13025

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

csev
Copy link
Contributor

@csev csev commented Nov 16, 2024

No description provided.

@csev csev marked this pull request as draft November 16, 2024 17:10
@csev csev requested a review from ern November 16, 2024 17:10
@paulgrudnitski
Copy link

This 100% reproduces the pnpservice claim that I have coded into my local Moodle 4.02. Very happy with this...and thank you.

However, what I thought was the proper encoding of the claim appears to be different than what is currently published in the Candidate Final draft. Consequently, I've opened an issue with the QTI Working Group to reconcile the differences between the Public Candidate Final doc and what we in the QTI Working Group had defined (this is the "spec" that I had been working off of).

I would not merge this until we (QTI Working Group and possibly the LTI Working Group) can reconcile this. I think we all want to avoid some churn (rework) on the claim.

@csev
Copy link
Contributor Author

csev commented Nov 17, 2024

@paulgrudnitski I had Claude Vervoort (LTI co-chair) check the actual PNP repo - not the draft spec online. Claude verified to me that both the Moodle code you have and the Sakai code I now have are correct. It is pretty clear the web page is just out of date. We can test that this patch works with your code from a branch. We can verify it and then merge it later when we triple check the spec.

@csev csev closed this Nov 17, 2024
@csev csev reopened this Nov 17, 2024
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.

2 participants