From 53a027c87c5fede5f7e49974a87b56fb4a865326 Mon Sep 17 00:00:00 2001 From: Denis Durepos Date: Thu, 31 Jul 2025 14:54:44 -0400 Subject: [PATCH] feat: Production readiness sanitization and access control refactoring - Remove all debug logging and statements for production deployment - Convert operational logging from info to debug level where appropriate - Clean up debug test files and commented code - Refactor access control helpers into centralized AccessControlMixin - Consolidate duplicated access control methods across controllers - Enforce team-based access control for all portal users - Fix access control logic to match expected security behavior - Move TODO.md to notes/ directory for better organization - All 76 tests passing with proper security enforcement Production ready: Clean codebase with centralized access control and no debug noise --- bemade_sports_clinic/__manifest__.py | 1 + bemade_sports_clinic/controllers/__init__.py | 1 + .../controllers/access_control_mixin.py | 167 +++++++++++++ .../controllers/patient_injury_portal.py | 98 ++------ .../controllers/player_management_portal.py | 31 +-- .../controllers/task_management_portal.py | 43 +--- .../controllers/team_management_portal.py | 57 +---- .../controllers/team_staff_portal.py | 23 +- bemade_sports_clinic/models/res_users.py | 28 +-- bemade_sports_clinic/{ => notes}/TODO.md | 53 ++++- .../tests/debug_mail_message.py | 157 ------------- bemade_sports_clinic/tests/test_rights.py | 5 +- .../views/sports_clinic_portal_views.xml.bak | 221 ------------------ 13 files changed, 267 insertions(+), 618 deletions(-) create mode 100644 bemade_sports_clinic/controllers/access_control_mixin.py rename bemade_sports_clinic/{ => notes}/TODO.md (66%) delete mode 100644 bemade_sports_clinic/tests/debug_mail_message.py delete mode 100644 bemade_sports_clinic/views/sports_clinic_portal_views.xml.bak diff --git a/bemade_sports_clinic/__manifest__.py b/bemade_sports_clinic/__manifest__.py index 4e67032..02e696d 100644 --- a/bemade_sports_clinic/__manifest__.py +++ b/bemade_sports_clinic/__manifest__.py @@ -84,6 +84,7 @@ "website": "https://www.bemade.org", "license": "LGPL-3", "depends": [ + "mail", # Required for mail.activity functionality "portal", "contacts", "phone_validation", # For phone number formatting in patient contacts diff --git a/bemade_sports_clinic/controllers/__init__.py b/bemade_sports_clinic/controllers/__init__.py index 56a904c..3f2e40f 100644 --- a/bemade_sports_clinic/controllers/__init__.py +++ b/bemade_sports_clinic/controllers/__init__.py @@ -1,3 +1,4 @@ +from . import access_control_mixin from . import team_staff_portal from . import patient_injury_portal from . import team_management_portal diff --git a/bemade_sports_clinic/controllers/access_control_mixin.py b/bemade_sports_clinic/controllers/access_control_mixin.py new file mode 100644 index 0000000..aac3d10 --- /dev/null +++ b/bemade_sports_clinic/controllers/access_control_mixin.py @@ -0,0 +1,167 @@ +# -*- coding: utf-8 -*- +# +# Bemade Inc. +# +# Copyright (C) October 2023 Bemade Inc. (). +# Author: Marc Durepos (Contact : marc@bemade.org) +# +# This program is under the terms of the Odoo Proprietary License v1.0 (OPL-1) +# It is forbidden to publish, distribute, sublicense, or sell copies of the Software +# or modified copies of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. +# IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, +# DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, +# ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +# DEALINGS IN THE SOFTWARE. +# + +from odoo import http, _ +from odoo.exceptions import UserError, AccessError, MissingError +from odoo.http import request + + +class AccessControlMixin: + """ + Mixin class providing common access control methods for portal controllers. + + This centralizes access control logic to avoid duplication across multiple controllers + and ensures consistent security checks throughout the portal interface. + """ + + def _check_team_access(self, team_id, check_staff=False): + """ + Verify the current user has access to this team. + + :param int team_id: ID of the team to check access for + :param bool check_staff: If True, only allow team staff members + :return: The team record if access is granted + :raises: MissingError if team not found + :raises: AccessError if user doesn't have permission + """ + team = request.env['sports.team'].browse(int(team_id)) + if not team.exists(): + raise MissingError(_("Team not found")) + + user = request.env.user + + # Check if user is a staff member of this team + is_team_staff = team.staff_ids.filtered( + lambda s: user.partner_id in s.user_ids.partner_id + ) + + # Check if user is a treatment professional with access + is_treatment_professional = request.env.user.has_group( + 'bemade_sports_clinic.group_portal_treatment_professional' + ) + + if check_staff and not is_team_staff: + # Only team staff can perform certain actions + raise AccessError(_("Only team staff members can perform this action.")) + + if not (is_team_staff or is_treatment_professional): + raise AccessError(_("You don't have permission to access this team.")) + + return team + + def _check_team_staff_access(self, team): + """ + Check if the current user is a staff member of the team. + + :param team: Team record to check staff access for + :return: Staff records if user is team staff, empty recordset otherwise + """ + user = request.env.user + return team.staff_ids.filtered( + lambda s: user.partner_id in s.user_ids.partner_id + ) + + def _check_treatment_professional_access(self): + """ + Check if the current user is a treatment professional. + + :return: True if user is a treatment professional or system admin + """ + return (request.env.user.has_group('bemade_sports_clinic.group_portal_treatment_professional') or + request.env.user.has_group('base.group_system')) + + def _check_access_to_patient(self, patient_id): + """ + Verify the user has access to this patient. + + :param int patient_id: ID of the patient to check access for + :return: The patient record if access is granted + :raises: UserError if user doesn't have permission or patient not found + """ + user = request.env.user + patient = request.env['sports.patient'].browse(int(patient_id)) + + if not patient.exists(): + raise UserError(_('Patient not found.')) + + # Check if user has access through team staff relationships (original task portal logic) + user_teams = user.partner_id.team_staff_rel_ids.mapped('team_id') + patient_teams = patient.team_ids + + # User must be staff on at least one of the patient's teams + has_team_access = bool(user_teams & patient_teams) + + # Treatment professionals still need to be staff on the patient's teams + # They don't get blanket access to all patients + if not has_team_access: + raise UserError(_('You do not have access to this patient.')) + + return patient + + def _check_access_to_injury(self, injury_id): + """ + Verify the user has access to this injury. + + :param int injury_id: ID of the injury to check access for + :return: The injury record if access is granted + :raises: UserError if user doesn't have permission or injury not found + """ + user = request.env.user + injury = request.env['sports.patient.injury'].browse(int(injury_id)) + + if not injury.exists(): + raise UserError(_('Injury not found.')) + + # Check if user has access through team staff relationships (original task portal logic) + user_teams = user.partner_id.team_staff_rel_ids.mapped('team_id') + patient_teams = injury.patient_id.team_ids + + # User must be staff on at least one of the patient's teams + has_team_access = bool(user_teams & patient_teams) + + # Treatment professionals still need to be staff on the patient's teams + # They don't get blanket access to all injuries + if not has_team_access: + raise UserError(_('You do not have access to this injury.')) + + return injury + + def _check_access_to_task_model(self, model_name, record_id): + """ + Verify the user has access to a task-related model record. + + :param str model_name: Name of the model ('sports.patient', 'sports.team', 'sports.patient.injury') + :param int record_id: ID of the record to check access for + :return: The record if access is granted + :raises: UserError if user doesn't have permission or record not found + """ + valid_models = ['sports.patient', 'sports.patient.injury', 'sports.team'] + if model_name not in valid_models: + raise UserError(_('Invalid model specified.')) + + if model_name == 'sports.patient': + return self._check_access_to_patient(record_id) + elif model_name == 'sports.patient.injury': + return self._check_access_to_injury(record_id) + elif model_name == 'sports.team': + return self._check_team_access(record_id) + + # This should never be reached due to the valid_models check above + raise UserError(_('Invalid model specified.')) diff --git a/bemade_sports_clinic/controllers/patient_injury_portal.py b/bemade_sports_clinic/controllers/patient_injury_portal.py index 3cbbb5d..9fb39a9 100644 --- a/bemade_sports_clinic/controllers/patient_injury_portal.py +++ b/bemade_sports_clinic/controllers/patient_injury_portal.py @@ -5,34 +5,16 @@ from odoo import http, fields, _ from odoo.exceptions import UserError, ValidationError from odoo.http import request from odoo.addons.portal.controllers.portal import CustomerPortal, pager +from .access_control_mixin import AccessControlMixin from datetime import datetime _logger = logging.getLogger(__name__) -class PatientInjuryPortal(CustomerPortal): +class PatientInjuryPortal(CustomerPortal, AccessControlMixin): """Controller for all injury reporting functionality in the portal""" - def _check_access_to_patient(self, patient_id): - """Verify the user has access to this patient""" - user = request.env.user - patient = request.env['sports.patient'].browse(int(patient_id)) - - # Check if user has access to any team this patient belongs to - patient_id_int = int(patient_id) - accessible_teams = request.env['sports.team'].search([ - ('staff_ids.user_ids', '=', user.id), - ('patient_ids', 'in', patient_id_int) - ]) - - # Medical professionals might have specific access - # Use request.env.user.has_group() directly to avoid security violations - is_medical = request.env.user.has_group('bemade_sports_clinic.group_portal_treatment_professional') - - if not patient.exists() or (not accessible_teams and not is_medical): - raise UserError(_('You do not have access to this patient.')) - - return patient + # Access control methods now inherited from AccessControlMixin @http.route(['/my/patient/injury/new'], type='http', auth='user', website=True) def create_injury_form(self, patient_id=None, **post): @@ -119,24 +101,21 @@ class PatientInjuryPortal(CustomerPortal): is_treatment_prof = request.env.user.has_group('bemade_sports_clinic.group_portal_treatment_professional') user = request.env.user - # Detailed logging of the current user's status - _logger.info(f"Current user: {user.name} (ID: {user.id})") - _logger.info(f"Is portal coach: {is_portal_coach}") - _logger.info(f"Is in treatment professional group: {is_treatment_prof}") + # Assign treatment professionals based on user role # If user is a treatment professional, add them to the treatment professionals # Only check group membership, not computed field if is_treatment_prof: - _logger.info(f"Adding current user {user.name} (ID: {user.id}) to treatment professionals") + # Add current user as treatment professional injury.write({ 'treatment_professional_ids': [(4, user.id)] }) else: - _logger.info(f"User {user.name} is not identified as a treatment professional, not adding to injury") + # User is not a treatment professional + pass - # Double-check who's assigned after our additions + # Get current treatment professionals treatment_profs = injury.treatment_professional_ids - _logger.info(f"Treatment professionals after assignment: {[u.name for u in treatment_profs]} (IDs: {treatment_profs.ids})") # Always try to assign team therapists regardless of who created the injury if True: @@ -150,20 +129,17 @@ class PatientInjuryPortal(CustomerPortal): ]) # Log debug info - _logger.info(f"Found {len(team_staff)} therapists/head therapists for team {selected_team_id}") + # Process team staff to find therapists for staff in team_staff: - _logger.info(f"Staff: {staff.partner_id.name}, Role: {staff.role}, User IDs: {[u.id for u in staff.user_ids]}") if not staff.user_ids: - _logger.info(f" - WARNING: No user_ids for {staff.partner_id.name}") # Try to find a user directly associated with this partner users = request.env['res.users'].sudo().search([('partner_id', '=', staff.partner_id.id)]) - _logger.info(f" - Found direct users: {[u.id for u in users]}") # Filter by role head_therapists = team_staff.filtered(lambda s: s.role == 'head_therapist') therapists = team_staff.filtered(lambda s: s.role == 'therapist') - _logger.info(f"Found {len(head_therapists)} head therapists and {len(therapists)} therapists") + # Separate head therapists from regular therapists # First try to assign head therapist, then any therapist from the selected team treatment_pros_assigned = False @@ -175,13 +151,14 @@ class PatientInjuryPortal(CustomerPortal): users = request.env['res.users'].search([('partner_id', '=', head_therapist.partner_id.id)]) if users: - _logger.info(f"Assigning head therapist: {head_therapist.partner_id.name} with user ID {users[0].id}") + # Assign head therapist to injury injury.write({ 'treatment_professional_ids': [(4, users[0].id)] }) treatment_pros_assigned = True else: - _logger.info(f"No user account found for head therapist: {head_therapist.partner_id.name}") + # No user account found for head therapist + pass # Try to assign regular therapist if no head therapist was assigned if not treatment_pros_assigned and therapists: @@ -190,13 +167,14 @@ class PatientInjuryPortal(CustomerPortal): users = request.env['res.users'].sudo().search([('partner_id', '=', therapist.partner_id.id)]) if users: - _logger.info(f"Assigning therapist: {therapist.partner_id.name} with user ID {users[0].id}") + # Assign therapist to injury injury.write({ 'treatment_professional_ids': [(4, users[0].id)] }) treatment_pros_assigned = True else: - _logger.info(f"No user account found for therapist: {therapist.partner_id.name}") + # No user account found for therapist + pass # If no therapist was assigned, log a warning if not treatment_pros_assigned: @@ -213,38 +191,7 @@ class PatientInjuryPortal(CustomerPortal): return request.render('bemade_sports_clinic.portal_injury_created', values) - def _check_access_to_injury(self, injury_id): - """Verify the user has access to this injury""" - user = request.env.user - injury = request.env['sports.patient.injury'].browse(int(injury_id)) - - # Check if user has access to this injury based on patient's team ownership - if not injury.exists(): - raise UserError(_('Injury not found.')) - - # Get the patient's teams - patient_teams = injury.patient_id.team_ids - - if patient_teams: - # Check if user is part of any of the patient's team staff - user_teams = request.env['sports.team.staff'].search([ - ('team_id', 'in', patient_teams.ids), - ('user_ids', '=', user.id) - ]) - - # Medical professionals might have specific access - # Use request.env.user.has_group() directly to avoid security violations - is_medical = request.env.user.has_group('bemade_sports_clinic.group_portal_treatment_professional') - - if not user_teams and not is_medical: - raise UserError(_('You do not have access to this injury.')) - else: - # If patient has no teams, only medical professionals can access - # Use request.env.user.has_group() directly to avoid security violations - if not request.env.user.has_group('bemade_sports_clinic.group_portal_treatment_professional'): - raise UserError(_('You do not have access to this injury.')) - - return injury + # _check_access_to_injury method now inherited from AccessControlMixin @http.route(['/my/injury/edit'], type='http', auth='user', website=True) def edit_injury_form(self, injury_id=None, **post): @@ -375,9 +322,7 @@ class PatientInjuryPortal(CustomerPortal): # Add a treatment note if provided if post.get('treatment_note') and is_treatment_prof: - _logger.info(f"DEBUG: About to create treatment note for injury {injury.id}") - _logger.info(f"DEBUG: injury.patient_id = {injury.patient_id} (ID: {injury.patient_id.id})") - _logger.info(f"DEBUG: injury.patient_id.partner_id = {injury.patient_id.partner_id} (ID: {injury.patient_id.partner_id.id if injury.patient_id.partner_id else 'None'})") + # Add treatment note for injury self._add_treatment_note(injury.patient_id, post.get('treatment_note'), injury) # Redirect back to the edit form with success message @@ -389,10 +334,7 @@ class PatientInjuryPortal(CustomerPortal): if not note_content.strip(): return False - _logger.info(f"DEBUG: _add_treatment_note called with patient={patient} (ID: {patient.id})") - _logger.info(f"DEBUG: patient model: {patient._name}") - if hasattr(patient, 'partner_id'): - _logger.info(f"DEBUG: patient.partner_id = {patient.partner_id} (ID: {patient.partner_id.id if patient.partner_id else 'None'})") + # Validate patient parameter # Create a new treatment note linked to patient, optionally to injury vals = { @@ -401,7 +343,7 @@ class PatientInjuryPortal(CustomerPortal): 'date': fields.Date.today(), 'user_id': request.env.user.id, } - _logger.info(f"DEBUG: About to create treatment note with vals: {vals}") + # Create treatment note with prepared values # If injury is provided, link the note to it if injury: diff --git a/bemade_sports_clinic/controllers/player_management_portal.py b/bemade_sports_clinic/controllers/player_management_portal.py index 5363052..576f951 100644 --- a/bemade_sports_clinic/controllers/player_management_portal.py +++ b/bemade_sports_clinic/controllers/player_management_portal.py @@ -3,32 +3,15 @@ from odoo import http, fields, _ from odoo.exceptions import UserError, ValidationError from odoo.http import request from odoo.addons.portal.controllers.portal import CustomerPortal, pager +from .access_control_mixin import AccessControlMixin _logger = logging.getLogger(__name__) -class PlayerManagementPortal(CustomerPortal): +class PlayerManagementPortal(CustomerPortal, AccessControlMixin): """Controller for player management functionality in the portal""" - def _check_access_to_patient(self, patient_id): - """Verify the user has access to this patient""" - user = request.env.user - patient = request.env['sports.patient'].browse(int(patient_id)) - - # Check if user has access to any team this patient belongs to - patient_id_int = int(patient_id) - accessible_teams = request.env['sports.team'].search([ - ('staff_ids.user_ids', '=', user.id), - ('patient_ids', 'in', patient_id_int) - ]) - - # Medical professionals might have specific access - is_medical = user.has_group('bemade_sports_clinic.group_portal_treatment_professional') - - if not patient.exists() or (not accessible_teams and not is_medical): - raise UserError(_('You do not have access to this patient.')) - - return patient + # Access control methods now inherited from AccessControlMixin @http.route(['/my/player/edit'], type='http', auth='user', website=True) def edit_player_form(self, patient_id, **post): @@ -54,10 +37,7 @@ class PlayerManagementPortal(CustomerPortal): # Access fields directly - field-level security is already defined # with appropriate groups for each field - # Debug log to check fields - _logger = logging.getLogger(__name__) - _logger.info(f"DEBUG - Allergies: {patient.allergies}") - _logger.info(f"DEBUG - Team Info Notes: {patient.team_info_notes}") + # Additional fields available to treatment professionals # Basic fields patient_info['date_of_birth'] = patient.date_of_birth @@ -75,8 +55,7 @@ class PlayerManagementPortal(CustomerPortal): # Add any other protected fields that should be available to treatment professionals # You can add more fields here as needed - # Debug log for the entire patient_info dictionary - _logger.info(f"DEBUG - patient_info: {patient_info}") + # Patient info prepared for treatment professional view # Get Canada and Canadian provinces/territories for address dropdowns canada = request.env['res.country'].search([('code', '=', 'CA')], limit=1) diff --git a/bemade_sports_clinic/controllers/task_management_portal.py b/bemade_sports_clinic/controllers/task_management_portal.py index c8090e2..35b3854 100644 --- a/bemade_sports_clinic/controllers/task_management_portal.py +++ b/bemade_sports_clinic/controllers/task_management_portal.py @@ -3,53 +3,16 @@ from odoo import http, fields, _ from odoo.exceptions import UserError, ValidationError from odoo.http import request from odoo.addons.portal.controllers.portal import CustomerPortal, pager +from .access_control_mixin import AccessControlMixin from datetime import timedelta _logger = logging.getLogger(__name__) -class TaskManagementPortal(CustomerPortal): +class TaskManagementPortal(CustomerPortal, AccessControlMixin): """Controller for task management functionality in the portal""" - def _check_access_to_task_model(self, model_name, record_id): - """Verify the user has access to the record""" - user = request.env.user - record = request.env[model_name].browse(int(record_id)) - - # Check if record exists - if not record.exists(): - raise UserError(_('Record not found.')) - - # For patient records, check team access - if model_name == 'sports.patient': - # Check if user has access through team staff relationships - user_teams = user.partner_id.team_staff_rel_ids.mapped('team_id') - patient_teams = record.team_ids - - # User must be staff on at least one of the patient's teams - if not (user_teams & patient_teams): - raise UserError(_('You do not have access to this patient.')) - - # For injury records, check team access through the patient - elif model_name == 'sports.patient.injury': - # Check if user has access through team staff relationships - user_teams = user.partner_id.team_staff_rel_ids.mapped('team_id') - patient_teams = record.patient_id.team_ids - - # User must be staff on at least one of the patient's teams - if not (user_teams & patient_teams): - raise UserError(_('You do not have access to this injury.')) - - # For team records, check if user is staff on the team - elif model_name == 'sports.team': - # Check if user has access through team staff relationships - user_teams = user.partner_id.team_staff_rel_ids.mapped('team_id') - - # User must be staff on this specific team - if record not in user_teams: - raise UserError(_('You do not have access to this team.')) - - return record + # Access control methods now inherited from AccessControlMixin @http.route(['/my/activities'], type='http', auth='user', website=True) def view_activities(self, model=None, res_id=None, **kw): diff --git a/bemade_sports_clinic/controllers/team_management_portal.py b/bemade_sports_clinic/controllers/team_management_portal.py index 4a0bd38..0deb23c 100644 --- a/bemade_sports_clinic/controllers/team_management_portal.py +++ b/bemade_sports_clinic/controllers/team_management_portal.py @@ -2,63 +2,18 @@ from odoo import http, _ from odoo.http import request from odoo.addons.portal.controllers.portal import CustomerPortal from odoo.exceptions import AccessError, MissingError, UserError, ValidationError +from .access_control_mixin import AccessControlMixin import logging _logger = logging.getLogger(__name__) -class TeamManagementPortal(CustomerPortal): +class TeamManagementPortal(CustomerPortal, AccessControlMixin): def _prepare_home_portal_values(self, counters): values = super()._prepare_home_portal_values(counters) return values - def _check_team_access(self, team_id, check_staff=False): - """Verify the current user has access to this team. - - :param int team_id: ID of the team to check access for - :param bool check_staff: If True, only allow team staff members - :return: The team record if access is granted - :raises: MissingError if team not found - :raises: AccessError if user doesn't have permission - """ - team = request.env['sports.team'].browse(int(team_id)) - if not team: - raise MissingError(_("Team not found")) - - user = request.env.user - - # Check if user is a staff member of this team - is_team_staff = team.staff_ids.filtered( - lambda s: user.partner_id in s.user_ids.partner_id - ) - - # Check if user is a treatment professional with access - # Use request.env.user.has_group() directly to avoid security violations - is_treatment_professional = request.env.user.has_group( - 'bemade_sports_clinic.group_portal_treatment_professional' - ) - - if check_staff and not is_team_staff: - # Only team staff can perform certain actions - raise AccessError(_("Only team staff members can perform this action.")) - - if not (is_team_staff or is_treatment_professional): - raise AccessError(_("You don't have permission to access this team.")) - - return team - - def _check_team_staff_access(self, team): - """Check if the current user is a staff member of the team""" - user = request.env.user - return team.staff_ids.filtered( - lambda s: user.partner_id in s.user_ids.partner_id - ) - - def _check_treatment_professional_access(self): - """Check if the current user is a treatment professional""" - return request.env.user.has_group( - 'bemade_sports_clinic.group_portal_treatment_professional' - ) or request.env.user.has_group('base.group_system') + # Access control methods now inherited from AccessControlMixin @http.route(['/my/team//player//request_removal'], type='http', auth="user", website=True, methods=['POST']) @@ -316,7 +271,7 @@ class TeamManagementPortal(CustomerPortal): if not existing_patient.active: existing_patient.write({'active': True}) action_taken.append("reactivated") - _logger.info( + _logger.debug( "Reactivated archived player %s for team %s by user %s", existing_patient.name, team.name, request.env.user.name ) @@ -327,7 +282,7 @@ class TeamManagementPortal(CustomerPortal): 'team_ids': [(4, team.id)], }) action_taken.append("added to team") - _logger.info( + _logger.debug( "Added existing player %s to team %s by user %s", existing_patient.name, team.name, request.env.user.name ) @@ -359,7 +314,7 @@ class TeamManagementPortal(CustomerPortal): patient = request.env['sports.patient'].create_portal_patient(patient_vals) # Log the action - _logger.info( + _logger.debug( "Created new player %s and added to team %s by user %s", patient.name, team.name, request.env.user.name ) diff --git a/bemade_sports_clinic/controllers/team_staff_portal.py b/bemade_sports_clinic/controllers/team_staff_portal.py index 7632547..4a32169 100644 --- a/bemade_sports_clinic/controllers/team_staff_portal.py +++ b/bemade_sports_clinic/controllers/team_staff_portal.py @@ -131,28 +131,7 @@ class TeamStaffPortal(CustomerPortal): user = http.request.env.user is_treatment_prof = user.has_group('bemade_sports_clinic.group_portal_treatment_professional') - # Debug output - import logging - _logger = logging.getLogger(__name__) - _logger.info(f"DEBUG - User: {user.name} (login: {user.login}) is treatment prof: {is_treatment_prof}") - _logger.info(f"DEBUG - User groups: {', '.join([g.name for g in user.groups_id])}") - _logger.info(f"DEBUG - XML ID check: {user.has_group('bemade_sports_clinic.group_portal_treatment_professional')}") - - # More detailed debugging - _logger.info(f"DEBUG - Player ID: {player_id}, Team ID: {team_id}") - if team_id: - _logger.info(f"DEBUG - Team name: {team.name if team else 'Team not found'}") - _logger.info(f"DEBUG - Team staff: {[(s.partner_id.name, s.role) for s in team.staff_ids]}") - _logger.info(f"DEBUG - Player teams: {[t.name for t in player.team_ids]}") - - # Check team staff role - staff_records = http.request.env['sports.team.staff'].sudo().search([('partner_id', '=', user.partner_id.id)]) - _logger.info(f"DEBUG - User's team staff roles: {[(s.team_id.name, s.role) for s in staff_records]}") - _logger.info(f"DEBUG - User's partner ID: {user.partner_id.id}") - - # Check if user should have therapist role - has_therapist_role = any(s.role in ['head_therapist', 'therapist'] for s in staff_records) - _logger.info(f"DEBUG - User has therapist role: {has_therapist_role}") + # Show all injuries to treatment professionals, but only active ones to coaches diff --git a/bemade_sports_clinic/models/res_users.py b/bemade_sports_clinic/models/res_users.py index 4aed86c..250deb3 100644 --- a/bemade_sports_clinic/models/res_users.py +++ b/bemade_sports_clinic/models/res_users.py @@ -38,19 +38,16 @@ class User(models.Model): def write(self, vals): """Override write to trigger treatment professional group assignment when portal access is granted.""" - _logger.info(f"DEBUG: res.users.write() called with vals: {vals}") - _logger.info(f"DEBUG: Processing {len(self)} users: {[u.login for u in self]}") + # Process user updates for portal access changes # Check if groups_id is being modified (portal access being granted/revoked) if 'groups_id' in vals: - _logger.info(f"DEBUG: groups_id is being modified: {vals['groups_id']}") # Get the portal group reference portal_group = self.env.ref('base.group_portal') - _logger.info(f"DEBUG: Portal group ID: {portal_group.id}") # Store old group memberships before making changes old_groups_by_user = {user.id: user.groups_id.ids for user in self} - _logger.info(f"DEBUG: Old groups by user: {old_groups_by_user}") + # Store old group memberships for comparison # Apply the changes first result = super().write(vals) @@ -59,23 +56,21 @@ class User(models.Model): for user in self: old_groups = old_groups_by_user[user.id] new_groups = user.groups_id.ids - _logger.info(f"DEBUG: User {user.login} - Old groups: {old_groups}, New groups: {new_groups}") + # Check if portal access was granted if portal_group.id in new_groups and portal_group.id not in old_groups: - _logger.info(f"DEBUG: Portal access granted to {user.login} - triggering group assignment") + # Portal access was just granted - trigger treatment professional group assignment # Portal access was just granted - trigger treatment professional group assignment staff_records = self.env['sports.team.staff'].search([ ('partner_id', '=', user.partner_id.id) ]) - _logger.info(f"DEBUG: Found {len(staff_records)} staff records for user {user.login}") if staff_records: - _logger.info(f"DEBUG: Staff roles: {[(s.team_id.name, s.role) for s in staff_records]}") staff_records._update_treatment_professional_group(user) - _logger.info(f"DEBUG: Group assignment completed for {user.login}") return result else: - _logger.info(f"DEBUG: groups_id not in vals, using normal write") + # No group changes, use normal write + pass # If groups_id is not being modified, use normal write return super().write(vals) @@ -83,29 +78,26 @@ class User(models.Model): @api.model_create_multi def create(self, vals_list): """Override create to trigger treatment professional group assignment when portal users are created.""" - _logger.info(f"DEBUG: res.users.create() called with vals_list: {vals_list}") + # Process user creation for portal access # Create the users first users = super().create(vals_list) # Get the portal group reference portal_group = self.env.ref('base.group_portal') - _logger.info(f"DEBUG: Portal group ID: {portal_group.id}") + # Check for portal access in created users # Check each created user to see if they were created with portal access for user in users: - _logger.info(f"DEBUG: Created user {user.login} with groups: {user.groups_id.ids}") + # Check if user was created with portal access if portal_group.id in user.groups_id.ids: - _logger.info(f"DEBUG: User {user.login} created with portal access - triggering group assignment") + # User was created with portal access - trigger treatment professional group assignment # User was created with portal access - trigger treatment professional group assignment staff_records = self.env['sports.team.staff'].search([ ('partner_id', '=', user.partner_id.id) ]) - _logger.info(f"DEBUG: Found {len(staff_records)} staff records for user {user.login}") if staff_records: - _logger.info(f"DEBUG: Staff roles: {[(s.team_id.name, s.role) for s in staff_records]}") staff_records._update_treatment_professional_group(user) - _logger.info(f"DEBUG: Group assignment completed for {user.login}") return users diff --git a/bemade_sports_clinic/TODO.md b/bemade_sports_clinic/notes/TODO.md similarity index 66% rename from bemade_sports_clinic/TODO.md rename to bemade_sports_clinic/notes/TODO.md index 66dd746..aef6200 100644 --- a/bemade_sports_clinic/TODO.md +++ b/bemade_sports_clinic/notes/TODO.md @@ -159,4 +159,55 @@ - [ ]Confirm followers added correctly when therapist adds player to team - [x]Remove Team from injury detail (including from model - was not there before) - [x]Fix treatment professional selection in injury detail -- [x]Add patient address add/edit to coach/therapist portal \ No newline at end of file +- [x]Add patient address add/edit to coach/therapist portal + + + +## DD & MD notes on how to move forward +- [ ] Calendar integration - add calendar events linked to tasks so that tasks block out therapist time (only really required for Steph, so can wait a bit) +- [ ] Project/product setup for billing: + - [ ] Create products for FEE-SIDELINE and FEE-TRANSPORT + - [ ] Config FEE-SIDELINE to create Project on confirmation + - [ ] Config FEE-TRANSPORT to create NOTHING on confirmation + - [ ] Set up SO per client cost center + - [ ] Put a FEE-SIDELINE product on the SO + - [ ] Put a FEE-TRANSPORT product on the SO + - [ ] Confirm SO then configure resulting Project (consider a project template to load on the FEE-SIDELINE) +- [ ] Portal user timesheet module (project_subcontractor_portal) + - [ ] Create portal user add timesheet modal + - [ ] Allow portal user to be selected as assignee on project tasks + - [ ] (optional?) Link portal users to Employee records? Probably required for timesheets to work +- [ ] Project task link to event (to block out calendar time re Appointments) +- [ ] Sanitize code for production readiness + - [x] Remove debug code + - [x] Remove debug logging (convert _logger.info to _logger.debug) + - [x] Remove debug print statements + - [x] Remove debug comments + - [x] Remove debug TODOs + - [x] Remove debug print statements + - [ ] Remove body_location field + - [ ] Remove injury_type field + - [ ] Remove severity field + - [ ] Re-establish no-update on security rules and anywhere else we temporarily removed it for debugging, if required + - [ ] Check status of MAIL_ACTIVITY_PORTAL_ACCESS.md and PORTAL_ACCESS_LIMITATIONS.md as well as test suite status (i.e. still have commented out tests?) + update notes at top of mail_activity_portal_rules.xml +- [ ] Sanity checks on functionality + - [ ] Check if tracking needs to be re-enabled on any notes fields (or convert the fields to not be html) + - [ ] Check if portal patient creation functionality actually accessible + - [ ] Check all translations present into french + - [ ] Check injury.document categories + - [ ] Bypass unverified stage for patient.injury when declared by a treatment professional +- [ ] Migration + - [ ] Get medsportsuroit db dump loaded into psql on fitcrew server + - [ ] Create module in bemade-tools + - [ ] See verajet/odoo migration module for example + - [ ] Use direct psql connection to suck out data and map as required + - [ ] Use Command.create() when required to create records with sub-records + - [ ] Set an environment (launch.json) up for testing the migration + - [ ] Staging + - [ ] Don't bother. Just run the whole migration on localhost (neutralizing the fit crew db each time) and test on LAN. + - [ ] Live migration + - [ ] Get a backup of the prod db + - [ ] Stop the server + - [ ] Run the migration locally + - [ ] Restore it back into the fitcrew server + - [ ] Cross fingers (while retesting) \ No newline at end of file diff --git a/bemade_sports_clinic/tests/debug_mail_message.py b/bemade_sports_clinic/tests/debug_mail_message.py deleted file mode 100644 index 91bd489..0000000 --- a/bemade_sports_clinic/tests/debug_mail_message.py +++ /dev/null @@ -1,157 +0,0 @@ -#!/usr/bin/env python3 - -from odoo.tests.common import TransactionCase -from odoo import fields -import logging - -_logger = logging.getLogger(__name__) - -class TestMailMessageDebug(TransactionCase): - - def setUp(self): - super().setUp() - - # Create test data similar to the failing test - self.therapist_user = self.env['res.users'].create({ - 'name': 'Test Therapist', - 'login': 'therapist@test.com', - 'email': 'therapist@test.com', - 'groups_id': [(6, 0, [self.env.ref('bemade_sports_clinic.group_portal_treatment_professional').id])] - }) - - # Create team and add therapist as staff - self.team = self.env['sports.team'].create({ - 'name': 'Test Team', - 'sport': 'football', - }) - - self.env['sports.team.staff'].create({ - 'team_id': self.team.id, - 'partner_id': self.therapist_user.partner_id.id, - 'role': 'therapist', - }) - - # Create authorized patient - self.authorized_patient = self.env['sports.patient'].create({ - 'first_name': 'John', - 'last_name': 'Doe', - 'team_ids': [(6, 0, [self.team.id])], - }) - - # Create activity type - self.patient_activity_type = self.env['mail.activity.type'].create({ - 'name': 'Patient Activity', - 'res_model_id': self.env['ir.model']._get_id('sports.patient'), - }) - - def test_debug_mail_message_access(self): - """Debug test to understand mail.message access control""" - - _logger.info("=== DEBUGGING MAIL.MESSAGE ACCESS ===") - - # Step 1: Create activity and complete it to generate message - _logger.info("Step 1: Creating and completing activity...") - activity = self.env['mail.activity'].create({ - 'activity_type_id': self.patient_activity_type.id, - 'summary': 'Test activity for messages', - 'user_id': self.therapist_user.id, - 'date_deadline': fields.Date.today(), - 'res_model_id': self.env['ir.model']._get_id('sports.patient'), - 'res_id': self.authorized_patient.id, - }) - - # Complete activity to generate message - activity.action_feedback(feedback='Activity completed successfully') - - # Step 2: Check if message was created - _logger.info("Step 2: Checking if message was created...") - all_messages = self.env['mail.message'].sudo().search([ - ('model', '=', 'sports.patient'), - ('res_id', '=', self.authorized_patient.id) - ]) - _logger.info(f"Found {len(all_messages)} messages for patient {self.authorized_patient.id}") - - if all_messages: - message = all_messages[0] - _logger.info(f"Message details: ID={message.id}, model={message.model}, res_id={message.res_id}, author_id={message.author_id.id}") - - # Step 3: Test patient access as therapist - _logger.info("Step 3: Testing patient access as therapist...") - patient_env = self.env['sports.patient'].with_user(self.therapist_user) - accessible_patients = patient_env.search([('id', '=', self.authorized_patient.id)]) - _logger.info(f"Therapist can access {len(accessible_patients)} patients (should be 1)") - - if accessible_patients: - _logger.info(f"Patient accessible: {accessible_patients[0].name}") - else: - _logger.error("PROBLEM: Therapist cannot access authorized patient!") - - # Step 4: Test message access as therapist using different methods - _logger.info("Step 4: Testing message access as therapist...") - message_env = self.env['mail.message'].with_user(self.therapist_user) - - # Method 1: Direct search - messages_direct = message_env.search([ - ('model', '=', 'sports.patient'), - ('res_id', '=', self.authorized_patient.id) - ]) - _logger.info(f"Direct search found {len(messages_direct)} messages") - - # Method 2: Browse specific message ID - if all_messages: - message_browse = message_env.browse(all_messages[0].id) - _logger.info(f"Browse message exists: {message_browse.exists()}") - - # Method 3: Check access manually - try: - message_browse.check_access('read') - _logger.info("Manual check_access('read') passed") - except Exception as e: - _logger.error(f"Manual check_access('read') failed: {e}") - - # Step 5: Debug record rule evaluation - _logger.info("Step 5: Debugging record rule evaluation...") - - # Check team staff relationships - team_staff_rels = self.therapist_user.partner_id.team_staff_rel_ids - _logger.info(f"Therapist has {len(team_staff_rels)} team staff relationships") - - if team_staff_rels: - team_ids = team_staff_rels.mapped('team_id') - _logger.info(f"Therapist is staff on teams: {team_ids.mapped('name')}") - - patient_ids = team_staff_rels.mapped('team_id.patient_ids.id') - _logger.info(f"Accessible patient IDs through teams: {patient_ids}") - - # Check if our patient is in the list - if self.authorized_patient.id in patient_ids: - _logger.info("✓ Authorized patient is in accessible patient list") - else: - _logger.error("✗ Authorized patient is NOT in accessible patient list") - - # Step 6: Test mail.message record rule domain manually - _logger.info("Step 6: Testing mail.message record rule domain manually...") - - # Simulate the record rule domain - domain = [ - '|', - # Messages on patients they have access to through teams - '&', - ('model', '=', 'sports.patient'), - ('res_id', 'in', self.therapist_user.partner_id.team_staff_rel_ids.mapped('team_id.patient_ids.id') or [0]), - '|', - # Messages on injuries they have access to through teams - '&', - ('model', '=', 'sports.patient.injury'), - ('res_id', 'in', self.therapist_user.partner_id.team_staff_rel_ids.mapped('team_id.patient_ids.injury_ids.id') or [0]), - # Messages authored by the user - ('author_id', '=', self.therapist_user.partner_id.id) - ] - - manual_messages = message_env.search(domain) - _logger.info(f"Manual domain search found {len(manual_messages)} messages") - - _logger.info("=== END DEBUG ===") - - # Final assertion to see what happens - self.assertTrue(len(messages_direct) > 0, "Should find messages with direct search") diff --git a/bemade_sports_clinic/tests/test_rights.py b/bemade_sports_clinic/tests/test_rights.py index 4a90a4b..f4b2d32 100644 --- a/bemade_sports_clinic/tests/test_rights.py +++ b/bemade_sports_clinic/tests/test_rights.py @@ -43,10 +43,7 @@ class TestRights(TransactionCase): ], } ) - # _logger.info( - # f"Treatment Pro Groups: " - # f"{cls.treatment_professional_user.groups_id.mapped('name')}" - # ) + def test_treatment_pro_has_access_only_to_staffed_teams(self): """A treatment professional should only have access to teams and, diff --git a/bemade_sports_clinic/views/sports_clinic_portal_views.xml.bak b/bemade_sports_clinic/views/sports_clinic_portal_views.xml.bak deleted file mode 100644 index c9ab6ef..0000000 --- a/bemade_sports_clinic/views/sports_clinic_portal_views.xml.bak +++ /dev/null @@ -1,221 +0,0 @@ - - - - - - - - - - - \ No newline at end of file