feat(cases): add slug field with slug-based case routing#45
Closed
feat(cases): add slug field with slug-based case routing#45
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR introduces slug support to the Case model, enabling URL-friendly identification. Changes include adding a slug field with validation constraints, updating the admin interface to display and manage slugs, extending the API to support slug-based lookups alongside ID-based access, and including slug in serializers for complete CRUD operations. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 OSV Scanner (2.3.5)Error: ENOENT: no such file or directory, scandir '/inmem/1276/nsjail-0216df87-e798-49a5-8b1d-d8b5dd2799f9/merged/.git/hooks' Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Refactor of PR #41 — slug only, no bigo/court_cases/missing_details. - Add slug CharField (max 50, unique, nullable, indexed) to Case model with RegexValidator (letters/numbers/--, cannot start with digit) - Slug is stripped/normalised to None if blank on save - Slug is required before publishing (enforced in clean()) - Admin list shows slug as an external link to jawafdehi.org/case/<slug> with admin search and fieldset support - CaseViewSet.get_object() resolves by numeric pk OR slug - slug field added to CaseSerializer (public API output) - slug field added to CaseCreateSerializer and CasePatchSerializer (caseworker API inputs) - Migration 0018_case_slug
damo-da
force-pushed
the
feat/case-slug-only
branch
from
14c1b99 to
6bfb52e
Compare
added 3 commits
April 3, 2026 16:47
- Rename slug_link -> case_link in CaseAdmin (short_description='Case') - Reset api_views.py to main and apply only get_object() slug routing, removing the accidental reversion of caseworker state-transition code that was already merged in PR #40
- Increase slug max_length from 50 to 100 across model, serializers,
caseworker serializers, and migration
- Update regex to {1,100} accordingly
- Update help_text example to '078-WC-0123-sunil-poudel' (court case
number as prefix, no redundant 'case-case-' prefix)
- Reset serializers.py to main + slug field only (removes accidental
revert of PR #42's JawafEntitySerializer unification)
- Remove '/state' from BLOCKED_PATH_PREFIXES (not part of this PR) - Restore state field in CasePatchSerializer (was accidentally dropped) - Keep only the slug field additions
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cases/api_views.py (1)
334-348:⚠️ Potential issue | 🟡 MinorException type mismatch in error handling.
Line 347 catches
Case.DoesNotExist, butget_object()raisesHttp404when no case is found (lines 173, 181). Theexcept Case.DoesNotExistblock will never execute sinceget_object()doesn't raise that exception.🔧 Remove dead code
- try: - case = self.get_object() - except Case.DoesNotExist: - return Response({"detail": "Not found."}, status=status.HTTP_404_NOT_FOUND) + case = self.get_object() # Raises Http404 if not foundThe
Http404exception is already handled by DRF's default exception handler, which returns a 404 response.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cases/api_views.py` around lines 334 - 348, The except block in partial_update is catching Case.DoesNotExist but get_object() raises Http404 (not Case.DoesNotExist), making that handler dead code; remove the unnecessary try/except entirely or change it to catch Http404 if you intend to handle it here—locate the partial_update method and the get_object() call and either delete the except Case.DoesNotExist branch or replace it with except Http404: and return the appropriate Response, ensuring you reference get_object(), partial_update, and Case.DoesNotExist when editing.
🧹 Nitpick comments (3)
cases/admin.py (1)
467-478: Consider making the external domain configurable.The hardcoded
https://jawafdehi.org/case/URL works but could become stale if the domain changes. Consider using Django settings for the base URL.💡 Optional: Use settings for external URL
# In settings.py JAWAFDEHI_PUBLIC_URL = "https://jawafdehi.org" # In admin.py from django.conf import settings def slug_link(self, obj): if not obj.slug: return "-" base_url = getattr(settings, 'JAWAFDEHI_PUBLIC_URL', 'https://jawafdehi.org') return format_html( '<a href="{}" target="_blank" rel="noopener noreferrer">{}</a>', f"{base_url}/case/{obj.slug}", obj.slug, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cases/admin.py` around lines 467 - 478, The slug_link method in admin (slug_link) uses a hardcoded base URL; make the external domain configurable by importing django.conf.settings and reading a setting (e.g., JAWAFDEHI_PUBLIC_URL) with a sensible default, then build the link using that base plus "/case/{obj.slug}" and keep the existing format_html output and attributes; update slug_link to use getattr(settings, 'JAWAFDEHI_PUBLIC_URL', 'https://jawafdehi.org') and ensure you add the settings constant in settings.py if desired.cases/serializers.py (1)
60-83: Incorrect OpenAPI schema type and duplicate query.
- Same schema issue as
get_alleged_cases- should be array of integers.- The
alleged_case_idsquery (lines 71-75) duplicates the logic fromget_alleged_cases. Consider caching or restructuring to avoid the redundant database query.🔧 Proposed fixes
Fix schema type:
- `@extend_schema_field`(OpenApiTypes.OBJECT) + `@extend_schema_field`({"type": "array", "items": {"type": "integer"}}) def get_related_cases(self, obj):Optional optimization to avoid duplicate query:
def get_alleged_cases(self, obj): from .models import RelationshipType cases = Case.objects.filter( entity_relationships__entity=obj, entity_relationships__relationship_type=RelationshipType.ACCUSED, state=CaseState.PUBLISHED, ) # Cache for use by get_related_cases self._alleged_case_ids = set(cases.values_list("id", flat=True)) return list(self._alleged_case_ids) def get_related_cases(self, obj): from .models import RelationshipType alleged_ids = getattr(self, '_alleged_case_ids', None) if alleged_ids is None: # Fallback if get_alleged_cases wasn't called first self.get_alleged_cases(obj) alleged_ids = self._alleged_case_ids related_cases = Case.objects.filter( entity_relationships__entity=obj, entity_relationships__relationship_type=RelationshipType.RELATED, state=CaseState.PUBLISHED, ).exclude(id__in=alleged_ids) return list(related_cases.values_list("id", flat=True))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cases/serializers.py` around lines 60 - 83, The OpenAPI schema for get_related_cases is incorrect (should be an array of integers) and the alleged_case_ids query duplicates get_alleged_cases; change the extend_schema_field on get_related_cases to an array-of-integers type (matching the fix for get_alleged_cases) and eliminate the duplicate DB query by caching results from get_alleged_cases: have get_alleged_cases store its IDs on self (e.g. self._alleged_case_ids) and in get_related_cases reuse that cached set, falling back to calling get_alleged_cases(obj) to populate the cache if missing, then filter RELATED cases excluding those IDs.cases/models.py (1)
571-578: Redundant validation with potentially misleading error message.The
RegexValidatoron the slug field already enforces^(?!\d)(cannot start with digit). If a slug passes field-level validation,re.match(r"^\d", self.slug)will never match. Similarly, a purely numeric slug would fail the regex validator before reaching this check.This validation is defensive (catches edge cases if field validation is bypassed), but the combined condition with
isnumeric()and "cannot start with a digit" in one error message is confusing—a numeric slug like "123" fails because it starts with a digit, not because it's "purely numeric."💡 Simplify or clarify the validation
If keeping the defensive check, consider clarifying:
if self.state == CaseState.PUBLISHED: if not self.slug: errors["slug"] = "Slug is required before publishing a case" - elif self.slug.isnumeric() or re.match(r"^\d", self.slug): - errors["slug"] = ( - "Slug cannot be purely numeric and cannot start with a digit" - ) + elif re.match(r"^\d", self.slug): + errors["slug"] = "Slug cannot start with a digit"The
isnumeric()check is redundant since any numeric string starts with a digit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cases/models.py` around lines 571 - 578, The slug validation in the model's clean/save block is redundant and confusing: remove the re.match(r"^\d", self.slug) check because the field-level RegexValidator already blocks slugs that start with a digit (and a purely numeric slug necessarily starts with a digit), or alternatively keep only a single defensive check and make the error message precise; update the code around CaseState.PUBLISHED and the slug checks so you either (A) only test for not self.slug and then rely on the field validator, or (B) keep a single defensive condition (e.g., if self.slug.isnumeric(): errors["slug"] = "Slug cannot be purely numeric") and drop the start-with-digit branch; reference the CaseState enum and the model's slug field/RegexValidator when making this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cases/api_views.py`:
- Around line 408-438: Wrap the multi-step update logic in a DB transaction to
ensure atomicity: start a transaction.atomic() covering the scalar field update
(Case.objects.filter(pk=pk).update(**scalar_updates)), the subsequent
refresh_from_db/relationship deletions, and all
CaseEntityRelationship.objects.create(...) calls so either all changes commit or
none do; optionally use Case.objects.select_for_update() to lock the row before
updating and then refresh_from_db() inside the same transaction to avoid race
conditions.
In `@cases/caseworker_serializers.py`:
- Around line 129-131: The serializer CasePatchSerializer declares a slug field
but partial_update's scalar_fields frozenset (in function/method partial_update)
does not include "slug", so PATCHed slug is validated then discarded; fix by
adding "slug" to the scalar_fields frozenset in partial_update so the patched
slug is persisted (alternatively, if slug should not be patchable, remove slug
from CasePatchSerializer to keep behavior and docs consistent).
In `@cases/serializers.py`:
- Around line 42-58: The decorator on get_alleged_cases incorrectly marks the
return as OBJECT; update the `@extend_schema_field` to declare a list of integers
(an array schema with integer items) so the OpenAPI docs show a list of case IDs
— e.g., replace OpenApiTypes.OBJECT with an array-of-integers schema using
drf-spectacular’s ARRAY with an integer child (or the library’s helper for
List[int]) in the decorator above get_alleged_cases to reflect list of ints.
---
Outside diff comments:
In `@cases/api_views.py`:
- Around line 334-348: The except block in partial_update is catching
Case.DoesNotExist but get_object() raises Http404 (not Case.DoesNotExist),
making that handler dead code; remove the unnecessary try/except entirely or
change it to catch Http404 if you intend to handle it here—locate the
partial_update method and the get_object() call and either delete the except
Case.DoesNotExist branch or replace it with except Http404: and return the
appropriate Response, ensuring you reference get_object(), partial_update, and
Case.DoesNotExist when editing.
---
Nitpick comments:
In `@cases/admin.py`:
- Around line 467-478: The slug_link method in admin (slug_link) uses a
hardcoded base URL; make the external domain configurable by importing
django.conf.settings and reading a setting (e.g., JAWAFDEHI_PUBLIC_URL) with a
sensible default, then build the link using that base plus "/case/{obj.slug}"
and keep the existing format_html output and attributes; update slug_link to use
getattr(settings, 'JAWAFDEHI_PUBLIC_URL', 'https://jawafdehi.org') and ensure
you add the settings constant in settings.py if desired.
In `@cases/models.py`:
- Around line 571-578: The slug validation in the model's clean/save block is
redundant and confusing: remove the re.match(r"^\d", self.slug) check because
the field-level RegexValidator already blocks slugs that start with a digit (and
a purely numeric slug necessarily starts with a digit), or alternatively keep
only a single defensive check and make the error message precise; update the
code around CaseState.PUBLISHED and the slug checks so you either (A) only test
for not self.slug and then rely on the field validator, or (B) keep a single
defensive condition (e.g., if self.slug.isnumeric(): errors["slug"] = "Slug
cannot be purely numeric") and drop the start-with-digit branch; reference the
CaseState enum and the model's slug field/RegexValidator when making this
change.
In `@cases/serializers.py`:
- Around line 60-83: The OpenAPI schema for get_related_cases is incorrect
(should be an array of integers) and the alleged_case_ids query duplicates
get_alleged_cases; change the extend_schema_field on get_related_cases to an
array-of-integers type (matching the fix for get_alleged_cases) and eliminate
the duplicate DB query by caching results from get_alleged_cases: have
get_alleged_cases store its IDs on self (e.g. self._alleged_case_ids) and in
get_related_cases reuse that cached set, falling back to calling
get_alleged_cases(obj) to populate the cache if missing, then filter RELATED
cases excluding those IDs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a206bb1f-e7ab-4017-8126-718b5e86e821
📒 Files selected for processing (7)
cases/admin.pycases/api_views.pycases/caseworker_serializers.pycases/migrations/0017_feedback_attachment.pycases/migrations/0018_case_slug.pycases/models.pycases/serializers.py
cases/api_views.py
Outdated
Comment on lines
+408
to
+438
| # Persist scalar field changes | ||
| scalar_updates = { | ||
| field: validated[field] for field in scalar_fields if field in validated | ||
| } | ||
| if scalar_updates: | ||
| Case.objects.filter(pk=pk).update(**scalar_updates) | ||
|
|
||
| # Persist entity relationship changes | ||
| case.refresh_from_db() | ||
| if "alleged_entity_ids" in validated: | ||
| case.entity_relationships.filter( | ||
| relationship_type=RelationshipType.ACCUSED | ||
| ).delete() | ||
| for entity_id in validated["alleged_entity_ids"]: | ||
| CaseEntityRelationship.objects.create( | ||
| case=case, | ||
| entity_id=entity_id, | ||
| relationship_type=RelationshipType.ACCUSED, | ||
| ) | ||
| if "related_entity_ids" in validated: | ||
| case.entity_relationships.filter( | ||
| relationship_type=RelationshipType.RELATED | ||
| ).delete() | ||
| for entity_id in validated["related_entity_ids"]: | ||
| CaseEntityRelationship.objects.create( | ||
| case=case, | ||
| entity_id=entity_id, | ||
| relationship_type=RelationshipType.RELATED, | ||
| ) | ||
|
|
||
| case.refresh_from_db() |
Contributor
There was a problem hiding this comment.
Missing transaction safety for multi-step updates.
The removal of transaction.atomic() means that if scalar field updates succeed but entity relationship updates fail (e.g., database error during CaseEntityRelationship.objects.create()), the case will be left in a partially updated state.
Consider wrapping the update operations in a transaction to ensure atomicity.
🔧 Proposed fix
+ with transaction.atomic():
# Persist scalar field changes
scalar_updates = {
field: validated[field] for field in scalar_fields if field in validated
}
if scalar_updates:
Case.objects.filter(pk=pk).update(**scalar_updates)
# Persist entity relationship changes
case.refresh_from_db()
if "alleged_entity_ids" in validated:
case.entity_relationships.filter(
relationship_type=RelationshipType.ACCUSED
).delete()
for entity_id in validated["alleged_entity_ids"]:
CaseEntityRelationship.objects.create(
case=case,
entity_id=entity_id,
relationship_type=RelationshipType.ACCUSED,
)
if "related_entity_ids" in validated:
case.entity_relationships.filter(
relationship_type=RelationshipType.RELATED
).delete()
for entity_id in validated["related_entity_ids"]:
CaseEntityRelationship.objects.create(
case=case,
entity_id=entity_id,
relationship_type=RelationshipType.RELATED,
)
- case.refresh_from_db()
+ case.refresh_from_db()
return Response(CaseSerializer(case).data, status=status.HTTP_200_OK)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # Persist scalar field changes | |
| scalar_updates = { | |
| field: validated[field] for field in scalar_fields if field in validated | |
| } | |
| if scalar_updates: | |
| Case.objects.filter(pk=pk).update(**scalar_updates) | |
| # Persist entity relationship changes | |
| case.refresh_from_db() | |
| if "alleged_entity_ids" in validated: | |
| case.entity_relationships.filter( | |
| relationship_type=RelationshipType.ACCUSED | |
| ).delete() | |
| for entity_id in validated["alleged_entity_ids"]: | |
| CaseEntityRelationship.objects.create( | |
| case=case, | |
| entity_id=entity_id, | |
| relationship_type=RelationshipType.ACCUSED, | |
| ) | |
| if "related_entity_ids" in validated: | |
| case.entity_relationships.filter( | |
| relationship_type=RelationshipType.RELATED | |
| ).delete() | |
| for entity_id in validated["related_entity_ids"]: | |
| CaseEntityRelationship.objects.create( | |
| case=case, | |
| entity_id=entity_id, | |
| relationship_type=RelationshipType.RELATED, | |
| ) | |
| case.refresh_from_db() | |
| with transaction.atomic(): | |
| # Persist scalar field changes | |
| scalar_updates = { | |
| field: validated[field] for field in scalar_fields if field in validated | |
| } | |
| if scalar_updates: | |
| Case.objects.filter(pk=pk).update(**scalar_updates) | |
| # Persist entity relationship changes | |
| case.refresh_from_db() | |
| if "alleged_entity_ids" in validated: | |
| case.entity_relationships.filter( | |
| relationship_type=RelationshipType.ACCUSED | |
| ).delete() | |
| for entity_id in validated["alleged_entity_ids"]: | |
| CaseEntityRelationship.objects.create( | |
| case=case, | |
| entity_id=entity_id, | |
| relationship_type=RelationshipType.ACCUSED, | |
| ) | |
| if "related_entity_ids" in validated: | |
| case.entity_relationships.filter( | |
| relationship_type=RelationshipType.RELATED | |
| ).delete() | |
| for entity_id in validated["related_entity_ids"]: | |
| CaseEntityRelationship.objects.create( | |
| case=case, | |
| entity_id=entity_id, | |
| relationship_type=RelationshipType.RELATED, | |
| ) | |
| case.refresh_from_db() |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cases/api_views.py` around lines 408 - 438, Wrap the multi-step update logic
in a DB transaction to ensure atomicity: start a transaction.atomic() covering
the scalar field update (Case.objects.filter(pk=pk).update(**scalar_updates)),
the subsequent refresh_from_db/relationship deletions, and all
CaseEntityRelationship.objects.create(...) calls so either all changes commit or
none do; optionally use Case.objects.select_for_update() to lock the row before
updating and then refresh_from_db() inside the same transaction to avoid race
conditions.
cases/caseworker_serializers.py
Outdated
Comment on lines
+129
to
+131
| class CasePatchSerializer(CaseEntityValidationMixin, serializers.Serializer): | ||
| state = serializers.ChoiceField(choices=CaseState.choices, required=False) | ||
| title = serializers.CharField(max_length=200) | ||
| slug = serializers.CharField(max_length=50, required=False, allow_blank=True) |
Contributor
There was a problem hiding this comment.
Slug field accepted by serializer but not persisted in PATCH.
The CasePatchSerializer accepts slug, but in cases/api_views.py the partial_update method's scalar_fields frozenset (lines 392-406) does not include "slug". This means the slug value will be validated but silently discarded on PATCH requests.
Either add "slug" to scalar_fields in partial_update, or remove it from CasePatchSerializer to avoid misleading API consumers.
🔧 Proposed fix in api_views.py
scalar_fields = frozenset(
[
"title",
+ "slug",
"short_description",
"description",
"thumbnail_url",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cases/caseworker_serializers.py` around lines 129 - 131, The serializer
CasePatchSerializer declares a slug field but partial_update's scalar_fields
frozenset (in function/method partial_update) does not include "slug", so
PATCHed slug is validated then discarded; fix by adding "slug" to the
scalar_fields frozenset in partial_update so the patched slug is persisted
(alternatively, if slug should not be patchable, remove slug from
CasePatchSerializer to keep behavior and docs consistent).
cases/serializers.py
Outdated
Comment on lines
+42
to
+58
| @extend_schema_field(OpenApiTypes.OBJECT) | ||
| def get_alleged_cases(self, obj): | ||
| """ | ||
| Get list of case IDs where this entity is alleged. | ||
|
|
||
| Only includes PUBLISHED cases. Uses the unified relationship system. | ||
| """ | ||
| from .models import RelationshipType | ||
|
|
||
| # Get cases where this entity has an 'alleged' relationship | ||
| cases = Case.objects.filter( | ||
| entity_relationships__entity=obj, | ||
| entity_relationships__relationship_type=RelationshipType.ACCUSED, | ||
| state=CaseState.PUBLISHED, | ||
| ) | ||
| ) | ||
|
|
||
| return list(cases.values_list("id", flat=True)) |
Contributor
There was a problem hiding this comment.
Incorrect OpenAPI schema type for list of integers.
@extend_schema_field(OpenApiTypes.OBJECT) is incorrect for a method returning a list of integers. This will generate misleading API documentation.
🔧 Proposed fix
- `@extend_schema_field`(OpenApiTypes.OBJECT)
+ `@extend_schema_field`({"type": "array", "items": {"type": "integer"}})
def get_alleged_cases(self, obj):📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @extend_schema_field(OpenApiTypes.OBJECT) | |
| def get_alleged_cases(self, obj): | |
| """ | |
| Get list of case IDs where this entity is alleged. | |
| Only includes PUBLISHED cases. Uses the unified relationship system. | |
| """ | |
| from .models import RelationshipType | |
| # Get cases where this entity has an 'alleged' relationship | |
| cases = Case.objects.filter( | |
| entity_relationships__entity=obj, | |
| entity_relationships__relationship_type=RelationshipType.ACCUSED, | |
| state=CaseState.PUBLISHED, | |
| ) | |
| ) | |
| return list(cases.values_list("id", flat=True)) | |
| `@extend_schema_field`({"type": "array", "items": {"type": "integer"}}) | |
| def get_alleged_cases(self, obj): | |
| """ | |
| Get list of case IDs where this entity is alleged. | |
| Only includes PUBLISHED cases. Uses the unified relationship system. | |
| """ | |
| from .models import RelationshipType | |
| # Get cases where this entity has an 'alleged' relationship | |
| cases = Case.objects.filter( | |
| entity_relationships__entity=obj, | |
| entity_relationships__relationship_type=RelationshipType.ACCUSED, | |
| state=CaseState.PUBLISHED, | |
| ) | |
| return list(cases.values_list("id", flat=True)) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cases/serializers.py` around lines 42 - 58, The decorator on
get_alleged_cases incorrectly marks the return as OBJECT; update the
`@extend_schema_field` to declare a list of integers (an array schema with integer
items) so the OpenAPI docs show a list of case IDs — e.g., replace
OpenApiTypes.OBJECT with an array-of-integers schema using drf-spectacular’s
ARRAY with an integer child (or the library’s helper for List[int]) in the
decorator above get_alleged_cases to reflect list of ints.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
cases/admin.py (1)
467-478: Consider making the external URL domain configurable.The external link URL
https://jawafdehi.org/case/{slug}is hardcoded. If the application is deployed to different environments (staging, development), the link will always point to production.💡 Suggested improvement
+from django.conf import settings + +# Add to settings.py: JAWAFDEHI_PUBLIC_URL = "https://jawafdehi.org" + def case_link(self, obj): """Display slug as an external link to the public case details page.""" if not obj.slug: return "-" + base_url = getattr(settings, 'JAWAFDEHI_PUBLIC_URL', 'https://jawafdehi.org') return format_html( '<a href="{}" target="_blank" rel="noopener noreferrer">{}</a>', - f"https://jawafdehi.org/case/{obj.slug}", + f"{base_url}/case/{obj.slug}", obj.slug, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cases/admin.py` around lines 467 - 478, The case_link helper currently hardcodes "https://jawafdehi.org/case/{slug}", which breaks non-production deployments; update the case_link method to read a configurable base URL (e.g., FRONTEND_BASE_URL or SITE_BASE_URL) from Django settings (import django.conf.settings) and build the external link using that base plus the obj.slug (ensure you handle trailing slashes consistently), then pass the constructed URL into format_html as before; update any related tests or docs to expect the configurable behavior.cases/models.py (1)
571-578: Redundant digit-start check in validate().Line 575 checks
re.match(r"^\d", self.slug)to reject slugs starting with a digit, but the field-levelRegexValidatoron line 426 already enforces this with the negative lookahead(?!\d). Theisnumeric()check is valid since purely numeric slugs would conflict with numeric PK lookups in the API, but there.matchportion is redundant.♻️ Suggested simplification
if self.state == CaseState.PUBLISHED: if not self.slug: errors["slug"] = "Slug is required before publishing a case" - elif self.slug.isnumeric() or re.match(r"^\d", self.slug): - errors["slug"] = ( - "Slug cannot be purely numeric and cannot start with a digit" - ) + elif self.slug.isnumeric(): + errors["slug"] = "Slug cannot be purely numeric"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cases/models.py` around lines 571 - 578, In validate() for Case model, remove the redundant re.match(r"^\d", self.slug) check because the slug field already has a RegexValidator enforcing the negative lookahead (?!\d); keep the self.slug.isnumeric() check to prevent purely numeric slugs and ensure errors["slug"] is set to the existing message ("Slug cannot be purely numeric and cannot start with a digit") or split into a clearer message if you prefer, while referencing CaseState.PUBLISHED, self.slug, and the slug field's RegexValidator so reviewers can find the related validation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cases/models.py`:
- Around line 571-578: PATCH requests currently ignore slug because the
scalar_fields frozenset used by the PATCH endpoint does not include "slug", and
CasePatchSerializer allows blank slugs without enforcing the model's
constraints; to fix, add "slug" to the scalar_fields set used by the PATCH bulk
UPDATE so submitted slugs are persisted, and add a validate_slug(self, value)
method to CasePatchSerializer that enforces the same rules as Case.clean()
(non-empty when publishing, not purely numeric, and not starting with a digit
via the same regex) or alternatively call instance.full_clean() before saving
the patched instance to ensure model-level validation is applied; update
references: scalar_fields, CasePatchSerializer, clean(), and slug.
---
Nitpick comments:
In `@cases/admin.py`:
- Around line 467-478: The case_link helper currently hardcodes
"https://jawafdehi.org/case/{slug}", which breaks non-production deployments;
update the case_link method to read a configurable base URL (e.g.,
FRONTEND_BASE_URL or SITE_BASE_URL) from Django settings (import
django.conf.settings) and build the external link using that base plus the
obj.slug (ensure you handle trailing slashes consistently), then pass the
constructed URL into format_html as before; update any related tests or docs to
expect the configurable behavior.
In `@cases/models.py`:
- Around line 571-578: In validate() for Case model, remove the redundant
re.match(r"^\d", self.slug) check because the slug field already has a
RegexValidator enforcing the negative lookahead (?!\d); keep the
self.slug.isnumeric() check to prevent purely numeric slugs and ensure
errors["slug"] is set to the existing message ("Slug cannot be purely numeric
and cannot start with a digit") or split into a clearer message if you prefer,
while referencing CaseState.PUBLISHED, self.slug, and the slug field's
RegexValidator so reviewers can find the related validation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cc876a0e-e0d9-4d10-a043-2caf7bbd1025
📒 Files selected for processing (6)
cases/admin.pycases/api_views.pycases/caseworker_serializers.pycases/migrations/0018_case_slug.pycases/models.pycases/serializers.py
🚧 Files skipped from review as they are similar to previous changes (2)
- cases/api_views.py
- cases/caseworker_serializers.py
Comment on lines
+571
to
+578
| # Slug is mandatory before publishing. | ||
| if self.state == CaseState.PUBLISHED: | ||
| if not self.slug: | ||
| errors["slug"] = "Slug is required before publishing a case" | ||
| elif self.slug.isnumeric() or re.match(r"^\d", self.slug): | ||
| errors["slug"] = ( | ||
| "Slug cannot be purely numeric and cannot start with a digit" | ||
| ) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify if slug is in scalar_fields for the PATCH endpoint
echo "=== Checking scalar_fields in api_views.py ==="
rg -n -A 20 'scalar_fields\s*=\s*frozenset' cases/api_views.py
echo ""
echo "=== Checking CasePatchSerializer slug field definition ==="
rg -n -B 2 -A 5 'class CasePatchSerializer' cases/caseworker_serializers.py
echo ""
echo "=== Checking for validate_slug method in CasePatchSerializer ==="
rg -n 'def validate_slug' cases/caseworker_serializers.pyRepository: Jawafdehi/JawafdehiAPI
Length of output: 1388
🏁 Script executed:
#!/bin/bash
# Check CaseEntityValidationMixin for slug validation
echo "=== Searching for CaseEntityValidationMixin ==="
rg -n 'class CaseEntityValidationMixin' cases/
echo ""
echo "=== Checking for validate_slug in entire codebase ==="
rg -n 'def validate_slug' cases/
echo ""
echo "=== Checking Case model's clean method for slug validation ==="
rg -n -A 30 'def clean' cases/models.py | head -50
echo ""
echo "=== Full content of CaseEntityValidationMixin ==="
rg -n -B 2 -A 30 'class CaseEntityValidationMixin' cases/Repository: Jawafdehi/JawafdehiAPI
Length of output: 4274
🏁 Script executed:
#!/bin/bash
# Check the full PATCH endpoint implementation to see if clean() is called
echo "=== Full PATCH endpoint implementation ==="
rg -n -A 50 'def patch' cases/api_views.py | grep -A 50 'def patch'
echo ""
echo "=== Check if there's any slug handling after scalar_fields ==="
rg -n -A 100 'scalar_updates = {' cases/api_views.py | head -80
echo ""
echo "=== Verify the exact Case model clean method around lines 571-578 ==="
sed -n '560,585p' cases/models.pyRepository: Jawafdehi/JawafdehiAPI
Length of output: 1348
🏁 Script executed:
#!/bin/bash
# Check the full PATCH endpoint implementation more carefully
echo "=== Full PATCH endpoint (lines 410-500) ==="
sed -n '410,500p' cases/api_views.py
echo ""
echo "=== Check if full_clean is called in the PATCH endpoint ==="
rg -n 'full_clean\|\.clean\(' cases/api_views.pyRepository: Jawafdehi/JawafdehiAPI
Length of output: 4002
🏁 Script executed:
#!/bin/bash
# Check if CasePatchSerializer calls model clean or has any slug validation
echo "=== Check entire CasePatchSerializer class ==="
rg -n -A 50 'class CasePatchSerializer' cases/caseworker_serializers.py
echo ""
echo "=== Search for any slug validation in entire codebase ==="
rg -n 'slug' cases/caseworker_serializers.py | head -20
echo ""
echo "=== Verify CasePatchSerializer has no custom validation methods ==="
rg -n 'def validate' cases/caseworker_serializers.py | grep -A 2 -B 2 'CasePatchSerializer'Repository: Jawafdehi/JawafdehiAPI
Length of output: 1906
Slug updates via caseworker PATCH API are silently discarded.
The PATCH endpoint (lines 420-442) uses a scalar_fields frozenset to determine which fields are persisted via bulk UPDATE. The slug field is excluded from this set, so slug changes submitted via CasePatchSerializer will be validated by the serializer but never persisted to the database.
Additionally, CasePatchSerializer (line 130) defines slug with allow_blank=True but provides no validate_slug() method to enforce the regex constraints defined in the model's clean() method. Since the PATCH endpoint does not call full_clean() on the Case model, invalid slugs can pass validation and be accepted by the API without triggering the model-level constraints.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cases/models.py` around lines 571 - 578, PATCH requests currently ignore slug
because the scalar_fields frozenset used by the PATCH endpoint does not include
"slug", and CasePatchSerializer allows blank slugs without enforcing the model's
constraints; to fix, add "slug" to the scalar_fields set used by the PATCH bulk
UPDATE so submitted slugs are persisted, and add a validate_slug(self, value)
method to CasePatchSerializer that enforces the same rules as Case.clean()
(non-empty when publishing, not purely numeric, and not starting with a digit
via the same regex) or alternatively call instance.full_clean() before saving
the patched instance to ensure model-level validation is applied; update
references: scalar_fields, CasePatchSerializer, clean(), and slug.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Refactor of #41 — slug only. All
bigo,court_cases, andmissing_detailschanges have been removed.Closes #41 (supersedes it).
Changes
Model (
cases/models.py)slugCharField (max 50, unique, nullable, indexed) withRegexValidator: letters/numbers/-, cannot start with a digitNonewhen blank onsave()clean())API (
cases/api_views.py)CaseViewSet.get_object()resolves by numeric pk or slug — e.g. both/api/cases/42/and/api/cases/sunil-poudel-corruption/workSerializers (
cases/serializers.py,cases/caseworker_serializers.py)slugadded toCaseSerializer(public read output)slugadded toCaseCreateSerializerandCasePatchSerializer(caseworker write inputs)Admin (
cases/admin.py)jawafdehi.org/case/<slug>in the list viewMigration
0018_case_slug— adds theslugfield onlySummary by CodeRabbit
New Features
API Updates
Enhancement