8000 [FIX] base_kanban_stage: extend search_domain only for fields belong to 'base.kanban.stage' by tafaRU · Pull Request #982 · OCA/server-tools · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[FIX] base_kanban_stage: extend search_domain only for fields belong to 'base.kanban.stage' #982

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

[FIX] base_kanban_stage: extend search_domain only for fields belong to 'base.kanban.stage' #982

wants to merge 1 commit into from

Conversation

tafaRU
Copy link
Member
@tafaRU tafaRU commented Sep 14, 2017

This PR is intended to cover the following scenario:

  • have a module as [10.0][ADD] Add calendar_event_kanban_stage module #964 [1]
  • have another module [2] where the model calendar.event is further extended with other fields
  • use the kanban view created in [1] in which the search view has a filter with domain based on a field added by [2]

Without this I get the following error:

File "/home/tafaru/dev/envs/buildout/demo10-community/parts/odoo/addons/calendar/models/calendar.py", line 1454, in read_group
    return super(Meeting, self.with_context(virtual_id=False)).read_group(domain, fields, groupby, offset=offset, limit=limit, orderby=orderby, lazy=lazy)
  File "/home/tafaru/dev/envs/buildout/demo10-community/parts/odoo/odoo/models.py", line 1932, in read_group
    result = self._read_group_raw(domain, fields, groupby, offset=offset, limit=limit, orderby=orderby, lazy=lazy)
  File "/home/tafaru/dev/envs/buildout/demo10-community/parts/odoo/odoo/models.py", line 2045, in _read_group_raw
    aggregated_fields, count_field, result, read_group_order=order,
  File "/home/tafaru/dev/envs/buildout/demo10-community/parts/odoo/odoo/models.py", line 1686, in _read_group_fill_results
    groups = getattr(self, field.group_expand)(groups, domain, order)
  File "/home/tafaru/dev/envs/buildout/demo10-community/parts/server-tools/base_kanban_stage/models/base_kanban_abstract.py", line 113, in _read_group_stage_ids
    return stages.search(search_domain, order=order)
  File "/home/tafaru/dev/envs/buildout/demo10-community/parts/odoo/odoo/models.py", line 1518, in search
    res = self._search(args, offset=offset, limit=limit, order=order, count=count)
  File "/home/tafaru/dev/envs/buildout/demo10-community/parts/odoo/odoo/models.py", line 4220, in _search
    query = self._where_calc(args)
  File "/home/tafaru/dev/envs/buildout/demo10-community/parts/odoo/odoo/models.py", line 4019,
8000
 in _where_calc
    e = expression.expression(domain, self)
  File "/home/tafaru/dev/envs/buildout/demo10-community/parts/odoo/odoo/osv/expression.py", line 643, in __init__
    self.parse()
  File "/home/tafaru/dev/envs/buildout/demo10-community/parts/odoo/odoo/osv/expression.py", line 821, in parse
    raise ValueError("Invalid field %r in leaf %r" % (left, str(leaf)))
ValueError: Invalid field u'related_installed_component' in leaf "<osv.ExtendedLeaf: (u'related_installed_component', u'!=', False) on base_kanban_stage (ctx: )>"

Does this belong to https://github.com/OCA/server-tools/tree/10.0/base_kanban_stage#known-issues--roadmap?

In that case, could you please explain me better the reason for https://github.com/OCA/server-tools/blob/10.0/base_kanban_stage/models/base_kanban_abstract.py#L109?

Thank you in advance.

@tafaRU
Copy link
Member Author
tafaRU commented Sep 14, 2017

@lasley, could you please give me a feedback?

@yajo yajo added this to the 10.0 milestone Sep 14, 2017
@lasley
Copy link
Contributor
lasley commented Sep 14, 2017

@obulkin - Do you know the answer to #982 (comment)?

@obulkin
Copy link
Contributor
obulkin commented Sep 14, 2017

Hi, @tafaRU. IIRC, the note in the README was added as a precaution because I wasn't sure that the current _read_group_stage_ids logic was handling domains correctly, and this issue definitely confirms that it isn't.

However, I don't think the fix here is correct. Your approach is to basically screen out invalid fields, but the fields are invalid in the first place because the search in the method is being done against the base.kanban.stage model while the search domains in the view are meant to apply to calendar.event. This means that even fields that happen to be present in both cases shouldn't just be directly added to search_domain. Instead, the approach should probably be something along the lines of:

  1. Get all records of model in view (calendar.event in this case) that fit domain coming from the view
  2. Get all kanban stages corresponding to those records
  3. Replace search_domain with domain that will only return these stages

Thanks for creating a PR to clean up my mess here! 😄

@obulkin
Copy link
Contributor
obulkin commented Sep 14, 2017

Alternatively, domain could just be ignored, as the approach above would result in some valid calendar.event stages being hidden, and this would make it harder to move the filtered event records to a different stage. A search of the Odoo code base actually reveals a number of _read_group_stage_ids implementations that do this.

@tafaRU
Copy link
Member Author
tafaRU commented Sep 15, 2017

Hi @obulkin, thanks a lot for your detailed answers!

If Iknow what you mean with:

Alternatively, domain could just be ignored

I'd modify _read_group_stage_ids implementation as follows:

    @api.multi
    def _read_group_stage_ids(self, stages, domain, order):
        search_domain = [('res_model_id.model', '=', self._name)]
        return stages.search(search_domain, order=order)

I remain awaiting a confirmation from you before to modifiy the PR.

Let me know. Thanks.

@obulkin
Copy link
Contributor
obulkin commented Sep 15, 2017

@tafaRU - Yup. That's what I had in mind.

tafaRU added a commit to tafaRU/server-tools that referenced this pull request Sep 18, 2017
@tafaRU
[FIX] base_kanban_stage: ignore domain in _read_group_stage_ids overr…
7fd88e9 
…ide.

See OCA#982 for further info
@tafaRU tafaRU mentioned this pull request Sep 18, 2017
Merged
@tafaRU
Copy link
Member Author
tafaRU commented Sep 18, 2017

@obulkin, thanks for the confirmation 😉
Please review #989.

@tafaRU
Copy link
Member Author
tafaRU commented Sep 18, 2017

I close this in favour of #989.

@tafaRU tafaRU closed this Sep 18, 2017
@tafaRU tafaRU deleted the 10.0-base_kanban_stage-fix branch September 18, 2017 07:32
tafaRU added a commit to tafaRU/server-tools that referenced this pull request Sep 18, 2017
@tafaRU
[FIX] base_kanban_stage: ignore domain in _read_group_stage_ids overr…
820709c 
…ide.

See OCA#982 for further info
AaronHForgeFlow pushed a commit to ForgeFlow/server-tools that referenced this pull request Sep 17, 2019
@tafaRU @AaronHForgeFlow
[FIX] base_kanban_stage: ignore domain in _read_group_stage_ids overr…
4f45d58 
…ide.

See OCA#982 for further info
MiquelRForgeFlow pushed a commit to ForgeFlow/server-tools that referenced this pull request Sep 27, 2019
@tafaRU @MiquelRForgeFlow
[FIX] base_kanban_stage: ignore domain in _read_group_stage_ids overr…
52f912e 
…ide.

See OCA#982 for further info
Tardo pushed a commit to Tecnativa/server-tools that referenced this pull request Dec 5, 2019
@tafaRU @Tardo
[FIX] base_kanban_stage: ignore domain in _read_group_stage_ids overr…
c91a1fe 
…ide.

See OCA#982 for further info
mileo pushed a commit to kmee/server-tools that referenced this pull request Mar 31, 2020
@tafaRU @mileo
[FIX] base_kanban_stage: ignore domain in _read_group_stage_ids overr…
742f2a5 
…ide.

See OCA#982 for further info
mileo pushed a commit to kmee/server-tools that referenced this pull request Jun 24, 2020
@tafaRU @mileo
[FIX] base_kanban_stage: ignore domain in _read_group_stage_ids overr…
21cc2dc 
…ide.

