Commit c32fedcd authored by Sean Hammond's avatar Sean Hammond

Validate domainModel not vm.annotation

When saving an annotation to the server, the object that we actually send to
the server is:

1. `domainModel`, after being updated by `updateDomainModel()`, in the case
   of creating a new annotation, or

2. The temporary local variable `updatedDomainModel` in the case of editing
   an existing annotation

`vm.annotation` is never the object that gets sent to the server.

So pass `domainModel` or `updatedDomainModel` to `validate()`, not
`vm.annotation`.
parent 803aaa6b
...@@ -191,21 +191,21 @@ function updateViewModel(domainModel, vm, permissions) { ...@@ -191,21 +191,21 @@ function updateViewModel(domainModel, vm, permissions) {
* @param {object} annotation The annotation to be validated * @param {object} annotation The annotation to be validated
* *
*/ */
function validate(annotation, permissions) { function validate(domainModel) {
if (!angular.isObject(annotation)) { if (!angular.isObject(domainModel)) {
return; return;
} }
permissions = permissions || {}; var permissions = domainModel.permissions || {};
var readPermissions = permissions.read || []; var readPermissions = permissions.read || [];
var targets = annotation.target || []; var targets = domainModel.target || [];
if (annotation.tags && annotation.tags.length) { if (domainModel.tags && domainModel.tags.length) {
return annotation.tags.length; return domainModel.tags.length;
} }
if (annotation.text && annotation.text.length) { if (domainModel.text && domainModel.text.length) {
return annotation.text.length; return domainModel.text.length;
} }
var worldReadable = false; var worldReadable = false;
...@@ -650,10 +650,6 @@ function AnnotationController( ...@@ -650,10 +650,6 @@ function AnnotationController(
return flash.info('Please sign in to save your annotations.'); return flash.info('Please sign in to save your annotations.');
} }
if (!validate(vm.annotation, domainModel.permissions)) {
return flash.info('Please add text or a tag before publishing.');
}
// Update stored tags with the new tags of this annotation. // Update stored tags with the new tags of this annotation.
var newTags = vm.annotation.tags.filter(function(tag) { var newTags = vm.annotation.tags.filter(function(tag) {
var tags = domainModel.tags || []; var tags = domainModel.tags || [];
...@@ -664,6 +660,11 @@ function AnnotationController( ...@@ -664,6 +660,11 @@ function AnnotationController(
switch (vm.action) { switch (vm.action) {
case 'create': case 'create':
updateDomainModel(domainModel, vm.annotation); updateDomainModel(domainModel, vm.annotation);
if (!validate(domainModel)) {
return flash.info('Please add text or a tag before publishing.');
}
var onFulfilled = function() { var onFulfilled = function() {
$rootScope.$emit('annotationCreated', domainModel); $rootScope.$emit('annotationCreated', domainModel);
view(); view();
...@@ -677,6 +678,11 @@ function AnnotationController( ...@@ -677,6 +678,11 @@ function AnnotationController(
case 'edit': case 'edit':
var updatedModel = angular.copy(domainModel); var updatedModel = angular.copy(domainModel);
updateDomainModel(updatedModel, vm.annotation); updateDomainModel(updatedModel, vm.annotation);
if (!validate(updatedModel)) {
return flash.info('Please add text or a tag before publishing.');
}
onFulfilled = function() { onFulfilled = function() {
angular.copy(updatedModel, domainModel); angular.copy(updatedModel, domainModel);
$rootScope.$emit('annotationUpdated', domainModel); $rootScope.$emit('annotationUpdated', domainModel);
......
...@@ -342,7 +342,7 @@ describe('annotation.js', function() { ...@@ -342,7 +342,7 @@ describe('annotation.js', function() {
var i; var i;
var values = [2, 'foo', true, null]; var values = [2, 'foo', true, null];
for (i = 0; i < values.length; i++) { for (i = 0; i < values.length; i++) {
assert.equal(validate(values[i]), undefined); assert.equal(validate(values[i]));
} }
}); });
...@@ -401,8 +401,8 @@ describe('annotation.js', function() { ...@@ -401,8 +401,8 @@ describe('annotation.js', function() {
it('returns false for public highlights', function() { it('returns false for public highlights', function() {
assert.equal( assert.equal(
validate( validate(
{text: undefined, tags: undefined, target: [1, 2, 3]}, {text: undefined, tags: undefined, target: [1, 2, 3],
{read: ['group:__world__']} permissions: {read: ['group:__world__']}}
), ),
false); false);
}); });
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment