-
-
Notifications
You must be signed in to change notification settings - Fork 456
[18.0][IMP] crm_phonecall: Add computed partner city/state/zip fields #663
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
Conversation
a4bc6f1 to
b6f0b2c
Compare
Jaimermaccione
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Anxo82 Tested in a local environment.
- Verified that the fields City, State, and ZIP Code are correctly populated from the contact record.
- These values are visible both in the Crm.phonecall List View and the Crm.phonecall Form View.
Pending
- It is currently not possible to filter or group by these fields in the List View.
- In the Call Analysis View:
- Attempting to group by these fields results in an error.
RPC_ERROR
Odoo Server Error
Occured on localhost:18069 on model crm.phonecall.report on 2025-07-29 13:33:04 GMT
Traceback (most recent call last):
File "/opt/odoo/custom/src/odoo/odoo/http.py", line 2123, in _transactioning
return service_model.retrying(func, env=self.env)
File "/opt/odoo/custom/src/odoo/odoo/service/model.py", line 156, in retrying
result = func()
File "/opt/odoo/custom/src/odoo/odoo/http.py", line 2090, in _serve_ir_http
response = self.dispatcher.dispatch(rule.endpoint, args)
File "/opt/odoo/custom/src/odoo/odoo/http.py", line 2338, in dispatch
result = self.request.registry['ir.http']._dispatch(endpoint)
File "/opt/odoo/custom/src/odoo/odoo/addons/base/models/ir_http.py", line 333, in _dispatch
result = endpoint(**request.params)
File "/opt/odoo/custom/src/odoo/odoo/http.py", line 754, in route_wrapper
result = endpoint(self, *args, **params_ok)
File "/opt/odoo/auto/addons/web/controllers/dataset.py", line 36, in call_kw
return call_kw(request.env[model], method, args, kwargs)
File "/opt/odoo/custom/src/odoo/odoo/api.py", line 535, in call_kw
result = getattr(recs, name)(*args, **kwargs)
File "/opt/odoo/custom/src/odoo/odoo/models.py", line 2887, in read_group
rows = self._read_group(domain, annotated_groupby.values(), annotated_aggregates.values(), offset=offset, limit=limit, order=orderby)
File "/opt/odoo/custom/src/odoo/odoo/models.py", line 2021, in _read_group
row_values = self.env.execute_query(query.select(*[groupby_terms[spec] for spec in groupby], *select_terms))
File "/opt/odoo/custom/src/odoo/odoo/api.py", line 993, in execute_query
self.cr.execute(query)
File "/opt/odoo/custom/src/odoo/odoo/sql_db.py", line 354, in execute
res = self._obj.execute(query, params)
psycopg2.errors.UndefinedColumn: column crm_phonecall_report.partner_state does not exist
LINE 1: SELECT "crm_phonecall_report"."partner_state", COUNT(*), SUM...
^
The above server error caused the following client error:
RPC_ERROR: Odoo Server Error
RPC_ERROR
at makeErrorFromResponse (http://localhost:18069/web/assets/c60ed5f/web.assets_web.min.js:3148:163)
at XMLHttpRequest.<anonymous> (http://localhost:18069/web/assets/c60ed5f/web.assets_web.min.js:3153:13))
-
Trying to filter them shows the warning:
"The domain is not valid, please correct it."
Would you mind taking a look when you get a chance?
Thanks!
906d370 to
f1b861f
Compare
|
@Jaimermaccione changes done! Could you review? Thanks |
Jaimermaccione
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Anxo82 LGMT! I've reviewed the changes and everything looks good. The filters and groupings by city, state/province, and ZIP code are working correctly both in the tree view and the call analysis report.
Great work, thanks!
|
There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. |
ValentinVinagre
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍🏻
|
@HaraldPanten or @rafaelbn whenever you have time, could you please review and merge this PR? |
HaraldPanten
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/ocabot merge patch
|
@HaraldPanten The merge process could not start, because command `git merge --no-ff -m 'Merge PR #663 into 18.0 Signed-off-by HaraldPanten' tmp-pr-663` failed with output: |
|
@Anxo82 Could you resolve conflicts? THX. |
f1b861f to
3b80ddc
Compare
|
@HaraldPanten The conflict has been resolved. Could you please try merging again? |
|
/ocabot merge patch |
|
On my way to merge this fine PR! |
|
Congratulations, your PR was merged at dbfaa59. Thanks a lot for contributing to OCA. ❤️ |
|
@Anxo82 @HaraldPanten @ValentinVinagre this is provoking a collapse on existing databases, as the new fields, being stored, are computed on module update, and on big DBs, this takes a lot. This kind of changes should be accompanied of pre-init/post-init hooks, and a migration script. |
| def _compute_partner_fields(self): | ||
| for rec in self: | ||
| partner = rec.partner_id | ||
| rec.partner_city = partner.city or "" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are you putting or ""? And why these are not simply related fields?
|
I'm reverting this PR in #715, as it's very harmful, provoking the effects commented, but also not seeing the value of incrementing the size of the DB for storing data that is very easily obtained for example with a SQL view and add a pivot view with that "report", or Odoo has the option for a partner to show in the widget directly the address. We can discuss the goal of these changes and select the best option in a new PR. |
|
@pedrobaeza thanks for the feedback. We’ll review this in detail and look for a more optimal solution that aligns with your suggestions. We’ll come back with a new PR once we have a clearer proposal. |
@HaraldPanten @Jaimermaccione