Skip to content

Count the redirection forms only for current year#653

Merged
danniel merged 3 commits intomainfrom
bugfix/archive-form-totals
Feb 17, 2026
Merged

Count the redirection forms only for current year#653
danniel merged 3 commits intomainfrom
bugfix/archive-form-totals

Conversation

@danniel
Copy link
Collaborator

@danniel danniel commented Feb 16, 2026

Fixes #650

Summary by CodeRabbit

  • Bug Fixes

    • NGO account redirection metrics now count only donations from January 1 of the current year onward, ensuring year-to-date redirection statistics are accurate.
  • Chores

    • Added a reliable start-of-year calculation to support consistent year-to-date reporting.

@coderabbitai
Copy link

coderabbitai bot commented Feb 16, 2026

No actionable comments were generated in the recent review. 🎉


📝 Walkthrough

Walkthrough

This PR adds a start-of-year helper and uses it to restrict redirections/donor counts to donors created on or after January 1 of the current year in both the model and the view.

Changes

Cohort / File(s) Summary
Calendar utility
backend/editions/calendar.py
Added january_first() returning a timezone-aware datetime for January 1st of the current year at 00:00:00.
Donor/redirections counting (model)
backend/donations/models/ngos.py
Changed Ngo.redirections_count to count only donors with date_created__gte=january_first() instead of all donors.
Donor/redirections counting (view)
backend/donations/views/ngo_account/redirections.py
Annotated redirections_count using Count("donor", filter=Q(date_created__gte=january_first())) (added Q import and january_first() usage).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 I nibble at dates and tidy the year,
January’s start now makes the counts clear,
Old forms tucked away, the new ones take stage,
Fresh numbers hop forward onto the page. 🥕📅

🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title clearly and specifically describes the main change: restricting redirection form counts to the current year, which directly addresses the bug in issue #650.
Linked Issues check ✅ Passed The changes correctly implement the primary requirement from issue #650 by filtering redirections_count to only include donors from the current year (January 1 onwards) in both the model and view layer.
Out of Scope Changes check ✅ Passed All changes are directly related to implementing the year-filtering functionality required by issue #650; no unrelated modifications are present.
Merge Conflict Detection ✅ Passed ✅ No merge conflicts detected when merging into main

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

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch bugfix/archive-form-totals

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

@greptile-apps
Copy link

greptile-apps bot commented Feb 16, 2026

Greptile Summary

This PR filters the redirection forms count to only include the current year by introducing a january_first() utility and applying it as a filter in both the Cause.redirections_count model property and the NgoRedirectionsView query annotation.

  • Bug in model property: Cause.redirections_count in ngos.py passes the datetime returned by january_first() directly to filter(), which expects a Q object or keyword argument — this will raise a TypeError at runtime. The view in redirections.py correctly uses Q(date_created__gte=january_first()).
  • The january_first() utility in editions/calendar.py works correctly but is missing a return type annotation, unlike other functions in the same module.
  • Note: The existing DonorCurrentYearManager uses date_created__year=timezone.now().year for year filtering, while this PR uses date_created__gte=january_first(). Both achieve similar results but with slightly different semantics (__year matches the exact year, __gte includes everything from Jan 1 onward).

Confidence Score: 1/5

  • This PR contains a runtime error in the model property that will break the redirections_count feature wherever the property is accessed directly.
  • The Cause.redirections_count property passes a bare datetime to QuerySet.filter() instead of wrapping it in a Q object with a field lookup, which will cause a TypeError. The view-level change is correct, but the model-level change is broken.
  • backend/donations/models/ngos.py has a critical bug on line 583 that needs to be fixed before merging.

Important Files Changed

Filename Overview
backend/donations/models/ngos.py Critical bug: redirections_count property passes a datetime to filter() instead of a Q object or keyword argument, which will cause a TypeError at runtime.
backend/donations/views/ngo_account/redirections.py Correctly filters donor count by current year using Q(date_created__gte=january_first()) within a Count annotation.
backend/editions/calendar.py Adds january_first() utility that returns a timezone-aware datetime for January 1st of the current year. Missing return type annotation unlike other functions in the file.

