Fix Bug that makes hours visually "disappear"

[Acidicts]

I think it this bit

/app/helper/application_helper.rb

def format_seconds(seconds, include_days: false)
    return "0s" if seconds.nil? || seconds <= 0

    days = seconds / 86400
    hours = (seconds % 86400) / 3600
    minutes = (seconds % 3600) / 60
    secs = seconds % 60

    parts = []
    parts << "#{days}d" if include_days && days > 0
    parts << "#{hours}h" if hours > 0 || (include_days && days > 0)
    parts << "#{minutes}m" if minutes > 0 || parts.any?
    parts << "#{secs}s" if parts.empty?

    parts.join(" ")
  end

I'm presuming include_days is false which means hours is put only until it reaches 24hr
parts << "#{hours}h" if hours > 0 || (include_days && days > 0) where it should be parts << "#{hours}h" if hours > 0 || (! include_days && days > 0)

My Version is:

def format_seconds(seconds, include_days = false)
    return "0s" if seconds.nil? || seconds <= 0

    days = seconds / 86400
    hours = include_days ? (seconds % 86400) / 3600 : seconds / 3600
    
    minutes = (seconds % 3600) / 60
    secs = seconds % 60

    parts = []
    parts << "#{days}d" if include_days && days > 0
    parts << "#{hours}h" if hours > 0 || (!include_days && days > 0)
    parts << "#{minutes}m" if minutes > 0 || parts.any?
    
    parts << "#{secs}s" if parts.empty?

    parts.join(" ")
end

[Copilot]

Pull request overview

This PR fixes a bug where hours would "disappear" from the time display after exceeding 24 hours. The issue occurred when include_days was false - the function was calculating hours as a remainder after removing days (hours % 24), rather than showing total hours.

Key Changes:

• Modified hours calculation to show total hours when include_days is false, and hours within a day when include_days is true
• Updated the condition for displaying hours to properly handle both modes

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Changing the parameter from a keyword argument (include_days:) to a positional argument (include_days =) breaks existing code. The codebase has multiple calls using the keyword argument syntax (e.g., format_seconds(data[:aho], include_days: true) in app/views/admin/fulfillment_dashboard/index.html.erb). These calls will fail with an ArgumentError after this change. Keep the keyword argument syntax to maintain backward compatibility.

Suggested change

	def format_seconds(seconds, include_days = false)
	def format_seconds(seconds, include_days = false, **kwargs)
	include_days = kwargs.fetch(:include_days, include_days)

[Copilot]

This blank line change doesn't serve a clear purpose and deviates from the original formatting. Consider keeping the original whitespace structure for consistency unless there's a specific style guide requirement.

[Copilot]

This blank line change doesn't serve a clear purpose and deviates from the original formatting. Consider keeping the original whitespace structure for consistency unless there's a specific style guide requirement.

[maxwofford]

I like the time changes, but your PR touches a bunch of other stuff around CI runs. can you just cherrypick the change that should be made so i can merge that?

[maxwofford]

I just moved this to #796!