Skip to content

Resources: make cover_image field non-nullable#19

Merged
AdityaKhatri merged 1 commit intodevelopfrom
fix/make-image-field-required
Apr 9, 2026
Merged

Resources: make cover_image field non-nullable#19
AdityaKhatri merged 1 commit intodevelopfrom
fix/make-image-field-required

Conversation

@sudip-khanal
Copy link
Copy Markdown
Contributor

@sudip-khanal sudip-khanal commented Apr 3, 2026

Changes

  • Make Casestudy cover_image field non-nullable
  • Update the description of N/A enum

This PR doesn't introduce any:

  • temporary files, auto-generated files or secret keys
  • n+1 queries
  • flake8 issues
  • print
  • typos
  • unwanted comments

This PR contains valid:

  • tests
  • permission checks (tests here too)

Summary by CodeRabbit

  • Updates
    • Case study cover images are now required and must be provided.
    • Tool availability label text expanded for clearer user-facing descriptions.
    • Catalog management now includes descriptive guidance: after adding/updating a catalog, related questions are automatically populated to the tool.

@sudip-khanal sudip-khanal changed the title fix(resources): make cover_image field non-nullable Resources: make cover_image field non-nullable Apr 3, 2026
@@ -1,4 +1,4 @@
# Generated by Django 5.2.9 on 2026-03-17 08:23
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why is this migration renamed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Because there are two migration files with the same name.
image

@AdityaKhatri
Copy link
Copy Markdown
Contributor

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 8, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 8, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 733d5ff9-b9f1-45f2-b228-08ee9f70fc25

📥 Commits

Reviewing files that changed from the base of the PR and between 1f4988b and ebe51a8.

📒 Files selected for processing (4)
  • apps/resources/migrations/0004_alter_casestudy_cover_image_requestdemo.py
  • apps/resources/models.py
  • apps/tool_picker/migrations/0005_alter_tool_catalogs.py
  • apps/tool_picker/models.py
💤 Files with no reviewable changes (1)
  • apps/resources/models.py
🚧 Files skipped from review as they are similar to previous changes (2)
  • apps/tool_picker/models.py
  • apps/resources/migrations/0004_alter_casestudy_cover_image_requestdemo.py

📝 Walkthrough

Walkthrough

Migration and model updates make CaseStudy.cover_image non-nullable and set an explicit ImageField upload path/default; Tool.catalogs gains help_text and OrdinalTypeEnum.NOT_AVAILABLE label text expanded, with a new migration altering the Tool.catalogs field.

Changes

Cohort / File(s) Summary
Resources — CaseStudy cover image
apps/resources/migrations/0004_alter_casestudy_cover_image_requestdemo.py, apps/resources/models.py
Migration dependency updated; CaseStudy.cover_image altered to a models.ImageField with default='', upload_to='case_studies/', explicit verbose name and preserve_default=False. Model field removed null=True, blank=True, making it required.
Tool Picker — catalogs & enum
apps/tool_picker/models.py, apps/tool_picker/migrations/0005_alter_tool_catalogs.py
Added help_text to Tool.catalogs ManyToManyField and changed OrdinalTypeEnum.NOT_AVAILABLE label to a longer descriptive string; new migration updates the Tool.catalogs field with related_name='tool_catalogs' and the help text.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hopped through fields and fixed a name,
A cover image shed its nullable frame.
Catalogs now whisper what they do,
Enums speak clearly—hi from me to you! 🥕✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive The description is incomplete. It mentions two changes but only partially addresses the template. Missing are related issue/PR numbers, detailed explanations of why changes were made, and reasoning for not including tests. Add context about related issues (Addresses section), explain the rationale for the changes, and clarify why tests were not included in this PR.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: making the CaseStudy cover_image field non-nullable, which is the primary focus of the PR.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/make-image-field-required

Comment @coderabbitai help to get the list of available commands and usage tips.

@sudip-khanal sudip-khanal force-pushed the fix/make-image-field-required branch from 1f4988b to ebe51a8 Compare April 8, 2026 09:51
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
apps/tool_picker/models.py (1)

189-192: Replace the standalone string block with a real class docstring/comment.

Line 189 adds a no-op string expression in the class body; it won’t be part of OrdinalTypeEnum.__doc__ and can be flagged by linters as “string statement has no effect.”

Proposed cleanup
 class OrdinalTypeEnum(models.IntegerChoices):
-    """Enum representing scale of ordinal type question."""
-
-    """
-        Note: The enum labels are intentionally descriptive and user-friendly so they can be
-        used directly in the frontend UI and remain consistent with design requirements.
-    """
+    """Enum representing scale of ordinal type question.
+
+    Note: Enum labels are intentionally descriptive and user-friendly so they can be used
+    directly in the frontend UI and remain consistent with design requirements.
+    """
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/tool_picker/models.py` around lines 189 - 192, The descriptive text is
currently a standalone string expression inside the class body (causing a no-op
and linter warning); update OrdinalTypeEnum so the text becomes a real class
docstring by placing the triple-quoted string immediately after the class header
(so OrdinalTypeEnum.__doc__ is populated), or alternatively convert the text to
a comment (prefix with #) above the class; remove the existing stray string
expression to eliminate the no-op.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@apps/tool_picker/models.py`:
- Around line 189-192: The descriptive text is currently a standalone string
expression inside the class body (causing a no-op and linter warning); update
OrdinalTypeEnum so the text becomes a real class docstring by placing the
triple-quoted string immediately after the class header (so
OrdinalTypeEnum.__doc__ is populated), or alternatively convert the text to a
comment (prefix with #) above the class; remove the existing stray string
expression to eliminate the no-op.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 74efd0aa-f903-4f0b-a08f-e2d202ca3502

📥 Commits

Reviewing files that changed from the base of the PR and between 94652b5 and 1f4988b.

📒 Files selected for processing (3)
  • apps/resources/migrations/0004_alter_casestudy_cover_image_requestdemo.py
  • apps/resources/models.py
  • apps/tool_picker/models.py
💤 Files with no reviewable changes (1)
  • apps/resources/models.py

@AdityaKhatri AdityaKhatri merged commit af77cb2 into develop Apr 9, 2026
3 checks passed
@AdityaKhatri AdityaKhatri deleted the fix/make-image-field-required branch April 9, 2026 05:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants