Annotation line in graphs. #371

stevanradakovic · 2018-10-11T10:46:49Z

Add UI for adding/updating annotation in build pages.
Add annotation lines in metrics with corresponding label.

Fixes #179.

squad/frontend/static/download.conf

mwasilew · 2018-10-11T13:28:24Z

squad/frontend/urls.py

@@ -36,4 +36,5 @@
    url(r'^(%s)/(%s)/build/([^/]+)/testrun/([^/]+)/metadata$' % ((slug_pattern,) * 2), views.test_run_metadata, name='testrun_metadata'),
    url(r'^(%s)/(%s)/build/([^/]+)/testrun/([^/]+)/attachments/([^/]+)$' % ((slug_pattern,) * 2), views.attachment, name='attachment'),
    url(r'^testjob/(\d+)$', views.test_job, name='test_job'),
+    url(r'^(%s)/(%s)/update-annotation/$' % ((slug_pattern,) * 2), views.update_annotation, name='update_annotation'),


do we consider annotations 'sensitive'? If not, maybe create REST endpoint and update using PUT requests?

mwasilew · 2018-10-16T11:01:02Z

oups, I think I merged in wrong order. Could you please rebase?

mwasilew · 2018-10-17T10:09:04Z

It seems to work. I have one doubt though. When I set annotation for the build on the chart I'm getting as many labels as there are environments. Is that expected? I think there should be only one annotation label no matter how many environments.

squad/frontend/static/squad/annotation.js

squad/frontend/templates/squad/build-nav.html

squad/frontend/static/squad/charts.js

mwasilew · 2018-10-18T08:27:23Z

squad/frontend/static/squad/charts.js

