Merge lp:~milo/linaro-ci-dashboard/linkify-next-loop into lp:linaro-ci-dashboard

[Milo Casagrande]

Thsi is a tentative implementation of the final detail of the detail view.
At the moment it works only for the textfield-based views, but if the implementation looks good enough, it can be extended to the others too.

The code added is commented, so should be clear what has been done.
Basically, I resolved this using the @permalink annotation in the model, and retrieving the correct instance from the generic Loop model from the fields.

The same approach might be used also in the JavaScript that creates the link for the 'build' path in the detail view.

[lp:~milo/linaro-ci-dashboard/linkify-next-loop]

45. By Milo Casagrande on 2012-09-12

Fixed problem with link.

[Stevan Radaković (stevanr)]

Hey Milo, looks nice!

Is there anyway we can move the get_next_loop model in the view tho?
Also, since we don't need any of the other permalink methods except detail and build, let's remove them.
And as we've spoken on IRC, let's remove everything related to the build_path now...

review: Needs Fixing

[lp:~milo/linaro-ci-dashboard/linkify-next-loop]

46. By Milo Casagrande on 2012-09-12

Added permaling method, simplified templates, removed not necessary ones.

47. By Milo Casagrande on 2012-09-12

Moved get_next_loop function in the view.

48. By Milo Casagrande on 2012-09-12

Fixed part of the tests.

[Stevan Radaković (stevanr)]

Looks good.
Thanks.
Approve +1

review: Approve

[lp:~milo/linaro-ci-dashboard/linkify-next-loop]

49. By Milo Casagrande on 2012-09-12

Fixed test failing, added note.

=== modified file 'dashboard/frontend/android_build/models/android_loop.py'
--- dashboard/frontend/android_build/models/android_loop.py	2012-09-11 14:34:59 +0000
+++ dashboard/frontend/android_build/models/android_loop.py	2012-09-12 14:34:19 +0000
@@ -139,3 +139,15 @@
                                                       exclude=exclude,
                                                       capitalize=capitalize,
                                                       upper=True)
+
+    @models.permalink
+    def get_detail_url(self):
+        return 'AndroidLoopDetail', (), {'slug': self.name}
+
+    @models.permalink
+    def get_build_url(self):
+        return 'AndroidLoopBuild', (), {'slug': self.name}
+
+    @models.permalink
+    def get_update_url(self):
+        return 'AndroidLoopUpdate', (), {'slug': self.name}

=== removed file 'dashboard/frontend/android_build/templates/android_loop_detail.html'
--- dashboard/frontend/android_build/templates/android_loop_detail.html	2012-09-07 15:36:09 +0000
+++ dashboard/frontend/android_build/templates/android_loop_detail.html	1970-01-01 00:00:00 +0000
@@ -1,7 +0,0 @@
-{% extends "loop_detail.html" %}
-{% block scripts %}
-{{ block.super }}
-$("#update_button").click(function() {
-    window.location.href = "{% url AndroidLoopUpdate loop_detail.name %}";
-});
-{% endblock scripts %}

=== modified file 'dashboard/frontend/android_build/tests/test_android_build_clientresponse.py'
--- dashboard/frontend/android_build/tests/test_android_build_clientresponse.py	2012-09-10 08:04:06 +0000
+++ dashboard/frontend/android_build/tests/test_android_build_clientresponse.py	2012-09-12 14:34:19 +0000
@@ -66,8 +66,7 @@
         # Assert template names for detail page.
         template_names = [template.name for template in
                           response.templates]
-        self.assertEquals(template_names, ["android_loop_detail.html",
-                                           "loop_detail.html",
+        self.assertEquals(template_names, ["loop_detail.html",
                                            "base.html",
                                            "login.html",
                                            ])

=== modified file 'dashboard/frontend/android_build/tests/test_android_build_views.py'
--- dashboard/frontend/android_build/tests/test_android_build_views.py	2012-09-10 08:04:06 +0000
+++ dashboard/frontend/android_build/tests/test_android_build_views.py	2012-09-12 14:34:19 +0000
@@ -33,6 +33,8 @@
         self.mock_loop.loop_ptr.id = 0
         self.mock_loop.loop_ptr.loopbuild_set = MagicMock()
         self.mock_loop.loop_ptr.loopbuild_set.all = MagicMock()
+        self.mock_loop.next_loop = MagicMock()
+        self.mock_loop.next_loop.id = 1
 
     def test_create_view_get_context_data(self):
         android_loop_create_view = AndroidLoopCreateView()

=== modified file 'dashboard/frontend/android_build/views/android_loop_detail_view.py'
--- dashboard/frontend/android_build/views/android_loop_detail_view.py	2012-09-07 15:08:52 +0000
+++ dashboard/frontend/android_build/views/android_loop_detail_view.py	2012-09-12 14:34:19 +0000
@@ -22,5 +22,3 @@
 class AndroidLoopDetailView(LoopDetailView):
 
     model = AndroidLoop
-    template_name = "android_loop_detail.html"
-    build_path = 'android'

=== modified file 'dashboard/frontend/android_textfield_loop/models/android_textfield_loop.py'
--- dashboard/frontend/android_textfield_loop/models/android_textfield_loop.py	2012-09-07 12:49:10 +0000
+++ dashboard/frontend/android_textfield_loop/models/android_textfield_loop.py	2012-09-12 14:34:19 +0000
@@ -17,6 +17,7 @@
 
 from frontend.models.textfield_loop import TextFieldLoop
 from frontend.models.loop_reference import ANDROID_LOOP
+from django.db.models import permalink
 
 
 class AndroidTextFieldLoop(TextFieldLoop):
@@ -34,3 +35,15 @@
     def can_chain_into():
         chain = [ANDROID_LOOP]
         return chain
+
+    @permalink
+    def get_detail_url(self):
+        return 'AndroidTextFieldLoopDetail', (), {'slug': self.name}
+
+    @permalink
+    def get_build_url(self):
+        return 'AndroidTextFieldLoopBuild', (), {'slug': self.name}
+
+    @permalink
+    def get_update_url(self):
+        return 'AndroidTextFieldLoopUpdate', (), {'slug': self.name}

=== removed file 'dashboard/frontend/android_textfield_loop/templates/android_textfield_loop_detail.html'
--- dashboard/frontend/android_textfield_loop/templates/android_textfield_loop_detail.html	2012-09-10 13:30:37 +0000
+++ dashboard/frontend/android_textfield_loop/templates/android_textfield_loop_detail.html	1970-01-01 00:00:00 +0000
@@ -1,7 +0,0 @@
-{% extends "textfield_loop_detail.html" %}
-{% block scripts %}
-{{ block.super }}
-$("#update_button").click(function() {
-    window.location.href = "{% url AndroidTextFieldLoopUpdate loop_detail.name %}";
-});
-{% endblock scripts %}

=== modified file 'dashboard/frontend/android_textfield_loop/tests/test_android_textfield_clientresponse.py'
--- dashboard/frontend/android_textfield_loop/tests/test_android_textfield_clientresponse.py	2012-09-10 13:30:37 +0000
+++ dashboard/frontend/android_textfield_loop/tests/test_android_textfield_clientresponse.py	2012-09-12 14:34:19 +0000
@@ -65,8 +65,7 @@
         template_names = [template.name for template in
                           response.templates]
         self.assertEquals(template_names,
-            ["android_textfield_loop_detail.html",
-             "textfield_loop_detail.html",
+            ["textfield_loop_detail.html",
              "loop_detail.html",
              "base.html",
              "login.html",

=== modified file 'dashboard/frontend/android_textfield_loop/views/android_textfield_loop_detail_view.py'
--- dashboard/frontend/android_textfield_loop/views/android_textfield_loop_detail_view.py	2012-09-10 13:30:37 +0000
+++ dashboard/frontend/android_textfield_loop/views/android_textfield_loop_detail_view.py	2012-09-12 14:34:19 +0000
@@ -23,5 +23,4 @@
 class AndroidTextFieldLoopDetailView(LoopDetailView):
 
     model = AndroidTextFieldLoop
-    template_name = 'android_textfield_loop_detail.html'
-    build_path = 'android-text'
+    template_name = 'textfield_loop_detail.html'

=== modified file 'dashboard/frontend/integration_loop/models/integration_loop.py'
--- dashboard/frontend/integration_loop/models/integration_loop.py	2012-08-31 13:34:41 +0000
+++ dashboard/frontend/integration_loop/models/integration_loop.py	2012-09-12 14:34:19 +0000
@@ -42,5 +42,16 @@
             pass
 
     def __repr__(self):
-        return "IntegrationLoop(id=%d, branch=%s, ...)" % (self.id,
-                                                           self.branch)
+        return "IntegrationLoop(id=%d, branch=%s)" % (self.id, self.branch)
+
+    @models.permalink
+    def get_detail_url(self):
+        return 'IntegrationLoopDetail', (), {'slug': self.name}
+
+    @models.permalink
+    def get_build_url(self):
+        return 'IntegrationLoopBuild', (), {'slug': self.name}
+
+    @models.permalink
+    def get_update_url(self):
+        return 'IntegrationLoopUpdate', (), {'slug': self.name}

=== removed file 'dashboard/frontend/integration_loop/templates/integration_loop_detail.html'
--- dashboard/frontend/integration_loop/templates/integration_loop_detail.html	2012-09-07 15:40:57 +0000
+++ dashboard/frontend/integration_loop/templates/integration_loop_detail.html	1970-01-01 00:00:00 +0000
@@ -1,7 +0,0 @@
-{% extends "loop_detail.html" %}
-{% block scripts %}
-{{ block.super }}
-$("#update_button").click(function() {
-    window.location.href = "{% url IntegrationLoopUpdate loop_detail.name %}";
-});
-{% endblock scripts %}

=== modified file 'dashboard/frontend/integration_loop/views/integration_loop_detail_view.py'
--- dashboard/frontend/integration_loop/views/integration_loop_detail_view.py	2012-09-07 15:40:57 +0000
+++ dashboard/frontend/integration_loop/views/integration_loop_detail_view.py	2012-09-12 14:34:19 +0000
@@ -22,5 +22,3 @@
 class IntegrationLoopDetailView(LoopDetailView):
 
     model = IntegrationLoop
-    template_name = "integration_loop_detail.html"
-    build_path = 'integration_loop'

=== modified file 'dashboard/frontend/kernel_build/models/kernel_loop.py'
--- dashboard/frontend/kernel_build/models/kernel_loop.py	2012-08-30 21:49:13 +0000
+++ dashboard/frontend/kernel_build/models/kernel_loop.py	2012-09-12 14:34:19 +0000
@@ -123,3 +123,15 @@
         except:
             # TODO: Log error.
             pass
+
+    @models.permalink
+    def get_detail_url(self):
+        return 'KernelLoopDetail', (), {'slug': self.name}
+
+    @models.permalink
+    def get_build_url(self):
+        return 'KernelLoopBuild', (), {'slug': self.name}
+
+    @models.permalink
+    def get_update_url(self):
+        return 'KernelLoopUpdate', (), {'slug': self.name}

=== removed file 'dashboard/frontend/kernel_build/templates/kernel_loop_detail.html'
--- dashboard/frontend/kernel_build/templates/kernel_loop_detail.html	2012-09-11 12:36:18 +0000
+++ dashboard/frontend/kernel_build/templates/kernel_loop_detail.html	1970-01-01 00:00:00 +0000
@@ -1,7 +0,0 @@
-{% extends "loop_detail.html" %}
-{% block scripts %}
-{{ block.super }}
-$("#update_button").click(function() {
-    window.location.href = "{% url KernelLoopUpdate loop_detail.name %}";
-});
-{% endblock scripts %}

=== modified file 'dashboard/frontend/kernel_build/tests/test_kernel_build_clientresponse.py'
--- dashboard/frontend/kernel_build/tests/test_kernel_build_clientresponse.py	2012-09-10 07:43:51 +0000
+++ dashboard/frontend/kernel_build/tests/test_kernel_build_clientresponse.py	2012-09-12 14:34:19 +0000
@@ -70,8 +70,7 @@
         # Assert template names for detail page.
         template_names = [template.name for template in
                           response.templates]
-        self.assertEquals(template_names, ["kernel_loop_detail.html",
-                                           "loop_detail.html",
+        self.assertEquals(template_names, ["loop_detail.html",
                                            "base.html",
                                            "login.html",
                                            ])

=== modified file 'dashboard/frontend/kernel_build/views/kernel_loop_detail_view.py'
--- dashboard/frontend/kernel_build/views/kernel_loop_detail_view.py	2012-09-07 15:43:36 +0000
+++ dashboard/frontend/kernel_build/views/kernel_loop_detail_view.py	2012-09-12 14:34:19 +0000
@@ -22,5 +22,3 @@
 class KernelLoopDetailView(LoopDetailView):
 
     model = KernelLoop
-    template_name = 'kernel_loop_detail.html'
-    build_path = 'kernel'

=== modified file 'dashboard/frontend/models/loop.py'
--- dashboard/frontend/models/loop.py	2012-09-11 14:34:59 +0000
+++ dashboard/frontend/models/loop.py	2012-09-12 14:34:19 +0000
@@ -39,7 +39,7 @@
     next_loop = models.OneToOneField('self', blank=True, null=True,
                                      unique=True, on_delete=models.SET_NULL,
                                      related_name='previous_loop',
-                                     verbose_name='Next loop in chain')
+                                     verbose_name='Next loop')
 
     def __init__(self, *args, **kwargs):
         self.log = Logger.getClassLogger(self)
@@ -237,7 +237,8 @@
 
         :return A list of tuples.
         """
-        excluded_fields = ['name', 'server', 'type', 'id', 'loop_ptr']
+        excluded_fields = ['name', 'server', 'type', 'id', 'loop_ptr',
+                           'next_loop']
         return self._get_wanted_fields(exclude=excluded_fields,
                                        verbose_name=True,
                                        capitalize=True)
@@ -259,3 +260,15 @@
         This method should be overridden by subclasses.
         """
         return configuration
+
+    @models.permalink
+    def get_detail_url(self):
+        return ()
+
+    @models.permalink
+    def get_build_url(self):
+        return ()
+
+    @models.permalink
+    def get_update_url(self):
+        return ()

=== modified file 'dashboard/frontend/models/textfield_loop.py'
--- dashboard/frontend/models/textfield_loop.py	2012-09-11 07:29:13 +0000
+++ dashboard/frontend/models/textfield_loop.py	2012-09-12 14:34:19 +0000
@@ -155,7 +155,7 @@
         # and define a separate functions to retrieve the lines from the
         # HTML template.
         excluded_fields = ['name', 'server', 'type', 'id', 'loop_ptr',
-                           'values']
+                           'values', 'next_loop']
         return self._get_wanted_fields(exclude=excluded_fields,
                                        verbose_name=True,
                                        capitalize=True)

=== modified file 'dashboard/frontend/templates/loop_detail.html'
--- dashboard/frontend/templates/loop_detail.html	2012-09-10 13:30:37 +0000
+++ dashboard/frontend/templates/loop_detail.html	2012-09-12 14:34:19 +0000
@@ -9,7 +9,23 @@
         {% for name, value in loop_detail.get_details %}
             <div>{{ name }}&#58;&nbsp;{{ value }}</div>
         {% endfor %}
+        {% for verbose_name, name, obj in next_loop %}
+            <div>
+                {% if name == None %}
+                    {{ verbose_name }}&#58;&nbsp;{{ name }}
+                {% else %}
+                    {{ verbose_name }}&#58;&nbsp;<a href="{{ obj.get_detail_url }}">{{ name }}</a>
+                {% endif %}
+            </div>
+        {% endfor %}
     </div>
+    {% block values %}
+            {% comment %}
+            This is used in the textfield detail views, to give a spece
+            to represent the key<>value values inserte by the user in a better
+            way.
+            {% endcomment %}
+    {% endblock values %}
     <div>
         <button type="button" id="update_button">Update loop configuration</button>
     </div>
@@ -25,8 +41,11 @@
 {% endblock content %}
 {% block scripts %}
 $("#schedule_button").click(function() {
-    $.get("{% if request.is_secure %}https{% else %}http{% endif %}://{{ request.get_host }}/{{ build_path }}/build/{{ loop_detail.name }}/", function(data) {
+    $.get("{% if request.is_secure %}https{% else %}http{% endif %}://{{ request.get_host }}/{{ loop_detail.get_build_url }}", function(data) {
     $(data).prependTo($("#builds"));
     });
 });
-{% endblock scripts %}
\ No newline at end of file
+$("#update_button").click(function() {
+    window.location.href = "{{ loop_detail.get_update_url }}";
+});
+{% endblock scripts %}

=== modified file 'dashboard/frontend/templates/textfield_loop_detail.html'
--- dashboard/frontend/templates/textfield_loop_detail.html	2012-09-10 13:30:37 +0000
+++ dashboard/frontend/templates/textfield_loop_detail.html	2012-09-12 14:34:19 +0000
@@ -1,28 +1,9 @@
 {% extends "loop_detail.html" %}
-{% block content %}
-    <div class="header"><h2>Loop {{ loop_detail.name }}</h2></div>
-    <div class="header"><h4>Details</h4></div>
-    <div>
-        {% for name, value in loop_detail.get_details %}
-            <div>{{ name }}&#58;&nbsp;{{ value }}</div>
-        {% endfor %}
-    </div>
+{% block values %}
     <div class="header"><h4>Values</h4></div>
     <div class="textfield">
         {% for value in loop_detail.get_values %}
             <div>{{ value }}</div>
         {% endfor %}
     </div>
-    <div>
-        <button type="button" id="update_button">Update loop configuration</button>
-    </div>
-    <div class="header"><h4>Builds</h4></div>
-    <div id="builds">
-        {% for build in builds %}
-            {% include "build.html" %}
-        {% endfor %}
-    </div>
-    <div>
-        <button type="button" id="schedule_button">Schedule new build</button>
-    </div>
-{% endblock content %}
+{% endblock values %}

=== modified file 'dashboard/frontend/tests/test_clientresponse.py'
--- dashboard/frontend/tests/test_clientresponse.py	2012-09-10 08:21:41 +0000
+++ dashboard/frontend/tests/test_clientresponse.py	2012-09-12 14:34:19 +0000
@@ -108,8 +108,7 @@
         # Assert template names for detail page.
         template_names = [template.name for template in
                           response.templates]
-        self.assertEquals(template_names, ["integration_loop_detail.html",
-                                           "loop_detail.html",
+        self.assertEquals(template_names, ["loop_detail.html",
                                            "base.html",
                                            "login.html",
                                            ])

=== modified file 'dashboard/frontend/views/loop_detail_view.py'
--- dashboard/frontend/views/loop_detail_view.py	2012-09-10 07:36:24 +0000
+++ dashboard/frontend/views/loop_detail_view.py	2012-09-12 14:34:19 +0000
@@ -20,6 +20,7 @@
 from django.utils.decorators import method_decorator
 from frontend.models.loop import Loop
 
+
 class LoopDetailView(DetailView):
 
     # These two fields should *never* be overridden.
@@ -28,19 +29,44 @@
     # The following fields have to be overridden in each sub-class.
     model = Loop
     template_name = 'loop_detail.html'
-    # This is necessary for each loop to define the URL path for the build
-    # view. This must be overridden.
-    build_path = 'loop'
 
     @method_decorator(login_required)
     def dispatch(self, *args, **kwargs):
         return super(LoopDetailView, self).dispatch(*args, **kwargs)
 
     def get_context_data(self, **kwargs):
-        '''Get the context data passed to template.'''
+        """Get the context data passed to template."""
         context = super(LoopDetailView, self).get_context_data(**kwargs)
         loop_pointer = self.object.loop_ptr
         context['request'] = self.request
         context['builds'] = loop_pointer.loopbuild_set.all()
-        context['build_path'] = self.build_path
+        context['next_loop'] = self._get_next_loop()
         return context
+
+    def _get_next_loop(self):
+        """
+        Retrieves all the necessary information to visualize the next loop as
+        a link.
+
+        :return A list with a three-element tuple containing the verbose name
+        of the field, the name of the pointed loop, and the actual instance of
+        the pointed loop.
+        """
+        field = self.object._meta.get_field('next_loop')
+        pk = self.object.next_loop_id
+        verbose_name = field.verbose_name.capitalize()
+        # XXX changed to isinstance, since tests were failing: we need to
+        # mock Loop, not the model variable, in a better way in our tests.
+        if isinstance(pk, int):
+            # Get the pointer to the object we want to retrieve, and retrieve
+            # the actual instance. We can do this since all our loops inherits
+            # from the same base class (Loop), and we have the information
+            # about the loop type (in type). Django has a feature to retrieve
+            # the correct object using the lower-case version of the class
+            # name.
+            ptr = Loop.objects.get(pk=pk)
+            obj = getattr(ptr, ptr.type.lower())
+            other_loop = [(verbose_name, ptr.name, obj)]
+        else:
+            other_loop = [(verbose_name, None, None)]
+        return other_loop