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 sum of rest jobs printing time #1689

Merged

Conversation

mdziekon
Copy link
Contributor

@mdziekon mdziekon commented Dec 16, 2023

Description

This PR fixes a bug in the Jobs queue, where the "rest of jobs" does not correctly sum up estimated printing time of the rest files. Instead of summing up all of the rest jobs, it takes into account only the last element of the list.

Bug is caused by a simple omission of addition operator in code.

Steps to reproduce

  1. Place 5 random models into the Queue
    • (so that the next item in the queue will not be displayed directly)
  2. Add two more DISTINCT models with different estimated times
    1. One with estimated print time of eg. 5h
    2. One with estimated print time of eg. 1h

Expected behavior

  • Summed up estimation displays "6h of print time"

Actual behavior

  • Summed up estimation displays "1h of print time"

Notes

  • Bug does not occur when adding the same model multiple times, that is, when combinedIds internal feature is being used.

Related Tickets & Documents

N/A (no bug report, this PR is both a report and a fix)

Regression caused by: N/A (this never worked correctly, see blame)

Mobile & Desktop Screenshots/Recordings

N/A

[optional] Are there any post-deployment tasks we need to perform?

N/A

Signed-off-by: Michał Dziekoński <michal.dziekonski+github@gmail.com>
@meteyou meteyou merged commit c92f4c0 into mainsail-crew:develop Dec 16, 2023
10 checks passed
@meteyou
Copy link
Member

meteyou commented Dec 16, 2023

thx for this fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants