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

Add data migration with standard Canvas Placement values (#54) #221

Merged
merged 6 commits into from
Jun 24, 2022

Conversation

ssciolla
Copy link
Contributor

The PR aims to resolve #54.

@ssciolla ssciolla added enhancement New feature or request backend labels Jun 23, 2022
@ssciolla ssciolla requested review from jonespm and zqian June 23, 2022 16:12
from django.db import migrations


PLACEMENT_NAMES = [
Copy link
Member

Choose a reason for hiding this comment

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

can we provide a source (from IMS, Canvas, etc) for this list of placement options? I am not sure whether this list in the Canvas LTI course is up-to-date.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe @jonespm knows a better source, but he pointed me to the external one I linked to above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps we can come up with a more refined list from this? https://canvas.instructure.com/doc/api/file.tools_intro.html

Copy link
Member

Choose a reason for hiding this comment

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

The Canvas API doc page should be good.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I updated this list and the link. I took out ones if I didn't see any indication that the were supported. Maybe some of them are for us or in Canvas Commons, per the LTI doc @zqian shared before?

'Assignment Configuration',
'Assignment Menu',
'Course Home Sub Navigation',
'Course Settings Sub Navigation',
'Discussion Menu',
'File Menu',
'Global Navigation',
'Module Menu',
'Post Grades',
'Quiz Menu',
'Tool Configuration',
'Wiki Page Menu'

Copy link
Member

@jonespm jonespm Jun 24, 2022

Choose a reason for hiding this comment

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

Canvas says that the officially documented list of placements is the below list and others are made for "internal use".

  • Account Navigation
  • Assignment Selection
  • Collaboration
  • Course Navigation
  • Editor button
  • Homework Submission
  • Link Selection
  • Migration Selection
  • User Navigation

It looks like this is the list of all possible internal and documented placements

https://github.com/instructure/canvas-lms/blob/ec4f8f6fa6f888612fb111378fe51fd1df9cbcdd/ui/features/external_apps/react/components/ExternalToolPlacementList.js#L33

I think it's fine if we just have the ones that are "officially documented" which looks like your list here in the code.

Copy link
Member

@jonespm jonespm Jun 24, 2022

Choose a reason for hiding this comment

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

One of the reasons I opted to make these configurable verses having an internal list was because Canvas seems to add these on occasion and promotes them to being "officially documented" and used. I'm not sure how often this happens though. Eventually if we implement #9 this will probably need to be reworked.

@ssciolla ssciolla merged commit de65c5c into tl-its-umich-edu:main Jun 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add some migration file to pre-populate the tables with known values
3 participants