-
Notifications
You must be signed in to change notification settings - Fork 175
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
Issue/636 duplicate assignment #637
Open
dsavransky
wants to merge
6
commits into
ucfopen:develop
Choose a base branch
from
dsavransky:issue/636-Duplicate-Assignment
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
5e3c74d
fixing minor docstring errors in course.create_page
dsavransky d956264
Merge branch 'develop' into develop
Thetwam ed9a4bf
Merge remote-tracking branch 'upstream/develop' into develop
dsavransky dcff525
Merge branch 'ucfopen:develop' into develop
dsavransky 167d488
Merge branch 'ucfopen:develop' into develop
dsavransky d8a7d90
adding duplicate assignment endpoint coverage and tests for #636
dsavransky File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might be overthinking it, but if the
result_type
key is passed, it will return aQuiz
object rather than anAssignment
. I'm not that type-safe when writing, but this could be a case where someone runs into issues. @Thetwam is it worth adding a test?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. Yeah we'll want to handle that case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @bennettscience and @Thetwam. Happy to make changes, but I could use a little guidance. I've tested this out on my university's Canvas installation, and I can get two qualitatively different outputs if I pass the
result_type="Quiz"
kwarg. However, in each case, the duplicated assignment on Canvas is just an ordinary assignment - the only thing that's different are the keywords in the response json that's generated. Similarly, I can pass either of these outputs into an Assignment object or into a Quiz object, with no issues either way. I also get failures when trying to duplicate anything other than a regular assignment (e.g. a quiz or an external tool assignment).So: what exactly do you want to happen here? Do you want
Assignment.duplicate
to check the format of the returned json and return a Quiz object in the case whereresult_type="Quiz"
, or should it still always return an Assignment object regardless of inputs? In the latter case, maybe I'd need to manually set theis_quiz_assignment
attribute?I'm struggling a bit, as I don't really understand the utility or intent of the result_type functionality in the original API, so any help would be greatly appreciated. Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dsavransky Can you see if your instance has the
newquizzes_on_quiz_page
setting enabled? Re-reading the docs, that setting has to be enabled for the Quiz keyword to serialize it in aQuiz
object:You shouldn't need to specify the
is_quiz_assignment
attribute because that's sent from Canvas. Maybe that's a better indicator?I'm not sure what the intent was on Instructure's side other than to separate out New Quiz functions from Classic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have admin access to the Canvas instance I'm using for testing, so I'm not actually sure how to check that setting (open to suggestions). Here's what I observe: when I omit the result_type keyword, the JSON that's returned includes the
is_quiz_assignment
key. When I setresult_type="Quiz"
, that key is not in the returned JSON. Instead, it includes:'quiz_type': 'quizzes.next'
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bennettscience @Thetwam I'd like to eventually get this merged in if possible - could you please provide any guidance on your preferred solution here?