Have time.js use UTC-related getters/setters

[kemzeb]

Before this patch, we were using Date getter/setter methods that worked with local time to get a list of Sundays that are in the range of some start date and end date. The problem with this was that the Sundays are in Unix epoch time and when we changed the "startDate" argument that was passed to make sure it is on a Sunday, this change would be reflected when we convert it to Unix epoch time. More specifically, I observed that we may get different Unix epochs depending on your timezone when the returned list should rather be timezone-agnostic.

This led to issues in US timezones that caused the contributor, code frequency, and recent commit charts to not show any chart data. This fix resolves this by using getter/setter methods that work with UTC since it isn't dependent on timezones.

Fixes #30851.

[wxiaoguang]

Thank you for the fix.

TBH I didn't quite get the point how the bug occurs, and why UTC fixes the bug .... if the problem could be clarified, maybe we could also add some tests

[kemzeb]

Yes, after looking at this for some time I observed that the output of startDaysBetween() was different when I switched from PDT to China Standard Time. This should not be the case because each element in the array should be in Unix epoch time, which isn't dependent on timezones.

I'm am not well versed in working with Date-related APIs yet, but I believed that these differing values were caused by the while loop trying to modify the start date to be on a Sunday. Depending on your timezone, startDate would be given a different Unix epoch time because it may have been changed in the loop. I thought by working with UTC time, the start date won't be changed depending on your timezone.

If you see this approach as an issue please let me know!

Edit: I changed my PR description to better explain the problem at hand.

[wxiaoguang]

Take a quick look:

1. There is another getDay call in firstStartDateAfterDate , is it right or not?
2. There are tests in time.test.js, I think this case should be covered.

[kemzeb]

1. There is another getDay call in firstStartDateAfterDate , is it right or not?

Sorry I wasn't specific enough about describing the problem. It is caused by both startDaysBetween() generating bad Unix epochs and the following function failing to map to the backend's own "start of the week" list that we pass to it:

gitea/web_src/js/utils/time.js

Lines 39 to 47 in bb0e4ce

	export function fillEmptyStartDaysWithZeroes(startDays, data) {
	const result = {};
	for (const startDay of startDays) {
	result[startDay] = data[startDay] || {'week': startDay, 'additions': 0, 'deletions': 0, 'commits': 0};
	}
	return Object.values(result);
	}

This is what causes the chart data to not be displayed. Regarding your question, I don't think it would be an issue to use UTC-based methods here too. I'll make the changes.

2. There are tests in time.test.js, I think this case should be covered.

I agree, I'll go ahead and add one or two there.

[kemzeb]

Recent changes has firstStartDateAfterDate() use UTC-related methods.

Having some trouble writing a test for startDaysBetween() since I want the test to make sure that its output remains the same when working with Date objects that have different local times. There doesn't seem to be a way to change the timezone that local time refers to but I'll keep looking

[kemzeb]

Decided to refactor startDaysBetween() to use dayjs's utc plugin. By default dayjs() would assume we are working with local time when making calls like the add() call does (read about it here). This change no longer has us worry about working with local time here.

I manually tested with PDT and China Standard Time on all 3 Activity pages and it appears to work fine.

I wondered if the dayjs.extend() call would be a problem (i.e. if it would be used elsewhere in the future), but dayjs handles this case. See here.

I am still pondering how to test this. As I said this test should ensure that the output of this function remains the same when dealing with different timezones, but I haven't seen a way to temporarily change the timezone when working with Date or dayjs objects.

dayjs does have a timezone plugin but it is just meant for parsing and displaying timezones; it won't affect dayjs() (i.e. it will still use the local time).

[kemzeb]

So... after integrating dayjs's utc plugin it worked perfectly fine in my dev environment but when running the frontend tests it freezes when it reaches time.test.js (deadlock?). I decided to revert my changes to keep using the Date object for now as I not sure what caused the freeze.

Edit: Ohh I see the problem. The while was running in an infinite loop (I forgot that dayjs objects are immutable). When I was manually testing I may have been using an older cached version of the page. Well go back to using the dayjs approach.

[kemzeb]

Thanks! Looking at the main's commit history, it looks like embarrassingly forgot to set my git username and email on another machine I was using 😂.