Flowchart

flowchart TD
    A["january_first()"] -->|"returns datetime"| B["Jan 1st of current year"]
    B --> C{"Used in view (redirections.py)"}
    B --> D{"Used in model (ngos.py)"}
    C -->|"Q(date_created__gte=datetime)"| E["Count annotation with filter ✅"]
    D -->|"filter(datetime) — no field lookup"| F["TypeError at runtime ❌"]
    E --> G["NgoRedirectionsView causes list"]
    F --> H["Cause.redirections_count property"]
Loading

Last reviewed commit: 3fa8417

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

3 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Copy link

@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.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
backend/donations/views/ngo_account/redirections.py (1)

8-29: ⚠️ Potential issue | 🟡 Minor

Fix import ordering to satisfy CI.
The pipeline reports the import block is unsorted. Reorder the Django ORM imports (and let the formatter handle the block).

♻️ Suggested order
-from django.db.models import Count, F, OuterRef, QuerySet, Subquery, Q
+from django.db.models import Count, F, OuterRef, Q, QuerySet, Subquery
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/donations/views/ngo_account/redirections.py` around lines 8 - 29, The
import block at the top is unsorted; reorder the Django-related imports so they
are grouped and alphabetized per the project's import style (e.g.,
django.db.models imports: Count, F, OuterRef, QuerySet, Subquery, Q should be
sorted alphabetically; django.db.models.functions JSONObject; django.http
imports: Http404, HttpRequest, HttpResponseNotAllowed, QueryDict;
django.shortcuts redirect; django.urls reverse, reverse_lazy;
django.utils.decorators method_decorator; django.utils.translation gettext_lazy
as _), then run the formatter/isort to ensure CI passes—adjust the import lines
that include Count, F, OuterRef, QuerySet, Subquery, Q, JSONObject, Http404,
HttpRequest, HttpResponseNotAllowed, QueryDict, redirect, reverse, reverse_lazy,
method_decorator, and gettext_lazy accordingly.
🤖 Fix all issues with AI agents
Verify each finding against the current code and only fix it if needed.


In `@backend/donations/models/ngos.py`:
- Around line 582-583: The redirections_count method is passing a raw datetime
as a positional arg to donor_set.filter (invalid); update redirections_count to
call donor_set.filter with a field lookup keyword on the donor model's datetime
field (for example use the created_at field with a date or range lookup such as
created_at__date=<january_first()> or created_at__gte=<january_first()>) and
then .count(); reference the redirections_count function, donor_set.filter call,
and january_first() helper when making the change.

In `@backend/donations/views/ngo_account/redirections.py`:
- Around line 8-29: The import block at the top is unsorted; reorder the
Django-related imports so they are grouped and alphabetized per the project's
import style (e.g., django.db.models imports: Count, F, OuterRef, QuerySet,
Subquery, Q should be sorted alphabetically; django.db.models.functions
JSONObject; django.http imports: Http404, HttpRequest, HttpResponseNotAllowed,
QueryDict; django.shortcuts redirect; django.urls reverse, reverse_lazy;
django.utils.decorators method_decorator; django.utils.translation gettext_lazy
as _), then run the formatter/isort to ensure CI passes—adjust the import lines
that include Count, F, OuterRef, QuerySet, Subquery, Q, JSONObject, Http404,
HttpRequest, HttpResponseNotAllowed, QueryDict, redirect, reverse, reverse_lazy,
method_decorator, and gettext_lazy accordingly.

@danniel danniel requested a review from tudoramariei February 16, 2026 21:26
@danniel danniel merged commit 886eaeb into main Feb 17, 2026
9 checks passed
@danniel danniel deleted the bugfix/archive-form-totals branch February 17, 2026 11:00
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.

Display error when seeing the number of forms included in the ANAF Archive

2 participants