Refactor case handling: update URLs and views to support case_number and case_id interchangeably, enhancing backward compatibility and maintainability

This commit is contained in:
Ross
2025-11-17 11:05:13 +00:00
parent 81cd109449
commit 6e658aca93
8 changed files with 270 additions and 38 deletions
+7
View File
@@ -1443,6 +1443,13 @@ class CaseDetail(models.Model):
def __str__(self) -> str:
return f"{self.case} -> {self.collection}"
def get_case_index(self):
"""Returns the question index (number) in the collection (0 indexed)"""
case_details = list(self.collection.casedetail_set.all().order_by("sort_order"))
return case_details.index(self)
def get_question_schema(self):
return json.dumps(self.question_schema)
@@ -30,7 +30,7 @@ document.getElementById('save-viewerstate').addEventListener('click', function()
document.getElementById('viewerstate-save-response').innerHTML = "<span class='text-danger'>Viewer state could not be retrieved.</span>";
return;
}
fetch("{% url 'atlas:collection_case_displaysetup' casedetail.collection.pk casedetail.case.pk %}", {
fetch({% if case_number %}"{% url 'atlas:collection_case_displaysetup' casedetail.collection.pk case_number=case_number %}"{% else %}"{% url 'atlas:collection_case_displaysetup_legacy' casedetail.collection.pk casedetail.case.pk %}"{% endif %}, {
method: "POST",
headers: {
"Content-Type": "application/json",
@@ -47,7 +47,7 @@ document.getElementById('save-viewerstate').addEventListener('click', function()
});
});
document.getElementById('reset-viewerstate').addEventListener('click', function() {
fetch("{% url 'atlas:collection_case_displaysetup' casedetail.collection.pk casedetail.case.pk %}", {
fetch({% if case_number %}"{% url 'atlas:collection_case_displaysetup' casedetail.collection.pk case_number=case_number %}"{% else %}"{% url 'atlas:collection_case_displaysetup_legacy' casedetail.collection.pk casedetail.case.pk %}"{% endif %}, {
method: "POST",
headers: {
"Content-Type": "application/json",
@@ -66,9 +66,9 @@
<div>
{% if question_completed %}
<iframe id="viewer" style="width: 100%; height: 100%; border: none; padding: 0px; min-height: 700px" src="/ohif/viewer/dicomjson?url=https://www.penracourses.org.uk{% url 'atlas:collection_case_dicom_json_review' collection.pk case.pk %}"></iframe>
<iframe id="viewer" style="width: 100%; height: 100%; border: none; padding: 0px; min-height: 700px" src="/ohif/viewer/dicomjson?url=https://www.penracourses.org.uk{% if case_number %}{% url 'atlas:collection_case_dicom_json_review' collection.pk case_number=case_number %}{% else %}{% url 'atlas:collection_case_dicom_json_review_legacy' collection.pk case.pk %}{% endif %}"></iframe>
{% else %}
<iframe id="viewer" style="width: 100%; height: 100%; border: none; padding: 0px; min-height: 700px" src="/ohif/viewer/dicomjson?url=https://www.penracourses.org.uk{% url 'atlas:collection_case_dicom_json' collection.pk case.pk %}"></iframe>
<iframe id="viewer" style="width: 100%; height: 100%; border: none; padding: 0px; min-height: 700px" src="/ohif/viewer/dicomjson?url=https://www.penracourses.org.uk{% if case_number %}{% url 'atlas:collection_case_dicom_json' collection.pk case_number=case_number %}{% else %}{% url 'atlas:collection_case_dicom_json_legacy' collection.pk case.pk %}{% endif %}"></iframe>
{% endif %}
</div>
@@ -77,10 +77,10 @@
{% if collection.show_ohif_viewer_link %}
<div>
{% if question_completed %}
<a target="_blank" href="/ohif/viewer/dicomjson?url=https://www.penracourses.org.uk{% url 'atlas:collection_case_dicom_json_review' collection.pk case.pk %}" title="Click to open the case in the advanced viewer. This will open in a new tab/window."><button class="viewer"><i class="bi bi-collection"></i> Launch advanced viewer.</button></a>
{% if question_completed %}
<a target="_blank" href="/ohif/viewer/dicomjson?url=https://www.penracourses.org.uk{% if case_number %}{% url 'atlas:collection_case_dicom_json_review' collection.pk case_number=case_number %}{% else %}{% url 'atlas:collection_case_dicom_json_review_legacy' collection.pk case.pk %}{% endif %}" title="Click to open the case in the advanced viewer. This will open in a new tab/window."><button class="viewer"><i class="bi bi-collection"></i> Launch advanced viewer.</button></a>
{% else %}
<a target="_blank" href="/ohif/viewer/dicomjson?url=https://www.penracourses.org.uk{% url 'atlas:collection_case_dicom_json' collection.pk case.pk %}" title="Click to open the case in the advanced viewer. This will open in a new tab/window."><button class="viewer"><i class="bi bi-collection"></i> Launch advanced viewer.</button></a>
<a target="_blank" href="/ohif/viewer/dicomjson?url=https://www.penracourses.org.uk{% if case_number %}{% url 'atlas:collection_case_dicom_json' collection.pk case_number=case_number %}{% else %}{% url 'atlas:collection_case_dicom_json_legacy' collection.pk case.pk %}{% endif %}" title="Click to open the case in the advanced viewer. This will open in a new tab/window."><button class="viewer"><i class="bi bi-collection"></i> Launch advanced viewer.</button></a>
{% endif %}
</div>
{% endif %}
@@ -24,9 +24,14 @@
<a href="{% url 'atlas:case_detail' case.id %}" class="btn btn-sm btn-outline-primary">View case</a>
</div>
{% if not added %}
<form class="row g-2" hx-post="{% url 'atlas:collection_case_priors' collection.id casedetail.case.pk %}"
hx-target="#prior-card-{{case.id}}" hx-swap="outerHTML">
{% if not added %}
<form class="row g-2"
{% if case_number %}
hx-post="{% url 'atlas:collection_case_priors' collection.id case_number=case_number %}"
{% else %}
hx-post="{% url 'atlas:collection_case_priors_legacy' collection.id casedetail.case.pk %}"
{% endif %}
hx-target="#prior-card-{{case.id}}" hx-swap="outerHTML">
{% csrf_token %}
<input type="hidden" name="prior_case_id" value="{{case.id}}" />
<div class="col-12">
@@ -44,8 +49,13 @@
</div>
</form>
{% else %}
<form hx-post="{% url 'atlas:collection_case_priors' collection.id casedetail.case.pk %}"
hx-target="#prior-card-{{case.id}}" hx-swap="outerHTML">
<form
{% if case_number %}
hx-post="{% url 'atlas:collection_case_priors' collection.id case_number=case_number %}"
{% else %}
hx-post="{% url 'atlas:collection_case_priors_legacy' collection.id casedetail.case.pk %}"
{% endif %}
hx-target="#prior-card-{{case.id}}" hx-swap="outerHTML">
{% csrf_token %}
<input type="hidden" name="remove" value="{{case.id}}" />
<div class="mb-2 small text-muted">Current relation: <strong>{{relation}}</strong></div>
@@ -2,7 +2,7 @@
<div class="d-flex justify-content-between align-items-center mb-3">
<div>
{% if previous %}
<a class="btn btn-outline-secondary btn-sm" href="{% url nav_link_view collection.id previous.id %}">&larr; Previous</a>
<a class="btn btn-outline-secondary btn-sm" href="{% url nav_link_view collection.id case_number|add:-1 %}">&larr; Previous</a>
{% endif %}
</div>
<div class="text-center">
@@ -14,7 +14,7 @@
</div>
<div>
{% if next %}
<a class="btn btn-outline-secondary btn-sm" href="{% url nav_link_view collection.id next.id %}">Next &rarr;</a>
<a class="btn btn-outline-secondary btn-sm" href="{% url nav_link_view collection.id case_number|add:1 %}">Next &rarr;</a>
{% endif %}
</div>
@@ -1,14 +1,14 @@
<div class="btn-group btn-group-sm me-2" role="group" aria-label="management-actions">
<a class="btn btn-outline-secondary" href="{% url 'atlas:collection_case_displaysetup' casedetail.collection.pk casedetail.case.pk %}" title="Setup default display">
<a class="btn btn-outline-secondary" href="{% url 'atlas:collection_case_displaysetup' casedetail.collection.pk casedetail.get_case_index %}" title="Setup default display">
<i class="bi bi-display"></i>
</a>
<a class="btn btn-outline-secondary" href="{% url 'atlas:collection_case_details' casedetail.collection.pk casedetail.case.pk %}" title="Case details">
<a class="btn btn-outline-secondary" href="{% url 'atlas:collection_case_details' casedetail.collection.pk casedetail.get_case_index %}" title="Case details">
<i class="bi bi-info-square"></i>
</a>
{% if collection.collection_type == "QUE" %}
<a class="btn btn-outline-secondary" href='{% url "atlas:collection_case_questions" casedetail.collection.pk casedetail.case.pk %}' title="Manage questions">
<a class="btn btn-outline-secondary" href='{% url "atlas:collection_case_questions" casedetail.collection.pk casedetail.get_case_index %}' title="Manage questions">
{% if casedetail.question_schema %}
<i class="bi bi-question-square text-success"></i>
{% else %}
+58 -6
View File
@@ -168,30 +168,56 @@ urlpatterns = [
),
path("exam/<int:pk>/groups", views.ExamGroupsUpdate.as_view(), name="exam_groups_edit"),
path(
"collection/<int:exam_id>/case/<int:case_id>/details",
"collection/<int:exam_id>/case/<int:case_number>/details",
views.collection_case_details,
name="collection_case_details",
),
# Backwards-compatible redirect from legacy case_id-style URLs
path(
"collection/<int:exam_id>/case/<int:case_id>/questions",
"collection/<int:exam_id>/case/<int:case_id>/details",
views.redirect_collection_case_details_by_id,
name="collection_case_details_legacy",
),
path(
"collection/<int:exam_id>/case/<int:case_number>/questions",
views.collection_case_questions,
name="collection_case_questions",
),
path(
"collection/<int:exam_id>/case/<int:case_id>/priors",
"collection/<int:exam_id>/case/<int:case_id>/questions",
views.redirect_collection_case_questions_by_id,
name="collection_case_questions_legacy",
),
path(
"collection/<int:exam_id>/case/<int:case_number>/priors",
views.collection_case_priors,
name="collection_case_priors",
),
path(
"collection/<int:exam_id>/case/<int:case_id>/dicom_json",
"collection/<int:exam_id>/case/<int:case_id>/priors",
views.redirect_collection_case_priors_by_id,
name="collection_case_priors_legacy",
),
path(
"collection/<int:exam_id>/case/<int:case_number>/dicom_json",
views.collection_case_dicom_json,
name="collection_case_dicom_json",
),
path(
"collection/<int:exam_id>/case/<int:case_id>/dicom_json/review",
"collection/<int:exam_id>/case/<int:case_id>/dicom_json",
views.redirect_collection_case_dicom_json_by_id,
name="collection_case_dicom_json_legacy",
),
path(
"collection/<int:exam_id>/case/<int:case_number>/dicom_json/review",
views.collection_case_dicom_json_review,
name="collection_case_dicom_json_review",
),
path(
"collection/<int:exam_id>/case/<int:case_id>/dicom_json/review",
views.redirect_collection_case_dicom_json_review_by_id,
name="collection_case_dicom_json_review_legacy",
),
path(
"collection/<int:exam_id>/question_schemas",
views.collection_question_schemas,
@@ -272,26 +298,52 @@ urlpatterns = [
views.collection_case_view,
name="collection_case_view",
),
# legacy path that used case PK; redirect to canonical case_number URL
path(
"collection/<int:collection_id>/<int:case_id>/display_setup",
"collection/<int:pk>/<int:case_id>",
views.redirect_collection_case_view_by_id,
name="collection_case_view_legacy",
),
path(
"collection/<int:collection_id>/<int:case_number>/display_setup",
views.collection_case_displaysetup,
name="collection_case_displaysetup",
),
path(
"collection/<int:collection_id>/<int:case_id>/display_setup",
views.redirect_collection_case_displaysetup_by_id,
name="collection_case_displaysetup_legacy",
),
path(
"collection/<int:pk>/<int:case_number>/take/<int:cid>/<str:passcode>",
views.collection_case_view_take,
name="collection_case_view_take",
),
path(
"collection/<int:pk>/<int:case_id>/take/<int:cid>/<str:passcode>",
views.redirect_collection_case_view_take_by_id,
name="collection_case_view_take_legacy",
),
path(
"collection/<int:pk>/<int:case_number>/take/",
views.collection_case_view_take_user,
name="collection_case_view_take_user",
),
path(
"collection/<int:pk>/<int:case_id>/take/",
views.redirect_collection_case_view_take_user_by_id,
name="collection_case_view_take_user_legacy",
),
path(
"collection/<int:pk>/<int:case_number>/take/answers",
views.collection_case_view_take_user_answers,
name="collection_case_view_take_user_answers",
),
path(
"collection/<int:pk>/<int:case_id>/take/answers",
views.redirect_collection_case_view_take_user_answers_by_id,
name="collection_case_view_take_user_answers_legacy",
),
path(
"collection/<int:pk>/json_edit",
views.GenericExamViews.exam_json_edit,
+178 -15
View File
@@ -2755,10 +2755,101 @@ def collection_take_start(request, pk, cid=None, passcode=None):
)
def _case_number_for_collection_and_caseid(collection_pk, case_id):
"""Helper: given a collection PK and a case PK, return the case index (case_number).
Raises Http404 if the case is not found in the collection.
"""
collection = get_object_or_404(CaseCollection, pk=collection_pk)
try:
# attempt to resolve the case object (may raise Http404)
case_obj = get_case_for_case_detail(case_id)
except Http404:
raise
try:
return collection.get_index_of_case(case_obj)
except ValueError:
raise Http404(f"Case {case_id} not in collection {collection_pk}")
@login_required
def redirect_collection_case_details_by_id(request, exam_id, case_id):
case_number = _case_number_for_collection_and_caseid(exam_id, case_id)
return redirect("atlas:collection_case_details", exam_id=exam_id, case_number=case_number)
@login_required
def redirect_collection_case_questions_by_id(request, exam_id, case_id):
case_number = _case_number_for_collection_and_caseid(exam_id, case_id)
return redirect("atlas:collection_case_questions", exam_id=exam_id, case_number=case_number)
@login_required
def redirect_collection_case_priors_by_id(request, exam_id, case_id):
case_number = _case_number_for_collection_and_caseid(exam_id, case_id)
return redirect("atlas:collection_case_priors", exam_id=exam_id, case_number=case_number)
@login_required
def redirect_collection_case_dicom_json_by_id(request, exam_id, case_id):
case_number = _case_number_for_collection_and_caseid(exam_id, case_id)
return redirect("atlas:collection_case_dicom_json", exam_id=exam_id, case_number=case_number)
@login_required
def redirect_collection_case_dicom_json_review_by_id(request, exam_id, case_id):
case_number = _case_number_for_collection_and_caseid(exam_id, case_id)
return redirect("atlas:collection_case_dicom_json_review", exam_id=exam_id, case_number=case_number)
@login_required
def redirect_collection_case_view_by_id(request, pk, case_id):
case_number = _case_number_for_collection_and_caseid(pk, case_id)
return redirect("atlas:collection_case_view", pk=pk, case_number=case_number)
@login_required
def redirect_collection_case_displaysetup_by_id(request, collection_id, case_id):
case_number = _case_number_for_collection_and_caseid(collection_id, case_id)
return redirect("atlas:collection_case_displaysetup", collection_id=collection_id, case_number=case_number)
@login_required
def redirect_collection_case_view_take_by_id(request, pk, case_id, cid, passcode):
case_number = _case_number_for_collection_and_caseid(pk, case_id)
return redirect("atlas:collection_case_view_take", pk=pk, case_number=case_number, cid=cid, passcode=passcode)
@login_required
def redirect_collection_case_view_take_user_by_id(request, pk, case_id):
case_number = _case_number_for_collection_and_caseid(pk, case_id)
return redirect("atlas:collection_case_view_take_user", pk=pk, case_number=case_number)
@login_required
def redirect_collection_case_view_take_user_answers_by_id(request, pk, case_id):
case_number = _case_number_for_collection_and_caseid(pk, case_id)
return redirect("atlas:collection_case_view_take_user_answers", pk=pk, case_number=case_number)
@user_is_collection_author_or_atlas_editor
def collection_case_priors(request, exam_id, case_id):
casedetail = CaseDetail.objects.get(case=case_id, collection=exam_id)
collection = casedetail.collection
# Support either case_number (index) or case_id (case PK) in the URL/kwargs.
# Backwards compatibility: callers may pass case_id; prefer case_number when present.
casedetail = None
collection = None
# Try treating the parameter as a case PK first, falling back to case index.
try:
casedetail = CaseDetail.objects.get(case=case_id, collection=exam_id)
collection = casedetail.collection
except Exception:
collection = get_object_or_404(CaseCollection, pk=exam_id)
try:
case_obj = collection.get_case_by_index(int(case_id))
casedetail = CaseDetail.objects.get(case=case_obj, collection=collection)
except Exception:
raise Http404("Case not found in collection")
if request.htmx:
# Ensure we can render the updated single-card partial and return it so HTMX
@@ -2892,9 +2983,17 @@ def collection_case_priors(request, exam_id, case_id):
@user_is_collection_author_or_atlas_editor
def collection_case_questions(request, exam_id, case_id):
casedetail = CaseDetail.objects.get(case=case_id, collection=exam_id)
collection = casedetail.collection
# Support either case_number (index) or case_id (case PK)
try:
casedetail = CaseDetail.objects.get(case=case_id, collection=exam_id)
collection = casedetail.collection
except Exception:
collection = get_object_or_404(CaseCollection, pk=exam_id)
try:
case_obj = collection.get_case_by_index(int(case_id))
casedetail = CaseDetail.objects.get(case=case_obj, collection=collection)
except Exception:
raise Http404("Case not found in collection")
if not collection.collection_type == "QUE":
raise Http404("Collection not in question mode")
@@ -3005,9 +3104,19 @@ def collection_case_questions(request, exam_id, case_id):
@user_is_collection_author_or_atlas_editor
def collection_case_details(request, exam_id, case_id):
casedetail = CaseDetail.objects.get(case=case_id, collection=exam_id)
collection = casedetail.collection
# Accept either case_number (index into collection) or case_id (case PK).
casedetail = None
collection = None
try:
casedetail = CaseDetail.objects.get(case=case_id, collection=exam_id)
collection = casedetail.collection
except Exception:
collection = get_object_or_404(CaseCollection, pk=exam_id)
try:
case_obj = collection.get_case_by_index(int(case_id))
casedetail = CaseDetail.objects.get(case=case_obj, collection=collection)
except Exception:
raise Http404("Case not found in collection")
if request.method == "POST":
form = CaseDetailForm(request.POST, instance=casedetail)
@@ -3303,12 +3412,14 @@ def collection_case_view_take_answers(
)
def collection_case_view_take_user(request, pk: int, case_number: int):
return collection_case_view_take(request, pk, case_number)
def collection_case_view_take_user(request, pk, case_number=None, case_id=None):
# Accept either case_number (index) or case_id (case PK) and forward both to
# the handler. This allows URLs that use either name to keep working.
return collection_case_view_take(request, pk, case_number=case_number, cid=None, passcode=None, case_id=case_id)
def collection_case_view_take(
request, pk: int, case_number: int, cid=None, passcode=None
request, pk, case_number=None, cid=None, passcode=None, case_id=None
):
"""
Displays the page from which the user takes a case in the collection
@@ -3328,6 +3439,21 @@ def collection_case_view_take(
"""
collection = get_object_or_404(CaseCollection, pk=pk)
# If caller provided a case_id (case PK) instead of a case_number (index),
# translate it into the index used by collection.get_case_by_index().
if case_id is not None and case_number is None:
try:
case_obj = get_case_for_case_detail(case_id)
except Http404:
raise
try:
case_number = collection.get_index_of_case(case_obj)
except ValueError:
raise Http404(f"Case {case_id} not in collection {pk}")
# Ensure we have a case_number to work with
if case_number is None:
raise Http404("No case specified")
form = None
answer: None | CidReportAnswer | UserReportAnswer = None
@@ -3567,7 +3693,7 @@ def collection_case_view_take(
@user_is_collection_author_or_atlas_editor
def collection_case_view(request, pk, case_number):
def collection_case_view(request, pk, case_number=None, case_id=None):
"""Edit/management overview of the case (as part of the collection)
Args:
@@ -3583,6 +3709,19 @@ def collection_case_view(request, pk, case_number):
"""
collection = get_object_or_404(CaseCollection, pk=pk)
# Support either case_number (index) or case_id (case PK) in the URL/kwargs.
if case_id is not None and case_number is None:
try:
case_obj = get_case_for_case_detail(case_id)
except Http404:
raise
try:
case_number = collection.get_index_of_case(case_obj)
except ValueError:
raise Http404(f"Case {case_id} not in collection {pk}")
if case_number is None:
raise Http404("No case specified")
form = None
answer = None
@@ -3635,8 +3774,18 @@ def use_dates_as_descriptions(request, pk):
def collection_case_dicom_json_review(request, exam_id, case_id):
return collection_case_dicom_json(request, exam_id, case_id, review=True)
def collection_case_dicom_json(request, exam_id, case_id, review=False):
casedetail = CaseDetail.objects.get(case=case_id, collection=exam_id)
# Support either case_number (index) or case_id (case PK)
try:
casedetail = CaseDetail.objects.get(case=case_id, collection=exam_id)
except Exception:
collection = get_object_or_404(CaseCollection, pk=exam_id)
try:
case_obj = collection.get_case_by_index(int(case_id))
casedetail = CaseDetail.objects.get(case=case_obj, collection=collection)
except Exception:
raise Http404("Case not found in collection")
if review:
priors = casedetail.caseprior_set.exclude(prior_visibility="NO")
@@ -4517,12 +4666,26 @@ def series_bulk_delete(request):
@user_is_collection_author_or_atlas_editor
@csrf_exempt # Only if you have CSRF issues; otherwise, keep CSRF protection
def collection_case_displaysetup(request, collection_id, case_id):
def collection_case_displaysetup(request, collection_id, case_number=None, case_id=None):
"""
View to set up the display options for a specific case in a collection.
This can include setting which series to display, annotations, etc.
"""
casedetail = get_object_or_404(CaseDetail, case__pk=case_id, collection__pk=collection_id)
# Accept either case_number (index into collection) or case_id (case PK).
casedetail = None
try:
# If a case_id was passed explicitly, try that first
if case_id is not None:
casedetail = get_object_or_404(CaseDetail, case__pk=case_id, collection__pk=collection_id)
else:
# Otherwise the route most likely supplied case_number; try resolving by index
collection = get_object_or_404(CaseCollection, pk=collection_id)
case_obj = collection.get_case_by_index(int(case_number))
casedetail = CaseDetail.objects.get(case=case_obj, collection=collection)
except Http404:
raise
except Exception:
raise Http404("Case not found in collection")
if request.method == "POST":
data = json.loads(request.body.decode("utf-8"))