Merge lp:~openerp-dev/openerp-web/7.0-opw-601491ContextGroupByPropagationIssue-msh into lp:openerp-web/7.0

Proposed by Mohammed Shekha(Open ERP)
Status: Merged
Merged at revision: 4181
Proposed branch: lp:~openerp-dev/openerp-web/7.0-opw-601491ContextGroupByPropagationIssue-msh
Merge into: lp:openerp-web/7.0
Diff against target: 0 lines
To merge this branch: bzr merge lp:~openerp-dev/openerp-web/7.0-opw-601491ContextGroupByPropagationIssue-msh
Reviewer Review Type Date Requested Status
Stefan Rijnhart (Opener) (community) Approve
Guewen Baconnier @ Camptocamp (community) Approve
Ronald Portier (Therp) (community) Approve
Nicolas Bessi - Camptocamp (community) no test, code review Approve
Xavier (Open ERP) Pending
Martin Trigaux (OpenERP) Pending
Review via email: mp+197475@code.launchpad.net

Description of the change

Hello,

Fixed the issue of traceback on clicking kanban link from project.project kanban.

Demo: Open Kanban view of prject and -> group by parent -> now click on Task link.
Result: Traceback
Expected: It should open Task kanban view.

Reason: When we do grouping it adds groupby in context and when we click on link of kanban it excutes the action same as button action performed on form/list view, the issue is that do_execute_action propagate the group_by to next action, which is not required, propagation of group_by in context is required.

Say for example I have a button in form view type action, I perform action, so do_execute_action is called which in turn loads the action for some other object, so if we pass group_by in context then next action will try to do grouping on that field and fails if that field is not available in database, even if field with same name available by chance then even we didn't define to do grouping on action still it will do grouping.

In our case we do grouping on parent_id field of project.project, and then we performs action by clicking Task link so do_execute_action loads Task action and propagate the current dataset context by extending action context + current dataset context(current dataset context has group_by parent_id field) and hence read_group is called with group_by parent_id field to load kanban view of project.task which is wrong.

There is no need to propagate group_by as well as search_default values in context when dataset.model and action.model is different, hence removed the those keys of current dataset context before updating action context with current dataset context.

Thanks.

To post a comment you must log in.
4084. By Launchpad Translations on behalf of openerp

Launchpad automatic translations update.

4085. By Launchpad Translations on behalf of openerp

Launchpad automatic translations update.

4086. By Michel Meyer

[FIX] events handling ordering courtesy of Michel Meyer

lead to errors during the validation of rows in list o2ms.

See https://bugs.launchpad.net/openerp-web/+bug/1182101/comments/20
for an extensive description of the events and issue.

4087. By Xavier (Open ERP)

[FIX] issue when tabbing too much at end of editable list row

Tabbing is intercepted by keydown_TAB, which — if the current cell is
the last active field of the row — will then call _next:476. _next
then calls save_edition:300 which "takes a lock" (more precisely
serializes access to its body) and within its body checks if an
edition is active (:303) and returns immediately if not (:304).

The problem here is when a second tab event arrives during the
potentially extremely long save_edition body (since for toplevel lists
it needs to perform a complete RPC call): the overall state of the
list has not changed so the second event *also* goes into _next, then
into save_edition. There it's serialized with the ongoing call and
thus inactive until said ongoing call's termination, and reaches the
body after the current edition has been wound down. As a result, the
body of _next (:408) gets the resolution of ``$.when()``, which is
``null`` and the first condition blows up.

There are 3 possible ways to fix this:

* adding a check in keydown_TAB's handler to see whether a _next call
  is ongoing. This requires adding a state flag to the object and does
  not protect (or cooperate with) _next calls from outside this
  specific handler, unless they are modified in turn.

* alter save_edition to *fail* in case there's no ongoing edition:
  this part was originally in ensure_saved which does not care whether
  a save was necessary or not and does not propagate save information,
  so ``$.when()`` made sense. In save_edition, there are really 3
  different outcomes: the save succeeded, the save failed (or
  potentially part of save's postprocessing failed, for the current
  implementation) and the save was unnecessary. But deferred only
  provide 1 bit of state (success or failure), so the last state has
  to be merged into either success or failure.

  Both make sense, to an extent. Changing from one to the other (as
  necessary here) could break existing code and have more extensive
  effects than expected.

* the simplest and least far-raging change is to just alter the
  save_edition().then handler to ignore cases where save_edition()
  results in no saveinfo, this can be assumed to be a
  bailed-out/unnecessary save call.

For simplicity, the 3rd solution was picked here although with more
extensive tests &al I'd have preferred trying out 2nd.

4088. By Martin Trigaux (OpenERP)

[FIX] css: avoid tabs in row below to move when selected (opw 601379)

4089. By Xavier (Open ERP)

[FIX] weird behavior when drag&dropping a row during edition in editable listview

When dropping, would simultanously stop the edition and try a write
(so 2 writes on the same record) and generally screw up the state of
all the things, ending up with an empty row and a weird (and
incorrect) warning.

This can be fixed by preventing resequencing during the creation or
edition of a record (row) inline.

For simplicity, implemented by looking up .ui-sortable descendants —
there are no utility methods for handling that and, aside from the
class, there's no good way to know if sortability was enabled on a
list body or not (as far as I can see, jquery-ui's sortable has no API
to query that) — and using jquery-ui's sortable API for enabling and
disabling sortable on the fly.

4090. By xmo

[FIX] prevent field going to be misplaced when going from readonly to writable in editable list view

4091. By Launchpad Translations on behalf of openerp

Launchpad automatic translations update.

4092. By Martin Trigaux (OpenERP)

[MERGE] [FIX] One2ManyListView: reload line data without saving it when m2o record has changed in edition mode (eg: changing product name in sale order line) (opw #600224)

4093. By Martin Trigaux (OpenERP)

[MERGE] [FIX] Grouped ListView: avoid removing info in row title (such as total) when removing page numbers (if grouped view contains more than 80 elements) (opw 594708)

Revision history for this message
Guewen Baconnier @ Camptocamp (gbaconnier-c2c) wrote :

Thanks for the fix! Tested, works well.

review: Approve (test)
Revision history for this message
Nicolas Bessi - Camptocamp (nbessi-c2c-deactivatedaccount) wrote :

LGTM

review: Approve (no test, code review)
Revision history for this message
Stefan Rijnhart (Opener) (stefan-opener) wrote :

Thanks!

review: Approve (test)
4094. By Martin Trigaux (OpenERP)

[FIX] view_list_editable: toggle visibility (using force_visibility attribute) for readonly changes, using effective_invisibility instead of invisibility attribute (opw 601970)

4095. By Xavier ALT

[FIX] web: only show in 'Advanced Search', fields that are actually searcheable and avoid duplicate 'ID' field

4096. By Xavier (Open ERP)

[MERGE] fixes for non-direct IME

4097. By Launchpad Translations on behalf of openerp

Launchpad automatic translations update.

4098. By Denis Ledoux (OpenERP)

[FIX]web: status bar alignment, sometimes wrong on google chrome

Revision history for this message
Mohammed Shekha(Open ERP) (msh-openerp) wrote :

Hello Martin,

Thanks for the pointing our the issue, previous fix was removing context of action, I improved the fix here.

Regards,
Mohammed Shekha

Revision history for this message
Mohammed Shekha(Open ERP) (msh-openerp) wrote :

Hello,

Added a pair and object function as version 7.0 uses 1.3.1 which doesn't contains this methods, yes we can achieve our goal of filtering properties based on regular expression using other logic but for sake of simplicity used pairs and object.

Thanks.

4099. By Mohammed Shekha(OpenERP)<email address hidden>

[FIX]Web: Refixed the issue of context propagation, also removed the view references of current dataset.

Revision history for this message
Mohammed Shekha(Open ERP) (msh-openerp) wrote :

Hello,

Re-fixed the issue and also removed the view references of current dataset context, if viewname_view_ref is available in dataset context then it will create a problem, field_view_get will try to fetch the view which is given as view_ref in context.

This will fix the bug https://bugs.launchpad.net/openerp-web/+bug/1263888.

Thanks.

Revision history for this message
Ronald Portier (Therp) (rportier1962) wrote :

Tested on the ocb-web/7.0 branch.

Works!

review: Approve
Revision history for this message
Guewen Baconnier @ Camptocamp (gbaconnier-c2c) wrote :

Tested with the new changes (also for the bug lp:1263888), works well. Thanks

review: Approve
Revision history for this message
Stefan Rijnhart (Opener) (stefan-opener) wrote :

I tested the new version of the fix and I confirm that it does not have the regression from the first version while still fixing the related bug. Thank you very much.

review: Approve
4100. By Martin Trigaux (OpenERP)

[FIX] update to latest version saas code with updated underscore lib

4101. By Martin Trigaux (OpenERP)

sync with 7.0

Revision history for this message
Martin Trigaux (OpenERP) (mat-openerp) wrote :

Hello,

I have updated this branch to match the current version in trunk. The underscorejs lib has been updated so rewriting these method was not necessary anymore.

revno: 4181 [merge]
revision-id: <email address hidden>

Preview Diff

Empty