Skip to content

use ET timezone consistently in the admin UI #693

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.

Sign up for GitHub

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

Open
jtmkrueger wants to merge 2 commits into
base: main
Choose a base branch
from
Open

use ET timezone consistently in the admin UI #693

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f29049a
use ET timezone consistently in the admin UI
jtmkrueger Feb 3, 2026
0e3cdf1
use session TZ; fix tests; change to using MST in new tests
jtmkrueger Feb 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/api-umbrella/admin-ui/app/utils/data-tables-helpers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ export default {

renderTime(value, type) {
if(type === 'display' && value && value !== '-') {
return moment(value).format('YYYY-MM-DD HH:mm:ss');
return moment(value).tz('America/New_York').format('YYYY-MM-DD HH:mm:ss z');
Copy link
Member

@GUI GUI Feb 4, 2026

Choose a reason for hiding this comment

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

There's a system-wide analytics.timezone configuration setting that is used throughout the system to determine this type of default time zone to be used. This system setting is exposed during the login in some session data, and that's how the charts know what time zone to use:

api-umbrella/src/api-umbrella/admin-ui/app/routes/stats/base.js

Line 26 in 1cb2bd5

let timezone = this.session.data.authenticated.analytics_timezone;

So to remain consistent with this configuration setting, I'd maybe recommend using that same setting here instead of hard-coding it to ET (even though for the api.data.gov deploy, it's hard-coded to ET):

Suggested change
return moment(value).tz('America/New_York').format('YYYY-MM-DD HH:mm:ss z');
return moment(value).tz(this.session.data.authenticated.analytics_timezone).format('YYYY-MM-DD HH:mm:ss z');

Copy link
Collaborator Author

@jtmkrueger jtmkrueger Feb 5, 2026

Choose a reason for hiding this comment

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

Oh I didn't spot this, cool! Changed to utilize. Since there's no this.session context here I had to refactor some things, and change how renderTime is structured. Used the same pattern implemented in other functions like renderLinkedList here in data-tables-helpers.js, but then had to adapt call locations to use the new method signature. I didn't go with a plain options but just set things up to explicitly require passing in this.session.data.authenticated.analytics_timezone when calling renderTime. This required touching a number of other files including injecting session into relevant results-table.js's. If you think there's a cleaner way let me know!

}

return value;
Expand Down
7 changes: 3 additions & 4 deletions src/api-umbrella/utils/time.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ local null = ngx.null
local date = icu_date.new()
local format_iso8601 = icu_date.formats.pattern("yyyy-MM-dd'T'HH:mm:ssZZZZZ")
local format_iso8601_ms = icu_date.formats.pattern("yyyy-MM-dd'T'HH:mm:ss.SSSZZZZZ")
local format_csv = icu_date.formats.pattern("yyyy-MM-dd HH:mm:ss")
local format_postgres = icu_date.formats.pattern("yyyy-MM-dd HH:mm:ss.SSSSxxx")
local format_postgres_no_millis = icu_date.formats.pattern("yyyy-MM-dd HH:mm:ssxxx")

Expand Down Expand Up @@ -80,7 +79,7 @@ function _M.timestamp_ms_to_csv(timestamp)
end

date:set_millis(timestamp)
return date:format(format_csv)
return date:format(format_iso8601_ms)
end

function _M.iso8601_to_timestamp(string)
Expand Down Expand Up @@ -116,7 +115,7 @@ function _M.iso8601_to_csv(string)
end

date:parse(format_iso8601, string)
return date:format(format_csv)
return date:format(format_iso8601)
end

function _M.iso8601_ms_to_csv(string)
Expand All @@ -125,7 +124,7 @@ function _M.iso8601_ms_to_csv(string)
end

date:parse(format_iso8601_ms, string)
return date:format(format_csv)
return date:format(format_iso8601_ms)
end

function _M.opensearch_to_csv(value)
Expand Down
22 changes: 22 additions & 0 deletions test/admin_ui/test_stats_logs.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,4 +305,26 @@ def test_filter_options
"is not null",
]
end

def test_table_displays_time_in_eastern_timezone
FactoryBot.create(:log_item, :request_at => Time.parse("2015-01-16T06:06:28.816Z").utc, :request_method => "OPTIONS")
LogItem.refresh_indices!

admin_login
visit "/admin/#/stats/logs?search=&start_at=2015-01-12&end_at=2015-01-18&interval=day"
refute_selector(".busy-blocker")

assert_text("2015-01-16 01:06:28 EST")
end

def test_table_displays_time_in_eastern_timezone_during_dst
FactoryBot.create(:log_item, :request_at => Time.parse("2015-07-16T06:06:28.816Z").utc, :request_method => "OPTIONS")
LogItem.refresh_indices!

admin_login
visit "/admin/#/stats/logs?search=&start_at=2015-07-12&end_at=2015-07-18&interval=day"
refute_selector(".busy-blocker")

assert_text("2015-07-16 02:06:28 EDT")
end
Copy link
Member

@GUI GUI Feb 4, 2026

Choose a reason for hiding this comment

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

Thanks for adding this test coverage! Related to my last comment about leveraging the analytics.timezone setting, the test environment is hard-coded to use America/Denver:

api-umbrella/config/test.yml

Lines 6 to 7 in 1cb2bd5

analytics:
timezone: America/Denver

So if you start leveraging that setting, I think you'll need to update these tests to use MDT/MST for their frame of reference. There's technically ways you could also change the timezone in specific tests, but I think that's probably overkill here, so I think just updating these tests to verify the mountain time assumption is sufficient.

Copy link
Collaborator Author

@jtmkrueger jtmkrueger Feb 5, 2026

Choose a reason for hiding this comment

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

Yep, I updated the tests to use MDT

end
22 changes: 22 additions & 0 deletions test/apis/admin/stats/test_logs.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,4 +226,26 @@ def test_no_results_non_existent_indices
"recordsTotal" => 0,
}, data)
end

def test_csv_timestamps_in_iso8601_utc_format
Copy link
Member

@GUI GUI Feb 4, 2026

Choose a reason for hiding this comment

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

This test looks good, but there's one other semi-related test that's failing now: https://github.com/NatLabRockies/api-umbrella/actions/runs/21634777879/job/62359925312#step:5:385

api-umbrella/test/apis/admin/stats/test_users.rb

Lines 99 to 112 in 1cb2bd5

@user1.created_at.utc.strftime("%Y-%m-%d %H:%M:%S"),
"2",
"2015-01-16 00:00:00",
@user1.use_description,
], csv[1])
assert_equal([
@user2.email,
@user2.first_name,
@user2.last_name,
@user2.website,
@user2.registration_source,
@user2.created_at.utc.strftime("%Y-%m-%d %H:%M:%S"),
"1",
"2015-01-17 00:00:00",

I think you'll need to update those tests to call user1.created_at.utc.iso8601 and then updated the hard-coded "2015-01-16 00:00:00" strings into ISO8601 format.

Copy link
Collaborator Author

@jtmkrueger jtmkrueger Feb 5, 2026

Choose a reason for hiding this comment

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

Ah, thank you! I was trying to figure out what I was missing on these failures. Fixed!

FactoryBot.create(:log_item, :request_at => Time.parse("2015-01-16T06:06:28.816Z").utc, :request_user_agent => unique_test_id)
LogItem.refresh_indices!

response = Typhoeus.get("https://127.0.0.1:9081/admin/stats/logs.csv", http_options.deep_merge(admin_session).deep_merge({
:params => {
"start_at" => "2015-01-13",
"end_at" => "2015-01-18",
"interval" => "day",
"start" => "0",
"length" => "10",
},
}))

assert_response_code(200, response)

csv = CSV.parse(response.body)
assert_equal(2, csv.length)
assert_equal("Time", csv[0][0])
assert_equal("2015-01-16T06:06:28.816Z", csv[1][0])
end
end
Loading