mirror of
https://github.com/discourse/discourse.git
synced 2024-12-21 19:16:51 +08:00
4406bbb020
Under certain conditions, a recurring automation can end up in a state with no pending automation records, causing it to not execute again until manually triggered.
We use the `RRule` gem to calculate the next execution date and time for recurring automations. The gem takes the interval, frequency, start date, and a time range, and returns all dates/times within this range that meet the recurrence rule. For example:
```ruby
RRule::Rule
.new("FREQ=DAILY;INTERVAL=1", dtstart: Time.parse("2023-01-01 07:30:00 UTC"))
.between(Time.zone.now, Time.zone.now + 2.days)
# => [Sat, 14 Sep 2024 07:30:00.000000000 UTC +00:00, Sun, 15 Sep 2024 07:30:00.000000000 UTC +00:00]
```
However, if the time component of the first point provided to `.between()` is slightly ahead of the start date (e.g., `dtstart`), the first date/time returned by `RRule` can fall outside the specified range by the same subsecond amount. For instance:
```ruby
RRule::Rule
.new("FREQ=DAILY;INTERVAL=1", dtstart: Time.parse("2023-01-01 07:30:00 UTC"))
.between(Time.parse("2023-01-01 07:30:00.999 UTC"), Time.parse("2023-01-03 07:30:00 UTC"))
.first
# => Sun, 01 Jan 2023 07:30:00.000000000 UTC +00:00
```
Here, the start date/time given to `.between()` is 999 milliseconds after 07:30:00, but the first date returned is exactly 07:30:00 without the 999 milliseconds. This causes the next recurring date to fall into the past if the automation executes within a subsecond of the start time, leading to the automation stalling.
I'm not sure why `RRule` does this, but it seems intentional judging by the source of the `.between()` method:
b9911b7147/lib/rrule/rule.rb (L28-L32)
This commit fixes the issue by selecting the first date ahead of the current time from the list returned by `RRule`, rather than the first date directly.
Internal topic: t/138045.
468 lines
15 KiB
Ruby
468 lines
15 KiB
Ruby
# frozen_string_literal: true
|
||
|
||
describe "Recurring" do
|
||
fab!(:user)
|
||
fab!(:topic)
|
||
fab!(:automation) do
|
||
Fabricate(
|
||
:automation,
|
||
trigger: DiscourseAutomation::Triggers::RECURRING,
|
||
script: "nothing_about_us",
|
||
)
|
||
end
|
||
|
||
def upsert_period_field!(interval, frequency)
|
||
metadata = { value: { interval: interval, frequency: frequency } }
|
||
automation.upsert_field!("recurrence", "period", metadata, target: "trigger")
|
||
end
|
||
|
||
it "allows manual trigger" do
|
||
triggerable = DiscourseAutomation::Triggerable.new(automation.trigger)
|
||
expect(triggerable.settings[DiscourseAutomation::Triggerable::MANUAL_TRIGGER_KEY]).to eq(true)
|
||
end
|
||
|
||
describe "scheduling next pending automations" do
|
||
context "with daily frequency" do
|
||
it "doesn't fail to schedule if the current time is within subsecond of the time component of start_date" do
|
||
automation.upsert_field!(
|
||
"start_date",
|
||
"date_time",
|
||
{ value: Time.parse("2022-11-01 07:30:00 UTC") },
|
||
target: "trigger",
|
||
)
|
||
automation.upsert_field!(
|
||
"recurrence",
|
||
"period",
|
||
{ value: { interval: 1, frequency: "day" } },
|
||
target: "trigger",
|
||
)
|
||
|
||
freeze_time(Time.parse("2023-09-13 07:30:00.141775363 UTC")) { automation.trigger! }
|
||
|
||
pending_automations = automation.reload.pending_automations
|
||
expect(pending_automations.count).to eq(1)
|
||
expect(pending_automations.first.execute_at).to eq_time(
|
||
Time.parse("2023-09-14 07:30:00 UTC"),
|
||
)
|
||
end
|
||
end
|
||
|
||
context "with weekday frequency" do
|
||
it "doesn't fail to schedule if the current time is within subsecond of the time component of start_date" do
|
||
automation.upsert_field!(
|
||
"start_date",
|
||
"date_time",
|
||
{ value: Time.parse("2024-10-01 23:59:59 UTC") },
|
||
target: "trigger",
|
||
)
|
||
automation.upsert_field!(
|
||
"recurrence",
|
||
"period",
|
||
{ value: { interval: 1, frequency: "weekday" } },
|
||
target: "trigger",
|
||
)
|
||
|
||
freeze_time(Time.parse("2024-10-01 23:59:59.141775363 UTC")) { automation.trigger! }
|
||
|
||
pending_automations = automation.reload.pending_automations
|
||
expect(pending_automations.count).to eq(1)
|
||
expect(pending_automations.first.execute_at).to eq_time(
|
||
Time.parse("2024-10-02 23:59:59 UTC"),
|
||
)
|
||
end
|
||
end
|
||
|
||
context "with weekly frequency" do
|
||
it "doesn't fail to schedule if the current time is within subsecond of the time component of start_date" do
|
||
automation.upsert_field!(
|
||
"start_date",
|
||
"date_time",
|
||
{ value: Time.parse("2024-09-15 23:59:59 UTC") },
|
||
target: "trigger",
|
||
)
|
||
automation.upsert_field!(
|
||
"recurrence",
|
||
"period",
|
||
{ value: { interval: 1, frequency: "week" } },
|
||
target: "trigger",
|
||
)
|
||
|
||
freeze_time(Time.parse("2024-09-15 23:59:59.141775363 UTC")) { automation.trigger! }
|
||
|
||
pending_automations = automation.reload.pending_automations
|
||
expect(pending_automations.count).to eq(1)
|
||
expect(pending_automations.first.execute_at).to eq_time(
|
||
Time.parse("2024-09-22 23:59:59 UTC"),
|
||
)
|
||
end
|
||
end
|
||
|
||
context "with monthly frequency" do
|
||
it "doesn't fail to schedule if the current time is within subsecond of the time component of start_date" do
|
||
automation.upsert_field!(
|
||
"start_date",
|
||
"date_time",
|
||
{ value: Time.parse("2023-10-01 07:30:00 UTC") },
|
||
target: "trigger",
|
||
)
|
||
automation.upsert_field!(
|
||
"recurrence",
|
||
"period",
|
||
{ value: { interval: 1, frequency: "month" } },
|
||
target: "trigger",
|
||
)
|
||
|
||
freeze_time(Time.parse("2023-10-01 07:30:00.141775363 UTC")) { automation.trigger! }
|
||
|
||
pending_automations = automation.reload.pending_automations
|
||
expect(pending_automations.count).to eq(1)
|
||
expect(pending_automations.first.execute_at).to eq_time(
|
||
Time.parse("2023-11-05 07:30:00 UTC"),
|
||
)
|
||
end
|
||
end
|
||
|
||
context "with yearly frequency" do
|
||
it "doesn't fail to schedule if the current time is within subsecond of the time component of start_date" do
|
||
automation.upsert_field!(
|
||
"start_date",
|
||
"date_time",
|
||
{ value: Time.parse("2023-01-01 07:30:00 UTC") },
|
||
target: "trigger",
|
||
)
|
||
automation.upsert_field!(
|
||
"recurrence",
|
||
"period",
|
||
{ value: { interval: 1, frequency: "year" } },
|
||
target: "trigger",
|
||
)
|
||
|
||
freeze_time(Time.parse("2023-01-01 07:30:00.141775363 UTC")) { automation.trigger! }
|
||
|
||
pending_automations = automation.reload.pending_automations
|
||
expect(pending_automations.count).to eq(1)
|
||
expect(pending_automations.first.execute_at).to eq_time(
|
||
Time.parse("2024-01-01 07:30:00 UTC"),
|
||
)
|
||
end
|
||
end
|
||
end
|
||
|
||
describe "updating trigger" do
|
||
context "when date is in future" do
|
||
before { freeze_time Time.parse("2021-06-04 10:00 UTC") }
|
||
|
||
it "creates a pending trigger with execute_at set to the start_date" do
|
||
expect {
|
||
automation.upsert_field!(
|
||
"start_date",
|
||
"date_time",
|
||
{ value: 2.hours.from_now },
|
||
target: "trigger",
|
||
)
|
||
upsert_period_field!(1, "hour")
|
||
}.to change { automation.pending_automations.count }.by(1)
|
||
|
||
expect(automation.pending_automations.last.execute_at).to be_within_one_second_of(
|
||
2.hours.from_now,
|
||
)
|
||
end
|
||
end
|
||
|
||
context "when date is in past" do
|
||
it "doesn’t create a pending trigger" do
|
||
expect {
|
||
automation.upsert_field!(
|
||
"start_date",
|
||
"date_time",
|
||
{ value: 2.hours.ago },
|
||
target: "trigger",
|
||
)
|
||
}.not_to change { automation.pending_automations.count }
|
||
end
|
||
end
|
||
end
|
||
|
||
context "when updating automation" do
|
||
fab!(:automation) do
|
||
Fabricate(:automation, trigger: DiscourseAutomation::Triggers::RECURRING, script: "test")
|
||
end
|
||
|
||
before do
|
||
DiscourseAutomation::Scriptable.add("test") do
|
||
triggerables [DiscourseAutomation::Triggers::RECURRING]
|
||
field :test, component: :text
|
||
end
|
||
|
||
automation.upsert_field!(
|
||
"start_date",
|
||
"date_time",
|
||
{ value: 2.hours.from_now },
|
||
target: "trigger",
|
||
)
|
||
upsert_period_field!(1, "week")
|
||
|
||
automation.upsert_field!("test", "text", { value: "something" }, target: "script")
|
||
end
|
||
|
||
context "when interval changes" do
|
||
before { freeze_time(DateTime.parse("2024-05-23")) }
|
||
|
||
context "when start_date is in the future" do
|
||
before do
|
||
automation.upsert_field!(
|
||
"start_date",
|
||
"date_time",
|
||
{ value: 5.days.from_now },
|
||
target: "trigger",
|
||
)
|
||
end
|
||
|
||
it "recreates pending automation with execute_at set to start_date" do
|
||
upsert_period_field!(4, "week")
|
||
|
||
expect(automation.pending_automations.count).to eq(1)
|
||
expect(automation.pending_automations.last.execute_at).to be_within_one_second_of(
|
||
5.days.from_now,
|
||
)
|
||
end
|
||
end
|
||
|
||
context "when start_date is in the past" do
|
||
before do
|
||
automation.upsert_field!(
|
||
"start_date",
|
||
"date_time",
|
||
{ value: 4.days.ago },
|
||
target: "trigger",
|
||
)
|
||
end
|
||
|
||
it "recreates pending automation with execute_at set to the first occurrence date after the current time" do
|
||
upsert_period_field!(3, "day")
|
||
|
||
expect(automation.pending_automations.count).to eq(1)
|
||
expect(automation.pending_automations.last.execute_at).to be_within_one_second_of(
|
||
Time.zone.now + 2.days,
|
||
)
|
||
end
|
||
end
|
||
end
|
||
|
||
context "when frequency changes" do
|
||
before { freeze_time(DateTime.parse("2024-05-23")) }
|
||
|
||
context "when start_date is in the future" do
|
||
before do
|
||
automation.upsert_field!(
|
||
"start_date",
|
||
"date_time",
|
||
{ value: 2.hours.from_now },
|
||
target: "trigger",
|
||
)
|
||
end
|
||
|
||
it "recreates pending automation with execute_at set to start_date" do
|
||
upsert_period_field!(1, "hour")
|
||
|
||
expect(automation.pending_automations.count).to eq(1)
|
||
expect(automation.pending_automations.last.execute_at).to be_within_one_second_of(
|
||
2.hours.from_now,
|
||
)
|
||
end
|
||
end
|
||
|
||
context "when start_date is in the past" do
|
||
before do
|
||
automation.upsert_field!(
|
||
"start_date",
|
||
"date_time",
|
||
{ value: 3.days.ago },
|
||
target: "trigger",
|
||
)
|
||
end
|
||
|
||
it "recreates pending automation with execute_at set to the first occurrence date after the current time" do
|
||
upsert_period_field!(2, "hour")
|
||
|
||
expect(automation.pending_automations.count).to eq(1)
|
||
expect(automation.pending_automations.last.execute_at).to be_within_one_second_of(
|
||
Time.zone.now + 2.hour,
|
||
)
|
||
end
|
||
end
|
||
end
|
||
|
||
context "when a non recurrence related field changes" do
|
||
it "doesn't reset the pending automations" do
|
||
expect {
|
||
automation.upsert_field!("test", "text", { value: "somethingelse" }, target: "script")
|
||
}.to_not change { automation.pending_automations.last.execute_at }
|
||
expect(automation.pending_automations.count).to eq(1)
|
||
end
|
||
|
||
context "when there are no existing pending automations" do
|
||
before { automation.pending_automations.destroy_all }
|
||
|
||
it "creates a new one" do
|
||
expect {
|
||
automation.upsert_field!("test", "text", { value: "somethingelse" }, target: "script")
|
||
}.to change { automation.pending_automations.count }.by(1)
|
||
end
|
||
end
|
||
end
|
||
end
|
||
|
||
context "when trigger is called" do
|
||
before do
|
||
freeze_time Time.zone.parse("2021-06-04 10:00")
|
||
automation.fields.insert!(
|
||
{
|
||
name: "start_date",
|
||
component: "date_time",
|
||
metadata: {
|
||
value: 2.hours.ago,
|
||
},
|
||
target: "trigger",
|
||
created_at: Time.now,
|
||
updated_at: Time.now,
|
||
},
|
||
)
|
||
metadata = { value: { interval: "1", frequency: "week" } }
|
||
automation.fields.insert!(
|
||
{
|
||
name: "recurrence",
|
||
component: "period",
|
||
metadata: metadata,
|
||
target: "trigger",
|
||
created_at: Time.now,
|
||
updated_at: Time.now,
|
||
},
|
||
)
|
||
end
|
||
|
||
it "creates the next iteration" do
|
||
expect { automation.trigger! }.to change { automation.pending_automations.count }.by(1)
|
||
|
||
pending_automation = automation.pending_automations.last
|
||
|
||
start_date = Time.parse(automation.trigger_field("start_date")["value"])
|
||
expect(pending_automation.execute_at).to be_within_one_minute_of(start_date + 7.days)
|
||
end
|
||
|
||
describe "every_month" do
|
||
before { upsert_period_field!(1, "month") }
|
||
|
||
it "creates the next iteration one month later" do
|
||
automation.trigger!
|
||
|
||
pending_automation = automation.pending_automations.last
|
||
expect(pending_automation.execute_at).to be_within_one_minute_of(
|
||
Time.parse("2021-07-02 08:00:00 UTC"),
|
||
)
|
||
end
|
||
end
|
||
|
||
describe "every_day" do
|
||
before do
|
||
automation.upsert_field!(
|
||
"start_date",
|
||
"date_time",
|
||
{ value: 1.minute.from_now },
|
||
target: "trigger",
|
||
)
|
||
upsert_period_field!(1, "day")
|
||
end
|
||
|
||
it "creates the next iteration one day later" do
|
||
automation.trigger!
|
||
|
||
pending_automation = automation.pending_automations.last
|
||
start_date = Time.parse(automation.trigger_field("start_date")["value"])
|
||
expect(pending_automation.execute_at).to be_within_one_minute_of(start_date)
|
||
end
|
||
end
|
||
|
||
describe "every_weekday" do
|
||
it "creates the next iteration one day after without Saturday/Sunday" do
|
||
upsert_period_field!(1, "weekday")
|
||
automation.trigger!
|
||
|
||
pending_automation = automation.pending_automations.last
|
||
start_date = Time.parse(automation.trigger_field("start_date")["value"])
|
||
expect(pending_automation.execute_at).to be_within_one_minute_of(start_date + 3.day)
|
||
end
|
||
|
||
it "creates the next iteration three days after without Saturday/Sunday" do
|
||
now = DateTime.parse("2022-05-19").end_of_day
|
||
start_date = now - 1.hour
|
||
freeze_time now
|
||
|
||
automation.pending_automations.destroy_all
|
||
automation.upsert_field!(
|
||
"start_date",
|
||
"date_time",
|
||
{ value: start_date },
|
||
target: "trigger",
|
||
)
|
||
upsert_period_field!(3, "weekday")
|
||
|
||
automation.trigger!
|
||
|
||
pending_automation = automation.pending_automations.last
|
||
expect(pending_automation.execute_at).to be_within_one_minute_of(start_date + 5.days)
|
||
end
|
||
end
|
||
|
||
describe "every_hour" do
|
||
before { upsert_period_field!(1, "hour") }
|
||
|
||
it "creates the next iteration one hour later" do
|
||
automation.trigger!
|
||
|
||
pending_automation = automation.pending_automations.last
|
||
expect(pending_automation.execute_at).to be_within_one_minute_of(
|
||
(Time.zone.now + 1.hour).beginning_of_hour,
|
||
)
|
||
end
|
||
end
|
||
|
||
describe "every_minute" do
|
||
before { upsert_period_field!(1, "minute") }
|
||
|
||
it "creates the next iteration one minute later" do
|
||
automation.trigger!
|
||
|
||
pending_automation = automation.pending_automations.last
|
||
expect(pending_automation.execute_at).to be_within_one_minute_of(
|
||
(Time.zone.now + 1.minute).beginning_of_minute,
|
||
)
|
||
end
|
||
end
|
||
|
||
describe "every_year" do
|
||
before { upsert_period_field!(1, "year") }
|
||
|
||
it "creates the next iteration one year later" do
|
||
automation.trigger!
|
||
|
||
pending_automation = automation.pending_automations.last
|
||
start_date = Time.parse(automation.trigger_field("start_date")["value"])
|
||
expect(pending_automation.execute_at).to be_within_one_minute_of(start_date + 1.year)
|
||
end
|
||
end
|
||
|
||
describe "every_other_week" do
|
||
before { upsert_period_field!(2, "week") }
|
||
|
||
it "creates the next iteration two weeks later" do
|
||
automation.trigger!
|
||
|
||
pending_automation = automation.pending_automations.last
|
||
start_date = Time.parse(automation.trigger_field("start_date")["value"])
|
||
expect(pending_automation.execute_at).to be_within_one_minute_of(start_date + 2.weeks)
|
||
end
|
||
end
|
||
end
|
||
end
|