Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix incorrect display of "days ago" on activity page #1382

Merged
merged 3 commits into from
Oct 23, 2021

Conversation

flaix
Copy link
Member

@flaix flaix commented Oct 23, 2021

The timeAgo does not work correctly when in GMT time zone because of how the daysAgo method in TimeUtils works.
This PR fixes that by changing to checking for actual full calendar days. To cover this with unit tests (as date issues are rather tricky), the methods in TimeUtils need to be overridden to make them testable in a reproducible way. And not have the test result dependent on the time of day the test is executed.
This fixes issued #800 and #1248.

Add tests for `timeAgo` to analyse issue gitblit-org#1248.
The tests are dependent on when they run as they time functions use the
current date and time. To make them testable in a reproducible way, we
need the ability to pass in what we think is "now". So add overloaded
methods that take a `now` parameter so that we can pass in the current
time.
For some reason the `TimeUtilsTest` class is, like almost all tests, in
the `com.gitblit.tests` package. But this way all methods in classes
which we might predominately need for tests have to be public.
So move the unit test class `TimeUtilsTest` to the same package as the
class it is testing, i.e. `com.gitblit.utils.TimeUtils`.
This way we ca set the new added methods which get the current time
passed in to be at least not public.
The `daysAgo` method seemed to want to normalize on a calendar day? I
can't really tell what it was trying to do, but the problem is that it
does not take into account any time shift due to time zones so it never
really worked outside of GMT.
So instead a new `calendarDaysAgo` method is added (because I am unsure
on what the `daysAgo` method is trying to do. It can probably be removed).
The new method cleanly calculates difference in calendar days because it
normalizes the two given time stamps on the same time zone.

The `timeAgo` method now used the new method. This fixes gitblit-org#1248.
@flaix flaix added the hacktoberfest-accepted Accepted Hacktoberfest contribution, will merge later. label Oct 23, 2021
@flaix flaix merged commit 583e15e into gitblit-org:master Oct 23, 2021
@flaix flaix deleted the wip-fix-daysAgo branch October 24, 2021 00:01
@flaix flaix added this to the 1.9.2 milestone Nov 25, 2021
@flaix flaix linked an issue Nov 25, 2021 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest-accepted Accepted Hacktoberfest contribution, will merge later.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant