Merge lp:~cjohnston/qa-dashboard/user-edit-profile into lp:qa-dashboard

[Chris Johnston]

Adds the ability for a user to edit their profile

[Andy Doan (doanac)]

1) This looks a little shady:
216 + try:
217 + profile = UserProfile.objects.get(user=request.user)
218 + except UserProfile.DoesNotExist:
219 + profile = UserProfile(user=request.user)
Why would request.user not exist?

2) This seems too verbose:
165 - <li>Logged in as: <a href="http://launchpad.net/~{{ user.username }}">{{ user.username }}</a></li>
166 + <li>Logged in as: <strong><a href="http://launchpad.net/~{{ user.username }}">{{ user.username }}</a></strong></li>
167 + <li><a href="{% url 'edit_profile' %}">Edit profile</a>

Why not just have the logged-in link take you to your profile. ie:

  <li>Logged in as: <a href="{% url 'edit_profile' %}">{{ user.username }}</a></li>

[Chris Johnston (cjohnston)]

> 1) This looks a little shady:
> 216 + try:
> 217 + profile = UserProfile.objects.get(user=request.user)
> 218 + except UserProfile.DoesNotExist:
> 219 + profile = UserProfile(user=request.user)
> Why would request.user not exist?
>

It isn't trying request.user, it's trying the UserProfile, which a user may or may not have.

> 2) This seems too verbose:
> 165 - <li>Logged in as: <a href="http://launchpad.net/~{{ user.username
> }}">{{ user.username }}</a></li>
> 166 + <li>Logged in as: <strong><a href="http://launchpad.net/~{{
> user.username }}">{{ user.username }}</a></strong></li>
> 167 + <li><a href="{% url 'edit_profile' %}">Edit profile</a>
>
> Why not just have the logged-in link take you to your profile. ie:
>
> <li>Logged in as: <a href="{% url 'edit_profile' %}">{{ user.username
> }}</a></li>

Is that obvious enough? Would people know that they can click it to change their settings?

[lp:~cjohnston/qa-dashboard/user-edit-profile]

645. By Chris Johnston on 2013-10-21

Add ability for a user to edit their profile

[Andy Doan (doanac)]

> > 2) This seems too verbose:
> > 165 - <li>Logged in as: <a href="http://launchpad.net/~{{ user.username
> > }}">{{ user.username }}</a></li>
> > 166 + <li>Logged in as: <strong><a href="http://launchpad.net/~{{
> > user.username }}">{{ user.username }}</a></strong></li>
> > 167 + <li><a href="{% url 'edit_profile' %}">Edit profile</a>
> >
> > Why not just have the logged-in link take you to your profile. ie:
> >
> > <li>Logged in as: <a href="{% url 'edit_profile' %}">{{ user.username
> > }}</a></li>
>
> Is that obvious enough? Would people know that they can click it to change
> their settings?

Seemed so to me. Maybe we should ask a few other people though. I guess I just didn't see the point in a link that takes you to launchpad (and off our site)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:645
http://10.97.0.26:8080/job/dashboard-ci/210/
Executed test runs:

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/dashboard-ci/210/rebuild

review: Approve (continuous-integration)

[Joe Talbott (joetalbott)]

On Mon, Oct 21, 2013 at 08:09:21PM -0000, Andy Doan wrote:
> > > 2) This seems too verbose:
> > > 165 - <li>Logged in as: <a href="http://launchpad.net/~{{ user.username
> > > }}">{{ user.username }}</a></li>
> > > 166 + <li>Logged in as: <strong><a href="http://launchpad.net/~{{
> > > user.username }}">{{ user.username }}</a></strong></li>
> > > 167 + <li><a href="{% url 'edit_profile' %}">Edit profile</a>
> > >
> > > Why not just have the logged-in link take you to your profile. ie:
> > >
> > > <li>Logged in as: <a href="{% url 'edit_profile' %}">{{ user.username
> > > }}</a></li>
> >
> > Is that obvious enough? Would people know that they can click it to change
> > their settings?
>
> Seemed so to me. Maybe we should ask a few other people though. I guess I just didn't see the point in a link that takes you to launchpad (and off our site)

