Skip to content
Snippets Groups Projects
Commit d69039d5 authored by Cam Saül's avatar Cam Saül
Browse files

Merge pull request #236 from metabase/fix_annotation_creation

fix annotation creation
parents f9987894 11678f1f
Branches
Tags
No related merge requests found
......@@ -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"]
......
......@@ -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
......@@ -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
......@@ -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
......@@ -14,7 +14,7 @@
<span class="text-grey-3"> added an annotation for </span>
<span class="text-brand">{{getAnnotationTimeframe(annotation)}}</span>
</p>
<p class="text-grey-5">{{annotation.title}}</p>
<p class="text-grey-4">{{annotation.body}}</p>
</div>
</li>
......@@ -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>
......@@ -7,7 +7,8 @@
[metabase.db :refer :all]
[metabase.models.hydrate :refer :all]
(metabase.models [annotation :refer [Annotation annotation-general annotation-description]]
[org :refer [Org]])
[org :refer [Org]]
[user :refer [User]])
[metabase.util :as util]))
......@@ -34,7 +35,7 @@
(sel :many Annotation :organization_id org :object_type_id object_model :object_id object_id (korma/order :start :DESC))
;; default is to return all annotations
(sel :many Annotation :organization_id org (korma/order :start :DESC)))
(hydrate :author)))
(simple-batched-hydrate User :author_id :author)))
(defendpoint POST "/" [:as {{:keys [organization start end title body annotation_type object_model object_id]
......
......@@ -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
......
(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 sensitive-fields
"Fields that we should censor before logging."
#{:password})
(defn- scrub-sensitive-fields
"Replace values of fields in `sensitive-fields` with `\"**********\"` before logging."
[request]
(clojure.walk/prewalk (fn [form]
(if-not (and (vector? form)
(= (count form) 2)
(keyword? (first form))
(contains? sensitive-fields (first form)))
form
[(first form) "**********"]))
request))
(def ^:private only-display-output-on-error
"Set this to `false` to see all API responses."
true)
(defn log-api-call
"Middleware to log `:request` and/or `:response` by passing corresponding OPTIONS."
......@@ -18,14 +41,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 +56,21 @@
(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 (or (string? body) (coll? body))
(str "\n" (with-out-str (pprint (scrub-sensitive-fields body))))))))
(defn- log-response [{:keys [uri request-method]} {:keys [status body]} elapsed-time]
(let [log-error (fn [& args] (log/error (apply str args))) ; inconveniently these are not macros
log-debug (fn [& args] (log/debug (apply str args)))
log-warn (fn [& args] (log/warn (apply str args)))
[error? color-fn log-fn] (cond
(>= status 500) [true color/red log-error]
(= status 403) [true color/red log-warn]
(>= status 400) [true color/red log-debug]
:else [false color/green log-debug])]
(log-fn (color-fn (format "%s %s %d (%d ms)" (.toUpperCase (name request-method)) uri status elapsed-time)
(when (or error? (not only-display-output-on-error))
(when (or (string? body) (coll? body))
(str "\n" (with-out-str (pprint body)))))))))
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment