Skip to content

fix(spp_session_tracking): harden security, improve views, and add comprehensive tests #100

Open
gonzalesedwin1123 wants to merge 3 commits into19.0from
fix/spp-session-tracking-improvements
Open

fix(spp_session_tracking): harden security, improve views, and add comprehensive tests #100
gonzalesedwin1123 wants to merge 3 commits into19.0from
fix/spp-session-tracking-improvements

Conversation

@gonzalesedwin1123
Copy link
Member

Summary

  • Fix broken session user write access (ACL was blocking record rule) and add co-facilitator write support
  • Add multi-company record rules, attendance record rules, state transition guards, and write() override
  • Restructure form/list/kanban/search views following OpenSPP UI patterns; add graph and pivot views
  • Add 63 tests (security, constraints, coverage) achieving 95%+ coverage
  • Fix spp_case_session XPath to use hasclass() and avoid duplicate button_box

Changes

Security

  • Fix ACL: grant session users write permission (was 0, now 1) so record rules can scope access
  • Add record rules for co-facilitator write access on sessions and attendance
  • Add multi-company global rules for spp.session and spp.session.type
  • Add attendance record rules (user: read all, write/create own; manager: full CRUD)

Models

  • Add ondelete="restrict" on session_type_id, facilitator_id, participant_id, company_id
  • Add ondelete="cascade" on topic session_type_id
  • Add time validation constraint (end time must be after start time)
  • Add state transition guards via action methods and write() override
  • 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
  • Add unique code constraint on session types via models.Constraint

Views

  • Form: add oe_title, button_box with attendance stat, ribbons, named groups/pages, cancel confirmation
  • List: add row decorations, badge widget on state, optional columns, default_order
  • Kanban: add progressbar, proper
    layout, disable quick_create
  • Calendar: disable quick_create
  • Search: add separators, co-facilitated filter, this month filter, group-by options
  • New graph and pivot views
  • Session type: restructure form, add list decorations, search separator

Cross-module

  • Fix spp_case_session button_box XPath from expr="//sheet/group[1]" to hasclass('oe_button_box') to avoid duplicate button boxes

Tests (63 total)

  • 19 security tests (user read/write/create/delete, co-facilitator, manager, multi-company)
  • 6 constraint tests (unique attendance, time validation, ondelete restrict)
  • 22 session tests (state workflow, computed fields, action_view_sessions, multi-record write)
  • 16 attendance tests (rate computation, edge cases, related fields, topics)

Documentation

  • Update DESCRIPTION.md with corrected security info and new view types
  • Add USAGE.md UI testing guide (65+ test cases across 12 areas)

Test plan

  • ./scripts/test_single_module.sh spp_session_tracking — 63/63 passed
  • ./scripts/test_single_module.sh spp_case_session — 29/29 passed
  • Pre-commit linting (ruff, ruff-format, prettier) — all clean
  • QA: Follow readme/USAGE.md for manual UI testing

@gemini-code-assist
Copy link

Summary of Changes

Hello, 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

  • Security Hardening: Fixed ACLs to grant session users write permission, enabling record rules to scope access. Added comprehensive record rules for co-facilitator write access on sessions and attendance, along with multi-company global rules for spp.session and spp.session.type. Implemented detailed attendance record rules for users (read all, write/create own) and managers (full CRUD).
  • UI/UX Improvements: Restructured form, list, kanban, and search views to align with OpenSPP UI patterns. Introduced new graph and pivot views for better data visualization. Enhanced list view with row decorations and state badges, and kanban view with progress bars and improved layout. Added new search filters for co-facilitated sessions and 'this month'.
  • Model Enhancements and Data Integrity: Added ondelete="restrict" to critical Many2one fields (session_type_id, facilitator_id, participant_id, company_id) and ondelete="cascade" for session_type_id on topics. Implemented a time validation constraint to ensure end time is after start time. Introduced state transition guards via action methods and an override of the write() method to prevent invalid state changes. Fixed an N+1 query issue in _compute_counts using read_group and added a unique code constraint on session types.
  • Comprehensive Testing: Added 63 new tests covering security, constraints, and workflow, achieving over 95% coverage. This includes 19 security tests, 6 constraint tests, 22 session tests (state workflow, computed fields, multi-record write), and 16 attendance tests (rate computation, edge cases, related fields, topics).
  • Cross-Module Compatibility: Fixed the spp_case_session button box XPath to use hasclass() instead of a positional index, resolving an issue with duplicate button boxes.
  • Documentation Updates: Updated DESCRIPTION.md with corrected security information and new view types. Added a detailed USAGE.md UI testing guide with over 65 test cases across 12 functional areas.
