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

Take and forward an optional assignment id in the LTI Name and Roles API #6617

Merged
merged 2 commits into from
Sep 3, 2024

Conversation

marcospri
Copy link
Member

@marcospri marcospri commented Aug 29, 2024

For:


Pass the value to the LTI service which should return only the roster relevant to the assignment.

More context and testing instructions over:

@@ -1,9 +1,6 @@
"""
Service to talk to the Name and Roles LTIA API.

We only implement this now as a way to obtain the LTI Advantage Complete
Copy link
Member Author

Choose a reason for hiding this comment

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

Not anymore!

"""
query = {}
if resource_link_id:
query["rlid"] = resource_link_id
Copy link
Member Author

Choose a reason for hiding this comment

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

rlid is the name in the API but that's a bit cryptic. It's called resource link ID everywhere else.

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps add a comment there to clarify that for the next person? Even adding a link to some documentation would be good, if such thing exists.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, added a note to the issue for this, I'll do a follow up to avoid some rebase hell.

@marcospri marcospri marked this pull request as ready for review September 2, 2024 16:24
"""
query = {}
if resource_link_id:
query["rlid"] = resource_link_id
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps add a comment there to clarify that for the next person? Even adding a link to some documentation would be good, if such thing exists.

@marcospri marcospri merged commit 7064177 into main Sep 3, 2024
9 checks passed
@marcospri marcospri deleted the names-roles-resource-link branch September 3, 2024 13:16
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