Skip to content

Conversation

@tlambert03
Copy link
Owner

This refactoring improves the multiple nullable ForeignKey pattern for spectrum ownership with the following enhancements:

Centralization & DRY:

  • Add OWNER_FIELD_CONFIG and OWNER_FIELD_NAMES constants as single source of truth for owner field configuration
  • Derive SpectrumForm.lookup from OWNER_FIELD_CONFIG to eliminate duplication
  • Add get_owner_field_name() and get_category_for_owner_field() helper methods

Database Integrity:

  • Add CheckConstraint to enforce "exactly one owner" at database level
  • Auto-populate category field from owner type in save() method
  • Better validation error messages

Code Quality:

  • Refactor sluglist() to use dynamic field iteration with next()
  • Update filter_owner() and find_similar_owners() to use constants
  • Add type hints to properties (owner_set, owner)
  • Change owner property to @cached_property for performance
  • Improve docstrings and add inline documentation

Documentation:

  • Document rationale for multiple nullable ForeignKeys vs GenericForeignKey pattern
  • Add comments explaining why category field is kept despite being derivable
  • Reference Luke Plant's blog post for future maintainers

No breaking changes - full backward compatibility maintained.

claude and others added 2 commits November 10, 2025 14:08
This refactoring improves the multiple nullable ForeignKey pattern for
spectrum ownership with the following enhancements:

**Centralization & DRY:**
- Add OWNER_FIELD_CONFIG and OWNER_FIELD_NAMES constants as single
  source of truth for owner field configuration
- Derive SpectrumForm.lookup from OWNER_FIELD_CONFIG to eliminate
  duplication
- Add get_owner_field_name() and get_category_for_owner_field()
  helper methods

**Database Integrity:**
- Add CheckConstraint to enforce "exactly one owner" at database level
- Auto-populate category field from owner type in save() method
- Better validation error messages

**Code Quality:**
- Refactor sluglist() to use dynamic field iteration with next()
- Update filter_owner() and find_similar_owners() to use constants
- Add type hints to properties (owner_set, owner)
- Change owner property to @cached_property for performance
- Improve docstrings and add inline documentation

**Documentation:**
- Document rationale for multiple nullable ForeignKeys vs
  GenericForeignKey pattern
- Add comments explaining why category field is kept despite being
  derivable
- Reference Luke Plant's blog post for future maintainers

No breaking changes - full backward compatibility maintained.
@tlambert03 tlambert03 marked this pull request as draft November 14, 2025 15:20
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.

3 participants