Merge pull request #694 from willkg/1214272-hb-products

[bug 1214272] Rework analyzer products views

fjord/analytics/analyzer_views.py

@@ -19,13 +19,15 @@
 
 from elasticsearch_dsl import F
 
+from django.core.paginator import Paginator, PageNotAnInteger, EmptyPage
+from django.core.urlresolvers import reverse_lazy
 from django.http import HttpResponse
-from django.shortcuts import get_object_or_404, render
+from django.shortcuts import render
 from django.utils.encoding import force_bytes
-from django.views.generic.edit import FormView
+from django.views.generic.edit import CreateView, UpdateView
 from django.utils.decorators import method_decorator
 
-from fjord.analytics.forms import ProductsUpdateForm
+from fjord.analytics.forms import ProductsCreateForm
 from fjord.analytics.utils import counts_to_options, zero_fill
 from fjord.base.templatetags.jinja_helpers import locale_name
 from fjord.base.utils import (
@@ -384,8 +386,7 @@ def empty_to_unknown(text):
 
     (original_search.aggs
      .bucket('histogram', 'date_histogram', field='created', interval='day')
-     .bucket('per_sentiment', 'terms', field='happy')
-    )
+     .bucket('per_sentiment', 'terms', field='happy'))
 
     results = original_search.execute()
     buckets = results.aggregations['histogram']['buckets']
@@ -424,48 +425,55 @@ def empty_to_unknown(text):
     })
 
 
-class ProductsUpdateView(FormView):
-    """An administrator view for showing, adding, and updating the products."""
+class ProductCreateView(CreateView):
+    model = Product
     template_name = 'analytics/analyzer/products.html'
-    form_class = ProductsUpdateForm
-    success_url = 'products'
+    success_url = reverse_lazy('analytics_products')
+    form_class = ProductsCreateForm
 
     @method_decorator(check_new_user)
     @method_decorator(analyzer_required)
     def dispatch(self, *args, **kwargs):
-        return super(ProductsUpdateView, self).dispatch(*args, **kwargs)
-
-    def get(self, request, *args, **kwargs):
-        if 'pk' in request.GET:
-            self.object = get_object_or_404(Product, pk=request.GET['pk'])
-        return super(ProductsUpdateView, self).get(request, *args, **kwargs)
+        return super(ProductCreateView, self).dispatch(*args, **kwargs)
 
     def get_context_data(self, **kwargs):
-        context = super(ProductsUpdateView, self).get_context_data(**kwargs)
-        context['products'] = Product.objects.all()
+        context = super(ProductCreateView, self).get_context_data(**kwargs)
+        page = self.request.GET.get('page')
+        paginator = Paginator(Product.objects.order_by('id'), 25)
+        try:
+            products = paginator.page(page)
+        except PageNotAnInteger:
+            products = paginator.page(1)
+        except EmptyPage:
+            products = paginator.page(paginator.num_pages)
+
+        context['products'] = products
+        context['update'] = False
         return context
 
-    def get_form_kwargs(self):
-        kwargs = super(ProductsUpdateView, self).get_form_kwargs()
-        if hasattr(self, 'object'):
-            kwargs['instance'] = self.object
-        return kwargs
 
-    def form_valid(self, form):
+class ProductUpdateView(UpdateView):
+    model = Product
+    template_name = 'analytics/analyzer/products.html'
+    success_url = reverse_lazy('analytics_products')
+    form_class = ProductsCreateForm
+
+    @method_decorator(check_new_user)
+    @method_decorator(analyzer_required)
+    def dispatch(self, *args, **kwargs):
+        return super(ProductUpdateView, self).dispatch(*args, **kwargs)
+
+    def get_context_data(self, **kwargs):
+        context = super(ProductUpdateView, self).get_context_data(**kwargs)
+        page = self.request.GET.get('page')
+        paginator = Paginator(Product.objects.order_by('id'), 25)
         try:
-            instance = Product.objects.get(db_name=form.data.get('db_name'))
-            instance.slug = form.data.get('slug') or instance.slug
-            instance.display_name = (form.data.get('display_name') or
-                                     instance.display_name)
-            instance.notes = form.data.get('notes') or instance.notes
-            instance.enabled = form.data.get('enabled') or False
-            instance.on_dashboard = form.data.get('on_dashboard') or False
-            instance.on_picker = form.data.get('on_picker') or False
-            instance.browser = form.data.get('browser') or u''
-            instance.browser_data_browser = (
-                form.data.get('browser_data_browser') or u''
-            )
-            self.object = instance.save()
-        except Product.DoesNotExist:
-            self.object = form.save()
-        return super(ProductsUpdateView, self).form_valid(form)
+            products = paginator.page(page)
+        except PageNotAnInteger:
+            products = paginator.page(1)
+        except EmptyPage:
+            products = paginator.page(paginator.num_pages)
+
+        context['products'] = products
+        context['update'] = True
+        return context

fjord/analytics/forms.py

@@ -3,7 +3,7 @@
 from fjord.feedback.models import Product
 
 
-class ProductsUpdateForm(forms.ModelForm):
+class ProductsCreateForm(forms.ModelForm):
     class Meta:
         model = Product
         fields = [

fjord/analytics/jinja2/analytics/analyzer/products.html

@@ -1,52 +1,99 @@
 {% extends "analytics/analyzer/dashboard.html" %}
+{# Note: This is not l10n-ized since it's only available to analyzers for now. #}
+
 {% block content_middle %}
   <div class="block">
     <h2>Analytics: Products</h2>
-    <table class="summarytable">
-      <tr>
-        <th>id</th>
-        <th>enabled?</th>
-        <th>on dashboard?</th>
-        <th>on picker?</th>
-        <th>display name</th>
-        <th>db name</th>
-        <th>slug</th>
-        <th>automatic translations?</th>
-        <th>feedback product url</th>
-        <th>browser</th>
-        <th>browser data browser</th>
-        <th>notes</th>
-      </tr>
-      {% for prod in products %}
+    {% if not update %}
+      <h2>Total products: {{ products.paginator.count }}</h2>
+
+      <div class="pager">
+        {% if next_page %}
+          <a class="older" href="{{ request.get_full_path()|urlparams(page=products.previous_page_number) }}">
+            Older
+          </a>
+        {% else %}
+          Older
+        {% endif %}
+        | Page {{ products.number }} of {{ products.paginator.num_pages }} |
+        {% if prev_page %}
+          <a class="newer" href="{{ request.get_full_path()|urlparams(page=products.next_page_number) }}">
+            Newer
+          </a>
+        {% else %}
+          Newer
+        {% endif %}
+      </div>
+
+      <table class="summarytable">
         <tr>
-          <td>
-            <a href="{{ url('analytics_products') }}?pk={{ prod.id }}">Edit</a>
-            {{ prod.id }}
-          </td>
-          <td>{{ prod.enabled }}</td>
-          <td>{{ prod.on_dashboard }}</td>
-          <td>{{ prod.on_picker }}</td>
-          <td>{{ prod.display_name }}</td>
-          <td>{{ prod.db_name }}</td>
-          <td>{{ prod.slug }}</td>
-          <td>{{ prod.translation_system }}</td>
-          {# FIXME - URL is hard-coded so it doesn't include the locale. Need better way to do this. #}
-          <td><a href="https://input.mozilla.org/feedback/{{ prod.slug }}">https://input.mozilla.org/feedback/{{ prod.slug }}</a></td>
-          <td>{{ prod.browser }}</td>
-          <td>{{ prod.browser_data_browser }}</td>
-          <td>{{ prod.notes }}</td>
+          <th>id</th>
+          <th>enabled?</th>
+          <th>on dashboard?</th>
+          <th>on picker?</th>
+          <th>display name</th>
+          <th>db name</th>
+          <th>slug</th>
+          <th>automatic translations?</th>
+          <th>feedback product url</th>
+          <th>browser</th>
+          <th>browser data browser</th>
+          <th>notes</th>
+          <th></th>
         </tr>
-      {% endfor %}
-    </table>
-    <p>
-      Click on "Edit" above to edit an existing product or fill out
-      the fields below to create a new product.
-    </p>
-    <p class="warning">
-      Note: If you go to update an existing product and you change the
-      db_name, it will create a new product. You can't change the
-      db_name for an existing product since it'll orphan responses.
-    </p>
+        {% for prod in products %}
+          <tr>
+            <td>
+              {{ prod.id }}
+            </td>
+            <td>{{ prod.enabled }}</td>
+            <td>{{ prod.on_dashboard }}</td>
+            <td>{{ prod.on_picker }}</td>
+            <td>{{ prod.display_name }}</td>
+            <td>{{ prod.db_name }}</td>
+            <td>{{ prod.slug }}</td>
+            <td>{{ prod.translation_system }}</td>
+            {# FIXME - URL is hard-coded so it doesn't include the locale. Need better way to do this. #}
+            <td><a href="https://input.mozilla.org/feedback/{{ prod.slug }}">https://input.mozilla.org/feedback/{{ prod.slug }}</a></td>
+            <td>{{ prod.browser }}</td>
+            <td>{{ prod.browser_data_browser }}</td>
+            <td>{{ prod.notes }}</td>
+            <td>
+              <a href="{{ url('analytics_products_update', pk=prod.id) }}">Update</a>
+            </td>
+          </tr>
+        {% endfor %}
+      </table>
+
+      <div class="pager">
+        {% if next_page %}
+          <a class="older" href="{{ request.get_full_path()|urlparams(page=products.previous_page_number) }}">
+            Older
+          </a>
+        {% else %}
+          Older
+        {% endif %}
+        | Page {{ products.number }} of {{ products.paginator.num_pages }} |
+        {% if prev_page %}
+          <a class="newer" href="{{ request.get_full_path()|urlparams(page=products.next_page_number) }}">
+            Newer
+          </a>
+        {% else %}
+          Newer
+        {% endif %}
+      </div>
+    {% endif %}
+
+    {% if update %}
+      <p class="warning">
+        Note: If you go to update an existing product and you change the
+        db_name, it will create a new product. You can't change the
+        db_name for an existing product since it'll orphan responses.
+      </p>
+    {% endif %}
+
+    <h2>{% if update %}Update{% else %}Create{% endif %} Product</h2>
+    <p><span>{% if update %}<a href="{{ url('analytics_products') }}">&lt; Back to products</a>{% endif %}</span></p>
     <form class="analyzer-form" action="" method="POST">
       {% csrf_token %}
       {{ form.as_p() }}

fjord/analytics/tests/test_analyzer_views.py

@@ -395,7 +395,8 @@ def test_update_product(self):
         }
 
         resp = self.client.post(
-            reverse('analytics_products'), data, follow=True
+            reverse('analytics_products_update', args=(prod.id,)),
+            data, follow=True
         )
         assert resp.status_code == 200
         assert data['display_name'] in resp.content

fjord/analytics/urls.py

@@ -1,6 +1,6 @@
 from django.conf.urls import patterns, url
 
-from fjord.analytics.analyzer_views import ProductsUpdateView
+from fjord.analytics.analyzer_views import ProductCreateView, ProductUpdateView
 
 
 urlpatterns = patterns(
@@ -25,8 +25,12 @@
     # Analytics dashboard
     url(r'^analytics/?$', 'analytics_dashboard',
         name='analytics_dashboard'),
-    url(r'^analytics/products/?$', ProductsUpdateView.as_view(),
+    url(r'^analytics/products/?$',
+        ProductCreateView.as_view(),
         name='analytics_products'),
+    url(r'^analytics/products/(?P<pk>\d+)/update/$',
+        ProductUpdateView.as_view(),
+        name='analytics_products_update'),
     url(r'^analytics/search/?$', 'analytics_search',
         name='analytics_search'),
 )

fjord/feedback/migrations/0008_auto_20151014_0820.py

@@ -0,0 +1,19 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('feedback', '0007_auto_20150915_0813'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='product',
+            name='db_name',
+            field=models.CharField(help_text='Do not change this after you have created the product. You will orphan all the existing feedback!', max_length=50),
+        ),
+    ]

fjord/feedback/models.py

@@ -64,7 +64,11 @@ class Product(ModelBase):
 
     # We're not using foreign keys, so when we save something to the
     # database, we use this name
-    db_name = models.CharField(max_length=50)
+    db_name = models.CharField(
+        max_length=50,
+        help_text=(u'Do not change this after you have created the product. '
+                   u'You will orphan all the existing feedback!')
+    )
 
     # This is the slug used in the feedback product urls; we don't use
     # the SlugField because we don't require slugs be unique