-
Notifications
You must be signed in to change notification settings - Fork 174
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
base: develop
Are you sure you want to change the base?
Issue/636 duplicate assignment #637
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #637 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 73 73
Lines 3732 3735 +3
=========================================
+ Hits 3732 3735 +3
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
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 a Quiz
object rather than an Assignment
. 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 where result_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 the is_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 a Quiz
object:
When the root account has the feature ‘newquizzes_on_quiz_page` enabled and this argument is set to “Quiz” the response will be serialized into a quiz format(quizzes);
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 set result_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?
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.
Proposed Changes
Fixes #636 .