Ability to edit user profiles #36
Conversation
…and get_account_card + lint fixes
Codecov Report
@@ Coverage Diff @@
## develop #36 +/- ##
===========================================
+ Coverage 90.1% 90.28% +0.17%
===========================================
Files 74 74
Lines 2739 2800 +61
Branches 148 150 +2
===========================================
+ Hits 2468 2528 +60
Misses 248 248
- Partials 23 24 +1
Continue to review full report at Codecov.
|
src/foobar/api.py
Outdated
| return None | ||
|
|
||
|
|
||
| def get_account_card(card_id): |
There was a problem hiding this comment.
I would argue that the name is a bit inaccurate. It sounds like the function is supposed to return the card that is associated with given account, while it is doing the opposite. Maybe get_account_for_card or get_account_by_card instead?
src/foobar/api.py
Outdated
| card_obj = get_card(card_id) | ||
| if card_obj is not None: | ||
| return card_obj.account | ||
|
|
||
|
|
||
| def set_account(account_id, **kwargs): |
There was a problem hiding this comment.
update_account would be more appropriate here imo.
src/foobar/forms.py
Outdated
|
|
||
|
|
||
| class EditProfileForm(forms.Form): | ||
| name = forms.CharField(label="Account Name", max_length=128) |
There was a problem hiding this comment.
The labels should be translated.
| @@ -0,0 +1,4 @@ | |||
| {% extends 'profile/base_profile.html' %} | |||
| {% block content %} | |||
| <div class="account_form" style="text-align:center;">Invalid QRcode <br> Login to get a new </div> | |||
There was a problem hiding this comment.
The text here should be translated.
| </head> | ||
| <body> | ||
| <div id="header" > | ||
| Profile Details |
| <form class="account_form" method="post" action=""> | ||
| {% csrf_token %} | ||
| {{ form }} | ||
| <input type="submit" name="save_changes" value="Save" id="submit_changes" value="{{ account_email }}"> |
src/foobar/views.py
Outdated
|
|
||
| def edit_profile(request, token): | ||
| form_class = EditProfileForm(request.POST or None) | ||
| list(messages.get_messages(request)) |
There was a problem hiding this comment.
Some temporary code that you forgot to remove before committing?
src/foobar/views.py
Outdated
| return render(request, "profile/bad_request.html") | ||
|
|
||
| if request.method == 'POST': | ||
| if 'save_changes' in request.POST: |
There was a problem hiding this comment.
This check is not needed as there is only one form on the page.
src/foobar/views.py
Outdated
| name=form_class.cleaned_data['name'], | ||
| email=form_class.cleaned_data['email']) | ||
| messages.add_message(request, messages.INFO, | ||
| 'Successfully Saved') |
src/foobar/models.py
Outdated
|
|
||
| @property | ||
| def isComplete(self): | ||
| return bool(self.email) |
There was a problem hiding this comment.
Our convention here is underscore case (i.e. is_complete here)
There was a problem hiding this comment.
It is not just our convention. PEP8 does actually say that the underscore case is the preferred naming method.
Method Names and Instance Variables
Use the function naming rules: lowercase with words separated by underscores as necessary to improve readability.
src/foobar/tests/test_views.py
Outdated
| token = signing.dumps({'id': str(account_obj.id)}) | ||
| url = reverse('edit_profile', kwargs={'token': token}) | ||
| bad_token = reverse('edit_profile', kwargs={'token': 'bad'}) | ||
| cl = self.client |
There was a problem hiding this comment.
Any particular reason to store the test client into cl?
Otherwise I would argue we want to be explicit(for instance by doing self.client.get).
|
Now that people can update their e-mail addresses, we should make it possible to see and edit them in the admin panel. |
|
🚀 |
No description provided.