-
-
Notifications
You must be signed in to change notification settings - Fork 878
[16.0][FIX] project_key: handle project search safely by id, key, or name #1561
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
base: 16.0
Are you sure you want to change the base?
Conversation
606a7eb to
d7ba96a
Compare
|
Blocked by : #1562 Once that PR is merged, the CI will be green. |
d7ba96a to
c7fafba
Compare
|
@AaronHForgeFlow @yajo , the PR is ready for review at your convenience. |
AaronHForgeFlow
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.
👍 Code LGTM
| args = list(args or []) | ||
| search_domain = [] | ||
|
|
||
| if name: | ||
| search_domain = [ | ||
| ("key", operator, name), | ||
| ("name", operator, name), | ||
| ] | ||
| if name.isdigit(): | ||
| search_domain.append(("id", "=", int(name))) | ||
|
|
||
| if len(search_domain) > 1: | ||
| domain = ["|"] * (len(search_domain) - 1) + search_domain | ||
| else: | ||
| domain = search_domain | ||
| else: | ||
| domain = [] | ||
|
|
||
| return self._search(domain + args, limit=limit, access_rights_uid=name_get_uid) |
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.
issue: IIUC, you don't need to redo everything that _name_search will do automatically when _rec_names_search is properly configured.
Besides, overriding without calling super() is a general bad practice.
Check this please:
| args = list(args or []) | |
| search_domain = [] | |
| if name: | |
| search_domain = [ | |
| ("key", operator, name), | |
| ("name", operator, name), | |
| ] | |
| if name.isdigit(): | |
| search_domain.append(("id", "=", int(name))) | |
| if len(search_domain) > 1: | |
| domain = ["|"] * (len(search_domain) - 1) + search_domain | |
| else: | |
| domain = search_domain | |
| else: | |
| domain = [] | |
| return self._search(domain + args, limit=limit, access_rights_uid=name_get_uid) | |
| args = list(args or []) | |
| if name.isdigit(): | |
| args.insert(0, ("id", "=", int(name))) | |
| return super()._name_search(name, args, operator, limit, name_get_uid) |
|
Oh, and please add a test. 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. |
issue :-
The module extended project.project with _rec_names_search = ["key", "name", "id"].
Including "id" in _rec_names_search caused SQL errors when users searched with text values (e.g., "inter").
Odoo generated invalid queries like:
"project_project"."id" = 'inter'
which failed because id is an integer field.
Steps to Reproduce the Error :-
Field: Project
Operator: is equal to
Value: any string (e.g., "inter")
Apply the filter.
You will get the following error:
ERROR: invalid input syntax for type integer: "inter"
LINE 1: ...->>'en_US' = 'inter')) OR ("project_project"."id" = 'inter')) AN...
Odoo.-.My.Timesheets.mp4