Bug 1074989 fix saving user details remote and local #46
Bug 1074989 fix saving user details remote and local #46
Conversation
var delete_setting = endpoint + 'delete_setting/'; | ||
delete_setting += '?format=json'; // TODO bug 1088014 | ||
return request_with_auth(delete_setting, 'POST', setting) | ||
.then(function(response) { |
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.
We need to be able to set this in one go. Please work with Mike in bug 1113056 to get this.
Currently, this code fails for me on the delete because it sends a 404 with the error: "No matching user setting found."
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.
Yes, I got into the same scenario with metadata and worked around that for now. We definitely need to change the way this works.
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 will add the same workaround I did for the metadata stuff here, unless you feel we should wait for the server side bits before continuing with this.
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.
In general, I wouldn't mind landing a workaround until the server side parts is fixed. But this workaround fails because we get a 404 from the delete call when we've never created those settings. Let's wait for bug 1118037 to land.
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.
Yes, I also have a temp workaround for the 404 on delete. Let me add then, and you can have a look. Seeing I already did it for the metadata PR, it's not really extra work from my side.
I need to clean this one... |
@@ -154,6 +166,29 @@ | |||
endpoint += '?format=json'; // TODO bug 1088014 | |||
|
|||
return request(endpoint, 'GET').then(JSON.parse); | |||
}, | |||
|
|||
update_settings: function(user, setting) { |
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.
update_user_settings to avoid confusion
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.
You haven't addressed this.
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 was waiting on feedback on the below so that I could address all three in one commit.
…-details-remote-and-local Bug 1074989 fix saving user details remote and local
@rik r? This one depends on #45