use anonymous user ID when calling the edxnotes retirement endpoint#18305
use anonymous user ID when calling the edxnotes retirement endpoint#18305
Conversation
bmedx
left a comment
bmedx
left a comment
There was a problem hiding this comment.
Ah dang. Sad that it doesn't 404 or something when it doesn't find a user like Forums does.
|
Your PR has finished running tests. |
|
Could we make it 404 on not found so it's a bit more uniform? |
|
Unfortunately, the notes service doesn't keep track of users, only notes. So, if the service were to return "Not Found", that would imply any one of the following:
#1 is solid grounds for a 404, but I'd argue #2 isn't. |
|
We should go through and look but I believe the forums service makes the same assumption. If a user hasn't create a forms post, they don't exist in that IDA and a 404 is returned. |
|
Forums users are created on registration these days, but if that fails we eat the error and attempt to create them at some point when they try to use the forums. Post time maybe? Either way, I think I'm with Troy on this one, if there's no concept of a user it seems weird to error that there isn't one. |
|
EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production on Monday, June 04, 2018. |
|
EdX Release Notice: This PR has been deployed to the production environment. |
5 participants
I was thrown off by the function signature of delete_all_notes_for_user taking a user_id but not explaining that it's in fact an anonymous user id (according to the model definition in the edx-notes-api repo). I simplified it to just take a django User object, and put the anonymous user magic within.
Testing this has been a bear, without loadtest being available. I'll look into deploying notes in my devstack, but honestly I think it'll be faster to let this hit stage and test there since we're currently the only consumers of this endpoint.