I'm +1 for staying on our site.

[lp:~cjohnston/qa-dashboard/user-edit-profile]

646. By Chris Johnston on 2013-10-21

Update per review

[Chris Johnston (cjohnston)]

review: Needs Resubmitting

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:646
http://10.97.0.26:8080/job/dashboard-ci/211/
Executed test runs:

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/dashboard-ci/211/rebuild

review: Approve (continuous-integration)

[Andy Doan (doanac)]

+1 by me

review: Approve

=== added file 'common/forms.py'
--- common/forms.py	1970-01-01 00:00:00 +0000
+++ common/forms.py	2013-10-21 21:02:25 +0000
@@ -0,0 +1,24 @@
+# QA Dashboard
+# Copyright 2012-2013 Canonical Ltd.
+
+# This program is free software: you can redistribute it and/or modify it
+# under the terms of the GNU Affero General Public License version 3, as
+# published by the Free Software Foundation.
+
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranties of
+# MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
+# PURPOSE.  See the GNU Affero General Public License for more details.
+
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+from django.forms import ModelForm
+
+from common.models import UserProfile
+
+
+class EditProfileForm(ModelForm):
+    class Meta:
+        model = UserProfile
+        fields = ['use_private_url', ]

=== added file 'common/static/css/new-style.css'
--- common/static/css/new-style.css	1970-01-01 00:00:00 +0000
+++ common/static/css/new-style.css	2013-10-21 21:02:25 +0000
@@ -0,0 +1,84 @@
+form fieldset {
+    border-radius : 4px;
+    background-repeat : no-repeat;
+    background-color : #efeeec;
+    background-position : -15px -15px;
+    border : 0;
+    margin-bottom : 8px;
+    padding : 15px 20px 15px 94px;
+}
+form fieldset h3 {
+    border-bottom : 1px dotted #dfdcd9;
+    margin-bottom : 9px;
+    padding-bottom : 10px;
+}
+form fieldset li:first-child {
+    margin-top : 0;
+}
+form input[type="text"], form textarea {
+    border-radius : 2px;
+    background : #fff;
+    border : #999 solid 1px;
+    font-family : Ubuntu, Arial, "libra sans", sans-serif;
+    display : block;
+    padding : 4px;
+}
+form input:focus, form textarea:focus {
+    border : #000 solid 1px;
+}
+form textarea[readonly="readonly"] {
+    color : #999;
+}
+form input[type="checkbox"], form input[type="radio"] {
+    margin : 0;
+    width : auto;
+    display: block;
+}
+form input[type="checkbox"] + label, form input[type="radio"] + label {
+    display : inline;
+    margin-left : 5px;
+    vertical-align : middle;
+    width : auto;
+}
+form input[type="submit"] {
+    font-size : 1em;
+    margin-bottom : 0.75em;
+    border-radius : 4px;
+    background-color : #dd4814;
+    box-shadow : none;
+    border : 0;
+    color : #fff;
+    display : block;
+    padding : 10px 14px;
+    text-shadow : none;
+    width : auto;
+    margin-bottom : 0;
+}
+form input[type="submit"]:hover {
+    background : #dd4814;
+}
+form label {
+    cursor : pointer;
+    display : block;
+    margin-bottom : 4px;
+}
+form label span {
+    color : #df382c;
+}
+form ul {
+    margin-left : 0;
+}
+form li {
+    list-style : none outside none;
+    margin-top : 14px;
+}
+form button[type="submit"] {
+    border : 0;
+    display : inline-block;
+    font-family : Ubuntu, Arial, "libra sans", sans-serif;
+    text-decoration : none;
+    font-weight : 300;
+}
+form input[type="reset"] {
+    display : none;
+}

=== added file 'common/templates/form.html'
--- common/templates/form.html	1970-01-01 00:00:00 +0000
+++ common/templates/form.html	2013-10-21 21:02:25 +0000
@@ -0,0 +1,26 @@
+{% extends "smokeng/smoke_layout.html" %}
+
+{% block page_name %}{{ form_title }}{%endblock %}
+{% block content %}
+<div class="grid_7">
+        <form action="{{ request.path_info }}" method="POST">{% csrf_token %}
+            <fieldset class='fieldset-1'>
+                <h3>{{ form_title }}</h3>
+                {{ form.as_ul }}
+                <input type="submit" name="submit" value="Save" class="submit-button" />
+            </fieldset>
+        </form>
+</div>
+<script type="text/javascript"><!--
+$(document).ready(function(){
+    $('span[rel*=help]').colorTip({color:'orange'});
+});
+--></script>
+<style>
+form ul {
+  height: 12em;
+  overflow-y: scroll;
+  overflow-x: hidden;
+}
+</style>
+{% endblock %}

=== modified file 'common/templates/layout.html'
--- common/templates/layout.html	2013-10-11 20:43:53 +0000
+++ common/templates/layout.html	2013-10-21 21:02:25 +0000
@@ -9,6 +9,7 @@
     <link href='{% static "css/grid.css" %}' rel='stylesheet' type='text/css' />
     <link href='{% static "css/jquery.dataTables.css" %}' rel='stylesheet' type='text/css' />
     <link href='{% static "css/style.css" %}' rel='stylesheet' type='text/css' />
+    <link href='{% static "css/new-style.css" %}' rel='stylesheet' type='text/css' />
     <title>{% block page_name %}{% endblock %} | Ubuntu CI Dashboard</title>
     <script src='http://ajax.googleapis.com/ajax/libs/jquery/1.7.2/jquery.min.js' type="text/javascript"></script>
     <script src='{% static "js/jquery.dataTables.min.js" %}' type="text/javascript"></script>
@@ -23,7 +24,7 @@
         <div class="header-login">
             <ul>
             {% if user.is_authenticated %}
-                <li>Logged in as: <a href="http://launchpad.net/~{{ user.username }}">{{ user.username }}</a></li>
+                <li>Logged in as: <strong><a href="{% url 'edit_profile' %}">{{ user.username }}</a></strong></li>
                 {% if user.is_staff %}
                     <li><a href="/admin" title="Admin">Admin</a></li>
                 {% endif %}

=== modified file 'common/tests/__init__.py'
--- common/tests/__init__.py	2013-10-08 15:22:19 +0000
+++ common/tests/__init__.py	2013-10-21 21:02:25 +0000
@@ -20,3 +20,4 @@
 from test_plugins import *
 from test_buildids import *
 from test_myserializer import *
+from test_profile import *

