Skip to content

Conversation

@AanshOjha
Copy link

Description

This PR addresses the task of extracting hardcoded numbers (magic numbers) to named constants at module level to improve code readability and maintainability across model mixins.

Changes Made

Files Modified

backend/apps/owasp/models/mixins/project.py

  • Added DEFAULT_HEALTH_SCORE = 100
  • Replaced hardcoded 100 with DEFAULT_HEALTH_SCORE in idx_health_score property

backend/apps/github/models/mixins/release.py

  • Added DESCRIPTION_MAX_LENGTH = 1000
  • Replaced hardcoded 1000 with DESCRIPTION_MAX_LENGTH in idx_description property

Implementation Notes

  • All constants follow Python naming convention (UPPER_CASE_WITH_UNDERSCORES)
  • Constants are placed at module level, immediately after imports and before class definitions
  • All hardcoded numeric values with business logic meaning have been extracted
  • Code functionality remains unchanged - this is a pure refactoring

Testing

  • Tests pass: make check-test
  • No breaking changes introduced

Related Issue

Closes #2649

- Extracted DEFAULT_HEALTH_SCORE = 100 in project.py
- Extracted DESCRIPTION_MAX_LENGTH = 1000 in release.py

Fixes OWASP#2649
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 1, 2025

Summary by CodeRabbit

  • Refactor
    • Updated internal constants for release description and health score handling.

✏️ Tip: You can customize this high-level summary in your review settings.

Walkthrough

Two files extract hardcoded magic numbers to named constants: DESCRIPTION_MAX_LENGTH = 1000 in release mixins and DEFAULT_HEALTH_SCORE = 100 in OWASP project mixins, with corresponding usages updated to reference the new constants.

Changes

Cohort / File(s) Change Summary
Magic number constant extraction
backend/apps/github/models/mixins/release.py, backend/apps/owasp/models/mixins/project.py
Added module-level constants: DESCRIPTION_MAX_LENGTH = 1000 and DEFAULT_HEALTH_SCORE = 100; replaced hardcoded values with constants in idx_description and idx_health_score methods respectively

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Verify that DEFAULT_HEALTH_SCORE = 100 accurately reflects the previous hardcoded behavior, particularly regarding the production environment conditional in idx_health_score
  • Confirm that the conditional logic in idx_health_score (using the constant when settings.IS_PRODUCTION_ENVIRONMENT is true) maintains intended behavior for both environments

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: extracting hardcoded numbers to named constants in model mixins, which is the primary focus of the changeset.
Description check ✅ Passed The description is well-detailed and directly related to the changeset, explaining the changes made, implementation notes, testing results, and linking to the related issue.
Linked Issues check ✅ Passed The PR successfully addresses all coding requirements from issue #2649: extracted magic numbers (100 and 1000) to named constants (DEFAULT_HEALTH_SCORE and DESCRIPTION_MAX_LENGTH) in the specified model mixin files with proper naming conventions.
Out of Scope Changes check ✅ Passed All changes are in-scope: only two constants were added and utilized in their respective files, with no unrelated modifications outside the stated objectives of extracting magic numbers.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 28ef47f and e37f0a6.

📒 Files selected for processing (2)
  • backend/apps/github/models/mixins/release.py (2 hunks)
  • backend/apps/owasp/models/mixins/project.py (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
backend/apps/github/models/mixins/release.py (1)
backend/apps/common/utils.py (1)
  • truncate (203-215)
backend/apps/owasp/models/mixins/project.py (1)
backend/apps/owasp/models/project.py (1)
  • health_score (139-141)
🔇 Additional comments (2)
backend/apps/github/models/mixins/release.py (1)

7-7: Magic number extraction for description length looks correct

DESCRIPTION_MAX_LENGTH is clearly named, placed with other module-level definitions, and used in idx_description without changing behavior. This keeps the truncation logic readable and centralized for future adjustment.

Also applies to: 40-42

backend/apps/owasp/models/mixins/project.py (1)

14-14: Health score default constant is well-factored and preserves behavior

Extracting DEFAULT_HEALTH_SCORE and using it in idx_health_score keeps the production placeholder value explicit and centralized, without changing the existing IS_PRODUCTION_ENVIRONMENT-gated behavior.

Also applies to: 41-44

Tip

✨ Issue Enrichment is now available for GitHub issues!

CodeRabbit can now help you manage issues more effectively:

  • Duplicate Detection — Identify similar or duplicate issues
  • Related Issues & PRs — Find relevant issues and PR's from your repository
  • Suggested Assignees — Find the best person to work on the issue
  • Implementation Planning — Generate detailed coding plans for engineers and agents
Disable automatic issue enrichment

To disable automatic issue enrichment, add the following to your .coderabbit.yaml:

issue_enrichment:
  auto_enrich:
    enabled: false

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.

❤️ Share

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

@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 1, 2025

@AanshOjha
Copy link
Author

FYI: Some flaky frontend e2e tests exist, unrelated to these changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Extract Magic Numbers to Named Constants

1 participant