@@ -25,7 +25,6 @@ function ChartsController($scope, $http, $location, $compile) {
                minDate = data[name][0][0]


I'm getting the following error:
TypeError: "data[name][0] is undefined"

terceiro · 2018-10-22T18:51:23Z

hi, I'm sorry but after #377 was merged there a few merge conflicts here. that is a pretty big PR so delaying much longer would make things even worse.

terceiro · 2018-10-22T18:52:07Z

(basically the template changes bow have to be done against the *.jinja2 counterparts of the templates you changed originally)

stevanradakovic · 2018-10-23T09:54:30Z

@terceiro I've rebased the PR against the jinja2 update.
AFAIK, Milosz was to do a final review regarding my latest changes on Thursday, but since he's not here this week it'd be great if you can go through this PR so it doesn't sit around for too long. Thanks!

terceiro

The UI looks good, thanks!

I felt adding annotations while looking at the chart is a bit cumbersome. I have to go back to the list of builds, look for the one I want by its number, then add the annotation there. ISTR that clicking on the circle (the points in the graph) used to lead you to the corresponding build page, maybe that got lost at some point?

Also, when you have two annotations that overlap, the ilne from one of them can go over the label for the other:

Is it possible to make all the vertical lines on one z-index, and the labels on another?

I have also some specific comments on the code.

terceiro · 2018-10-24T14:34:52Z

squad/api/urls.py

@@ -21,4 +21,5 @@
    url(r'^data/(%s)/(%s)' % ((slug_pattern,) * 2), data.get),
    url(r'^resubmit/([0-9]+)', ci.resubmit_job),
    url(r'^forceresubmit/([0-9]+)', ci.force_resubmit_job),
+    url(r'^update-annotation/(%s)/(%s)/(%s)$' % ((slug_pattern,) * 3), views.update_annotation, name='update_annotation'),


also IMO it would probably be better to use the REST API here.

specially seeing the implementation of this view below, since it has nothing special. it would be better to not have to maintain that code.

terceiro · 2018-10-24T14:37:27Z

squad/core/queries.py

        )
        entry[environment] = [
-            [int(p['test_run__build__datetime'].timestamp()), p['result'], p['test_run__build__version']] for p in series
+            [int(p['test_run__build__datetime'].timestamp()), p['result'], p['test_run__build__version'], "" if p['test_run__build__annotation__description'] is None else p['test_run__build__annotation__description']] for p in series


you can use p['test_run__build__annotation__description'] or "" which is simpler and shorter

terceiro · 2018-10-24T14:37:39Z

squad/core/queries.py

        ).annotate(
            pass_percentage=100 * Sum('tests_pass') / Sum(tests_total)
        ).order_by('test_run__build__datetime')

        results[environment] = [
-            [int(s['test_run__build__datetime'].timestamp()), s['pass_percentage'], s['test_run__build__version']]
+            [int(s['test_run__build__datetime'].timestamp()), s['pass_percentage'], s['test_run__build__version'], "" if s['test_run__build__annotation__description'] is None else s['test_run__build__annotation__description']]


terceiro · 2018-10-24T14:58:22Z

squad/frontend/views.py

@@ -9,6 +9,7 @@
 from django.contrib.auth.decorators import login_required
 from django.http import HttpResponse, Http404
 from django.shortcuts import render, get_object_or_404, redirect
+from django.urls import reverse


this seems unused

stevanradakovic · 2018-10-25T10:22:51Z

We have the chart click event, it should work for opening build pages, otherwise there's a bug in master. The overlapping is a bug in the annotation plugin chartjs/chartjs-plugin-annotation#94 so we can maybe fix it upstream at some point.
I'll take a look at the code comments, thanks!

terceiro · 2018-10-25T14:16:40Z

just tested the click and it works on master

terceiro · 2018-10-25T14:20:40Z

(and it also works on your branch; not sure why it didn't work for me yesterday)

terceiro · 2018-10-25T14:24:06Z

I found what seems to be a error that is triggered whenever I hit "reload" having a few environments and one metrics selected:

 [Show/hide message details.] ReferenceError: None is not defined[Learn More] metrics:194:75
ReferenceError: "DATA is not defined"

this is Firefox 62, and it happens consistently on your branch, but not on master.

Add UI for adding/updating annotation in build pages. Add annotation lines in metrics with corresponding label. Fixes Linaro#179.

stevanradakovic · 2018-10-29T21:30:08Z

Fixed all reported issues with the latest push.

mwasilew · 2018-10-31T14:15:06Z

Looks good from my perspective.

stevanradakovic force-pushed the issue-179 branch from fb12d3f to b853ef0 Compare October 11, 2018 11:05

stevanradakovic requested review from terceiro and mwasilew October 11, 2018 12:23

stevanradakovic force-pushed the issue-179 branch from b853ef0 to 367c1f9 Compare October 11, 2018 12:34

mwasilew reviewed Oct 11, 2018

View reviewed changes

squad/frontend/static/download.conf Outdated Show resolved Hide resolved

mwasilew reviewed Oct 11, 2018

View reviewed changes

stevanradakovic force-pushed the issue-179 branch from 367c1f9 to 3d11c99 Compare October 12, 2018 09:56

stevanradakovic force-pushed the issue-179 branch from 3d11c99 to 8afc804 Compare October 16, 2018 11:08

stevanradakovic force-pushed the issue-179 branch from 8afc804 to 1d5f6ac Compare October 18, 2018 08:05

mwasilew suggested changes Oct 18, 2018

View reviewed changes

stevanradakovic force-pushed the issue-179 branch from 1d5f6ac to 8f5daa2 Compare October 18, 2018 11:39

stevanradakovic force-pushed the issue-179 branch from 8f5daa2 to 23782a5 Compare October 23, 2018 09:52

terceiro requested changes Oct 24, 2018

View reviewed changes

stevanradakovic force-pushed the issue-179 branch from 23782a5 to 7527184 Compare October 25, 2018 13:17

stevanradakovic force-pushed the issue-179 branch from 7527184 to 66c6910 Compare October 26, 2018 11:22

Annotation line in graphs.

8c1f21c

Add UI for adding/updating annotation in build pages. Add annotation lines in metrics with corresponding label. Fixes Linaro#179.

stevanradakovic force-pushed the issue-179 branch from 66c6910 to 8c1f21c Compare October 26, 2018 13:29

mwasilew approved these changes Oct 31, 2018

View reviewed changes

stevanradakovic mentioned this pull request Oct 31, 2018

Add tooltips on chart sliders. #386

Merged

terceiro approved these changes Oct 31, 2018

View reviewed changes

terceiro merged commit f0606e2 into Linaro:master Oct 31, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Annotation line in graphs. #371

Annotation line in graphs. #371

stevanradakovic commented Oct 11, 2018

mwasilew Oct 11, 2018

mwasilew commented Oct 16, 2018

mwasilew commented Oct 17, 2018

mwasilew Oct 18, 2018

terceiro commented Oct 22, 2018

terceiro commented Oct 22, 2018

stevanradakovic commented Oct 23, 2018 •

edited

Loading

terceiro left a comment

terceiro Oct 24, 2018

terceiro Oct 24, 2018

terceiro Oct 24, 2018

terceiro Oct 24, 2018

terceiro Oct 24, 2018

stevanradakovic commented Oct 25, 2018

terceiro commented Oct 25, 2018

terceiro commented Oct 25, 2018

terceiro commented Oct 25, 2018 •

edited

Loading

stevanradakovic commented Oct 29, 2018

mwasilew commented Oct 31, 2018

		@@ -25,7 +25,6 @@ function ChartsController($scope, $http, $location, $compile) {
		minDate = data[name][0][0]

Annotation line in graphs. #371

Annotation line in graphs. #371

Conversation

stevanradakovic commented Oct 11, 2018

mwasilew Oct 11, 2018

Choose a reason for hiding this comment

mwasilew commented Oct 16, 2018

mwasilew commented Oct 17, 2018

mwasilew Oct 18, 2018

Choose a reason for hiding this comment

terceiro commented Oct 22, 2018

terceiro commented Oct 22, 2018

stevanradakovic commented Oct 23, 2018 • edited Loading

terceiro left a comment

Choose a reason for hiding this comment

terceiro Oct 24, 2018

Choose a reason for hiding this comment

terceiro Oct 24, 2018

Choose a reason for hiding this comment

terceiro Oct 24, 2018

Choose a reason for hiding this comment

terceiro Oct 24, 2018

Choose a reason for hiding this comment

terceiro Oct 24, 2018

Choose a reason for hiding this comment

stevanradakovic commented Oct 25, 2018

terceiro commented Oct 25, 2018

terceiro commented Oct 25, 2018

terceiro commented Oct 25, 2018 • edited Loading

stevanradakovic commented Oct 29, 2018

mwasilew commented Oct 31, 2018

stevanradakovic commented Oct 23, 2018 •

edited

Loading

terceiro commented Oct 25, 2018 •

edited

Loading