[Views] Warn if view omits key

Warn if a view definition does not include a key,
and prune it from the list of views. This avoids
an underlying cause of WTD-628, which is fixed for
plots specifically in the previous commit but could
recur if other views were to omit keys.
This commit is contained in:
Victor Woeltjen
2015-01-06 10:14:38 -08:00
parent 0ef6c59643
commit 13f552ae81
3 changed files with 40 additions and 3 deletions

View File

@ -67,7 +67,7 @@
"provides": "viewService", "provides": "viewService",
"type": "provider", "type": "provider",
"implementation": "views/ViewProvider.js", "implementation": "views/ViewProvider.js",
"depends": [ "views[]" ] "depends": [ "views[]", "$log" ]
} }
], ],
"types": [ "types": [

View File

@ -37,7 +37,25 @@ define(
* @constructor * @constructor
* @param {View[]} an array of view definitions * @param {View[]} an array of view definitions
*/ */
function ViewProvider(views) { function ViewProvider(views, $log) {
// Views without defined keys cannot be used in the user
// interface, and can result in unexpected behavior. These
// are filtered out using this function.
function validate(view) {
var key = view.key;
// Leave a log message to support detection of this issue.
if (!key) {
$log.warn([
"No key specified for view in ",
(view.bundle || {}).path,
"; omitting this view."
].join(""));
}
return key;
}
// Check if an object has all capabilities designated as `needs` // Check if an object has all capabilities designated as `needs`
// for a view. Exposing a capability via delegation is taken to // for a view. Exposing a capability via delegation is taken to
@ -79,6 +97,9 @@ define(
}); });
} }
// Filter out any key-less views
views = views.filter(validate);
return { return {
/** /**
* Get all views which are applicable to this domain object. * Get all views which are applicable to this domain object.

View File

@ -25,6 +25,7 @@ define(
delegates = {}, delegates = {},
delegation, delegation,
mockDomainObject = {}, mockDomainObject = {},
mockLog,
provider; provider;
beforeEach(function () { beforeEach(function () {
@ -38,6 +39,7 @@ define(
mockDomainObject.useCapability = function (c, v) { mockDomainObject.useCapability = function (c, v) {
return capabilities[c] && capabilities[c].invoke(v); return capabilities[c] && capabilities[c].invoke(v);
}; };
mockLog = jasmine.createSpyObj("$log", ["warn", "info", "debug"]);
capabilities = {}; capabilities = {};
delegates = {}; delegates = {};
@ -48,7 +50,7 @@ define(
} }
}; };
provider = new ViewProvider([viewA, viewB, viewC]); provider = new ViewProvider([viewA, viewB, viewC], mockLog);
}); });
it("reports views provided as extensions", function () { it("reports views provided as extensions", function () {
@ -71,6 +73,20 @@ define(
.toEqual([viewA, viewC]); .toEqual([viewA, viewC]);
}); });
it("warns if keys are omitted from views", function () {
// Verify that initial construction issued no warning
expect(mockLog.warn).not.toHaveBeenCalled();
// Recreate with no keys; that view should be filtered out
expect(
new ViewProvider(
[viewA, { some: "bad view" }],
mockLog
).getViews(mockDomainObject)
).toEqual([viewA]);
// We should have also received a warning, to support debugging
expect(mockLog.warn).toHaveBeenCalledWith(jasmine.any(String));
});
}); });
} }
); );