Prepare models before new framework#572
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
This PR prepares the codebase for future database schema expansion by adding slug fields to Category and Skill models, improving the populate_db.py script with better validation and error handling, and adding comprehensive test coverage for the models module. The changes introduce two abstract base classes (NamedModel and SluggedModel) that provide shared functionality for name, description, and auto-generated slug fields.
Changes:
- Added abstract base classes
NamedModelandSluggedModelto provide shared fields and behavior - Added unique slug fields to Category and Skill models with auto-generation on save
- Refactored populate_db.py to use a centralized
add_object_to_dbfunction with validation and logging - Added comprehensive test coverage for all model classes and validation logic
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| main/models.py | Introduced abstract base classes (NamedModel, SluggedModel) and updated Category, Skill, and SkillLevel models to use them, adding slug fields with auto-generation |
| main/migrations/0011_category_slug_skill_slug_alter_skilllevel_level.py | Schema migration adding slug fields to Category and Skill models and making SkillLevel.level unique |
| scripts/populate_db.py | Refactored to use new add_object_to_db helper function with validation, error handling, and logging for robustness |
| tests/main/test_models.py | Added comprehensive test coverage for all models including validation logic, abstract base classes, and model relationships |
| tests/scripts/test_populate_db.py | Enhanced tests with validation error scenarios and added tests for the new add_object_to_db function |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
| if self.category_id is None: | ||
| raise ValidationError({"category": _("A skill must belong to a category.")}) |
There was a problem hiding this comment.
Wouldn't modify the category field above with null=False, blank=False do the same job?
There was a problem hiding this comment.
No, those are the default values, so it is already set. This is only required because of the below check, where I'm checking self.category.parent_category is not None, which will error if self.category is None. This happens in the Admin if you try to create a skill without selecting a category.
There was a problem hiding this comment.
It's odd that the Admin itself does not respect the requirement that a certain field must not be none/null. Ok, so be it...
There was a problem hiding this comment.
The admin does check it, but my custom clean method is run regardless, which is what causes the problem
Description
This PR updates the models and the populate_db script in preparation for expanding the database (#568). These changes include:
populate_db.pyslugfield to most of the existing models through the creation of two abstract parent classes:NamedModelandSluggedModelmodels.pyFixes #571
Type of change
Key checklist
python -m pytest)mkdocs serve)pre-commit run --all-files)Further checks