Skip to content

Commit c7de312

Browse files
committed
Extract enabled_tracker_ids and simplify comments
The tracker id normalization was duplicated between enabled_for? and any_tracker_enabled?. The zone label and drift check comments are reworded in plainer English so non-native contributors can follow the reasoning (#20).
1 parent 5796a8d commit c7de312

2 files changed

Lines changed: 24 additions & 21 deletions

File tree

lib/redmine_issue_datetime.rb

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,17 +16,23 @@ def self.setup
1616
# optional list columns out of the column picker on an instance that has not
1717
# enabled the plugin anywhere.
1818
def self.any_tracker_enabled?
19-
Array(settings['tracker_ids']).reject(&:blank?).any?
19+
enabled_tracker_ids.any?
2020
end
2121

2222
def self.settings
2323
Setting.plugin_redmine_issue_datetime
2424
end
2525

26+
# The tracker ids the plugin is enabled for. The setting is stored as an
27+
# array of strings and contains a blank entry from the form's hidden field.
28+
def self.enabled_tracker_ids
29+
Array(settings['tracker_ids']).reject(&:blank?).map(&:to_i)
30+
end
31+
2632
def self.enabled_for?(tracker_id)
2733
return false if tracker_id.blank?
2834

29-
Array(settings['tracker_ids']).reject(&:blank?).map(&:to_i).include?(tracker_id.to_i)
35+
enabled_tracker_ids.include?(tracker_id.to_i)
3036
end
3137

3238
def self.time_step_minutes
@@ -46,22 +52,20 @@ def self.reference_zone
4652
ActiveSupport::TimeZone['UTC']
4753
end
4854

49-
# Short name of the reference zone, for labelling times in the UI.
50-
#
51-
# Times are stored as instants and always shown on one clock, the instance's
52-
# reference zone, rather than each viewer's own. For site-based work that is
53-
# the useful choice: a dispatcher and someone at the site must mean the same
54-
# wall-clock time by "09:15". The label is what keeps that unambiguous.
55+
# Zone labels for the UI.
5556
#
56-
# Two labels, because the honest label depends on what is being labelled.
57+
# Times are always shown on one clock, the instance's reference zone, never
58+
# converted to each viewer's own zone. For work that happens at a physical
59+
# place, everyone must mean the same wall-clock time by "09:15", and the
60+
# visible zone label is what makes that unambiguous.
5761
#
58-
# zone_abbreviation needs an instant and has no default: the abbreviation is
59-
# daylight-saving dependent, so "now" would be wrong whenever the thing on
60-
# screen is in another part of the year. Use it only where a specific time is
61-
# displayed.
62+
# There are two label methods because the correct label depends on context:
6263
#
63-
# zone_name is for anywhere that covers many dates or none yet - a list header,
64-
# an empty form - where no single abbreviation can be correct.
64+
# - zone_abbreviation(at) is daylight-saving aware, so it requires the
65+
# instant being displayed (deliberately no default: "now" would be wrong
66+
# for a time in another part of the year). Use it next to a specific time.
67+
# - zone_name is season-independent. Use it where many dates are covered,
68+
# or none yet: list headers, empty forms.
6569
def self.zone_abbreviation(at)
6670
at.in_time_zone(reference_zone).strftime('%Z')
6771
end

lib/redmine_issue_datetime/drift_check.rb

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,11 @@ def initialize(zone: RedmineIssueDatetime.reference_zone)
3030

3131
# Every disagreement, ordered by issue then field.
3232
#
33-
# The scan is batched so a large instance never loads every row at once, and
34-
# deliberately unordered: find_each batches by primary key and silently
35-
# ignores an order clause (raising outright when
36-
# ActiveRecord.error_on_ignored_order is set). The findings are the problems,
37-
# so there are few of them, and sorting those at the end is both cheap and
38-
# actually honoured.
33+
# The scan is batched so a large instance never loads every row at once.
34+
# It is deliberately unordered: find_each batches by primary key and
35+
# silently ignores an order clause (or raises when
36+
# ActiveRecord.error_on_ignored_order is set). Sorting the few findings
37+
# at the end is cheap and, unlike an order clause here, actually applied.
3938
def findings
4039
results = []
4140
IssueDatetime.includes(:issue).find_each(batch_size: 500) do |record|

0 commit comments

Comments
 (0)