Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

[Fix bug 844764] A user should be able to view a list of invitations #854

Closed
wants to merge 1 commit into from

3 participants

@leogau

No description provided.

mozillians/phonebook/helpers.py
@@ -49,3 +49,10 @@ def langcode_to_name(code, locale=None):
except KeyError:
return code
return lang
+
+@register.inclusion_tag('phonebook/includes/invite_card.html')
+@jinja2.contextfunction
+def invite_card(context, invite):
+ d = dict(context.items())
+ d.update(invite=invite)
+ return d
@penthu
penthu added a note

  • W292 no newline at end of file
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
mozillians/phonebook/helpers.py
@@ -49,3 +49,10 @@ def langcode_to_name(code, locale=None):
except KeyError:
return code
return lang
+
+@register.inclusion_tag('phonebook/includes/invite_card.html')
@penthu
penthu added a note

  • E302 expected 2 blank lines, found 1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@glogiotatidis

@leogau Thanks for contributing! First I'll have to assign this bug to you so can you please make a comment in the bug?

@glogiotatidis

Nevermind I just saw your comment. Thanks!

@glogiotatidis glogiotatidis changed the title from NOTREADY - [Fix bug 844764] A user should be able to view a list of invitations to [Fix bug 844764] A user should be able to view a list of invitations
mozillians/phonebook/urls.py
@@ -20,6 +20,7 @@
url(r'^search/$', 'views.search', name='search'),
url(r'^vouch/$', 'views.vouch', name='vouch'),
url(r'^invite/$', 'views.invite', name='invite'),
+ url(r'^delete_invite/(?P<invite_pk>\d+)/$', 'views.delete_invite', name='delete_invite'),
@glogiotatidis Owner

Let's make this url ^invite/(?P<invite_pk>\d+)/delete/$ so that we respect invite's namespace and be prettier.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
mozillians/phonebook/views.py
((5 lines not shown))
return render(request, 'phonebook/invite.html',
- {'invite_form': invite_form})
+ {'invite_form': invite_form, 'invites': invites})
+
+
+@require_POST
+def delete_invite(request, invite_pk):
@glogiotatidis Owner

The way this is now one can delete invites from other mozillians.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
mozillians/phonebook/views.py
((5 lines not shown))
return render(request, 'phonebook/invite.html',
- {'invite_form': invite_form})
+ {'invite_form': invite_form, 'invites': invites})
+
+
+@require_POST
+def delete_invite(request, invite_pk):
+ deleted_invite = Invite.objects.get(pk=invite_pk)
+ msg = _(u"%s's invitation to Mozillians has been revoked. You can "
+ u"invite %s again if you like." %
+ (deleted_invite.recipient, deleted_invite.recipient))
+ messages.success(request, msg)
+ deleted_invite.delete()
@glogiotatidis Owner

Let's first delete, than add success message. This way if anything breaks users do not get a false message.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
mozillians/templates/phonebook/invite.html
@@ -35,4 +35,12 @@
</p>
</form>
+ {% if invites %}
+ <h1>{{ _('Your Invites') }}</h1>
+
+ {% for invite in invites %}
+ {{ invite_card(invite) }}
@glogiotatidis Owner

Since invite_card is only used here here is no need for a separate file. I suggest that you bring the contents of invite_card.html into this file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
mozillians/templates/phonebook/invite.html
@@ -35,4 +35,12 @@
</p>
</form>
+ {% if invites %}
@glogiotatidis Owner

if invites.exist()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
mozillians/templates/phonebook/includes/invite_card.html
@@ -0,0 +1,22 @@
+<div class="card">
+ <ul>
+ <li>
+ <a title="{{ invite.recipient }}" href="mailto:{{ invite.recipient }}">
+ <i class="icon-envelope"></i> {{ invite.recipient }}
+ </a>
+ </li>
+ <li>
+ Sent: {{ invite.created }}
+ </li>
+ {% if invite.redeemed %}
+ <li>
@glogiotatidis Owner