Changelog
  • spp_case_session/views/session_views.xml
    • Updated XPath expression for the button box to use hasclass('oe_button_box') for more robust element selection.
  • spp_session_tracking/data/session_data.xml
    • Renamed required_attendance_pct field to required_attendance_percentage in session type demo data.
  • spp_session_tracking/models/session.py
    • Added imports for logging, UserError, ValidationError, and _ for improved error handling and internationalization.
    • Applied ondelete="restrict" to session_type_id, facilitator_id, and company_id fields to prevent accidental deletion of related records.
    • Implemented a _check_time_range constraint to validate that end_time is always after start_time.
    • Adjusted _compute_duration to ensure duration is non-negative.
    • Introduced _VALID_TRANSITIONS to define allowed state changes.
    • Overrode the write method to enforce state transition rules, raising UserError for invalid changes.
    • Modified action_start, action_complete, and action_cancel methods to include state checks, logging, and user-friendly error messages for invalid transitions.
  • spp_session_tracking/models/session_attendance.py
    • Applied ondelete="restrict" to the participant_id field.
    • Removed default=False from is_attended and is_excused boolean fields.
  • spp_session_tracking/models/session_topic.py
    • Applied ondelete="cascade" to the session_type_id field.
  • spp_session_tracking/models/session_type.py
    • Added import for _ for internationalization.
    • Renamed required_attendance_pct field to required_attendance_percentage.
    • Applied ondelete="restrict" to the company_id field.
    • Added a _unique_code constraint to ensure session type codes are unique.
    • Updated _compute_counts to use read_group for efficient session counting, addressing potential N+1 query issues.
    • Modified action_view_sessions to use translated names and updated the default view_mode from tree to list.
  • spp_session_tracking/readme/DESCRIPTION.md
    • Updated the security section to reflect co-facilitator write access and multi-company rules.
    • Expanded the list of available views to include graph and pivot views.
    • Clarified the management of topics within session types.
  • spp_session_tracking/readme/USAGE.md
    • Added a new comprehensive UI testing guide covering module installation, menu structure, session type management, session creation, state workflow, attendance, various views, search/filtering, security, data integrity, cross-module integration, and edge cases.
  • spp_session_tracking/security/ir.model.access.csv
    • Updated access_spp_session_user to grant write permission (from 0 to 1) for session users, enabling record rules to control access.
  • spp_session_tracking/security/session_rules.xml
    • Added multi-company global record rules for spp.session and spp.session.type to ensure data isolation.
    • Modified rule_session_user_facilitated to extend write access to co-facilitators.
    • Added rule_session_manager_full for spp.session to grant managers full CRUD access.
    • Introduced new record rules for spp.session.attendance to define user (read all, write/create own session attendance) and manager (full CRUD) permissions.
  • spp_session_tracking/tests/init.py
    • Imported new test modules: test_security and test_constraints.
  • spp_session_tracking/tests/test_attendance.py
    • Imported Command for cleaner record creation in tests.
    • Added SessionTopic to setUpClass for topic-related tests.
    • Updated expected_participant_ids creation to use Command.link.
    • Added new tests for attendance rate calculation with zero expected participants, no records, and all excused participants.
    • Included tests for topic assignment with and without topic tracking enabled on session types.
    • Added tests for related fields (session_date, session_type_id) and comprehensive field coverage for attendance and topics.
  • spp_session_tracking/tests/test_constraints.py
    • Added a new test file to verify database and model constraints, including duplicate attendance, invalid time ranges, and ondelete restrictions for session types and participants, and unique session type codes.
  • spp_session_tracking/tests/test_security.py
    • Added a new test file with extensive security tests covering group hierarchy, admin access, user read/write permissions (including co-facilitators), manager full access, and multi-company isolation.
  • spp_session_tracking/tests/test_session.py
    • Imported Command and UserError for test setup and assertion.
    • Updated expected_participant_ids creation to use Command.link.
    • Added comprehensive tests for valid and invalid state transitions (action_start, action_complete, action_cancel).
    • Included tests for computed field edge cases, such as duration calculation with partial time inputs.
    • Added tests for session_type_session_count, action_view_sessions return values, session type frequency, archiving, and multi-record state write operations.
  • spp_session_tracking/views/session_type_views.xml
    • Restructured the session type form view by moving the description field into the main group and converting the 'Topics' page into a conditional group.
    • Renamed view_session_type_tree to view_session_type_list and added decoration-muted for inactive records.
    • Renamed required_attendance_pct to required_attendance_percentage in the form and list views.
    • Added a separator to the search view for better organization.
    • Removed an outdated Odoo 19 comment regarding view mode changes.
  • spp_session_tracking/views/session_views.xml
    • Added action_session_attendance_popup for displaying attendance records in a modal window.
    • Added a confirmation prompt to the 'Cancel' button in the session form view.
    • Integrated web_ribbon widgets to visually indicate 'Completed' and 'Cancelled' session states.
    • Restructured the session form view with an oe_button_box for attendance statistics, an oe_title for the session name, and named group and column elements for clarity.
    • Updated the participant list to use a standard list widget and added visual decorations (decoration-success, decoration-warning, decoration-muted) to the attendance list.
    • Renamed view_session_tree to view_session_list, set default_order by date, and added decoration attributes and widget="badge" for state display.
    • Made location and area_id fields optional in the list view.
    • Disabled quick-create functionality in the calendar view using quick_create="0".
    • Enhanced the kanban view with quick_create="false", a progressbar for state distribution, and a cleaner div based layout for cards.
    • Added a separator and new filters ('Cancelled', 'My Co-facilitated Sessions') to the session search view.
    • Introduced new view_session_graph and view_session_pivot views for analytical reporting.
    • Updated action_session_view and action_my_sessions to include the new graph and pivot view modes.
