Skip to content

Conversation

@skools-here
Copy link

@skools-here skools-here commented Oct 22, 2025

fixes: #4567

  1. Updated custom_tags.py and page_stats.html to show same and actual page views

Summary by CodeRabbit

  • Bug Fixes
    • Page views now default to a broader dataset when no specific page is selected, improving the page views chart and total views.
    • Daily aggregation now uses inclusive calendar-day bounds, ensuring per-day and overall view totals are correctly calculated and reducing missing or misattributed counts.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 22, 2025

Walkthrough

Template now requests site-wide page view data by passing None to get_page_views; the tag was updated to filter by an inclusive calendar-day range (created__date__range) and to apply the path filter only when a URL path is provided, then aggregate daily views.

Changes

Cohort / File(s) Summary
Template page views call
website/templates/includes/page_stats.html
Changed tag invocation from {% get_page_views current_url_path 30 %} to {% get_page_views None 30 %} so the widget requests site-wide page view data.
Custom tag date & path filtering
website/templatetags/custom_tags.py
Reworked get_page_views to use local date bounds (start_day/end_day) and an inclusive created__date__range; build a base queryset for that range and apply path__icontains=url_path only when url_path is provided; aggregate with .values("created__date").annotate(total_views=Sum("count")).order_by("created__date") and initialize daily keys from date objects.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Template as page_stats.html
  participant Tag as get_page_views
  participant DB as IP table
  Note over Template,Tag: Widget requests site-wide views (url_path = None)
  Template->>Tag: get_page_views(None, days=30)
  Tag->>DB: queryset: filter(created__date__range = start_day..end_day)
  alt url_path provided
    Tag->>DB: add filter(path__icontains = url_path)
  end
  Tag->>DB: values(created__date) -> annotate(total_views=Sum(count)) -> order_by(created__date)
  DB-->>Tag: daily aggregated counts
  Tag-->>Template: return JSON view_counts_dict
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Pre-merge checks and finishing touches

✅ 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 "Fixed mismatch between page views in bottom left widget and homepage stats" is clear, concise, and directly summarizes the main objective of the changes. It accurately describes the primary issue being addressed—reconciling the difference between page view counts displayed in two separate UI components. The title is specific enough to convey the nature of the fix without being overly verbose or generic.
Linked Issues Check ✅ Passed The code changes directly address the primary objective of issue #4567 to resolve the page view count mismatch. The key modification—passing None instead of current_url_path to the get_page_views tag in page_stats.html—shifts the aggregation from per-URL-path to site-wide data collection, which aligns with the suggested fix to make both components use consistent, site-wide aggregation logic. The corresponding updates to custom_tags.py to conditionally filter by path (when provided) support this change by enabling site-wide aggregation when no path is specified. These modifications directly target the root cause of inconsistent queryset scoping identified in the issue.
Out of Scope Changes Check ✅ Passed All code changes in this pull request are directly related to fixing the page view mismatch issue. Modifications to page_stats.html and custom_tags.py are focused on aligning the widget aggregation logic to use site-wide data instead of per-path data, which is the core objective of issue #4567. No unrelated changes, refactorings, or feature additions appear to be included in the changeset. The scope remains narrowly focused on the files and logic necessary to reconcile the displayed page view counts.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ 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: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between bddd9c0 and e25c818.

📒 Files selected for processing (1)
  • website/templatetags/custom_tags.py (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • website/templatetags/custom_tags.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Run Tests

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.

Copy link
Contributor

@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

🧹 Nitpick comments (2)
website/templatetags/custom_tags.py (2)

103-109: Cache the site-wide aggregation to avoid a heavy query on every page render.

The widget appears on all pages; without caching this will re-aggregate frequently. Add a short TTL cache keyed by scope and days.

Example (outside this hunk):

from django.core.cache import cache

cache_key = f"pageviews:{(url_path or 'all')}:{days}:{end_day.isoformat()}"
cached = cache.get(cache_key)
if cached is not None:
    return cached
# ...build view_counts_dict...
json_data = json.dumps(view_counts_dict)
cache.set(cache_key, json_data, 300)  # 5 minutes
return json_data

112-115: Optional: use TruncDate for database-agnostic grouping.

This avoids backend-specific “__date” extraction quirks.

Example:

from django.db.models.functions import TruncDate

daily_views = (
    queryset
    .annotate(day=TruncDate("created"))
    .values("day")
    .annotate(total_views=models.Sum("count"))
    .order_by("day")
)
# and read entry["day"] downstream
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between 2d9030d and 2e0bffb.

📒 Files selected for processing (2)
  • website/templates/includes/page_stats.html (1 hunks)
  • website/templatetags/custom_tags.py (1 hunks)
🔇 Additional comments (1)
website/templates/includes/page_stats.html (1)

4-4: Manually verify homepage uses identical 30-day window. I didn’t find a separate homepage calculation; ensure the homepage template calls get_page_views(None, 30) (last 30 calendar days, inclusive).

@github-project-automation github-project-automation bot moved this from Backlog to Ready in 📌 OWASP BLT Project Board Oct 22, 2025
@skools-here
Copy link
Author

Hey @DonnieBLT could you please review this PR !

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

Labels

None yet

Projects

Status: Ready

Development

Successfully merging this pull request may close these issues.

Fix mismatch between page views in bottom left widget and homepage stats

1 participant