fix(chart): use milliseconds for commit activity timestamps (fixes #1…#1651
fix(chart): use milliseconds for commit activity timestamps (fixes #1…#1651arnavsharma990 wants to merge 3 commits intoscalacenter:mainfrom
Conversation
|
It looks like the PR introduces (or surfaces) a regression in This suggests that Could you:
Once that’s addressed, the rest of the suite passes, so this should unblock the PR. |
There was a problem hiding this comment.
Pull request overview
This pull request addresses a timestamp unit mismatch causing the Commit Activity chart to display dates starting from 1970 (Unix Epoch) and additionally refactors the ArtifactService to handle cases where no versions are available.
Key changes:
- Fixed timestamp conversion in commit activity chart from seconds to milliseconds
- Refactored
computeLatestVersionto returnOption[Version]with proper empty sequence handling - Added comprehensive test coverage for the refactored
computeLatestVersionmethod
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| modules/webclient/src/main/scala/scaladex/client/Sparkline.scala | Updated frontend to parse commit activity timestamps as milliseconds using Instant.ofEpochMilli instead of ofEpochSecond |
| modules/template/src/main/twirl/scaladex/view/project/project.scala.html | Updated template to convert timestamps to milliseconds using toEpochMilli instead of getEpochSecond |
| modules/server/src/main/scala/scaladex/server/service/ArtifactService.scala | Changed computeLatestVersion to return Option[Version] and added proper handling for empty version sequences with appropriate logging |
| modules/server/src/test/scala/scaladex/server/service/ArtifactServiceTests.scala | Added new test suite with comprehensive coverage for computeLatestVersion method including edge cases for empty sequences, stable/unstable version preferences |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Thanks @kannupriyakalra for guiding the approach to fix this, Verified locally: server/test, server/testOnly scaladex.server.service.ArtifactServiceTests, and testOnly scaladex.server.route.api.OldSearchApiTests all pass. PTAL. |
|
@arnavsharma990 a test case is failing, please check |
|
CI needed a tiny formatting tweak (import spacing). Fixed it now. |
Description
The "Commit Activity" chart on project pages was displaying dates starting from 1970 (Unix Epoch). This was caused by a unit mismatch: the server was providing a timestamp in Seconds, but the frontend client (and Chart.js) expected Milliseconds.
Changes
project.scala.html: Updated the template to explicitly convert the start date to Milliseconds using.toEpochMillibefore rendering it to the HTML attribute.Sparkline.scala: Updated the Scala.js frontend to parse thedata-commit-activity-starting-dayattribute usingInstant.ofEpochMilli.Verification
./cs launch sbt -- compileto ensure type safety for the new time conversions.