Remove request.POST save - incompatible with DRF v3.6.3. #15584
Conversation
|
As you can see by the code below, modifying request.POST for a rest_framework.request.Request is non-standard and requires a workaround: |
|
The request.POST attribute changed to a |
| @@ -60,7 +60,6 @@ class BulkEnrollView(APIView): | |||
| def post(self, request): | |||
| serializer = BulkEnrollmentSerializer(data=request.data) | |||
| if serializer.is_valid(): | |||
| request.POST = request.data | |||
There was a problem hiding this comment.
i see that self.request gets passed in to students_update_enrollment below, and that function accesses request.POST. however, i don't understand if self.request and request are the same object?
There was a problem hiding this comment.
It appears they are:
ipdb> id(request)
140610886734992
ipdb> id(self.request)
140610886734992
but it also appears that request.POST is the same as request.data:
ipdb> p request.POST
<QueryDict: {u'action': [u'enroll'], u'courses': [u'org.1/course_1/Run_1'], u'identifiers': [u'[email protected]'], u'email_students': [u'False']}>
ipdb> p request.data
<QueryDict: {u'action': [u'enroll'], u'courses': [u'org.1/course_1/Run_1'], u'identifiers': [u'[email protected]'], u'email_students': [u'False']}>
ipdb> id(request.POST)
140610886285712
ipdb> id(request.data)
140610886285712
So I'm unsure if there's a reason why this line exists.
|
@doctoryes I'm currently verifying this change |
There was a problem hiding this comment.
thanks for the clarification on that @doctoryes. approving this, if something comes up from @bdero's verification, i suggest addressing those issues in https://github.com/edx/edx-platform/pull/15579.
|
@doctoryes @brittneyexline I've tested this carefully, although it's difficult to test some aspects completely with this bug still present. 👍 I don't see a reason why this should be necessary as well. @macdiesel This feature was initially built by another dev group and I didn't notice this line during the initial rebasing. It's possible that this was needed for a prior DRF version as the blame says it was introduced in a commit that fixes the feature for a version range of DRF. |
|
Thanks for the prompt review, @bdero . |
|
EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production on Tuesday, July 18, 2017. |
|
EdX Release Notice: This PR has been deployed to the production environment. |
DRF v3.6.3 doesn't allow
request.POSTto be changed for DRF Requests. This change makes the Python unittests pass again.@bdero Was there a reason for this statement? The
requestisn't accessed further in this method.