-
Notifications
You must be signed in to change notification settings - Fork 4
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
added fields #6
added fields #6
Conversation
@@ -9,7 +9,7 @@ | |||
"license": "AGPL-3", | |||
"website": "https://github.com/OCA/partner-contact", | |||
"depends": ["base"], | |||
"data": ["cron/update_cron.xml", "views/smart_tagger_view.xml"], | |||
"data": ["cron/update_cron.xml", "views/smart_tagger_view.xml", "views/res_partner_category_view_extension.xml"], |
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.
You forgot to include file views/res_partner_category_view_extension.xml
in your commit. But anyway, if you are modifying the view of res.partner.category
object, you can directly do it in the same inheritance already existing in file views/smart_tagger_view.xml
. No need to create multiple views for the same object inside the same module.
@@ -1,2 +1,4 @@ | |||
# License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl). | |||
from . import res_partner_category | |||
from . import res_partner_category_extension |
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.
Again, no need to create a new class inheritance. The same class is already extendend in res_partner_category
file, and you should put your additions inside it.
author_of_the_tag = fields.Many2one('res.users', string="Author") | ||
department_that_uses_the_tag = fields.Many2one('hr.department', string="Department") | ||
description_of_the_tag = fields.Text(string="Description") |
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.
There are field naming conventions based upon their type. Especially for relational field, they should be suffixed by _id
or _ids
. I propose this:
author_of_the_tag = fields.Many2one('res.users', string="Author") | |
department_that_uses_the_tag = fields.Many2one('hr.department', string="Department") | |
description_of_the_tag = fields.Text(string="Description") | |
user_id = fields.Many2one('res.users', "Author", domain=[("share", "=", False)]) # domain is here to avoid putting external users | |
department_id = fields.Many2one('hr.department', "Department") | |
description = fields.Char() # We don't want a large description, char field should be enough. |
author_of_the_tag = fields.Many2one('res.users', string="Author") | ||
department_that_uses_the_tag = fields.Many2one('hr.department', string="Department") | ||
description_of_the_tag = fields.Text(string="Description") | ||
valid_until = fields.Date(string="Valid until") |
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.
No need to specify the string attribute if it's the same as the field name (_
is automatically replaced by spaces in the UI)
valid_until = fields.Date(string="Valid until") | |
valid_until = fields.Date() |
records_to_activate = self.search([('valid_until', '>', today), ('active', '=', False)]) | ||
records_to_activate.write({'active': True}) | ||
|
||
|
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 wouldn't mind activating records that are archived. If they are so, it's probably because of a manual action performed by someone, and we wouldn't want to change that automatically.
records_to_activate = self.search([('valid_until', '>', today), ('active', '=', False)]) | |
records_to_activate.write({'active': True}) |
No description provided.