Code inside template tags should be indented by two spaces in. Here and in other places too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
mozillians/phonebook/helpers.py
@@ -49,3 +49,11 @@ def langcode_to_name(code, locale=None):
except KeyError:
return code
return lang
+
+
+@register.inclusion_tag('phonebook/includes/invite_card.html')
+@jinja2.contextfunction
+def invite_card(context, invite):
@glogiotatidis Owner

Can you please explain why this is needed and where it's used?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@leogau

@glogiotatidis I've updated based on feedback. Please take a look let me know what you think.

Thanks,
Leo

mozillians/phonebook/views.py
((5 lines not shown))
return render(request, 'phonebook/invite.html',
- {'invite_form': invite_form})
+ {'invite_form': invite_form, 'invites': invites})
+
+
+@require_POST
+def delete_invite(request, invite_pk):
+ profile = request.user.userprofile
+ try:
+ deleted_invite = Invite.objects.get(pk=invite_pk, inviter=profile)
+ except Invite.DoesNotExist:
+ return
@glogiotatidis Owner

Django views should return a Web response. For example here you could raise Http404. There is a shortcut use you use, that does what you want:

Replace the whole try/except block with

deleted_invite = get_object_or_404(Invite, pk=invite_pk, inviter=profile)

https://docs.djangoproject.com/en/dev/topics/http/shortcuts/#get-object-or-404

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
mozillians/phonebook/views.py
@@ -297,8 +297,25 @@ def invite(request):
messages.success(request, msg)
return redirect('phonebook:home')
+ invites = Invite.objects.filter(inviter=profile)
@glogiotatidis Owner

I prefer using profile.invites to Invite.objects.filter(inviter=profile). It's shorter and less error prone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
mozillians/static/mozillians/css/main.less
@@ -1788,3 +1788,17 @@ body#invite {
input { margin-bottom: 25px; }
}
+
+body#invite .card {
+ float: left;
+ margin-right: 30px;
+}
+
+body#invite .card li {
+ list-style-type: none;
+ margin-left: 0;
+}
+
+body#invite .card button {
+ margin-top: 30px;
+}
@glogiotatidis Owner

Since this is less you can

body#invite {
    min-height: 500px;
    button[type=submit] {
        margin: 20px 0;
        .button;
    }
    textarea {
        @media (max-width: @breakMobileLandscape) {
            width: 80%;
        }
    }
    input { margin-bottom: 25px; }

    .card {
        float: left;
        margin-right: 30px;
    }

    .card li {
        list-style-type: none;
        margin-left: 0;
    }
    .card button {
        margin-top: 30px;
    }
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
mozillians/phonebook/views.py
((5 lines not shown))
return render(request, 'phonebook/invite.html',
- {'invite_form': invite_form})
+ {'invite_form': invite_form, 'invites': invites})
+
+
+@require_POST
+def delete_invite(request, invite_pk):
+ profile = request.user.userprofile
+ try:
+ deleted_invite = Invite.objects.get(pk=invite_pk, inviter=profile)
+ except Invite.DoesNotExist:
+ return
+
+ deleted_invite.delete()
+ msg = _(u"%s's invitation to Mozillians has been revoked. You can "
@glogiotatidis Owner

Function _() helps us translate strings. We first need to get the string translated and then replace format the arguments (i.e. replace %s with the arguments given). So this block should be like

    msg = (_(u"%s's invitation to Mozillians has been revoked."
             u"You can invite %s again if you like.") %
           (deleted_invite.recipient, deleted_invite.recipient))

Please note where the parenthesis start and end.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
mozillians/templates/phonebook/invite.html
((5 lines not shown))
+ <h1>{{ _('Your Invites') }}</h1>
+
+ {% for invite in invites %}
+ <div class="card">
+ <ul>
+ <li>
+ <a title="{{ invite.recipient }}" href="mailto:{{ invite.recipient }}">
+ <i class="icon-envelope"></i> {{ invite.recipient }}
+ </a>
+ </li>
+ <li>
+ Sent: {{ invite.created }}
+ </li>
+ {% if invite.redeemed %}
+ <li>
+ Redeemed: {{ invite.redeemed }}
@glogiotatidis Owner

Add a line with the redeemer and a link to the profile.

@glogiotatidis Owner

Redeemed, needs to be translatable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@glogiotatidis

@leogau Thanks for the update. We're almost there! Please make the suggested changes and ping me to review again.

@glogiotatidis

Also please change your commit message to [fix bug 844764] List user's invites

@leogau

@glogiotatidis Thanks for the feedback. I've made the updates and they're ready for review.

mozillians/phonebook/views.py
@@ -298,7 +298,20 @@ def invite(request):
return redirect('phonebook:home')
return render(request, 'phonebook/invite.html',
- {'invite_form': invite_form})
+ {'invite_form': invite_form, 'invites': profile.invites.all()})
+
+
+@require_POST
+def delete_invite(request, invite_pk):
+ profile = request.user.userprofile
+ deleted_invite = get_object_or_404(Invite, pk=invite_pk, inviter=profile)
@glogiotatidis Owner