Activity
  • The pull request author, gonzalesedwin1123, has implemented significant changes across security, UI, models, and testing.
  • New test files (test_security.py, test_constraints.py) were added to expand test coverage.
  • Existing test files (test_attendance.py, test_session.py) were updated with more comprehensive test cases, including state transitions and edge cases.
  • Documentation was updated with a detailed UI testing guide (USAGE.md) and refined security descriptions (DESCRIPTION.md).
  • Multiple XML view files were refactored to improve UI consistency and add new functionalities like graph and pivot views.
Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@codecov
Copy link

codecov bot commented Mar 12, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.50%. Comparing base (4854031) to head (998f5b1).

Additional details and impacted files

Impacted file tree graph

@@            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     
Flag Coverage Δ
spp_base_common 90.26% <ø> (ø)
spp_case_session 94.64% <ø> (ø)
spp_programs 45.51% <ø> (ø)
spp_security 66.66% <ø> (ø)
spp_session_tracking 99.25% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
spp_session_tracking/models/session.py 100.00% <100.00%> (ø)
spp_session_tracking/models/session_attendance.py 100.00% <100.00%> (ø)
spp_session_tracking/models/session_topic.py 100.00% <100.00%> (ø)
spp_session_tracking/models/session_type.py 100.00% <100.00%> (ø)

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +206 to +212
<list
default_order="date desc"
decoration-info="state == 'scheduled'"
decoration-warning="state == 'in_progress'"
decoration-success="state == 'completed'"
decoration-muted="state == 'cancelled'"
>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

Suggested change
<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">

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

Suggested change
<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.
@gonzalesedwin1123 gonzalesedwin1123 force-pushed the fix/spp-session-tracking-improvements branch from 72fefe9 to 998f5b1 Compare March 12, 2026 08:04
@gonzalesedwin1123 gonzalesedwin1123 marked this pull request as ready for review March 12, 2026 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant