New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIX] base_address_city: correct view_get with address fields #27114

Closed
wants to merge 1 commit into
base: 11.0
from

Conversation

Projects
None yet
4 participants
@kebeclibre
Contributor

kebeclibre commented Sep 19, 2018

Have a partner with children exceeding the limit of the kanban.
Edit,
Add a contact > save and quit
Save the partner

Before this commit, there was a traceback, because a check on parent_id was done to compute
the modifier readonly of the city fields

However, onchange gets rid of parent_id because child_ids and parent_id are two faces of the same coin
And if that check was not done, we would end up in an infinite recursion

After this commit, there is no traceback, as we retrieve the parent with another means
Also, in subviews, checking the parent might be overkill, because we literally know there is one
But you know, safety first

OPW 1886320

Description of the issue/feature this PR addresses:

Current behavior before PR:

Desired behavior after PR is merged:

--
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

@kebeclibre kebeclibre requested review from qdp-odoo and nim-odoo Sep 19, 2018

@kebeclibre

This comment has been minimized.

Contributor

kebeclibre commented Sep 19, 2018

@adr-odoo
For an enlightened advice :D

@robodoo robodoo added the seen 🙂 label Sep 20, 2018

@kebeclibre kebeclibre requested a review from rco-odoo Sep 20, 2018

@nim-odoo

Let's way for @rco-odoo input here... the clean fix should be a proper override in the XML instead of the awful workaround that was used in a first place.

@nim-odoo nim-odoo added the Blocked label Sep 20, 2018

@C3POdoo C3POdoo added the OE label Sep 20, 2018

@robodoo robodoo added the CI 🤖 label Sep 21, 2018

@robodoo robodoo removed the CI 🤖 label Nov 8, 2018

replace_node.attrib['attrs'] = new_attrs % parent_condition
for city_node in doc.xpath("//field[@name='city']"):
replacement_tree = etree.fromstring(replacement_xml)

This comment has been minimized.

@kebeclibre

kebeclibre Nov 8, 2018

Contributor

rebuild a tree here is necessary, otherwise at the next iteration of the xpath, replacement_tree would be the same and already consumed

@kebeclibre

This comment has been minimized.

Contributor

kebeclibre commented Nov 8, 2018

@nim-odoo
@rco-odoo

Following discussions with RCO, it appears that this approach is the best for a bug fix, considering

  • The module should have implemented inherited views
  • Specs of the feature are somewhat lacking

Possible solutions:

  • [ruled out] create inherited views
  • [ruled out] force injection of parent_id in the return of the onchange
  • keep what was designed, but improve the behavior

The two commits are not that different, and the best of the first can be integrated in the second

@robodoo robodoo added CI 🤖 and removed CI 🤖 labels Nov 8, 2018

@nim-odoo

This comment has been minimized.

Contributor

nim-odoo commented Nov 9, 2018

ok then... let's see when you have a final version of this

@robodoo robodoo removed the CI 🤖 label Nov 13, 2018

@kebeclibre

This comment has been minimized.

Contributor

kebeclibre commented Nov 13, 2018

@nim-odoo
I've got the final solution (third commit)

@robodoo robodoo added the CI 🤖 label Nov 13, 2018

[FIX] base_address_city: correct view_get with address fields
Have a partner with children exceeding the limit of the kanban.
Edit,
Add a contact > save and quit
Save the partner

Before this commit, there was a traceback, because a check on parent_id was done to compute
the modifier readonly of the city fields in subviews

However, python onchange gets rid of parent_id because child_ids and parent_id are two faces of the same coin
And if that check were not done, we would end up in an infinite recursion

After this commit, there is no traceback, as we stop checking on parent_id in the case of paged subviews
(list and kanban) because we literally know there is one

OPW 1886320

@robodoo robodoo removed the CI 🤖 label Nov 14, 2018

@kebeclibre

This comment has been minimized.

Contributor

kebeclibre commented Nov 14, 2018

robodoo r+

robodoo pushed a commit that referenced this pull request Nov 14, 2018

[FIX] base_address_city: correct view_get with address fields
Have a partner with children exceeding the limit of the kanban.
Edit,
Add a contact > save and quit
Save the partner

Before this commit, there was a traceback, because a check on parent_id was done to compute
the modifier readonly of the city fields in subviews

However, python onchange gets rid of parent_id because child_ids and parent_id are two faces of the same coin
And if that check were not done, we would end up in an infinite recursion

After this commit, there is no traceback, as we stop checking on parent_id in the case of paged subviews
(list and kanban) because we literally know there is one

OPW 1886320

closes #27114
@robodoo

This comment has been minimized.

Contributor

robodoo commented Nov 14, 2018

Merged, thanks!

@robodoo robodoo closed this Nov 14, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment