Add ability to disconnect elsewhere accounts. #1976
Conversation
First step towards fixing #639.
|
This is blocked by #2047, where the new version of Aspen changes how JSON error messages are served. |
Conflicts: gittip/elsewhere/__init__.py gittip/models/_mixin_elsewhere.py templates/connected-accounts.html tests/test_elsewhere.py tests/test_participant.py
|
I have merged master into this, and made a style change (the old style was broken). |
|
Error messages are now working. I think this PR is ready. |
| data = dict(platform=platform, user_id=user_id) | ||
| response = self.client.PxST('/alice/elsewhere.json', data, auth_as='alice') | ||
| assert response.code == 400 | ||
| assert "last login" in response.body |
There was a problem hiding this comment.
I think it'd be better to test for the full error message
There was a problem hiding this comment.
What if we raise a subclass of Response instead of an instance of Response? Then we could use pytest.raises to look for that specifically, instead of checking the response body.
That means we push aspen.Response down into the gittip library, which might feel wrong.
There was a problem hiding this comment.
I like testing for the error message itself, because that's what actually gets sent to the user. It ensures that a human-intended error message is being sent, as opposed to something useless like "ERR 234325"
| if len(accounts) == 1: | ||
| raise LastElsewhere() | ||
| c.one(""" | ||
| DELETE FROM elsewhere |
There was a problem hiding this comment.
Why delete from elsewhere here? Why not just set elsewhere.participant to a stub?
There was a problem hiding this comment.
DELETE FROM elsewhere is our current account disconnect process, so it's fine here. IRC
Add ability to disconnect elsewhere accounts.
I have separated elsewhere accounts into 'login' and the rest and I try to make sure there stays at least one login account. The UI is a bit simplistic but it is the best I could do - I am no UI person 😓. Please review.
Fix #639.