Merge lp:~openerp-dev/openerp-web/7.0-bug-1047911-fme into lp:openerp-web/7.0

Proposed by Fabien Meghazi (OpenERP)
Status: Merged
Merged at revision: 3749
Proposed branch: lp:~openerp-dev/openerp-web/7.0-bug-1047911-fme
Merge into: lp:openerp-web/7.0
Diff against target: 89 lines (+41/-10)
1 file modified
addons/web/static/src/js/views.js (+41/-10)
To merge this branch: bzr merge lp:~openerp-dev/openerp-web/7.0-bug-1047911-fme
Reviewer Review Type Date Requested Status
Xavier (Open ERP) (community) Approve
Review via email: mp+147686@code.launchpad.net

Description of the change

This branch fixes the active_id reload bug by pushing the active_id and/or active_ids in the state.
They are popped back on_load_state().

Also added jsdoc for ActionManager#do_action()

To post a comment you must log in.
Revision history for this message
Fabien Meghazi (OpenERP) (fme) wrote :
Revision history for this message
Xavier (Open ERP) (xmo-deactivatedaccount) wrote :

> state["active_ids"] = this.inner_action.context.active_ids.toString();

The toString() is a bit iffy, I've got to say I'd prefer an explicit join here, or some other type of serialization

> add_context.active_ids = state.active_ids.toString().split(',').map

Not sure what's toString() supposed to be here, isn't active_ids already a string?

Also the parsing might be a tad risky: what if we have "string" or "virtual" ids in that? They're going to be parseInt'd to NaN.

review: Needs Fixing
3751. By Fabien Meghazi (OpenERP)

[IMP] Improved code and doc

3752. By Fabien Meghazi (OpenERP)

[FIX] support virtual ids

Revision history for this message
Fabien Meghazi (OpenERP) (fme) wrote :

> > state["active_ids"] = this.inner_action.context.active_ids.toString();
>
> The toString() is a bit iffy, I've got to say I'd prefer an explicit join here, or some other type of serialization

I was relying on the ecma spec but I admit it's biffy. Sorry, I meant iffy.

> > add_context.active_ids = state.active_ids.toString().split(',').map
>
> Not sure what's toString() supposed to be here, isn't active_ids already a string?

No because jQuery BBQ does some parsing in some case. I've documented it.

> Also the parsing might be a tad risky: what if we have "string" or "virtual" ids in that? They're going to be parseInt'd to NaN.

Fixed that

3753. By Fabien Meghazi (OpenERP)

[END] This is the end of the world

Revision history for this message
Xavier (Open ERP) (xmo-deactivatedaccount) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'addons/web/static/src/js/views.js'
--- addons/web/static/src/js/views.js 2013-02-05 12:09:50 +0000
+++ addons/web/static/src/js/views.js 2013-02-12 13:32:20 +0000
@@ -190,6 +190,14 @@
190 });190 });
191 state = _.extend(params || {}, state);191 state = _.extend(params || {}, state);
192 }192 }
193 if (this.inner_action.context) {
194 if (this.inner_action.context.active_id) {
195 state["active_id"] = this.inner_action.context.active_id;
196 }
197 if (this.inner_action.context.active_ids) {
198 state["active_ids"] = this.inner_action.context.active_ids.join(',');
199 }
200 }
193 }201 }
194 if(!this.dialog) {202 if(!this.dialog) {
195 this.getParent().do_push_state(state);203 this.getParent().do_push_state(state);
@@ -212,8 +220,20 @@
212 } else {220 } else {
213 var run_action = (!this.inner_widget || !this.inner_widget.action) || this.inner_widget.action.id !== state.action;221 var run_action = (!this.inner_widget || !this.inner_widget.action) || this.inner_widget.action.id !== state.action;
214 if (run_action) {222 if (run_action) {
223 var add_context = {};
224 if (state.active_id) {
225 add_context.active_id = state.active_id;
226 }
227 if (state.active_ids) {
228 // The jQuery BBQ plugin does some parsing on values that are valid integers.
229 // It means that if there's only one item, it will do parseInt() on it,
230 // otherwise it will keep the comma seperated list as string.
231 add_context.active_ids = state.active_ids.toString().split(',').map(function(id) {
232 return parseInt(id, 10) || id;
233 });
234 }
215 this.null_action();235 this.null_action();
216 action_loaded = this.do_action(state.action);236 action_loaded = this.do_action(state.action, { additional_context: add_context });
217 $.when(action_loaded || null).done(function() {237 $.when(action_loaded || null).done(function() {
218 instance.webclient.menu.has_been_loaded.done(function() {238 instance.webclient.menu.has_been_loaded.done(function() {
219 if (self.inner_action && self.inner_action.id) {239 if (self.inner_action && self.inner_action.id) {
@@ -249,12 +269,25 @@
249 }269 }
250 });270 });
251 },271 },
272 /**
273 * Execute an OpenERP action
274 *
275 * @param {Number|String|Object} Can be either an action id, a client action or an action descriptor.
276 * @param {Object} [options]
277 * @param {Boolean} [options.clear_breadcrumbs=false] Clear the breadcrumbs history list
278 * @param {Function} [options.on_reverse_breadcrumb] Callback to be executed whenever an anterior breadcrumb item is clicked on.
279 * @param {Function} [options.on_close] Callback to be executed when the dialog is closed (only relevant for target=new actions)
280 * @param {Function} [options.action_menu_id] Manually set the menu id on the fly.
281 * @param {Object} [options.additional_context] Additional context to be merged with the action's context.
282 * @return {jQuery.Deferred} Action loaded
283 */
252 do_action: function(action, options) {284 do_action: function(action, options) {
253 options = _.defaults(options || {}, {285 options = _.defaults(options || {}, {
254 clear_breadcrumbs: false,286 clear_breadcrumbs: false,
255 on_reverse_breadcrumb: function() {},287 on_reverse_breadcrumb: function() {},
256 on_close: function() {},288 on_close: function() {},
257 action_menu_id: null,289 action_menu_id: null,
290 additional_context: {},
258 });291 });
259 if (action === false) {292 if (action === false) {
260 action = { type: 'ir.actions.act_window_close' };293 action = { type: 'ir.actions.act_window_close' };
@@ -269,15 +302,13 @@
269 }302 }
270303
271 // Ensure context & domain are evaluated and can be manipulated/used304 // Ensure context & domain are evaluated and can be manipulated/used
272 if (action.context) {305 var ncontext = new instance.web.CompoundContext(options.additional_context, action.context || {});
273 action.context = instance.web.pyeval.eval(306 action.context = instance.web.pyeval.eval('context', ncontext);
274 'context', action.context);307 if (action.context.active_id || action.context.active_ids) {
275 if (action.context.active_id || action.context.active_ids) {308 // Here we assume that when an `active_id` or `active_ids` is used
276 // Here we assume that when an `active_id` or `active_ids` is used309 // in the context, we are in a `related` action, so we disable the
277 // in the context, we are in a `related` action, so we disable the310 // searchview's default custom filters.
278 // searchview's default custom filters.311 action.context.search_disable_custom_filters = true;
279 action.context.search_disable_custom_filters = true;
280 }
281 }312 }
282 if (action.domain) {313 if (action.domain) {
283 action.domain = instance.web.pyeval.eval(314 action.domain = instance.web.pyeval.eval(