Let's make this get_object_or_404(Invite, pk=invite_pk, inviter=profile, redeemed=None) to make sure that one cannot delete already redeemed invites.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@leogau

@glogiotatidis That sounds good. Made the change.

mozillians/templates/phonebook/invite.html
@@ -35,4 +35,40 @@
</p>
</form>
+ {% if invites.exists() %}
+ <h1>{{ _('Your Invites') }}</h1>
+
+ {% for invite in invites %}
+ <div class="card">
+ <ul>
+ <li>
+ <a title="{{ invite.recipient }}" href="mailto:{{ invite.recipient }}">
+ <i class="icon-envelope"></i> {{ invite.recipient }}
+ </a>
+ </li>
+ <li>
+ Sent: {{ invite.created }}
@glogiotatidis Owner

Sent needs to be translatable. {{ _('Sent:') }}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
mozillians/templates/phonebook/invite.html
((8 lines not shown))
+ <div class="card">
+ <ul>
+ <li>
+ <a title="{{ invite.recipient }}" href="mailto:{{ invite.recipient }}">
+ <i class="icon-envelope"></i> {{ invite.recipient }}
+ </a>
+ </li>
+ <li>
+ Sent: {{ invite.created }}
+ </li>
+ {% if invite.redeemed %}
+ <li>
+ Redeemed: {{ invite.redeemed }}
+ </li>
+ <li>
+ Redeemer:
@glogiotatidis Owner

Needs to be translatable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
mozillians/phonebook/views.py
@@ -5,7 +5,7 @@
from django.contrib.auth.models import User
from django.core.paginator import Paginator, EmptyPage, PageNotAnInteger
from django.http import Http404, HttpResponseBadRequest
-from django.shortcuts import render
+from django.shortcuts import render, get_object_or_404
@glogiotatidis Owner

get_object_or_404 before render. We import in alphabetical order, you can find more info here http://mozweb.readthedocs.org/en/latest/coding.html#python

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@glogiotatidis

@leogau do you feel comfortable to write some tests for invite deletion?
We need:

  • One test with a valid invite actually gets deleted
  • One test with a valid invite but not belonging to the requester, that returns 404
  • One test with a already redeemed invite that returns 404
  • One test with an invalid invite.

There should live under mozillians/phonebook/tests/test_views_misc:InviteTests.

If that's over your head I'm happy to help.

@leogau

@glogiotatidis I'll give it a shot and ping you if I have trouble. Thanks!

@glogiotatidis glogiotatidis changed the title from [Fix bug 844764] A user should be able to view a list of invitations to WIP - [Fix bug 844764] A user should be able to view a list of invitations
@leogau

@glogiotatidis Added those tests. Let me know what you think.

@glogiotatidis glogiotatidis changed the title from WIP - [Fix bug 844764] A user should be able to view a list of invitations to [Fix bug 844764] A user should be able to view a list of invitations
@glogiotatidis glogiotatidis commented on the diff
mozillians/phonebook/tests/test_views_misc.py
@@ -12,7 +13,7 @@
from mozillians.common.tests import TestCase, requires_login, requires_vouch
from mozillians.phonebook.models import Invite
-from mozillians.phonebook.tests import _get_privacy_fields
+from mozillians.phonebook.tests import _get_privacy_fields, InviteFactory
@glogiotatidis Owner

We import in alphabetical order, capitals first. You can find more about our styling here http://mozweb.readthedocs.org/en/latest/coding.html#python

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@glogiotatidis glogiotatidis commented on the diff
mozillians/phonebook/tests/test_views_misc.py
@@ -87,6 +88,45 @@ def test_invite_already_vouched(self):
ok_('recipient' in response.context['invite_form'].errors)
eq_(Invite.objects.all().count(), 0)
+ def test_invite_delete(self):
+ user = UserFactory.create(userprofile={'is_vouched': True})
+ invite = InviteFactory.create(inviter=user.userprofile)
+ url = reverse('phonebook:delete_invite', prefix='/en-US/', kwargs={'invite_pk': invite.pk})
+ with self.login(user) as client:
+ response = client.post(url, follow=True)
+
+ eq_(Invite.objects.all().count(), 0)
+ self.assertTemplateUsed(response, 'phonebook/invite.html')
@glogiotatidis Owner

Let's just assert the status_code is 200 here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@glogiotatidis

Great job @leogau ! r+ :rocket: I noted a couple of nits which I'm going to fix and merge so this is not blocked. Thanks so much for this contribution!

@glogiotatidis

Merged here: 5cea6da

@leogau

Thanks @glogiotatidis ! Would you mind vouching for me on mozillians?

@glogiotatidis

Done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 30, 2014
  1. @leogau
This page is out of date. Refresh to see the latest.
View
42 mozillians/phonebook/tests/test_views_misc.py
@@ -1,4 +1,5 @@
import os.path
+from datetime import datetime
from django.contrib.auth.models import User
@@ -12,7 +13,7 @@
from mozillians.common.tests import TestCase, requires_login, requires_vouch
from mozillians.phonebook.models import Invite
-from mozillians.phonebook.tests import _get_privacy_fields
+from mozillians.phonebook.tests import _get_privacy_fields, InviteFactory
@glogiotatidis Owner

We import in alphabetical order, capitals first. You can find more about our styling here http://mozweb.readthedocs.org/en/latest/coding.html#python

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
from mozillians.users.managers import MOZILLIANS, PRIVILEGED
from mozillians.users.models import UserProfilePrivacyModel
from mozillians.users.tests import UserFactory
@@ -87,6 +88,45 @@ def test_invite_already_vouched(self):
ok_('recipient' in response.context['invite_form'].errors)
eq_(Invite.objects.all().count(), 0)
+ def test_invite_delete(self):
+ user = UserFactory.create(userprofile={'is_vouched': True})
+ invite = InviteFactory.create(inviter=user.userprofile)
+ url = reverse('phonebook:delete_invite', prefix='/en-US/', kwargs={'invite_pk': invite.pk})
+ with self.login(user) as client:
+ response = client.post(url, follow=True)
+
+ eq_(Invite.objects.all().count(), 0)
+ self.assertTemplateUsed(response, 'phonebook/invite.html')
@glogiotatidis Owner

