-
Notifications
You must be signed in to change notification settings - Fork 80
[FIX] util/report: Make 'name' argument optional #312
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/report: Make 'name' argument optional #312
Conversation
Pirols
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.
Left a couple of comments. Still, I think that providing a list/tuple of ids is a misuse of the function. So I'd expect the error to need correcting on the user-side. Otherwise, can you refer to where you encountered a relevant issue?
src/util/report.py
Outdated
| def get_anchor_link_to_record(model, id, name=None, action_id=None): | ||
| _validate_model(model) | ||
| if not name: | ||
| if isinstance(id, tuple): |
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.
If we want this, we should also check for lists:
| if isinstance(id, tuple): | |
| if isinstance(id, (tuple, list)): |
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.
I think this is not what we want. id must be an int.
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 this change?
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.
@depr-odoo remove this please.
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.
Hello @aj-fuentes,
it's done
|
Hello @Pirols, Some changes were suggested in the PR. Based on these updates, I’m inserting records into Table A and adding values from Table B. There are 2 case in method get_anchor_link_to_record
However, when I pass only the ID and model and call the method, a TypeError occurs because the name is required. |
|
upgradeci retry with always only crm |
eda3805 to
cfc92ef
Compare
This [commit](odoo#215) ensure that if the name argument is not provided, it will automatically generate a default value for name based on the model and id. When we call method get_anchor_link_to_record without name,it fails with ``` TypeError: get_anchor_link_to_record() missing 1 required positional argument: 'name' ``` Updated get_anchor_link_to_record to default 'name' to None if not provided.
cfc92ef to
d20f366
Compare
aj-fuentes
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
cc: @KangOl
KangOl
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.
@robodoo r+
This [commit](#215) ensure that if the name argument is not provided, it will automatically generate a default value for name based on the model and id. When we call method get_anchor_link_to_record without name,it fails with ``` TypeError: get_anchor_link_to_record() missing 1 required positional argument: 'name' ``` Updated get_anchor_link_to_record to default 'name' to None if not provided. closes #312 Signed-off-by: Christophe Simonis (chs) <chs@odoo.com>


This commit ensure that if the name argument is not provided, it will automatically generate a default value for name based on the model and id.
When we call method get_anchor_link_to_record without name, it fails with
Updated get_anchor_link_to_record to default 'name' to None if not provided.
Also added a check to ensure 'id' is not a tuple before proceeding with its use.