[16.0] [IMP] mgmtsystem_evaluation: only manager should reset to draft#768
[16.0] [IMP] mgmtsystem_evaluation: only manager should reset to draft#768luisDIXMIT wants to merge 1 commit into
Conversation
marcos-mendez
left a comment
There was a problem hiding this comment.
Automated Review -- Tests Failed
1. Root Cause
The test failure is due to a database connection error, not directly related to the XML change. However, the change introduces a new groups attribute on a button in mgmtsystem_evaluation.xml, which may cause access control issues if the group mgmtsystem.group_mgmtsystem_manager is not properly defined or loaded during the test run.
2. Suggested Fix
Ensure that the group mgmtsystem.group_mgmtsystem_manager exists and is correctly loaded before the test runs. If the group is defined in the module, confirm it's included in the manifest (__manifest__.py) under data or demo files. If it's missing, add it to the module's data files or verify that it's inherited from another module.
If the group is not defined in this module, and the test fails because of missing access rights, you should either:
- Add the group definition to
data/group.xml, or - Update the test to ensure the required group is available.
File to check/update: __manifest__.py, data/group.xml (if exists), or test setup.
3. Additional Code Issues
- No functional issues in the diff itself — the addition of
groups="mgmtsystem.group_mgmtsystem_manager"is valid Odoo syntax and does not introduce bugs. - However, the test environment may not be fully initialized, causing the connection failure. This is likely a test setup issue, not a code issue.
4. Test Improvements
Add a test case that ensures the button is only visible to users with the mgmtsystem.group_mgmtsystem_manager group. Use SavepointCase or TransactionCase to test visibility and access control.
Suggested test:
def test_button_visibility(self):
# Create a user without the manager group
user = self.env['res.users'].create({
'name': 'Test User',
'login': 'testuser@example.com',
'groups_id': [(6, 0, [self.env.ref('base.group_user').id])]
})
# Create an evaluation record
evaluation = self.env['mgmtsystem.evaluation'].create({
'name': 'Test Evaluation',
'state': 'done',
})
# Check that button is invisible to non-manager user
view = evaluation.with_user(user).fields_view_get(view_type='form')
button_visible = 'Back to draft' in view['arch']
self.assertFalse(button_visible)Reference OCA patterns:
- Use
SavepointCasefor tests that involve access rights and groups. - Use
TransactionCasefor more complex data setups.
✅ Conclusion: The XML change is valid. The test failure is likely due to a missing group or misconfigured test environment, not a code bug. Ensure mgmtsystem.group_mgmtsystem_manager is available in the test context.
⏰ This PR has been open for 32 days.
💤 Last activity was 32 days ago.
Every ignored PR is a contributor who might not come back. Review time matters. (OCA Aging Report)
Reciprocal Review Request
Hi everyone! I found some test failures on this PR and left detailed feedback above. I am happy to discuss or help debug. In the meantime, if any of you get a chance, I would appreciate a look at my open PR(s):
My open PRs across OCA:
- server-tools#3554 [MIG] datetime_formatter: Migration to 18.0
- server-tools#3548 [18.0][MIG] base_kanban_stage: Migration to 18.0
- hr-attendance#262 [16.0][ADD] Hr_attendance_idsecure: iDSecure (ControliD) attendance integration
- stock-logistics-workflow#2276 [16.0][ADD] stock_move_line_devaluation
- stock-logistics-workflow#2275 [16.0][ADD] Stock move line analytic account
- stock-logistics-workflow#2268 [16.0][ADD] stock_move_line_picking_partner
- purchase-workflow#2694 [16.0][IMP]Purchase workflow added to review state & exception fix
Reviewing each other's work helps the whole community move forward. Thank you!
Environment via OCA Neural Reviewer: Minikube + K8s Job + oca-ci/py3.10-odoo16.0 | Odoo 16.0
Automated review by OCA Neural Reviewer + qwen3-coder:30b
A normal user can perform all actions, but the only ones they should be allowed to do are initiating or canceling an evaluation.