See OCA#982 for further info
fshah-initos pushed a commit to initOS/server-tools that referenced this pull request Dec 11, 2020
@tafaRU @fshah-initos
[FIX] base_kanban_stage: ignore domain in _read_group_stage_ids overr…
c4e7ae9 
…ide.

See OCA#982 for further info
fshah-initos pushed a commit to initOS/server-tools that referenced this pull request Mar 4, 2021
@tafaRU @fshah-initos
[FIX] base_kanban_stage: ignore domain in _read_group_stage_ids overr…
40a038e 
…ide.

See OCA#982 for further info
hkapatel-initos pushed a commit to initOS/server-tools that referenced this pull request Jun 24, 2021
@tafaRU @hkapatel-initos
[FIX] base_kanban_stage: ignore domain in _read_group_stage_ids overr…
f62b29d 
…ide.

See OCA#982 for further info
hkapatel-initos pushed a commit to initOS/server-tools that referenced this pull request Jul 2, 2021
@tafaRU @hkapatel-initos
[FIX] base_kanban_stage: ignore domain in _read_group_stage_ids overr…
81a5e93 
…ide.

See OCA#982 for further info
MiquelRForgeFlow pushed a commit to ForgeFlow/server-tools that referenced this pull request Aug 4, 2021
@tafaRU @MiquelRForgeFlow
[FIX] base_kanban_stage: ignore domain in _read_group_stage_ids overr…
45a31da 
…ide.

See OCA#982 for further info
mtelahun pushed a commit to mtelahun/server-tools that referenced this pull request Sep 5, 2022
@tafaRU @mtelahun
[FIX] base_kanban_stage: ignore domain in _read_group_stage_ids overr…
1f06773 
…ide.

See OCA#982 for further info
AaronHForgeFlow pushed a commit to ForgeFlow/server-tools that referenced this pull request Dec 2, 2022
@tafaRU @AaronHForgeFlow
[FIX] base_kanban_stage: ignore domain in _read_group_stage_ids overr…
60d7f50 
…ide.

See OCA#982 for further info
odooNextev pushed a commit to odooNextev/server-tools that referenced this pull request May 30, 2023
@tafaRU @matteonext
[FIX] base_kanban_stage: ignore domain in _read_group_stage_ids overr…
c6a22d3 
…ide.

See OCA#982 for further info
AaronHForgeFlow pushed a commit to ForgeFlow/server-tools that referenced this pull request Aug 4, 2023
@tafaRU @AaronHForgeFlow
[FIX] base_kanban_stage: ignore domain in _read_group_stage_ids overr…
451a3ef 
…ide.

See OCA#982 for further info
AaronHForgeFlow pushed a commit to ForgeFlow/server-tools that referenced this pull request Oct 31, 2023
@tafaRU @AaronHForgeFlow
[FIX] base_kanban_stage: ignore domain in _read_group_stage_ids overr…
dd3e8c5 
…ide.

See OCA#982 for further info
SiesslPhillip pushed a commit to grueneerde/OCA-server-tools that referenced this pull request Nov 20, 2024
@bt-admin
Merge pull request OCA#982 from OCA/12.0
33a8c0f 
Syncing from upstream OCA/server-tools (12.0)
mileo pushed a commit to kmee/server-tools that referenced this pull request May 25, 2025
@tafaRU @mileo
[FIX] base_kanban_stage: ignore domain in _read_group_stage_ids overr…
3fbdaac 
…ide.

See OCA#982 for further info
AaronHForgeFlow pushed a commit to ForgeFlow/server-tools that referenced this pull request May 26, 2025
@tafaRU @AaronHForgeFlow
[FIX] base_kanban_stage: ignore domain in _read_group_stage_ids overr…
a79bb36 
…ide.

See OCA#982 for further info
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
needs review
Projects
None yet
Milestone
10.0
Development

Successfully merging this pull request may close these issues.
4 participants
@tafaRU @lasley @obulkin @yajo
0