Skip to content

Extend stats timeline to current day#3895

Merged
dgw merged 3 commits intoYOURLS:masterfrom
ntindicator:stats-time-range
Apr 24, 2025
Merged

Extend stats timeline to current day#3895
dgw merged 3 commits intoYOURLS:masterfrom
ntindicator:stats-time-range

Conversation

@ntindicator
Copy link
Copy Markdown
Contributor

Fixes issue #3695 - Traffic statistics is incorrect in case of no recent activity. The stats timeline stopped at the last click. This change ensures users see stats up to today. Now pads $list_of_days with zeros from the last click to today in
yourls_build_list_of_days().
Tested with multiple URLs to confirm all timeframes display correctly.

The stats timeline stopped at the last click. This change
ensures users see stats up to today. Now pads $list_of_days
with zeros from the last click to today in
yourls_build_list_of_days().
@dgw
Copy link
Copy Markdown
Member

dgw commented Apr 11, 2025

Looks like a fine approach, though you have removed whitespace that is part of our (under-documented?) format standards. I'd appreciate it if you unremoved those spaces :)

@LeoColomb Do you think we can ship this + #3893 as 1.10.1 soon? Broken plugin deactivation in particular is going to frustrate people if it sticks around too long in the stable release.

YOURLS style prefers no space before parentheses in
control structures. Fixed spacing in
yourls_build_list_of_days() (e.g., `if (` to `if(`) for
consistency, as requested in PR review.
@ntindicator
Copy link
Copy Markdown
Contributor Author

The white space removal was accidental, only noticed it after I'd already pushed the commit. Fixed it now :)

Copy link
Copy Markdown
Member

@LeoColomb LeoColomb left a comment

Choose a reason for hiding this comment

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

LGTM (but not tested).
@dgw Agreed 🚀

@dgw
Copy link
Copy Markdown
Member

dgw commented Apr 14, 2025

(but not tested)

Thankfully it's easy enough to copy-paste functions-infos.php over to my server to test it out.

I don't have a super active instance, so the highest-click-count example I can test ain't much. It's enough to confirm that this patch does what it says, though:

image

Compare to the old line chart, before I updated functions-infos.php from this PR branch:

image

Note that the "last 7 days" and "last 30 days" options disappeared after this patch, because there is no data within those time periods. That's probably appropriate. Why invite the user to view empty data?

What worries me a little more is that the click total I get by summing the points in the updated line graph view is only about half the actual total reported in the table to the right. (Probably that is from yourls_array_granularity() and not a result of this patch. It's just more likely as the link ages to drop meaningful values from the larger set.)

@dgw dgw added this to the 1.10.1 milestone Apr 14, 2025
@LeoColomb
Copy link
Copy Markdown
Member

What worries me a little more is that the click total I get by summing the points in the updated line graph view is only about half the actual total reported in the table to the right.

Can you check if the total in the right table corresponds to the number of:

  1. Click count in the database (I expect so)?
  2. The number of lines in the database log table for the corresponding link?

As the two might be different, it's eventually all fine.

@dgw
Copy link
Copy Markdown
Member

dgw commented Apr 17, 2025

Can you check if the total in the right table corresponds to the number of:

The total click count is correct. As I said before, it's probably yourls_array_granularity() messing with things to get a "smoother" graph at the expense of accuracy, as it says in the doc comment:

* This make less accurate but less messy graphs when too much values.

Changing that (to e.g. sum the values in each "bucket" instead of dropping the in-betweens) could be an interesting thought, but out of scope for this PR 😸

@LeoColomb
Copy link
Copy Markdown
Member

As I said before, it's probably yourls_array_granularity() messing with things to get a "smoother" graph at the expense of accuracy, as it says in the doc comment

Oh right, apologies (again!), I read your comment too quick.
I see now.

Right, so ready to merge?

Copy link
Copy Markdown
Member

@dgw dgw left a comment

Choose a reason for hiding this comment

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

Right, so ready to merge?

Yes :shipit:

@dgw dgw enabled auto-merge (squash) April 24, 2025 23:06
@dgw dgw merged commit 57e8c78 into YOURLS:master Apr 24, 2025
6 checks passed
tomtenuta pushed a commit to tomtenuta/YOURLS that referenced this pull request Nov 4, 2025
* Extend stats timeline to current day

The stats timeline stopped at the last click. This change
ensures users see stats up to today. Now pads $list_of_days
with zeros from the last click to today in
yourls_build_list_of_days().

* Remove spaces before parentheses in function

YOURLS style prefers no space before parentheses in
control structures. Fixed spacing in
yourls_build_list_of_days() (e.g., `if (` to `if(`) for
consistency, as requested in PR review.

---------

Co-authored-by: dgw <dgw@technobabbl.es>
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.

3 participants