Let's just assert the status_code is 200 here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+
+ def test_invite_delete_invalid_requester(self):
+ user = UserFactory.create(userprofile={'is_vouched': True})
+ invite = InviteFactory.create(inviter=user.userprofile)
+ url = reverse('phonebook:delete_invite', prefix='/en-US/', kwargs={'invite_pk': invite.pk})
+ invalid_requester = UserFactory.create(userprofile={'is_vouched': True})
+ with self.login(invalid_requester) as client:
+ response = client.post(url)
+
+ eq_(Invite.objects.all().count(), 1)
+ eq_(response.status_code, 404)
+
+ def test_invite_delete_redeemed(self):
+ user = UserFactory.create(userprofile={'is_vouched': True})
+ invite = InviteFactory.create(inviter=user.userprofile, redeemed=datetime.now())
+ url = reverse('phonebook:delete_invite', prefix='/en-US/', kwargs={'invite_pk': invite.pk})
+ with self.login(user) as client:
+ response = client.post(url)
+
+ eq_(Invite.objects.all().count(), 1)
+ eq_(response.status_code, 404)
+
+ def test_invite_delete_invalid_invite(self):
+ user = UserFactory.create(userprofile={'is_vouched': True})
+ url = reverse('phonebook:delete_invite', prefix='/en-US/', kwargs={'invite_pk': '1'})
+ with self.login(user) as client:
+ response = client.post(url)
+
+ eq_(response.status_code, 404)
+
class VouchTests(TestCase):
def test_vouch_get_method(self):
View
1  mozillians/phonebook/urls.py
@@ -20,6 +20,7 @@
url(r'^search/$', 'views.search', name='search'),
url(r'^vouch/$', 'views.vouch', name='vouch'),
url(r'^invite/$', 'views.invite', name='invite'),
+ url(r'^invite/(?P<invite_pk>\d+)/delete/$', 'views.delete_invite', name='delete_invite'),
url(r'^country/(?P<country>[A-Za-z]+)/$',
'views.list_mozillians_in_location', name='list_country'),
url(r'^country/(?P<country>[A-Za-z]+)/city/(?P<city>.+)/$',
View
17 mozillians/phonebook/views.py
@@ -5,7 +5,7 @@
from django.contrib.auth.models import User
from django.core.paginator import Paginator, EmptyPage, PageNotAnInteger
from django.http import Http404, HttpResponseBadRequest
-from django.shortcuts import render
+from django.shortcuts import get_object_or_404, render
from django.views.decorators.cache import cache_page, never_cache
from django.views.decorators.http import require_POST
@@ -298,7 +298,20 @@ def invite(request):
return redirect('phonebook:home')
return render(request, 'phonebook/invite.html',
- {'invite_form': invite_form})
+ {'invite_form': invite_form, 'invites': profile.invites.all()})
+
+
+@require_POST
+def delete_invite(request, invite_pk):
+ profile = request.user.userprofile
+ deleted_invite = get_object_or_404(Invite, pk=invite_pk, inviter=profile, redeemed=None)
+ deleted_invite.delete()
+
+ msg = (_(u"%s's invitation to Mozillians has been revoked. "
+ u"You can invite %s again if you like.") %
+ (deleted_invite.recipient, deleted_invite.recipient))
+ messages.success(request, msg)
+ return redirect('phonebook:invite')
@require_POST
View
13 mozillians/static/mozillians/css/main.less
@@ -1777,4 +1777,17 @@ body#invite {
}
input { margin-bottom: 25px; }
+ .card {
+ float: left;
+ margin-right: 30px;
+ }
+
+ .card li {
+ list-style-type: none;
+ margin-left: 0;
+ }
+
+ .card button {
+ margin-top: 30px;
+ }
}
View
36 mozillians/templates/phonebook/invite.html
@@ -35,4 +35,40 @@
</p>
</form>
+ {% if invites.exists() %}
+ <h1>{{ _('Your Invites') }}</h1>
+
+ {% for invite in invites %}
+ <div class="card">
+ <ul>
+ <li>
+ <a title="{{ invite.recipient }}" href="mailto:{{ invite.recipient }}">
+ <i class="icon-envelope"></i> {{ invite.recipient }}
+ </a>
+ </li>
+ <li>
+ {{ _('Sent:') }} {{ invite.created }}
+ </li>
+ {% if invite.redeemed %}
+ <li>
+ {{ _('Redeemed:') }} {{ invite.redeemed }}
+ </li>
+ <li>
+ {{ _('Redeemer:') }}
+ <a href="{{ url('phonebook:profile_view', invite.redeemer.user.username) }}">
+ {{ invite.redeemer.display_name }}
+ </a>
+ </li>
+ {% else %}
+ <form action="{{ url('phonebook:delete_invite', invite_pk=invite.pk) }}" method="POST">
+ {{ csrf() }}
+ <button type="submit" class="button remove">{{ _('Remove') }} <span>✖</span></button>
+ </form>
+ {% endif %}
+ </ul>
+ </div>
+ {% endfor %}
+
+ {% endif %}
+
{% endblock %}
Something went wrong with that request. Please try again.