Skip to content

Allows sorting Audiobooks on added date - #516

Closed
T4g1 wants to merge 1 commit into
Listenarrs:canaryfrom
T4g1:feature/sort-by-added
Closed

Allows sorting Audiobooks on added date#516
T4g1 wants to merge 1 commit into
Listenarrs:canaryfrom
T4g1:feature/sort-by-added

Conversation

@T4g1

@T4g1 T4g1 commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

Summary

Requested in #513
Allows user to sort audiobooks on added date and create custom filters for it too

Changes

Added

  • Audiobook now store when they were added
  • Possibility to sort audiobooks on added date
  • Possibility to create custom filters on added date

Testing

Manualy tested

Comment thread listenarr.api/Controllers/LibraryController.cs Outdated
Comment thread listenarr.api/Services/LibraryAddService.cs Outdated
Comment thread listenarr.domain/Models/Audiobook.cs Outdated
Comment thread fe/src/views/library/AudiobooksView.vue Outdated
Comment thread fe/src/types/index.ts Outdated
seriesNumber?: string
seriesMemberships?: AudiobookSeriesMembership[]
description?: string
added: Date

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

While this is a full UTC timestamp the JSON never delivers this as a date, it should be typed as a string.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure I follow what you mean, we expect it to be a Date fetched from the API, the transfer/conversion logic should not determine what data type we want to use right ?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The backend value is a DateTime, but once it comes through JSON the frontend receives a plain string, not a JavaScript Date. Nothing is converting it back automatically.

So added: Date is misleading right now because TS would allow a.added.getTime(), but that would fail at runtime because it's a string. Unless we add a mapping step that converts added with new Date(...), the accurate API type should be added: string.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Addressed. The frontend transport contract is now added?: string | null, matching the JSON payload rather than pretending it is a JavaScript Date. Consumers explicitly parse the serialized UTC string where timestamp semantics are needed.

Comment thread fe/src/components/domain/collection/CustomFilterModal.vue Outdated
Comment thread fe/src/views/library/CollectionView.vue Outdated
table: "Audiobooks",
type: "TEXT",
nullable: false,
defaultValue: new DateTime(2026, 1, 1, 0, 0, 0, 0, DateTimeKind.Unspecified));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Shouldn't we use DateTimeKing.Utc? It's what we use elsewhere. Also why 2026-1-1 as the default for existing rows? Why not DateTime.MinValue?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The reasoning was the following:

  • When a user enters a filtering date through the date picker, it will be relative to his timezone thus comparing to UTC could lead to strange behavior. We can fix that by converting the inputed date to UTC based on user known (?) timezone probably
  • The default here is to give something in the past (relative to the date this will be merged/released that is not too absurd for the user (should we want to display that added date somewhere in the future) so filtering will work as expected for the user but we can definitely go for DateTime.MinValue if you prefer, non tech people seeing 1970-01-01 will probably freak out comparing to seeing 2026-01-01

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We can fix that by converting the inputed date to UTC based on user known (?) timezone probably

Turns out the browser already handles timezone when working with date selector!

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The main thing I’m trying to avoid is introducing an Unspecified DateTime into a field that otherwise behaves like a UTC timestamp. If we keep 2026-01-01 as the backfill/default for existing rows, I think it should still be:
new DateTime(2026, 1, 1, 0, 0, 0, DateTimeKind.Utc)

I’m less attached to MinValue specifically. The important part is that whatever sentinel/default we choose should be UTC and intentional. DateTimeKind.Unspecified is the bit that can create inconsistent serialization/comparison behavior later.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Addressed by removing the fabricated sentinel entirely. The regenerated EF migration adds a nullable Added column with no default. After migration, startup repair backfills legacy rows only from durable evidence: earliest Added history first, then earliest AudiobookFile.CreatedAt as fallback. Rows with no evidence remain null, so we never invent a date. Known values are normalized back to UTC on SQLite materialization.

@T4g1
T4g1 requested a review from therobbiedavis April 20, 2026 15:22
@therobbiedavis therobbiedavis added this to the 0.3.0.0 milestone Apr 22, 2026
@github-project-automation github-project-automation Bot moved this to Not Started in Listenarr 0.3.0.0 Apr 22, 2026
@therobbiedavis therobbiedavis added the minor minor version bump - add functionality in a backward compatible manner label Apr 22, 2026
@therobbiedavis therobbiedavis moved this from Not Started to In review in Listenarr 0.3.0.0 Apr 22, 2026
@therobbiedavis therobbiedavis removed this from the 0.3.0.0 milestone Apr 22, 2026
@therobbiedavis therobbiedavis modified the milestone: 0.3.0.0 Apr 22, 2026
@T4g1 T4g1 self-assigned this Apr 26, 2026
Comment thread fe/src/views/library/AudiobooksView.vue Outdated
Comment on lines +1101 to +1102
av = new Date(a.added).getTime().toString()
bv = new Date(b.added).getTime().toString()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Converting it to string is going to break the comparison. Like I said: this will not sort as expected because it's trying to sort literal string like "Fri Apr 17 2026" and "Mon Apr 27 2026" which will not sort as expected.

We need to change this to:

av = new Date(a.added).getTime()
bv = new Date(b.added).getTime()

Then later before the return add a new conditional, like line 1109:

if (typeof av === 'number' && typeof bv === 'number') {
  return (av - bv) * (sortOrder.value === 'asc' ? 1 : -1)
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Addressed. Date Added sorting now parses the serialized timestamp with Date.parse() and compares the numeric epoch values. Missing or invalid legacy dates are kept last in both directions, and selecting Date Added defaults to descending/newest-first.

Comment thread fe/src/types/index.ts Outdated
seriesNumber?: string
seriesMemberships?: AudiobookSeriesMembership[]
description?: string
added: Date

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The backend value is a DateTime, but once it comes through JSON the frontend receives a plain string, not a JavaScript Date. Nothing is converting it back automatically.

So added: Date is misleading right now because TS would allow a.added.getTime(), but that would fail at runtime because it's a string. Unless we add a mapping step that converts added with new Date(...), the accurate API type should be added: string.

table: "Audiobooks",
type: "TEXT",
nullable: false,
defaultValue: new DateTime(2026, 1, 1, 0, 0, 0, 0, DateTimeKind.Unspecified));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The main thing I’m trying to avoid is introducing an Unspecified DateTime into a field that otherwise behaves like a UTC timestamp. If we keep 2026-01-01 as the backfill/default for existing rows, I think it should still be:
new DateTime(2026, 1, 1, 0, 0, 0, DateTimeKind.Utc)

I’m less attached to MinValue specifically. The important part is that whatever sentinel/default we choose should be UTC and intentional. DateTimeKind.Unspecified is the bit that can create inconsistent serialization/comparison behavior later.

@therobbiedavis therobbiedavis assigned therobbiedavis and unassigned T4g1 Jun 8, 2026
@therobbiedavis

Copy link
Copy Markdown
Collaborator

Is this still on your radar @T4g1?

@T4g1

T4g1 commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

Yes, its on hold but I can update it when you think we should merge it

@therobbiedavis
therobbiedavis requested a review from a team as a code owner June 11, 2026 12:58

Copy link
Copy Markdown
Collaborator

Superseded by #841.

I recreated this work on a maintainer-owned branch so the remaining hardening and review fixes can be landed directly without depending on contributor-fork write access. The replacement PR explicitly credits @T4g1 for the original feature/implementation and preserves the current #516 code state as its starting point.

Closing this PR as superseded rather than rejected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

minor minor version bump - add functionality in a backward compatible manner

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants