From b8c1571d948e593779b83ba78130c341f1e427cd Mon Sep 17 00:00:00 2001 From: David Monjoie Date: Wed, 14 Jun 2017 14:35:38 +0200 Subject: [PATCH] [FIX] web: prevent crash when giving wrong id in url The mock server had to be modified in order to test this behavior since it used to throw an error for invalid ids, while the actual server just ignores it and continues forth with the other ids. --- addons/web/static/src/js/chrome/view_manager.js | 2 +- .../static/src/js/views/basic/basic_model.js | 6 +++++- addons/web/static/tests/helpers/mock_server.js | 10 +++++----- .../web/static/tests/views/basic_model_tests.js | 17 +++++++++++++++++ 4 files changed, 28 insertions(+), 7 deletions(-) diff --git a/addons/web/static/src/js/chrome/view_manager.js b/addons/web/static/src/js/chrome/view_manager.js index bf2081d157b..a51fc911a99 100644 --- a/addons/web/static/src/js/chrome/view_manager.js +++ b/addons/web/static/src/js/chrome/view_manager.js @@ -307,7 +307,7 @@ var ViewManager = Widget.extend(ControlPanelMixin, { self.trigger('switch_mode', view_type, view_options); }).fail(function(e) { if (!(e && e.code === 200 && e.data.exception_type)) { - self.do_warn(_t("Error"), view.controller.display_name + _t(" view couldn't be loaded")); + self.do_warn(_t("Error"), view.label + _t(" view couldn't be loaded")); } // Restore internal state self.active_view = old_view; diff --git a/addons/web/static/src/js/views/basic/basic_model.js b/addons/web/static/src/js/views/basic/basic_model.js index d897a74c696..1ace44be50f 100644 --- a/addons/web/static/src/js/views/basic/basic_model.js +++ b/addons/web/static/src/js/views/basic/basic_model.js @@ -1399,7 +1399,8 @@ var BasicModel = AbstractModel.extend({ * @param {string} [optinos.viewType] the type of view for which the record * is fetched (usefull to load the adequate fields), by defaults, uses * record.viewType - * @returns {Deferred -> Object} resolves to the record + * @returns {Deferred -> Object} resolves to the record or is rejected in + * case no id given were valid ids */ _fetchRecord: function (record, options) { var self = this; @@ -1412,6 +1413,9 @@ var BasicModel = AbstractModel.extend({ context: _.extend({}, record.context, {bin_size: true}), }) .then(function (result) { + if (result.length === 0) { + return $.Deferred().reject(); + } result = result[0]; record.data = _.extend({}, record.data, result); }) diff --git a/addons/web/static/tests/helpers/mock_server.js b/addons/web/static/tests/helpers/mock_server.js index 29e53c735a7..b3d92f379c7 100644 --- a/addons/web/static/tests/helpers/mock_server.js +++ b/addons/web/static/tests/helpers/mock_server.js @@ -537,11 +537,11 @@ var MockServer = Class.extend({ ids = [ids]; } var fields = args[1] && args[1].length ? _.uniq(args[1].concat(['id'])) : Object.keys(this.data[model].fields); - var results = _.map(ids, function (id) { - var record = _.findWhere(self.data[model].records, {id: id}); - if (!record) { - throw "mock read: id does not exist..."; - } + var records = _.reduce(ids, function (records, id) { + var record = _.findWhere(self.data[model].records, {id: id}); + return record ? records.concat(record) : records + }, []); + var results = _.map(records, function (record) { var result = {}; for (var i = 0; i < fields.length; i++) { var field = self.data[model].fields[fields[i]]; diff --git a/addons/web/static/tests/views/basic_model_tests.js b/addons/web/static/tests/views/basic_model_tests.js index ca2f6f37f3e..cbe04f5b43b 100644 --- a/addons/web/static/tests/views/basic_model_tests.js +++ b/addons/web/static/tests/views/basic_model_tests.js @@ -92,6 +92,23 @@ QUnit.module('Views', { model.destroy(); }); + QUnit.test('rejects loading a record with invalid id', function (assert) { + assert.expect(1); + + this.params.res_id = 99; + + var model = createModel({ + Model: BasicModel, + data: this.data, + }); + + model.load(this.params).always(function (error) { + assert.strictEqual(this.state(), 'rejected', + "load should return a rejected deferred for an invalid id") + }); + model.destroy(); + }); + QUnit.test('notify change with many2one', function (assert) { assert.expect(2);