Resolve date alignment logic in project health metrics#3838
Resolve date alignment logic in project health metrics#3838saichethana28 wants to merge 2 commits intoOWASP:mainfrom
Conversation
Summary by CodeRabbit
WalkthroughReplaces ExtractMonth/TruncDate with TruncMonth for monthly aggregation, removes Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@backend/tests/apps/owasp/models/project_health_metrics_test.py`:
- Around line 182-186: The mock_now fixture is defined but never used; either
remove the unused fixture named mock_now or make the tests use it by adding
mock_now as a parameter to the tests that rely on current time (e.g.,
test_get_stats_monthly_grouping_logic and test_get_stats_empty_state) so
Django's timezone.now is patched to the fixed value when those tests run; locate
the fixture mock_now and the tests test_get_stats_monthly_grouping_logic and
test_get_stats_empty_state and either delete mock_now or add it to the tests'
parameter lists to ensure the filter that calls timezone.now() sees the frozen
time.
🧹 Nitpick comments (2)
backend/tests/apps/owasp/models/project_health_metrics_test.py (2)
119-151: Test is tightly coupled to ORM chain shape but functionally correct.The mock wiring mirrors the exact query-chain call order (
.annotate().filter().values().annotate().order_by()). This is fragile — any reordering inget_statswill silently break the mock and returnMockobjects instead of the expected data, potentially causing confusing test failures rather than clear assertion errors.Consider adding a defensive assertion (e.g., verifying
monthsentries are strings) or, if feasible, using an in-memory SQLite database withpytest-djangomarks for a more resilient integration-style test. That said, this is a common trade-off in Django unit tests, so it's fine to keep as-is if you prefer.
123-124: Mock month values are naive datetimes.Production code's
TruncMonthreturns timezone-aware datetimes. Usingtimezone.datetime(2024, 1, 1)here produces naive datetimes. This works becausestrftimedoesn't check awareness, but for consistency consider usingtimezone.make_aware(...)as done elsewhere in this file (e.g.,FIXED_DATEon line 14).
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
arkid15r
left a comment
There was a problem hiding this comment.
I verified that my code works as intended and resolves the issue as described
Could you attach a screenshot demonstrating fix is working?
|
Closing in favor of #3842 |



Proposed change
Resolves #3795
The project health metrics were previously miscalculating the time-series data for the "latest months," leading to inaccurate reporting of project activity over time. This PR updates the logic in the
ProjectHealthMetricsmodel to correctly identify and aggregate data points for the most recent months.Key changes:
project_health_metrics.pyto ensure consistent month-to-month alignment.Checklist
make check-testlocally: all warnings addressed, tests passed