Skip to content

Comment out broken Unique Visitors chart (#12823)#12831

Draft
Sadashii wants to merge 3 commits into
internetarchive:masterfrom
Sadashii:12823/fix/hide-unique-visitors
Draft

Comment out broken Unique Visitors chart (#12823)#12831
Sadashii wants to merge 3 commits into
internetarchive:masterfrom
Sadashii:12823/fix/hide-unique-visitors

Conversation

@Sadashii
Copy link
Copy Markdown
Collaborator

@Sadashii Sadashii commented Jun 1, 2026

Closes #12823

Dynamically hides the Unique Visitors stats charts on the homepage and the admin dashboard when visitor counts data is not available (such as when the upstream archive.org Graphite pipeline is down).

Technical

  • Wrapped the Unique Visitors chart and JSON embed script in openlibrary/templates/home/stats.html and on the admin stats page openlibrary/templates/admin/index.html with a $if stats["visitors"].get_summary(28): template conditional.
  • Added a clarifying comment note above the conditional blocks: $# Conditional check since the graphite source is currently unreliable, hiding the stat when it returns nothing.
  • Added a timeout=5 parameter to the Graphite request in openlibrary/core/admin.py to prevent hangs when the Graphite server is down.

Testing

Verify style checks:

pre-commit run --files openlibrary/core/admin.py openlibrary/templates/home/stats.html openlibrary/templates/admin/index.html

Screenshot

With valid response,
image

With missing/invalid response,
image

(Note: To be updated by the reviewer/submitter with a screenshot from local testing environment showing the homepage stats blocks cleanly rendered without the Unique Visitors chart).

Stakeholders

@mekarpeles

Copilot AI review requested due to automatic review settings June 1, 2026 11:31
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR appears to disable the “Unique Visitors” chart on the home stats page and stop fetching visitor counts from Graphite, with corresponding updates to the i18n message template.

Changes:

  • Commented out the “Unique Visitors” chart and its graph JSON payload in home/stats.html
  • Disabled Graphite-backed visitor count retrieval in openlibrary/core/admin.py (now returns an empty list)
  • Updated messages.pot to reflect removed/changed translatable strings and source references

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File Description
openlibrary/templates/home/stats.html Comments out the visitors chart block and its JSON graph data script
openlibrary/i18n/messages.pot Removes/adjusts message entries/source refs to match template/code changes
openlibrary/core/admin.py Replaces Graphite fetch implementation with a hardcoded empty result

Comment thread openlibrary/core/admin.py Outdated
Comment thread openlibrary/core/admin.py Outdated
Comment thread openlibrary/templates/home/stats.html Outdated
Comment thread openlibrary/templates/home/stats.html Outdated
Comment on lines 8526 to 8528
#: databarHistory.html databarView.html
#, python-format
msgid "Last edited by %(author_link)s"
@Sadashii Sadashii force-pushed the 12823/fix/hide-unique-visitors branch 3 times, most recently from 444fef9 to 2f4f439 Compare June 1, 2026 11:40
@mekarpeles
Copy link
Copy Markdown
Member

Thank you for the PR, @Sadashii!

A reviewer must first be assigned. There are currently 14 open PRs of equal or higher priority ahead of yours.

PR triage checklist (maintainers / Pam)
  • PR description — not empty; explains what the change does and how to verify it
  • References an issue — PR body contains a #NNN reference
    • Linked issue is triaged — has a Priority: * label (not just Needs: Triage)
    • Linked issue is assigned — has at least one assignee
  • Commit history clean — no WIP/fixup/conflict noise; commit messages are meaningful
  • CI passing — no failing check-runs
  • Test cases present — if the change touches substantive logic, test coverage exists or is explained
  • Proof of testing — PR body includes a description of what was tested, a screenshot, or a video

Note

This comment was automatically generated by Pam, Open Library's Project AI Manager, on behalf of @mekarpeles. Pam is designed to provide status visibility, perform basic project management functions and relevant codebase research, and provide actionable feedback so contributors aren't left waiting.

Comment thread openlibrary/core/admin.py Outdated
Comment thread openlibrary/templates/admin/index.html Outdated
@mekarpeles
Copy link
Copy Markdown
Member

This PR needs significant work and as is implemented I believe creates work for us. It partially un-implements a feature, while leaving some pieces intact and fully removing other parts, resulting in an incoherent solution that will not be accessible to others maintaining the project.

The code should simply be updated such that:

  • If the data is fetched and there is no data, don't render that chart.

Also, please kindly provide a screenshot in the PR to give reviewers confidence so we can put it on testing.

@mekarpeles mekarpeles marked this pull request as draft June 1, 2026 14:41
@mekarpeles mekarpeles added the Needs: Submitter Input Waiting on input from the creator of the issue/pr [managed] label Jun 1, 2026
@mekarpeles mekarpeles self-assigned this Jun 1, 2026
@Sadashii Sadashii force-pushed the 12823/fix/hide-unique-visitors branch from 2f4f439 to f9ed54a Compare June 1, 2026 15:05
@github-actions github-actions Bot removed the Needs: Submitter Input Waiting on input from the creator of the issue/pr [managed] label Jun 1, 2026
@Sadashii
Copy link
Copy Markdown
Collaborator Author

Sadashii commented Jun 1, 2026

Updated so when there is valid result the visual are shown, when the response is missing, the visual is also hidden.

@Sadashii Sadashii force-pushed the 12823/fix/hide-unique-visitors branch from f9ed54a to 0767328 Compare June 1, 2026 15:30
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.

Unique visitors broken on the site

3 participants