=== added file 'common/tests/test_profile.py'
--- common/tests/test_profile.py	1970-01-01 00:00:00 +0000
+++ common/tests/test_profile.py	2013-10-21 21:02:25 +0000
@@ -0,0 +1,116 @@
+# QA Dashboard
+# Copyright 2012-2013 Canonical Ltd.
+
+# This program is free software: you can redistribute it and/or modify it
+# under the terms of the GNU Affero General Public License version 3, as
+# published by the Free Software Foundation.
+
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranties of
+# MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
+# PURPOSE.  See the GNU Affero General Public License for more details.
+
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+from django import test as djangotest
+from django.test.client import Client
+from django.contrib.auth.models import User
+from django.core.urlresolvers import reverse
+
+from smokeng.models import SmokeImage
+
+
+class UserProfileTestCase(djangotest.TestCase):
+
+    def setUp(self):
+        self.c = Client()
+        self.user = User.objects.create(
+            username='testuser',
+            first_name='Test',
+            last_name='User',
+        )
+        self.user.set_password('password')
+        self.user.save()
+
+        self.image = SmokeImage.objects.create(
+            release="saucy",
+            arch="mako",
+            variant="touch_mir",
+            build_number="20130401",
+        )
+        self.assert_(self.image)
+
+    def tearDown(self):
+        self.c.logout()
+
+    def login(self):
+        logged_in = self.c.login(username='testuser', password='password')
+        self.assertTrue(logged_in)
+
+    def _get_page_url(self):
+        url = reverse(
+            'smokeng_overview',
+            args=(
+                self.image.release,
+            ),
+        )
+        return url
+
+    def test_not_logged_in_no_edit_profile_link(self):
+        """
+        Test that a user who is not logged in does not have a link to edit
+        their profile
+        """
+        response = self.c.get(self._get_page_url())
+        self.assertContains(response, '/edit/', 0)
+
+    def test_logged_in_edit_profile_link(self):
+        """
+        Test that a user that is logged in has a link to edit their profile
+        """
+        self.login()
+        response = self.c.get(self._get_page_url())
+        self.assertContains(response, '/edit/', 1)
+
+    def test_not_logged_in_edit_profile_url(self):
+        """
+        Test that a user who tries to edit their profile without being logged
+        in is redirected
+        """
+        response = self.c.get(reverse('edit_profile'))
+        self.assertEquals(response.status_code, 302)
+
+    def test_edit_profile_url(self):
+        """
+        Test edit profile url
+        """
+        self.login()
+        response = self.c.get(reverse('edit_profile'))
+        self.assertEquals(response.status_code, 200)
+        self.assertTemplateUsed(response, 'form.html')
+
+    def test_user_save_profile(self):
+        """
+        Tests that a user can update their profile
+        """
+        self.login()
+        response = self.c.get(reverse('edit_profile'))
+        self.assertEqual(response.status_code, 200)
+
+        # Define data to fill our the form
+        use_private_url = True
+        user = self.user
+        csrf_token = '%s' % response.context['csrf_token']
+
+        # Post the form
+        post = self.c.post(
+            reverse('edit_profile'),
+            data={
+                'csrfmiddlewaretoken': csrf_token,
+                'use_private_url': use_private_url,
+                'user': user,
+            }
+        )
+
+        self.assertEqual(post.status_code, 302)

=== modified file 'common/views.py'
--- common/views.py	2013-10-11 15:24:21 +0000
+++ common/views.py	2013-10-21 21:02:25 +0000
@@ -13,6 +13,7 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+from django.contrib.auth.decorators import login_required
 from django.views.decorators.http import require_GET
 from django.shortcuts import render_to_response
 from django.template import RequestContext
@@ -21,11 +22,9 @@
 from django.http import HttpResponseRedirect
 
 from common.plugin_helper import ExtensionManager
-
-from common.bread_crumbs import (
-    BreadCrumb,
-    BreadCrumbTrail,
-)
+from common.bread_crumbs import BreadCrumb, BreadCrumbTrail
+from common.models import UserProfile
+from common.forms import EditProfileForm
 
 
 @BreadCrumb("CI Dashboard")
@@ -51,3 +50,30 @@
 def logout_view(request):
     logout(request)
     return HttpResponseRedirect('/')
+
+
+@login_required
+def edit_profile(request):
+    form_title = 'Edit profile'
+    try:
+        profile = UserProfile.objects.get(user=request.user)
+    except UserProfile.DoesNotExist:
+        profile = UserProfile(user=request.user)
+
+    if request.method == 'POST':
+        form = EditProfileForm(data=request.POST, instance=profile)
+        if form.is_valid():
+            form.save()
+            return HttpResponseRedirect('/')
+    else:
+        form = EditProfileForm(instance=profile)
+
+    context = {
+        'form': form,
+        'form_title': form_title,
+    }
+    return render_to_response(
+        'form.html',
+        context,
+        RequestContext(request)
+    )

=== modified file 'qa_dashboard/urls.py'
--- qa_dashboard/urls.py	2013-10-11 15:24:21 +0000
+++ qa_dashboard/urls.py	2013-10-21 21:02:25 +0000
@@ -23,6 +23,7 @@
 urlpatterns = patterns(
     '',
     url(r'^$', 'common.views.index', name='index'),
+    url(r'^edit/$', 'common.views.edit_profile', name='edit_profile'),
     url(r'^admin/', include(admin.site.urls)),
     url(r'^\+oops$', 'common.views.cause_oops', name='cause-oops'),
     url(r'^logout$', 'common.views.logout_view', name='logout'),