fix(spp_session_tracking): harden security, improve views, and add comprehensive tests #100
fix(spp_session_tracking): harden security, improve views, and add comprehensive tests #100gonzalesedwin1123 wants to merge 3 commits into19.0from
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the session tracking module by bolstering its security mechanisms, refining the user interface, and improving data integrity. It introduces robust access controls for various user roles, ensuring appropriate permissions for managing sessions and attendance, including support for co-facilitators and multi-company environments. The user experience is upgraded with redesigned views and new analytical tools, while backend stability is improved through new validation rules and optimized data handling. The changes are thoroughly validated with an extensive suite of new automated tests and detailed manual testing instructions. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 19.0 #100 +/- ##
==========================================
+ Coverage 70.39% 70.50% +0.10%
==========================================
Files 674 681 +7
Lines 37005 37139 +134
==========================================
+ Hits 26051 26184 +133
- Misses 10954 10955 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Code Review
The pull request refactors session and session type models, renaming required_attendance_pct to required_attendance_percentage and adding ondelete="restrict" to several Many2one fields to enforce data integrity. It introduces a robust state machine for sessions with enforced transitions and validation for start/end times. The UI is enhanced with new graph and pivot views, improved list view decorations, and a dedicated attendance popup. Documentation and test coverage have been expanded to reflect these changes. Review comments suggest improving UI consistency by using decoration-danger for 'cancelled' sessions in the list view and disabling kanban card dragging to prevent user errors and align with the specified UX.
| <list | ||
| default_order="date desc" | ||
| decoration-info="state == 'scheduled'" | ||
| decoration-warning="state == 'in_progress'" | ||
| decoration-success="state == 'completed'" | ||
| decoration-muted="state == 'cancelled'" | ||
| > |
There was a problem hiding this comment.
There's a minor inconsistency in the visual representation of the 'Cancelled' state. Here in the list view, the row is styled with decoration-muted, while the form view ribbon and kanban progress bar use a red (danger) color. For a more consistent user experience across the module, consider using decoration-danger here as well.
| <list | |
| default_order="date desc" | |
| decoration-info="state == 'scheduled'" | |
| decoration-warning="state == 'in_progress'" | |
| decoration-success="state == 'completed'" | |
| decoration-muted="state == 'cancelled'" | |
| > | |
| <list | |
| default_order="date desc" | |
| decoration-info="state == 'scheduled'" | |
| decoration-warning="state == 'in_progress'" | |
| decoration-success="state == 'completed'" | |
| decoration-danger="state == 'cancelled'" |
| <field name="model">spp.session</field> | ||
| <field name="arch" type="xml"> | ||
| <kanban default_group_by="state"> | ||
| <kanban default_group_by="state" quick_create="false"> |
There was a problem hiding this comment.
While the backend correctly prevents invalid state transitions via the write() method, the UI currently allows users to attempt dragging a kanban card to another column, which results in an error. To improve the user experience and align with the USAGE.md which states cards cannot be dragged, you can disable this functionality directly in the view by adding group_drag="false" to the <kanban> tag.
| <kanban default_group_by="state" quick_create="false"> | |
| <kanban default_group_by="state" quick_create="false" group_drag="false"> |
- Fix ACL allowing session users to write own facilitated sessions - Add co-facilitator write access and multi-company record rules - Add attendance record rules scoped to facilitated sessions - Add state transition guards, write() override, and time validation - Add ondelete=restrict on session_type_id, facilitator_id, company_id - Fix N+1 query in _compute_counts using read_group - Remove PII from log messages (log rec.id only) - Rename required_attendance_pct to required_attendance_percentage - Restructure form with oe_title, button_box, ribbons, named groups - Add graph, pivot views; improve list, kanban, calendar, search views - Fix spp_case_session button_box XPath to use hasclass() - Add 63 tests (security, constraints, coverage) for 95%+ coverage - Update DESCRIPTION.md to reflect security and view changes
Replace #### headings with **bold text** for subsections to fix RST heading level inconsistency in oca-gen-addon-readme output.
72fefe9 to
998f5b1
Compare
Summary
Changes
Security
Models
Views
Cross-module
Tests (63 total)
Documentation
Test plan