Skip to content
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] util/fields : update field in context of custom search view #18

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

thni-odoo
Copy link
Contributor

@thni-odoo thni-odoo commented Dec 4, 2023

This commit aims to make views compatible which are generated when a search filter is created using studio, view looks like

<data>
  <xpath expr="xyz" position="after">
    <filter name="filter1"  context="{'group_by': 'renamed field'}"/>
    <filter name="filter2"  context="{'group_by': 'removed field'}"/>
  </xpath>
</data>

Currently after upgrade this view will be disabled because we are not updating the field in this view.

After this fix

view:

<data>
  <xpath expr="xyz" position="after">
    <filter name="filter1"  context="{'group_by': 'New Name'}"/>
    <filter name="filter2"  context="{}"/>
  </xpath>
</data>

Alternative upgrade PR
idk whether update_custom_filter is in correct file any suggestion where it should be ?

@robodoo
Copy link
Contributor

robodoo commented Dec 4, 2023

@thni-odoo thni-odoo force-pushed the master-update_filter_context-thni branch from 858ee54 to 7c2fe49 Compare December 11, 2023 11:41
@thni-odoo thni-odoo force-pushed the master-update_filter_context-thni branch from 7c2fe49 to 0f22f5b Compare January 3, 2024 04:45
@KangOl
Copy link
Contributor

KangOl commented Jan 4, 2024

upgradeci retry with always only hr

src/util/fields.py Outdated Show resolved Hide resolved
Comment on lines 65 to 66
class _Skip(Exception):
pass
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid duplication with code in domains.py. This should be factorized and moved to records.py.

@@ -1121,3 +1138,48 @@ def update_server_actions_fields(cr, src_model, dst_model=None, fields_mapping=N
) % {"src_model": src_model, "dst_model": dst_model, "actions": ", ".join(action_names)}

add_to_migration_reports(message=msg, category="Server Actions")


def update_custom_filter(cr, old, model, new=None):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Bad function name.
  • Not a fan of the magic None value for new for the removal. What about passing a callback method?
  • follow the same pattern as other function and should iterate over inherits.
Suggested change
def update_custom_filter(cr, old, model, new=None):
def adapt_search_views(cr, old, model, callback, skip_inherit=()):

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please provide a reference for callback method, I didn't get it.

WHERE {} ~ %s
AND type = 'search'
AND model = %s
AND active = 't'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why only active views?

Suggested change
AND active = 't'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to avoid False negative cases.

Comment on lines 1180 to 1184
except lxml.etree.XMLSyntaxError as e:
if e.msg.startswith("Opening and ending tag mismatch"):
# this view is already wrong, we don't change it
_logger.warning("Skipping custom filter adaptaiton for invalid view (id=%s):\n%s", view_id, e.msg)
continue
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, code copy/pasted from domains.py. Should be factorized.

@thni-odoo thni-odoo force-pushed the master-update_filter_context-thni branch 2 times, most recently from 5e83138 to f48f3f5 Compare March 15, 2024 10:18
This commit aims to make views compatible which are generated when a
search filter is created using studio, view looks like
<data>
  <xpath expr="xyz" position="after">
    <filter name="filter1"  context="{'group_by': 'renamed field'}"/>
    <filter name="filter2"  context="{'group_by': 'removed field'}"/>
  </xpath>
</data>

Currently after upgrade this view will be disabled because we are not updating
the field in this view.

After this fix

view:
<data>
  <xpath expr="xyz" position="after">
    <filter name="filter1"  context="{'group_by': 'New Name'}"/>
    <filter name="filter2"  context="{}"/>
  </xpath>
</data>
@thni-odoo thni-odoo force-pushed the master-update_filter_context-thni branch from f48f3f5 to 585940d Compare March 15, 2024 10:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants