Merge lp:~pedro.baeza/ocb-web/7.0-web_calendar-fix-1306136 into lp:ocb-web

Proposed by Pedro Manuel Baeza
Status: Rejected
Rejected by: Pedro Manuel Baeza
Proposed branch: lp:~pedro.baeza/ocb-web/7.0-web_calendar-fix-1306136
Merge into: lp:ocb-web
Diff against target: 12 lines (+1/-1)
1 file modified
addons/web_calendar/static/src/js/calendar.js (+1/-1)
To merge this branch: bzr merge lp:~pedro.baeza/ocb-web/7.0-web_calendar-fix-1306136
Reviewer Review Type Date Requested Status
Pedro Manuel Baeza Disapprove
Yannick Vaucher @ Camptocamp code review, no test Approve
Holger Brunn (Therp) Approve
Review via email: mp+215256@code.launchpad.net

Description of the change

[FIX] web_calendar: Error when deleting an element from calendar view.

It solves lp:1306136. See bug report for details.

To post a comment you must log in.
Revision history for this message
Holger Brunn (Therp) (hbrunn) wrote :

Since when can't we pass single IDs to the ORM functions? I think you should rather search where in the inheritance tree someone doesn't check if ids is a list or an integer and fix that.

review: Needs Information
Revision history for this message
Pedro Manuel Baeza (pedro.baeza) wrote :

Hi, Holger, there are a lot of write, unlink and so on overrides that doesn't comply with ids argument as integer/long, so I prefer to fix this in this way that add the check on a lot of places.

This discussion also happens in another place with Lionel for the browse method and he told me that v8 is going to deprecate all single ID calls.

Regards.

Revision history for this message
Holger Brunn (Therp) (hbrunn) wrote :

Yes, in 8.0 that's true, so I'm fine with the fix

review: Approve
Revision history for this message
Yannick Vaucher @ Camptocamp (yvaucher-c2c) wrote :

LGTM

review: Approve (code review, no test)
Revision history for this message
Holger Brunn (Therp) (hbrunn) wrote :

please make a pull request for https://github.com/OCA/OCB if you still consider this fix relevant.

Revision history for this message
Pedro Manuel Baeza (pedro.baeza) wrote :
review: Disapprove

Unmerged revisions

4180. By Pedro Manuel Baeza

[FIX] web_calendar: Error when deleting an element from calendar view.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'addons/web_calendar/static/src/js/calendar.js'
--- addons/web_calendar/static/src/js/calendar.js 2014-02-14 11:42:58 +0000
+++ addons/web_calendar/static/src/js/calendar.js 2014-04-10 17:03:19 +0000
@@ -533,7 +533,7 @@
533 var self = this;533 var self = this;
534 var index = this.dataset.get_id_index(event_id);534 var index = this.dataset.get_id_index(event_id);
535 if (index !== null) {535 if (index !== null) {
536 this.dataset.unlink(this.dataset.ids[index]);536 this.dataset.unlink([this.dataset.ids[index]]);
537 }537 }
538 },538 },
539});539});