From 523f9940233aba5b8651f63258b7519f85b481ea Mon Sep 17 00:00:00 2001
From: Cam Saul <cam@getluckybird.com>
Date: Thu, 19 Mar 2015 18:44:40 -0700
Subject: [PATCH] fix annotation creation. FANCY COLOR LOGGING FOR API CALLS

---
 project.clj                                   |   1 +
 .../admin/datasets/datasets.controllers.js    | 133 +++++++++++-------
 .../app/annotation/annotation.directives.js   |  21 +--
 .../app/annotation/annotation.services.js     |  36 +++--
 .../annotation/partials/annotation_form.html  |   4 +-
 src/metabase/core.clj                         |   4 +-
 src/metabase/middleware/log_api_call.clj      |  41 ++++--
 7 files changed, 150 insertions(+), 90 deletions(-)

diff --git a/project.clj b/project.clj
index 2ec1a05e2bd..313242a8f95 100644
--- a/project.clj
+++ b/project.clj
@@ -21,6 +21,7 @@
                  [cheshire "5.4.0"]                                   ; fast JSON encoding (used by Ring JSON middleware)
                  [clj-http-lite "0.2.1"]                              ; HTTP client; lightweight version of clj-http that uses HttpURLConnection instead of Apache
                  [clj-time "0.9.0"]                                   ; library for dealing with date/time
+                 [colorize "0.1.1" :exclusions [org.clojure/clojure]] ; string output with ANSI color codes (for logging)
                  [com.cemerick/friend "0.2.1"]                        ; auth library
                  [com.h2database/h2 "1.4.186"]                        ; embedded SQL database
                  [com.mattbertolini/liquibase-slf4j "1.2.1"]
diff --git a/resources/frontend_client/app/admin/datasets/datasets.controllers.js b/resources/frontend_client/app/admin/datasets/datasets.controllers.js
index 351063b8d73..d19f24e6270 100644
--- a/resources/frontend_client/app/admin/datasets/datasets.controllers.js
+++ b/resources/frontend_client/app/admin/datasets/datasets.controllers.js
@@ -5,12 +5,12 @@ var AdminDatasetsControllers = angular.module('corvusadmin.datasets.controllers'
 
 AdminDatasetsControllers.controller('AdminDatasetList', ['$scope', 'Metabase', function($scope, Metabase) {
 
-    $scope.syncDatabase = function (databaseId) {
+    $scope.syncDatabase = function(databaseId) {
         Metabase.db_sync_metadata({
             'dbId': databaseId
-        }, function (result) {
+        }, function(result) {
             // hmmm, what should we do here?
-        }, function (error) {
+        }, function(error) {
             console.log('error syncing database', error);
         });
     };
@@ -21,18 +21,20 @@ AdminDatasetsControllers.controller('AdminDatasetList', ['$scope', 'Metabase', f
 
             Metabase.db_list({
                 'orgId': org.id
-            }, function (dbs) {
+            }, function(dbs) {
                 _.map(dbs, function(db) {
                     $scope.databases.push(db);
                 });
 
                 _.map($scope.databases, function(db, index) {
                     var dbindex = index;
-                    Metabase.db_tables({'dbId': db.id}, function (tables) {
+                    Metabase.db_tables({
+                        'dbId': db.id
+                    }, function(tables) {
                         $scope.databases[dbindex].tables = tables;
                     });
                 });
-            }, function (error) {
+            }, function(error) {
                 console.log('error getting database list', error);
             });
         }
@@ -49,9 +51,14 @@ AdminDatasetsControllers.controller('AdminDatasetEdit', ['$scope', '$routeParams
         containment: '.EntityGroup',
         orderChanged: function(event) {
             // Change order here
-            var new_order = _.map($scope.fields, function(field){return field.id;});
-            Metabase.table_reorder_fields({'tableId': $routeParams.tableId, 'new_order': new_order});
-        },
+            var new_order = _.map($scope.fields, function(field) {
+                return field.id;
+            });
+            Metabase.table_reorder_fields({
+                'tableId': $routeParams.tableId,
+                'new_order': new_order
+            });
+        }
     };
 
 
@@ -68,8 +75,8 @@ AdminDatasetsControllers.controller('AdminDatasetEdit', ['$scope', '$routeParams
         }
     });
 
-    $scope.getIdFields = function(){
-            // fetch the ID fields
+    $scope.getIdFields = function() {
+        // fetch the ID fields
         Metabase.db_idfields({
             'dbId': $scope.table.db.id
         }, function(result) {
@@ -85,21 +92,21 @@ AdminDatasetsControllers.controller('AdminDatasetEdit', ['$scope', '$routeParams
 
     };
 
-    $scope.decorateWithTargets = function(){
+    $scope.decorateWithTargets = function() {
         console.log($scope.table);
-        $scope.table.fields.forEach(function(field){
-            if (field.target){
+        $scope.table.fields.forEach(function(field) {
+            if (field.target) {
                 field.target_id = field.target.id;
             }
         });
     };
 
-    $scope.syncMetadata = function () {
+    $scope.syncMetadata = function() {
         Metabase.table_sync_metadata({
             'tableId': $routeParams.tableId
-        }, function (result) {
+        }, function(result) {
             // nothing to do here really
-        }, function (error) {
+        }, function(error) {
             console.log('error doing metabase sync', error);
         });
     };
@@ -116,18 +123,22 @@ AdminDatasetsControllers.controller('AdminDatasetEdit', ['$scope', '$routeParams
         }
     };
 
-    $scope.inlineSpecialTypeChange = function(idx){
+    $scope.inlineSpecialTypeChange = function(idx) {
         // If we are changing the field from a FK to something else, we should delete any FKs present
         var field = $scope.table.fields[idx];
-        if (field.target_id && field.special_type != "fk"){
+        if (field.target_id && field.special_type != "fk") {
             // we have something that used to be an FK and is now not an FK
             // Let's delete its foreign keys
-            var fks = Metabase.field_foreignkeys({'fieldId': field.id}, function(result){
-                fks.forEach(function(fk){
+            var fks = Metabase.field_foreignkeys({
+                'fieldId': field.id
+            }, function(result) {
+                fks.forEach(function(fk) {
                     console.log("deleting ", fk);
-                    ForeignKey.delete({'fkID': fks[0].id}, function(result){
+                    ForeignKey.delete({
+                        'fkID': fks[0].id
+                    }, function(result) {
                         console.log("deleted fk");
-                    }, function(error){
+                    }, function(error) {
                         console.log("error deleting fk");
                     });
                 });
@@ -161,27 +172,39 @@ AdminDatasetsControllers.controller('AdminDatasetEdit', ['$scope', '$routeParams
             var field = $scope.table.fields[idx];
             var new_target_id = field.target_id;
 
-            var fks = Metabase.field_foreignkeys({'fieldId': field.id});
+            var fks = Metabase.field_foreignkeys({
+                'fieldId': field.id
+            });
 
-            if(fks.length > 0){
+            if (fks.length > 0) {
                 // delete this key
                 var relationship_id = 0;
-                 ForeignKey.delete({
-                     'fkID': fks[0].id
-                 }, function(result){
-                     console.log("Deleted FK");
-                     Metabase.field_addfk({"db": $scope.table.db.id, "fieldId":field.id,'target_field': new_target_id, "relationship": "Mt1"});
-
-                 }, function(error){
-                     console.log('Error deleting key ', error);
-                 });
-            }else{
-                Metabase.field_addfk({"db": $scope.table.db.id, "fieldId":field.id,'target_field': new_target_id, "relationship": "Mt1"});
+                ForeignKey.delete({
+                    'fkID': fks[0].id
+                }, function(result) {
+                    console.log("Deleted FK");
+                    Metabase.field_addfk({
+                        "db": $scope.table.db.id,
+                        "fieldId": field.id,
+                        'target_field': new_target_id,
+                        "relationship": "Mt1"
+                    });
+
+                }, function(error) {
+                    console.log('Error deleting key ', error);
+                });
+            } else {
+                Metabase.field_addfk({
+                    "db": $scope.table.db.id,
+                    "fieldId": field.id,
+                    'target_field': new_target_id,
+                    "relationship": "Mt1"
+                });
             }
         }
     };
 
-    $scope.deleteTarget = function(field, target){
+    $scope.deleteTarget = function(field, target) {
 
     };
 }]);
@@ -189,7 +212,7 @@ AdminDatasetsControllers.controller('AdminDatasetEdit', ['$scope', '$routeParams
 
 AdminDatasetsControllers.controller('AdminTableDependents', ['$scope', '$routeParams', '$location', 'Metabase', function($scope, $routeParams, $location, Metabase) {
 
- if ($routeParams.tableId) {
+    if ($routeParams.tableId) {
         Metabase.table_dependents({
             'tableId': $routeParams.tableId
         }, function(result) {
@@ -215,30 +238,30 @@ AdminDatasetsControllers.controller('AdminFieldDetail', ['$scope', '$routeParams
             // grab where this field is foreign keyed to
             Metabase.field_foreignkeys({
                 'fieldId': $routeParams.fieldId
-            }, function (result) {
+            }, function(result) {
                 $scope.fks = result;
-            }, function (error) {
+            }, function(error) {
                 console.log('error getting fks for field', error);
             });
 
             // grab summary data about our field
             Metabase.field_summary({
                 'fieldId': $routeParams.fieldId
-            }, function (result){
+            }, function(result) {
                 $scope.field_summary = result;
-            }, function (error){
+            }, function(error) {
                 console.log('error gettting field summary', error);
             });
 
             // grab our field values
             Metabase.field_values({
                 'fieldId': $routeParams.fieldId
-            }, function (result){
+            }, function(result) {
                 $scope.field_values = result;
-            }, function (error){
+            }, function(error) {
                 console.log('error getting field values', error);
             });
-        }, function (error) {
+        }, function(error) {
             console.log(error);
             if (error.status == 404) {
                 $location.path('/');
@@ -259,13 +282,13 @@ AdminDatasetsControllers.controller('AdminFieldDetail', ['$scope', '$routeParams
         }
     };
 
-    $scope.updateMappedValues = function(){
+    $scope.updateMappedValues = function() {
         Metabase.field_value_map_update({
             'fieldId': $routeParams.fieldId,
             'values_map': $scope.field_values.human_readable_values
-        }, function (result){
+        }, function(result) {
             // nothing to do
-        }, function (error){
+        }, function(error) {
             console.log('Error');
         });
     };
@@ -281,13 +304,17 @@ AdminDatasetsControllers.controller('AdminFieldDetail', ['$scope', '$routeParams
         $scope.fks = $scope.fks.slice(0);
         $scope.fks.push(relationship);
     };
-     $scope.deleteRelationship = function(relationship_id) {
+    $scope.deleteRelationship = function(relationship_id) {
         // this is here to clone the original array so that we can modify it
         // by default the deserialized data from an api response is immutable
-        ForeignKey.delete({'fkID': relationship_id}, function(result){
-            $scope.fks = _.reject($scope.fks, function(fk){return fk.id == relationship_id;});
-        }, function(error){
+        ForeignKey.delete({
+            'fkID': relationship_id
+        }, function(result) {
+            $scope.fks = _.reject($scope.fks, function(fk) {
+                return fk.id == relationship_id;
+            });
+        }, function(error) {
             console.log('Error deleting key ', error);
         });
     };
-}]);
+}]);
\ No newline at end of file
diff --git a/resources/frontend_client/app/annotation/annotation.directives.js b/resources/frontend_client/app/annotation/annotation.directives.js
index 2d1fa0e4d38..0b23d6cebf2 100644
--- a/resources/frontend_client/app/annotation/annotation.directives.js
+++ b/resources/frontend_client/app/annotation/annotation.directives.js
@@ -3,7 +3,7 @@
 var AnnotationDirectives = angular.module('corvus.annotation.directives', ['corvus.annotation.services', 'corvus.services']);
 
 AnnotationDirectives.directive('cvAnnotationForm', ['$filter', 'Annotation', 'AppState',
-    function ($filter, Annotation, AppState) {
+    function($filter, Annotation, AppState) {
         function link(scope, element, attr) {
 
             scope.DATE_FORMAT = "MMM dd, yyyy";
@@ -87,18 +87,23 @@ AnnotationDirectives.directive('cvAnnotationForm', ['$filter', 'Annotation', 'Ap
                     // we need to set the orgId before we save
                     scope.new_annotation.organization = AppState.model.currentOrg.id;
 
+                    // If title string is empty then remove it
+                    if (!scope.new_annotation.title || scope.new_annotation.title.length === 0) {
+                        delete scope.new_annotation.title;
+                    }
+
                     // make sure we handle start/end date formatting properly
                     scope.new_annotation.start = scope.startPicker.date.toISOString();
                     scope.new_annotation.end = scope.endPicker.date.toISOString();
 
-                    Annotation.create(scope.new_annotation, function (annotation) {
+                    Annotation.create(scope.new_annotation, function(annotation) {
                         scope.annotations.unshift(annotation);
                         // TODO: user confirmation
 
                         updateVisibleAnnotations();
 
                         prepNewAnnotation();
-                    }, function (error) {
+                    }, function(error) {
                         console.log('error creating annotation', error);
                     });
                 } else {
@@ -115,7 +120,7 @@ AnnotationDirectives.directive('cvAnnotationForm', ['$filter', 'Annotation', 'Ap
 
                 scope.new_annotation = {
                     'object_model': scope.objectModel,
-                    'object_id': scope.objectId
+                    'object_id': (scope.objectId ? Number.parseInt(scope.objectId) : null) // convert objectId to an int since @bindings are always strings
                 };
             };
 
@@ -125,17 +130,17 @@ AnnotationDirectives.directive('cvAnnotationForm', ['$filter', 'Annotation', 'Ap
                         'org': org.id,
                         'object_model': scope.objectModel,
                         'object_id': scope.objectId
-                    }, function (annotations) {
+                    }, function(annotations) {
                         scope.annotations = annotations;
 
                         updateVisibleAnnotations();
-                    }, function (error) {
+                    }, function(error) {
                         console.log('error getting annotations list', error);
                     });
                 }
             };
 
-            scope.$on('appstate:organization', function (event, org) {
+            scope.$on('appstate:organization', function(event, org) {
                 loadAnnotations(org);
             });
 
@@ -156,4 +161,4 @@ AnnotationDirectives.directive('cvAnnotationForm', ['$filter', 'Annotation', 'Ap
             link: link
         };
     }
-]);
+]);
\ No newline at end of file
diff --git a/resources/frontend_client/app/annotation/annotation.services.js b/resources/frontend_client/app/annotation/annotation.services.js
index f4e1857780a..d5ff629f147 100644
--- a/resources/frontend_client/app/annotation/annotation.services.js
+++ b/resources/frontend_client/app/annotation/annotation.services.js
@@ -3,35 +3,47 @@
 var AnnotationServices = angular.module('corvus.annotation.services', ['ngResource', 'ngCookies']);
 
 AnnotationServices.factory('Annotation', ['$resource', '$cookies',
-    function($resource, $cookies){
+    function($resource, $cookies) {
         return $resource('/api/annotation/:annotationId', {}, {
             list: {
                 url: '/api/annotation/',
-                method:'GET',
-                isArray:true
+                method: 'GET',
+                isArray: true
             },
             create: {
                 url: '/api/annotation',
-                method:'POST',
-                headers: {'X-CSRFToken': function() { return $cookies.csrftoken; }}
+                method: 'POST',
+                headers: {
+                    'X-CSRFToken': function() {
+                        return $cookies.csrftoken;
+                    }
+                }
             },
             get: {
                 url: '/api/annotation/:annotationId',
-                method:'GET'
+                method: 'GET'
             },
             update: {
                 url: '/api/annotation/:annotationId',
-                params:{
+                params: {
                     annotation_id: '@annotationId'
                 },
-                method:'PUT',
-                headers: {'X-CSRFToken': function() { return $cookies.csrftoken; }}
+                method: 'PUT',
+                headers: {
+                    'X-CSRFToken': function() {
+                        return $cookies.csrftoken;
+                    }
+                }
             },
             delete: {
                 url: '/api/annotation/:annotationId',
-                method:'DELETE',
-                headers: {'X-CSRFToken': function() { return $cookies.csrftoken; }},
+                method: 'DELETE',
+                headers: {
+                    'X-CSRFToken': function() {
+                        return $cookies.csrftoken;
+                    }
+                }
             }
         });
     }
-]);
+]);
\ No newline at end of file
diff --git a/resources/frontend_client/app/annotation/partials/annotation_form.html b/resources/frontend_client/app/annotation/partials/annotation_form.html
index 9be7823bff9..90bb0560fb0 100644
--- a/resources/frontend_client/app/annotation/partials/annotation_form.html
+++ b/resources/frontend_client/app/annotation/partials/annotation_form.html
@@ -30,7 +30,7 @@
 
         	<div class="mt3 clearfix">
         		<div class="float-right">
-        			<button class="Button Button--primary" ng-click="create()">Save</button>
+        			<button class="Button Button--primary" ng-disabled="!new_annotation.body || new_annotation.body.length === 0" ng-click="create()">Save</button>
         			<button class="Button" ng-click="toggleEditMode()">Cancel</button>
         		</div>
         		<div>
@@ -57,4 +57,4 @@
         </div>
     </div>
 
-</div>
\ No newline at end of file
+</div>
diff --git a/src/metabase/core.clj b/src/metabase/core.clj
index 376b4beaf48..7f9c1687e00 100644
--- a/src/metabase/core.clj
+++ b/src/metabase/core.clj
@@ -23,11 +23,11 @@
 (def app
   "The primary entry point to the HTTP server"
   (-> routes/routes
-      (log-api-call :request)
+      (log-api-call :request :response)
       format-response         ; [METABASE] Do formatting before converting to JSON so serializer doesn't barf
-      wrap-json-response      ; middleware to automatically serialize suitable objects as JSON in responses
       (wrap-json-body         ; extracts json POST body and makes it avaliable on request
         {:keywords? true})
+      wrap-json-response      ; middleware to automatically serialize suitable objects as JSON in responses
       wrap-keyword-params     ; converts string keys in :params to keyword keys
       wrap-params             ; parses GET and POST params as :query-params/:form-params and both as :params
       auth/wrap-sessionid     ; looks for a Metabase sessionid and assocs as :metabase-sessionid
diff --git a/src/metabase/middleware/log_api_call.clj b/src/metabase/middleware/log_api_call.clj
index b06ce8e9c94..3bc771acf21 100644
--- a/src/metabase/middleware/log_api_call.clj
+++ b/src/metabase/middleware/log_api_call.clj
@@ -1,10 +1,17 @@
 (ns metabase.middleware.log-api-call
   "Middleware to log API calls. Primarily for debugging purposes."
-  (:require [clojure.pprint :refer [pprint]]
-            [clojure.tools.logging :as log]))
+  (:require [clojure.math.numeric-tower :as math]
+            [clojure.pprint :refer [pprint]]
+            [clojure.tools.logging :as log]
+            [colorize.core :as color]))
 
 (declare api-call?
-         log-request)
+         log-request
+         log-response)
+
+(def ^:private only-display-output-on-error
+  "Set this to `false` to see out API responses."
+  true)
 
 (defn log-api-call
   "Middleware to log `:request` and/or `:response` by passing corresponding OPTIONS."
@@ -18,14 +25,13 @@
                 (when log-request?
                   (log-request request))
                 (let [start-time (System/nanoTime)
-                      response (handler request)]
-                  (when log-request?
-                    (let [elapsed-time (-> (- (System/nanoTime) start-time)
-                                           double
-                                           (/ 1000000.0))]
-                      (log/debug (str "Elapsed time: " elapsed-time " msecs"))))
+                      response (handler request)
+                      elapsed-time (-> (- (System/nanoTime) start-time)
+                                       double
+                                       (/ 1000000.0)
+                                       math/round)]
                   (when log-response?
-                    (log/debug (with-out-str (pprint response))))
+                    (log-response request response elapsed-time))
                   response))))))
 
 (defn- api-call?
@@ -34,6 +40,15 @@
   (and (>= (count uri) 4)
        (= (.substring uri 0 4) "/api")))
 
-(defn- log-request [{:keys [uri request-method params]}]
-  (log/debug (.toUpperCase (name request-method)) uri) (when-not (empty? params)
-                                                         (with-out-str (pprint params))))
+(defn- log-request [{:keys [uri request-method body]}]
+  (log/debug (color/blue (format "%s %s " (.toUpperCase (name request-method)) uri)
+                         (when-let [body-output (with-out-str (pprint body))]
+                           (str "\n" body-output)))))
+
+(defn- log-response [{:keys [uri request-method]} {:keys [status body]} elapsed-time]
+  (let [error? (>= status 400)
+        color-fn (if error? color/red color/green)]
+    (log/debug (color-fn (format "%s %s %d (%d ms)" (.toUpperCase (name request-method)) uri status elapsed-time)
+                         (when (and error? only-display-output-on-error)
+                           (when-let [body-output (with-out-str (pprint body))]
+                             (str "\n" body-output)))))))
-- 
GitLab