starred/audiobookshelf
1
0
Fork 0
mirror of https://github.com/advplyr/audiobookshelf.git synced 2026-05-30 23:40:40 +02:00
Code Issues 1.0k Packages Projects Releases 10 Wiki Activity

[PR #3832] [MERGED] Fixes #3817 Correct rounding and carry of minutes in client/plugins/utils.js::$elapsedPrettyExtended #4096

New Issue
Closed
opened 2026-04-25 00:18:18 +02:00 by adam · 0 comments
adam commented 2026-04-25 00:18:18 +02:00
Owner
Copy Link
Copy Source

📋 Pull Request Information

Original PR: https://github.com/advplyr/audiobookshelf/pull/3832
Author: @daneroo
Created: 1/13/2025
Status: ✅ Merged
Merged: 1/15/2025
Merged by: @advplyr

Base: master ← Head: fix_rounding_elapsedPrettyExtended

📝 Commits (1)

📊 Changes

2 files changed (+198 additions, -5 deletions)

View changed files

➕ client/cypress/tests/utils/ElapsedPrettyExtended.cy.js (+188 -0)
📝 client/plugins/utils.js (+10 -5)

📄 Description

Correct rounding and carry of minutes in client/plugins/utils.js:: Vue.prototype.$elapsedPrettyExtended

-Add cypress tests for Vue.prototype.$elapsedPrettyExtended function

Note: This is my first PR to the project, any feedback would be appreciated.

Brief summary

In the Audiobookshelf Year In Review, there was a time at which the App reported
"Listening Time" as "44d23h60m", but it should have been "Listening Time" as "45d"

Correct Display Bug Display
Year In Review OK Year In Review BUG

Which issue is fixed?

Fixes #3817

In-depth Description

In client/plugins/utils.js:: Vue.prototype.$elapsedPrettyExtended,
specifically when showSeconds=false, the code to round minutes up (when seconds>=30) did not account for the carry of minutes into hours.

The solution was to add a condition that if the rounded up minutes were now>=60, we should carry minutes into hours.

if (minutes && seconds && !showSeconds) {
  if (seconds >= 30) minutes++
  // new condition
  if (minutes >= 60) {
    hours++ // Increment hours if minutes roll over
    minutes -= 60 // adjust minutes
  }
}

Furthermore, this rounding condition was moved up in the function, before the conditional days handling, to prevent this carry condition from needing to be propagated to days as well.

How have you tested this?

I have added cypress tests (although these are pure functional/non-UI tests) to validate the current behavior of the $elapsedPrettyExtended function.

  • The specific test cases that fix this issue are identified.
  • All tests, except these, were also passed by the original function

Given that the current (existing) cypress tests are not all passing, and do not seem to be run in any GitHub Action CI tasks,
this is how you can run only the newly contributed tests:

cd client; npm ci; npm run generate
// normally just:
// npm test
// but to run only our new tests:
npm run compile-tailwind && npx cypress run --component --browser chrome --spec "cypress/tests/utils/ElapsedPrettyExtended.cy.js"

Test Results

  $elapsedPrettyExtended
    function is on the Vue Prototype
      ✓ exists as a function on Vue.prototype (9ms)
    param default values
      ✓ uses useDays=true showSeconds=true by default (8ms)
      ✓ only useDays=false overrides useDays but keeps showSeconds=true (4ms)
      ✓ explicit useDays=false showSeconds=false overrides both (3ms)
    useDays=false showSeconds=true
      ✓ 0s -> "" (3ms)
      ✓ 1h 1s -> 1h 1s (7ms)
      ✓ 25h 1s -> 25h 1s (4ms)
    useDays=true showSeconds=true
      ✓ 0s -> "" (4ms)
      ✓ 1h 1s -> 1h 1s (3ms)
      ✓ 25h 1s -> 1d 1h 1s (7ms)
    useDays=true showSeconds=false
      ✓ 0s -> "" (3ms)
      ✓ 1h -> 1h (3ms)
      ✓ 1h 1s -> 1h (3ms)
      ✓ 1h 1m -> 1h 1m (3ms)
      ✓ 25h -> 1d 1h (4ms)
      ✓ 25h 1s -> 1d 1h (3ms)
      ✓ 2d -> 2d (5ms)
    rounding useDays=true showSeconds=true
      ✓ 1s -> 1s (3ms)
      ✓ 29.9s -> 30s (3ms)
      ✓ 30s -> 30s (3ms)
      ✓ 30.1s -> 30s (6ms)
      ✓ 59.4s -> 59s (3ms)
      ✓ 59.5s -> 1m (3ms)
      ✓ 59m 29s -> 59m 29s (6ms)
      ✓ 59m 30s -> 59m 30s (2ms)
      ✓ 59m 59.5s -> 1h (3ms)
      ✓ 23h 59m 29s -> 23h 59m 29s (7ms)
      ✓ 23h 59m 30s -> 23h 59m 30s (3ms)
      ✓ 23h 59m 59.5s -> 1d (3ms)
      ✓ 44d 23h 59m 30s -> 44d 23h 59m 30s (5ms)
    rounding useDays=true showSeconds=false
      ✓ 1s -> "" (7ms)
      ✓ 29.9s -> "" (3ms)
      ✓ 30s -> "" (2ms)
      ✓ 30.1s -> "" (6ms)
      ✓ 59.4s -> "" (3ms)
      ✓ 59.5s -> 1m (3ms)
      ✓ 1m 29.5s -> 2m (3ms)
      ✓ 59m 29s -> 59m (3ms)
      ✓ 59m 30s -> 1h (3ms)
      ✓ 59m 59.5s -> 1h (6ms)
      ✓ 23h 59m 29s -> 23h 59m (3ms)
      ✓ 23h 59m 30s -> 1d (3ms)
      ✓ 23h 59m 59.5s -> 1d (6ms)
      ✓ 44d 23h 59m 30s -> 45d (2ms)
    empty values
      with days and seconds
        ✓ null input (6ms)
        ✓ undefined input (6ms)
        ✓ zero (3ms)
        ✓ rounds to zero (2ms)
      with days, no seconds
        ✓ null input (3ms)
        ✓ undefined input (2ms)
        ✓ zero (2ms)
        ✓ rounds to zero (3ms)
      no days, with seconds
        ✓ null input (4ms)
        ✓ undefined input (6ms)
        ✓ zero (3ms)
        ✓ rounds to zero (3ms)
      no days, no seconds
        ✓ null input (3ms)
        ✓ undefined input (3ms)
        ✓ zero (3ms)
        ✓ rounds to zero (3ms)


  60 passing (440ms)


  (Results)

  ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
  │ Tests:        60                                                                               │
  │ Passing:      60                                                                               │
  │ Failing:      0                                                                                │
  │ Pending:      0                                                                                │
  │ Skipped:      0                                                                                │
  │ Screenshots:  0                                                                                │
  │ Video:        false                                                                            │
  │ Duration:     0 seconds                                                                        │
  │ Spec Ran:     ElapsedPrettyExtended.cy.js                                                      │
  └────────────────────────────────────────────────────────────────────────────────────────────────┘


====================================================================================================

  (Run Finished)


       Spec                                              Tests  Passing  Failing  Pending  Skipped  
  ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
  │ ✔  ElapsedPrettyExtended.cy.js              441ms       60       60        -        -        - │
  └────────────────────────────────────────────────────────────────────────────────────────────────┘
    ✔  All specs passed!                        441ms       60       60        -        -        -

I also ran a brute force test (not submitted) to guarantee that the corrected function did not change the function's behaviour for cases other than those identified in this bug fix. i.e. I compared the value of the original function vs the fixed version for all values of 'seconds' in [0s, 101 days] in increments of 0.5 seconds

Screenshots

None

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/advplyr/audiobookshelf/pull/3832 **Author:** [@daneroo](https://github.com/daneroo) **Created:** 1/13/2025 **Status:** ✅ Merged **Merged:** 1/15/2025 **Merged by:** [@advplyr](https://github.com/advplyr) **Base:** `master` ← **Head:** `fix_rounding_elapsedPrettyExtended` --- ### 📝 Commits (1) - [`acda776`](https://github.com/advplyr/audiobookshelf/commit/acda776e3ea1d6903995f49b83ff83293d063e8c) Fixes #3817 ### 📊 Changes **2 files changed** (+198 additions, -5 deletions) <details> <summary>View changed files</summary> ➕ `client/cypress/tests/utils/ElapsedPrettyExtended.cy.js` (+188 -0) 📝 `client/plugins/utils.js` (+10 -5) </details> ### 📄 Description Correct rounding and carry of minutes in client/plugins/utils.js:: Vue.prototype.$elapsedPrettyExtended -Add cypress tests for Vue.prototype.$elapsedPrettyExtended function <!-- For Work In Progress Pull Requests, please use the Draft PR feature, see https://github.blog/2019-02-14-introducing-draft-pull-requests/ for further details. If you do not follow this template, the PR may be closed without review. Please ensure all checks pass. If you are a new contributor, the workflows will need to be manually approved before they run. --> Note: This is my first PR to the project, any feedback would be appreciated. ## Brief summary In the Audiobookshelf Year In Review, there was a time at which the App reported "Listening Time" as "44d23h60m", but it should have been "Listening Time" as "45d" | Correct Display | Bug Display | |:---------------:|:-----------:| | <img src="https://github.com/user-attachments/assets/a883099f-5439-4f40-b3c2-3719241df498" width="200" alt="Year In Review OK"> | <img src="https://github.com/user-attachments/assets/bfa344a3-786d-4366-9eb0-0ccd88a274c7" width="200" alt="Year In Review BUG"> | ## Which issue is fixed? Fixes #3817 ## In-depth Description <!-- Describe your solution in more depth. How does it work? Why is this the best solution? Does it solve a problem that affects multiple users or is this an edge case for your setup? --> In client/plugins/utils.js:: Vue.prototype.$elapsedPrettyExtended, specifically when showSeconds=false, the code to round minutes *up* (when seconds>=30) did not account for the carry of minutes into hours. The solution was to add a condition that if the rounded up minutes were now>=60, we should carry minutes into hours. ```js if (minutes && seconds && !showSeconds) { if (seconds >= 30) minutes++ // new condition if (minutes >= 60) { hours++ // Increment hours if minutes roll over minutes -= 60 // adjust minutes } } ``` Furthermore, this rounding condition was moved up in the function, before the conditional days handling, to prevent this carry condition from needing to be propagated to days as well. ## How have you tested this? I have added cypress tests (although these are pure functional/non-UI tests) to validate the current behavior of the `$elapsedPrettyExtended` function. - The specific test cases that fix this issue are identified. - All tests, except these, were also passed by the original function Given that the current (existing) cypress tests are _not all passing_, and do not seem to be run in any GitHub Action CI tasks, this is how you can run _only_ the newly contributed tests: ```bash cd client; npm ci; npm run generate // normally just: // npm test // but to run only our new tests: npm run compile-tailwind && npx cypress run --component --browser chrome --spec "cypress/tests/utils/ElapsedPrettyExtended.cy.js" ``` Test Results ```bash $elapsedPrettyExtended function is on the Vue Prototype ✓ exists as a function on Vue.prototype (9ms) param default values ✓ uses useDays=true showSeconds=true by default (8ms) ✓ only useDays=false overrides useDays but keeps showSeconds=true (4ms) ✓ explicit useDays=false showSeconds=false overrides both (3ms) useDays=false showSeconds=true ✓ 0s -> "" (3ms) ✓ 1h 1s -> 1h 1s (7ms) ✓ 25h 1s -> 25h 1s (4ms) useDays=true showSeconds=true ✓ 0s -> "" (4ms) ✓ 1h 1s -> 1h 1s (3ms) ✓ 25h 1s -> 1d 1h 1s (7ms) useDays=true showSeconds=false ✓ 0s -> "" (3ms) ✓ 1h -> 1h (3ms) ✓ 1h 1s -> 1h (3ms) ✓ 1h 1m -> 1h 1m (3ms) ✓ 25h -> 1d 1h (4ms) ✓ 25h 1s -> 1d 1h (3ms) ✓ 2d -> 2d (5ms) rounding useDays=true showSeconds=true ✓ 1s -> 1s (3ms) ✓ 29.9s -> 30s (3ms) ✓ 30s -> 30s (3ms) ✓ 30.1s -> 30s (6ms) ✓ 59.4s -> 59s (3ms) ✓ 59.5s -> 1m (3ms) ✓ 59m 29s -> 59m 29s (6ms) ✓ 59m 30s -> 59m 30s (2ms) ✓ 59m 59.5s -> 1h (3ms) ✓ 23h 59m 29s -> 23h 59m 29s (7ms) ✓ 23h 59m 30s -> 23h 59m 30s (3ms) ✓ 23h 59m 59.5s -> 1d (3ms) ✓ 44d 23h 59m 30s -> 44d 23h 59m 30s (5ms) rounding useDays=true showSeconds=false ✓ 1s -> "" (7ms) ✓ 29.9s -> "" (3ms) ✓ 30s -> "" (2ms) ✓ 30.1s -> "" (6ms) ✓ 59.4s -> "" (3ms) ✓ 59.5s -> 1m (3ms) ✓ 1m 29.5s -> 2m (3ms) ✓ 59m 29s -> 59m (3ms) ✓ 59m 30s -> 1h (3ms) ✓ 59m 59.5s -> 1h (6ms) ✓ 23h 59m 29s -> 23h 59m (3ms) ✓ 23h 59m 30s -> 1d (3ms) ✓ 23h 59m 59.5s -> 1d (6ms) ✓ 44d 23h 59m 30s -> 45d (2ms) empty values with days and seconds ✓ null input (6ms) ✓ undefined input (6ms) ✓ zero (3ms) ✓ rounds to zero (2ms) with days, no seconds ✓ null input (3ms) ✓ undefined input (2ms) ✓ zero (2ms) ✓ rounds to zero (3ms) no days, with seconds ✓ null input (4ms) ✓ undefined input (6ms) ✓ zero (3ms) ✓ rounds to zero (3ms) no days, no seconds ✓ null input (3ms) ✓ undefined input (3ms) ✓ zero (3ms) ✓ rounds to zero (3ms) 60 passing (440ms) (Results) ┌────────────────────────────────────────────────────────────────────────────────────────────────┐ │ Tests: 60 │ │ Passing: 60 │ │ Failing: 0 │ │ Pending: 0 │ │ Skipped: 0 │ │ Screenshots: 0 │ │ Video: false │ │ Duration: 0 seconds │ │ Spec Ran: ElapsedPrettyExtended.cy.js │ └────────────────────────────────────────────────────────────────────────────────────────────────┘ ==================================================================================================== (Run Finished) Spec Tests Passing Failing Pending Skipped ┌────────────────────────────────────────────────────────────────────────────────────────────────┐ │ ✔ ElapsedPrettyExtended.cy.js 441ms 60 60 - - - │ └────────────────────────────────────────────────────────────────────────────────────────────────┘ ✔ All specs passed! 441ms 60 60 - - - ``` I also ran a brute force test (not submitted) to guarantee that the corrected function did not change the function's behaviour for cases other than those identified in this bug fix. i.e. I compared the value of the original function vs the fixed version for all values of 'seconds' in `[0s, 101 days]` in increments of `0.5` seconds ## Screenshots None --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
adam added the pull-request label 2026-04-25 00:18:18 +02:00
adam closed this issue 2026-04-25 00:18:18 +02:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
authentication
backlog
bug
chapter editor
config-issue
ebooks
encoding/embedding
enhancement
help wanted
listening sessions & progress
planned
possible plugin
progress sync
pull-request
Mirrored from GitHub Pull Request
 sorting/filtering/searching
unable to reproduce
upload
users & permissions
waiting
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
adam (Adam Melkus)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/audiobookshelf#4096
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?