From 90c8f0b60d6c1a87c6ff5099a408155f2fbd4820 Mon Sep 17 00:00:00 2001 From: Ross Date: Mon, 13 Apr 2026 13:44:25 +0100 Subject: [PATCH] Enhance logging and error handling in collection access methods and WSGI configuration --- atlas/models.py | 5 ++++- atlas/views.py | 4 ++++ generic/models.py | 24 +++++++++++++++++++----- rad/wsgi.py | 13 +++++++++++-- 4 files changed, 38 insertions(+), 8 deletions(-) diff --git a/atlas/models.py b/atlas/models.py index b0812fa9..e0837f27 100644 --- a/atlas/models.py +++ b/atlas/models.py @@ -1256,6 +1256,7 @@ class Series(SeriesBase): class CaseCollection(ExamOrCollectionGenericBase): + app_name = "atlas" cases = models.ManyToManyField(Case, through="CaseDetail") @@ -1518,7 +1519,9 @@ class CaseCollection(ExamOrCollectionGenericBase): Extend base check_user_can_take to also require completion of any prerequisite collections. """ - # Perform the normal access checks first + logger.error("Checking if user can take collection with prerequisites") + logger.error(f"CID: {cid}, user: {user}, active_only: {active_only}") + #` Perform the normal access checks first super().check_user_can_take(cid, passcode, user=user, active_only=active_only) # If there are prerequisites, the user (or CID) must have completed them diff --git a/atlas/views.py b/atlas/views.py index 3812eb58..ad315f9b 100755 --- a/atlas/views.py +++ b/atlas/views.py @@ -4002,6 +4002,8 @@ def collection_take_start(request, pk, cid=None, passcode=None): valid_user = True except Http404: valid_user = False + except collection.InactiveException as e: + return HttpResponse(f"

{str(e)}

") except PrerequisiteRequired as e: # Show a friendly page linking to the required collection rather than 404 prereq = getattr(e, 'prereq', None) @@ -4034,6 +4036,8 @@ def collection_take_start(request, pk, cid=None, passcode=None): return render( request, "atlas/collection_review_start.html", template_variables ) + case _: + return HttpResponse(f"Unknown collection type {collection.collection_type}") def _case_number_for_collection_and_caseid(collection_pk, case_id): diff --git a/generic/models.py b/generic/models.py index acab5b0c..80e579f5 100644 --- a/generic/models.py +++ b/generic/models.py @@ -654,6 +654,8 @@ class ExamOrCollectionGenericBase(models.Model, AuthorMixin): e.g. user management """ + class InactiveException(Exception): + pass # Is this actually used? It is but should be replaced on the inherited class cid_users = GenericRelation("generic.CidUserExam") @@ -768,7 +770,9 @@ class ExamOrCollectionGenericBase(models.Model, AuthorMixin): Raises: Http404: If user does not have access """ + logger.debug(f"Checking user can take: {cid=}, passcode=****, user={user}, active_only={active_only}") if user is not None and self.is_author(user): + logger.debug("User is author, allowing access") return # If we are limiting by dates check that the current date is within the range @@ -778,24 +782,32 @@ class ExamOrCollectionGenericBase(models.Model, AuthorMixin): ): # Raise with an explanatory message so the frontend can # surface the reason to the end user. + logger.debug("Exam is outside of allowed dates, denying access") raise Http404("Exam not currently available (outside allowed dates)") # If it is not active no one can take elif active_only and not self.active: - raise Http404("Exam currently inactive") + logger.debug("Exam is not active, denying access") + raise self.InactiveException("Exam currently inactive") # If candidates only check they have access if self.candidates_only: + logger.debug("Exam is candidates only, checking CID/user access") if not self.check_cid_user(cid, passcode, user): + logger.debug("CID/user access check failed, denying access") raise Http404("Error accessing exam") return # Otherwise check that they are a valid user. if user is None: + logger.debug("No user provided, denying access") raise Http404("Error accessing exam") if not user.is_active: + logger.debug("User is not active, denying access") raise Http404("Error accessing exam") + logger.debug("User passed all checks, allowing access") + return def check_cid_user( @@ -808,7 +820,7 @@ class ExamOrCollectionGenericBase(models.Model, AuthorMixin): allow_authors: bool = True, ): """Checks if a user (cid or otherwise) is allowed to access (and take) the exam""" - print(f"Check cid user: {cid=}, {passcode=}, exam_id={self.pk}, {user=}, {user_id=}, {allow_authors=}") + logger.debug(f"Check cid user: {cid=}, {passcode=}, exam_id={self.pk}, {user=}, {user_id=}, {allow_authors=}") if user is not None and user.is_superuser: return True @@ -829,14 +841,16 @@ class ExamOrCollectionGenericBase(models.Model, AuthorMixin): return True if not self.valid_cid_users.exists() and not self.valid_user_users.exists(): + logger.debug("No valid CID or user users found") return False - - # Start by checking if the logged in user can access + logger.debug(f"Checking user access for user_id={user_id}") if user_id is not None: if self.valid_user_users.filter(pk=user_id).exists(): return True + else: + logger.debug("No valid user users found for user_id={user_id}") # Then test CID data if self.valid_cid_users.exists(): @@ -2284,4 +2298,4 @@ class FindingBase(models.Model): return get_pretty_json(json.loads(self.annotation_json)) def get_pretty_viewport_json(self): - return get_pretty_json(json.loads(self.viewport_json)) \ No newline at end of file + return get_pretty_json(json.loads(self.viewport_json)) diff --git a/rad/wsgi.py b/rad/wsgi.py index 558636c2..20ead4cf 100644 --- a/rad/wsgi.py +++ b/rad/wsgi.py @@ -22,9 +22,18 @@ try: # ensure directory exists log_dir = Path("/var/log/rad") log_dir.mkdir(parents=True, exist_ok=True) + # Determine stdout level: allow overriding with LOGURU_STDOUT_LEVEL env var, + # otherwise expose DEBUG when DEBUG env var is truthy, fall back to INFO. + raw_debug = os.environ.get("DEBUG", "0") + env_level = os.environ.get("LOGURU_STDOUT_LEVEL") + if env_level: + stdout_level = env_level + else: + stdout_level = "DEBUG" if raw_debug.lower() in ("1", "true", "yes") else "INFO" + # Add stdout sink (if not already added by other code) logger.remove() # remove default handlers to avoid duplicate lines - logger.add(sys.stdout, level="INFO", enqueue=True, backtrace=False, diagnose=False) + logger.add(sys.stdout, level=stdout_level, enqueue=True, backtrace=False, diagnose=False) # Add rotating file sink for persistence in container. Use JSON lines # (serialize=True) to make logs easy to query in Loki/Grafana. logger.add( @@ -35,7 +44,7 @@ try: enqueue=True, serialize=True, ) - logger.debug("Loguru logging configured with stdout and file sinks") + logger.debug("Loguru logging configured with stdout and file sinks (stdout=%s)", stdout_level) logging.root.setLevel(logging.DEBUG) except Exception: