Add case to shift_back function to handle day periods

[djanda97]

Changes

Fixes #1650
This PR adds a case to the shift_back function in lib/plausible/stats/query.ex to handle day periods.

Tests

• Automated tests have been added

Changelog

• This PR does not make a user-facing change

Documentation

• This change does not need a documentation update

Dark mode

• This PR does not change the UI

[bundlemon]

BundleMon

Unchanged files (7)

Status	Path	Size	Limits
✅	static/css/app.css	492.34KB	-
✅	static/js/dashboard.js	298.05KB	-
✅	static/js/app.js	12.13KB	-
✅	static/js/embed.host.js	5.58KB	-
✅	static/js/embed.content.js	5.06KB	-
✅	tracker/js/plausible.js	748B	-
✅	static/js/applyTheme.js	314B	-

No change in files bundle size

Final result: ✅

View report in BundleMon website ➡️

---

Current branch size history | Target branch size history

[djanda97]

Since the shift_back function handles month and year periods similarly, I tried following this format and adding another case for the function with the relevant parts swapped out for the day period. I haven't added an automated test case for these changes yet, but I wanted to open this PR to check if I'm on the right track. I would appreciate any feedback 😄

[ukutaht]

I haven't added an automated test case for these changes yet, but I wanted to open this PR to check if I'm on the right track. I would appreciate any feedback 😄

Yes I think you're on the right track :) We'd need the automated test cases in order to verify this change properly.

[vinibrsl]

Hey 👋!

First of all, thank you for your contribution, David!

We're building a comparisons feature that uses the shift_back functions extensively. In the context of this work we've refactored this function completely using a new strategy. The shift_back function itself does not exist anymore and it is equivalent to this clause of the do_compare function:

analytics/lib/plausible/stats/comparisons.ex

Lines 67 to 78 in 9a4c4f4

	defp do_compare(source_query, "previous_period", opts) do
	now = Keyword.fetch!(opts, :now)
	last = earliest(source_query.date_range.last, now)
	diff_in_days = Date.diff(source_query.date_range.first, last) - 1
	new_first = Date.add(source_query.date_range.first, diff_in_days)
	new_last = Date.add(last, diff_in_days)
	range = Date.range(new_first, new_last)
	{:ok, %Stats.Query{source_query | date_range: range}}
	end

The code itself is simpler because instead of having a case for each requested period, we limit the end date to the current day.

The bug your pull request is trying to solve is still there after the refactor, and I'm not sure this pull request actually fixes it. The shift back function was in fact shifting back 1 day for day periods, which is not entirely correct, as this does not take the time into account.

If you're still interested in tackling this issue, I'd be happy to give you more context and to help as needed. I'd suggest you to start over, as this function has been entirely refactored, and working on the time precison from a clean slate.

[djanda97]

Hi @vinibrsl! It has been a while since I've worked on this issue plus there wasn't much code that was included in this PR, so I agree that it would be best to start over. I'm still interested in working on #1650, so I'll take a look through analytics/lib/plausible/stats/comparisons.ex and its associated tests. After taking a closer look, I'll post here with any questions that I have! 😄

[djanda97]

Hi @vinibrsl, I haven't found the time to work on updating this PR.