[17.0][ADD] add hr_governance module #1455
Conversation
Luca-Policastro
force-pushed
the
C2C-2439
branch
5 times, most recently
from
6537631 to
a52ba41
Compare
| def _is_assigned_w_steering(self): | ||
| self.ensure_one() | ||
| steering_role = self.env.ref( | ||
| "hr_governance.steering_gct", raise_if_not_found=False |
There was a problem hiding this comment.
IMO we shouldn't have business code depend on configurable records like the role types.
Instead, we should perhaps have a "type"-like selection field on the role types, with predefined values like "steering", and use it instead.
That way the code wouldn't rely on given records being created (through xmlid), an rely instead on record configurations
not blocking, though. can be a future improvement
hr_governance/static/src/components/circlepack_colorlist.esm.js
Outdated
Show resolved
Hide resolved
rvalyi
changed the title
Luca-Policastro
force-pushed
the
C2C-2439
branch
from
a52ba41 to
5296eaf
Compare
hr_governance/__manifest__.py
Outdated
| # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). | ||
| { | ||
| "name": "HR Governance", | ||
| "version": "17.0.1.4.0", |
There was a problem hiding this comment.
| "version": "17.0.1.4.0", | |
| "version": "17.0.1.0.0", |
There was a problem hiding this comment.
Remove the migrations folder: a new module doesn't need any migration
hr_governance/models/__init__.py
Outdated
| from . import res_config_settings | ||
| from . import ir_ui_view | ||
| from . import governance_authority | ||
| from . import governance_expectation | ||
| from . import governance_role_type | ||
| from . import governance_circle | ||
| from . import res_users | ||
| from . import ir_http |
There was a problem hiding this comment.
Non blocking: sort them alphabetically to ease readability (unless there's a technical reason to sort them this way)
| rec.assigned_user_ids = [ | ||
| Command.link(mem.user_id.id) | ||
| for mem in rec.member_ids | ||
| if mem.user_id | ||
| ] |
There was a problem hiding this comment.
| rec.assigned_user_ids = [ | |
| Command.link(mem.user_id.id) | |
| for mem in rec.member_ids | |
| if mem.user_id | |
| ] | |
| rec.assigned_user_ids = rec.member_ids.user_id |
There was a problem hiding this comment.
I'm not sure to understand how this 2 codes do the same thing ?
There was a problem hiding this comment.
The old code creates a list of x2many fields' commands. Each command adds a res.users ID; the users are retrieved by cycling through the rec.member_ids recordset and checking whether the member record is linked to a user.
The new code directly retrieves all the users related to the members, and sets them to the field. It is equivalent as doing this:
users = self.env["res.users"]
for member in rec.member_ids:
if member.user_id:
users += member.user_id
rec.assigned_user_ids = users
| rec.assigned_user_ids = [ | ||
| Command.link(mem.member_id.mapped("user_id").id) | ||
| for mem in rec.member_rel_ids | ||
| if mem.member_id.mapped("user_id") | ||
| ] |
There was a problem hiding this comment.
| rec.assigned_user_ids = [ | |
| Command.link(mem.member_id.mapped("user_id").id) | |
| for mem in rec.member_rel_ids | |
| if mem.member_id.mapped("user_id") | |
| ] | |
| rec.assigned_user_ids = rec.member_rel_ids.member_id.user_id |
| """ | ||
| return random.randint(2, 11) | ||
|
|
||
| @api.depends("child_ids") |
There was a problem hiding this comment.
I would add is_root to the dependencies
| for rec in self: | ||
| rec.member_count = len(rec.member_ids) | ||
|
|
||
| @api.constrains("parent_id") |
|
|
||
| return records | ||
|
|
||
| def _get_fields(self): |
There was a problem hiding this comment.
I would rename this method to make it easier to understand what it does. For example: what are these fields used for?
| class HrEmployee(models.Model): | ||
| _inherit = "hr.employee" | ||
|
|
||
| circle_ids = fields.Many2manyCustom( | ||
| "governance.circle", | ||
| "governance_circle_member_rel", | ||
| "member_id", | ||
| "circle_id", | ||
| create_table=False, | ||
| string="Governance Circle", | ||
| ) | ||
|
|
||
|
|
||
| class GovernanceCircleMember(models.Model): | ||
| _name = "governance.circle.member.rel" | ||
| _description = "Roles and Members relation" | ||
| _rec_names_search = ["member_id.name"] | ||
|
|
||
| circle_id = fields.Many2one( | ||
| "governance.circle", string="Governance Circle", required=True, index=True | ||
| ) | ||
| member_id = fields.Many2one( | ||
| "hr.employee", string="Energized by", required=True, index=True | ||
| ) | ||
| percentage = fields.Float(string="Time dedicated (%)") | ||
| focus = fields.Text() | ||
| _sql_constraints = [ | ||
| ( | ||
| "unique_member_per_role", | ||
| "unique(member_id, circle_id)", | ||
| "This member is already assigned to the role/circle", | ||
| ), | ||
| ] |
There was a problem hiding this comment.
I would move these 2 models in 2 different files
| self._update_governance_circle("expectation", vals.get("expectation", "")) | ||
| if "authority" in vals: | ||
| self._update_governance_circle("authority", vals.get("authority", "")) | ||
| self.env.registry.clear_cache() |
There was a problem hiding this comment.
Clearing the cache every time a write() is called might be not optimal perf-wise. I would add an if block and check whether we actually need to do it (for example, cache is cleared only if we change a field used by an @ormcache-decorated method).
|
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. |
github-actions
bot
added
the
stale
|
superseded by #1521 |
4 participants
No description provided.