IntenseWebs/discourse
3
0
Fork 0
mirror of https://github.com/discourse/discourse.git synced 2025-02-25 18:55:32 -06:00
Code Issues Packages Projects Releases Wiki Activity

FIX: Preveint recurring automations from running before start_date (#26963)

Browse Source 
Some combinations of start_date and frequency/interval values can cause a recurring automation rule to either trigger before its start_date or never trigger. Example repros:

- Configure a recurring automation with hourly recurrence and a start_date several days ahead. What this will do is make the automation start running hourly immediately even though the start_date is several days ahead.

-  Configure a recurring automation with a weekly recurrence and a start_date several weeks ahead. This will result in the automation never triggering even after the start_date.

These 2 scenarios share the same cause which is that the automation plugin doesn't use the start_date as the date for the first run and instead uses the frequency/interval values from the current time to calculate the first run date.

This PR fixes this bug by adding an explicit check for start_date and using it as the first run's date if it's ahead of the current time.
This commit is contained in:
Osama Sayegh 2024-05-10 04:45:23 +03:00 committed by GitHub
parent 2a2eaf835f
commit 9ebf7c9c37
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 97 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
plugins/automation/lib/discourse_automation/triggers/recurring.rb
View File

@ -36,6 +36,11 @@ module DiscourseAutomation
end
start_date = Time.parse(start_date)
if start_date > Time.zone.now
automation.pending_automations.create!(execute_at: start_date)
return
end
byday = start_date.strftime("%A").upcase[0, 2]
interval = interval.to_i
interval_end = interval + 1

130
plugins/automation/spec/triggers/recurring_spec.rb
View File

@ -25,7 +25,7 @@ describe "Recurring" do
context "when date is in future" do
before { freeze_time Time.parse("2021-06-04 10:00 UTC") }
it "creates a pending trigger" do
it "creates a pending trigger with execute_at set to the start_date" do
expect {
automation.upsert_field!(
"start_date",
@ -34,10 +34,10 @@ describe "Recurring" do
target: "trigger",
)
upsert_period_field!(1, "hour")
}.to change { DiscourseAutomation::PendingAutomation.count }.by(1)
}.to change { automation.pending_automations.count }.by(1)
expect(DiscourseAutomation::PendingAutomation.last.execute_at).to be_within_one_minute_of(
1.hours.from_now,
expect(automation.pending_automations.last.execute_at).to be_within_one_second_of(
2.hours.from_now,
)
end
end
@ -51,7 +51,7 @@ describe "Recurring" do
{ value: 2.hours.ago },
target: "trigger",
)
}.not_to change { DiscourseAutomation::PendingAutomation.count }
}.not_to change { automation.pending_automations.count }
end
end
end
@ -78,35 +78,91 @@ describe "Recurring" do
automation.upsert_field!("test", "text", { value: "something" }, target: "script")
end
context "when start date changes" do
it "resets the pending automations" do
expect {
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: 1.day.from_now },
{ value: 5.days.from_now },
target: "trigger",
)
}.to change { DiscourseAutomation::PendingAutomation.last.execute_at }
expect(DiscourseAutomation::PendingAutomation.count).to eq(1)
end
end
end
context "when interval changes" do
it "resets the pending automations" do
expect { upsert_period_field!(2, "week") }.to change {
DiscourseAutomation::PendingAutomation.last.execute_at
}
expect(DiscourseAutomation::PendingAutomation.count).to eq(1)
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
it "resets the pending automations" do
expect { upsert_period_field!(1, "month") }.to change {
DiscourseAutomation::PendingAutomation.last.execute_at
}
expect(DiscourseAutomation::PendingAutomation.count).to eq(1)
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
@ -114,8 +170,8 @@ describe "Recurring" do
it "doesn't reset the pending automations" do
expect {
automation.upsert_field!("test", "text", { value: "somethingelse" }, target: "script")
}.to_not change { DiscourseAutomation::PendingAutomation.last.execute_at }
expect(DiscourseAutomation::PendingAutomation.count).to eq(1)
}.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
@ -124,7 +180,7 @@ describe "Recurring" do
it "creates a new one" do
expect {
automation.upsert_field!("test", "text", { value: "somethingelse" }, target: "script")
}.to change { DiscourseAutomation::PendingAutomation.count }.by(1)
}.to change { automation.pending_automations.count }.by(1)
end
end
end
@ -159,11 +215,9 @@ describe "Recurring" do
end
it "creates the next iteration" do
expect { automation.trigger! }.to change { DiscourseAutomation::PendingAutomation.count }.by(
1,
)
expect { automation.trigger! }.to change { automation.pending_automations.count }.by(1)
pending_automation = DiscourseAutomation::PendingAutomation.last
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)
@ -175,7 +229,7 @@ describe "Recurring" do
it "creates the next iteration one month later" do
automation.trigger!
pending_automation = DiscourseAutomation::PendingAutomation.last
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"),
)
@ -196,7 +250,7 @@ describe "Recurring" do
it "creates the next iteration one day later" do
automation.trigger!
pending_automation = DiscourseAutomation::PendingAutomation.last
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
@ -207,7 +261,7 @@ describe "Recurring" do
upsert_period_field!(1, "weekday")
automation.trigger!
pending_automation = DiscourseAutomation::PendingAutomation.last
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
@ -239,7 +293,7 @@ describe "Recurring" do
it "creates the next iteration one hour later" do
automation.trigger!
pending_automation = DiscourseAutomation::PendingAutomation.last
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,
)
@ -252,7 +306,7 @@ describe "Recurring" do
it "creates the next iteration one minute later" do
automation.trigger!
pending_automation = DiscourseAutomation::PendingAutomation.last
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,
)
@ -265,7 +319,7 @@ describe "Recurring" do
it "creates the next iteration one year later" do
automation.trigger!
pending_automation = DiscourseAutomation::PendingAutomation.last
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
@ -277,7 +331,7 @@ describe "Recurring" do
it "creates the next iteration two weeks later" do
automation.trigger!
pending_automation = DiscourseAutomation::PendingAutomation.last
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