forked from openedx/edx-platform
-
Notifications
You must be signed in to change notification settings - Fork 1
Lilac: Remote gradebook & Canvas cherry-picks #256
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
Merged
Merged
Conversation
This file contains hidden or 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
* Sync canvas enrollments * Added instructor dashboard button to push edX grades to Canvas * Fixed canvas grade syncing JS * Changed 'edX' reference to 'MITx' * Use EDIT_COURSE_ACCESS permission instead of is_staff * Use OVERRIDE_GRADES rule instead * Use instructor_tasks for canvas work (#183) * Add per_page to paginated requests * Static method * Fix email lowercase mismatch * Use instructor_tasks to handle canvas work * Fix transaction error * Decorator needs to be at the top * Attempt to fix polling of tasks * More task bug fixes * Fix typo * Fix course key bug * Update message for push edx grades command * Use course_key so sync_canvas_enrollments uses the same message * Format the submitted time * Don't return output to prevent task output max size error (#189) Co-authored-by: George Schneeloch <[email protected]> Co-authored-by: Gavin Sidebottom <[email protected]>
* Adding enrollment restriction for users based on social auth providers In order to prevent users outside of MIT creating a collaborator account and self-enrolling in courses we need to be able to restrict self-enrollment to users who authenticate using the MIT Kerberos SAML backend. This adds filtering of self-enrollment based on a feature flag that sets the allowed provider ids. (cherry picked from commit a4c53a1) * Bugfix for collaborator account access check Co-authored-by: Tobias Macey <[email protected]>
edX requires the full namespace for imports so importing e.g. `remote_gradebook.tasks` is no longer valid. Instead it requires `from lms.djangoapps.remote_gradebook import tasks`.
- Change submission type to "None" - Change published state to "False"
Posting to the remote gradebook via lmod proxy was failing due to a key error when parsing the CSV content. This was due to erroneously encoding a string object. This commit adds a check for the type of the header value to prevent double encoding it.
pdpinch
reviewed
Apr 15, 2021
pdpinch
reviewed
Apr 15, 2021
pdpinch
requested changes
Apr 15, 2021
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.
A couple of minor comments. Otherwise, I think we should merge this and see how it works on QA.
@pdpinch I have addressed the feedback so that's ready for review again @blarghmatey here is the isolated PR for that #257 |
Looks good to me |
blarghmatey
added a commit
that referenced
this pull request
Sep 7, 2021
* Canvas Commits (#223) * Sync canvas enrollments * Added instructor dashboard button to push edX grades to Canvas * Fixed canvas grade syncing JS * Changed 'edX' reference to 'MITx' * Use EDIT_COURSE_ACCESS permission instead of is_staff * Use OVERRIDE_GRADES rule instead * Use instructor_tasks for canvas work (#183) * Add per_page to paginated requests * Static method * Fix email lowercase mismatch * Use instructor_tasks to handle canvas work * Fix transaction error * Decorator needs to be at the top * Attempt to fix polling of tasks * More task bug fixes * Fix typo * Fix course key bug * Update message for push edx grades command * Use course_key so sync_canvas_enrollments uses the same message * Format the submitted time * Don't return output to prevent task output max size error (#189) Co-authored-by: George Schneeloch <[email protected]> Co-authored-by: Gavin Sidebottom <[email protected]> * Reimplemented legacy dashboard enrollment data functionality (60a6ddc) * Fix syntax error in import statement * Cherry-Picked: Enrolment commits (#228) * Adding enrollment restriction for users based on social auth providers In order to prevent users outside of MIT creating a collaborator account and self-enrolling in courses we need to be able to restrict self-enrollment to users who authenticate using the MIT Kerberos SAML backend. This adds filtering of self-enrollment based on a feature flag that sets the allowed provider ids. (cherry picked from commit a4c53a1) * Bugfix for collaborator account access check Co-authored-by: Tobias Macey <[email protected]> * Fixing import path for canvas_integration plugin * Fix import namespace for relative imprts edX requires the full namespace for imports so importing e.g. `remote_gradebook.tasks` is no longer valid. Instead it requires `from lms.djangoapps.remote_gradebook import tasks`. * #229 change submission type and published state on canvas (#232) - Change submission type to "None" - Change published state to "False" * Fixing encoding of header row for posting to remote gradebook Posting to the remote gradebook via lmod proxy was failing due to a key error when parsing the CSV content. This was due to erroneously encoding a string object. This commit adds a check for the type of the header value to prevent double encoding it. * fix the imports * address feedback Co-authored-by: Asad Iqbal <[email protected]> Co-authored-by: George Schneeloch <[email protected]> Co-authored-by: Gavin Sidebottom <[email protected]> Co-authored-by: sar <[email protected]> Co-authored-by: Arslan Ashraf <[email protected]> Co-authored-by: Tobias Macey <[email protected]>
arslanashraf7
added a commit
that referenced
this pull request
Oct 29, 2021
* Canvas Commits (#223) * Sync canvas enrollments * Added instructor dashboard button to push edX grades to Canvas * Fixed canvas grade syncing JS * Changed 'edX' reference to 'MITx' * Use EDIT_COURSE_ACCESS permission instead of is_staff * Use OVERRIDE_GRADES rule instead * Use instructor_tasks for canvas work (#183) * Add per_page to paginated requests * Static method * Fix email lowercase mismatch * Use instructor_tasks to handle canvas work * Fix transaction error * Decorator needs to be at the top * Attempt to fix polling of tasks * More task bug fixes * Fix typo * Fix course key bug * Update message for push edx grades command * Use course_key so sync_canvas_enrollments uses the same message * Format the submitted time * Don't return output to prevent task output max size error (#189) Co-authored-by: George Schneeloch <[email protected]> Co-authored-by: Gavin Sidebottom <[email protected]> * Reimplemented legacy dashboard enrollment data functionality (60a6ddc) * Fix syntax error in import statement * Cherry-Picked: Enrolment commits (#228) * Adding enrollment restriction for users based on social auth providers In order to prevent users outside of MIT creating a collaborator account and self-enrolling in courses we need to be able to restrict self-enrollment to users who authenticate using the MIT Kerberos SAML backend. This adds filtering of self-enrollment based on a feature flag that sets the allowed provider ids. (cherry picked from commit a4c53a1) * Bugfix for collaborator account access check Co-authored-by: Tobias Macey <[email protected]> * Fixing import path for canvas_integration plugin * Fix import namespace for relative imprts edX requires the full namespace for imports so importing e.g. `remote_gradebook.tasks` is no longer valid. Instead it requires `from lms.djangoapps.remote_gradebook import tasks`. * #229 change submission type and published state on canvas (#232) - Change submission type to "None" - Change published state to "False" * Fixing encoding of header row for posting to remote gradebook Posting to the remote gradebook via lmod proxy was failing due to a key error when parsing the CSV content. This was due to erroneously encoding a string object. This commit adds a check for the type of the header value to prevent double encoding it. * fix the imports * address feedback Co-authored-by: Asad Iqbal <[email protected]> Co-authored-by: George Schneeloch <[email protected]> Co-authored-by: Gavin Sidebottom <[email protected]> Co-authored-by: sar <[email protected]> Co-authored-by: Arslan Ashraf <[email protected]> Co-authored-by: Tobias Macey <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
fixes #255
Cherry-pick the following commits into one commit and test that it works on the mitx/lilac branch.
(most recent at the top)
608828d
e630376
e52d609
b331778
697ce49
1374507
63ceb85
ca3a1a6
I don't know how to test it but after making all these changes, my devstack is still functional. I had faced some import issues that I have fixed in the last commit to make it functional.
Screenshots