-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[12.0][MIG] base_name_search_improved #1419
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
Currently translated at 100.0% (2 of 2 strings) Translation: server-tools-9.0/server-tools-9.0-base_name_search_improved Translate-URL: https://translation.odoo-community.org/projects/server-tools-9-0/server-tools-9-0-base_name_search_improved/ca/
- Descriptor: change __openerp__.py file to __manifest__.py also update module
version from 9.0.1.0.0 to 11.0.1.0.0
- Update README using guideline
- Views: replace view tag openerp to odoo and rename files to match guideline
- Models
- update imports from openerp to odoo
- remove enconding line # -*- coding: utf-8 -*-
- update to make it compatible
- remove use of SUPERUSER_ID use sudo instead.
- rename class name to make it match with guideline.
- update methods to match api used in version 11.0
- fix pylint errors
- replace use of non exist self._model with self._name
- use `Model `is not None instead of if `Model` this becuase the last one is
a empty recordset and this one was evaluate to False and never was
patching the method.
- apply changes resquested/suggested in the PR by the reviewers.
|
Hey @wowerliu, thank you for your Pull Request. It looks like some users haven't signed our Contributor License Agreement, yet.
Appreciation of efforts, |
|
Please check Travis. |
elicoidal
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.
Thanks for the contribution
some details + travis
| Additional search fields can be configured at Settings > Technical > Database > Models, | ||
| using the "Name Search Fields" field. | ||
|
|
||
| .. figure:: https://raw.githubusercontent.com/OCA/server-tools/11.0/base_name_search_improved/images/image1.png |
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.
to be updated to v12
| Additionally, an Administrator can configure other fields to also lookup into. | ||
| For example, Customers could be additionally searched by City or Phone number. | ||
|
|
||
| .. figure:: https://raw.githubusercontent.com/OCA/server-tools/11.0/base_name_search_improved/images/image2.png |
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.
update here too
|
|
||
| def setUp(self): | ||
| super(NameSearchCase, self).setUp() | ||
| phone_field = self.env.ref('base.field_res_partner_phone') |
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.
Tests are failing because of this: the field XML Id changed to base.field_res_partner__phone
|
This error raises when search yields no results: |
how do you solve it ? |
|
|
||
| def _extend_name_results(self, domain, results, limit): | ||
| result_count = len(results) | ||
| if result_count < limit: |
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 result_count < limit: | |
| if not limit: | |
| domain += [('id', 'not in', [x[0] for x in results])] | |
| recs = self.search(domain) | |
| results.extend(recs.name_get()) | |
| elif result_count < limit: |
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.
This patch solves the typeerror
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.
Thank you very much dear
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.
👍 Functional review fine.
|
I see all passed, could anyone merge this branch? |
|
It requires 2 approvals. |
|
Any updates on this merge? |
|
/ocabot merge nobump |
|
This PR looks fantastic, let's merge it! |
|
@dreispt your merge command was aborted due to failed check(s), which you can inspect on this commit of 12.0-ocabot-merge-pr-1419-by-dreispt-bump-nobump. After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red. |
|
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. |
|
/ocabot merge nobump |
|
This PR looks fantastic, let's merge it! |
|
@dreispt your merge command was aborted due to failed check(s), which you can inspect on this commit of 12.0-ocabot-merge-pr-1419-by-dreispt-bump-nobump. After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red. |
|
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. |
Syncing from upstream OCA/server-tools (15.0)
No description provided.