mirror of
https://github.com/discourse/discourse.git
synced 2026-08-11 02:59:07 +08:00
de8f59edf5Cleaning up what @Lillinator was working on over here: https://github.com/discourse/discourse/pull/39917 Description: Context / Bug: Currently, when an admin sets a monthly recurring automation, the script attempts to calculate the "Nth Weekday" (e.g., "The 3rd Tuesday") using BYDAY instead of the actual calendar date. This causes two major issues for administrators: 1. Shifting Dates: Users expect a monthly report to run on the exact same calendar date (e.g., the 15th). Instead, the date shifts back and forth depending on when the "3rd Tuesday" or "2nd Friday" falls in the following month. 2. Silent Failures: If an admin sets a start date on the 30th or 31st of a month (e.g., the 5th Tuesday), the automation generates a rule like BYDAY=5TU. Months that only have 4 Tuesdays will completely fail to find a valid execution date and silently skip the entire month. Changes: * Replaced the BYDAY string calculation with BYMONTHDAY=#{start_date.day}. Monthly schedules will now reliably trigger on the exact calendar date matching user expectations. * Removed the now-unused count while-loop that was calculating the weekday occurrence. * Minor cleanup: Swapped Time.now for Time.zone.now in the RRule bounds check for standard Rails timezone consistency. Test Updates: * Updated two existing assertions in recurring_spec.rb. The original tests were explicitly (and incorrectly) expecting the buggy Nth-weekday drift. I corrected the assertion dates to expect the proper exact calendar day. Files Touched: * plugins/discourse-automation/lib/discourse_automation/triggers/recurring.rb * plugins/discourse-automation/spec/lib/discourse_automation/triggers/recurring_spec.rb
584 lines
18 KiB
Ruby
Vendored
584 lines
18 KiB
Ruby
Vendored
# 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-01 07:30:00 UTC"),
|
||
)
|
||
end
|
||
|
||
it "clamps to the end of the month if the target month is too short" do
|
||
automation.upsert_field!(
|
||
"start_date",
|
||
"date_time",
|
||
{ value: Time.parse("2023-01-31 07:30:00 UTC") },
|
||
target: "trigger",
|
||
)
|
||
automation.upsert_field!(
|
||
"recurrence",
|
||
"period",
|
||
{ value: { interval: 1, frequency: "month" } },
|
||
target: "trigger",
|
||
)
|
||
|
||
freeze_time(Time.parse("2023-01-31 07:30:00.141775363 UTC")) { automation.trigger! }
|
||
|
||
pending_automations = automation.reload.pending_automations
|
||
expect(pending_automations.first.execute_at).to eq_time(
|
||
Time.parse("2023-02-28 07:30:00 UTC"),
|
||
)
|
||
end
|
||
|
||
it "rejects a zero interval" do
|
||
expect {
|
||
automation.upsert_field!(
|
||
"recurrence",
|
||
"period",
|
||
{ value: { interval: "0", frequency: "month" } },
|
||
target: "trigger",
|
||
)
|
||
}.to raise_error(ActiveRecord::RecordInvalid)
|
||
end
|
||
|
||
it "rejects a negative interval" do
|
||
expect {
|
||
automation.upsert_field!(
|
||
"recurrence",
|
||
"period",
|
||
{ value: { interval: "-1", frequency: "month" } },
|
||
target: "trigger",
|
||
)
|
||
}.to raise_error(ActiveRecord::RecordInvalid)
|
||
end
|
||
|
||
it "preserves the selected day when it is the last day of February" do
|
||
automation.upsert_field!(
|
||
"start_date",
|
||
"date_time",
|
||
{ value: Time.parse("2023-02-28 07:30:00 UTC") },
|
||
target: "trigger",
|
||
)
|
||
automation.upsert_field!(
|
||
"recurrence",
|
||
"period",
|
||
{ value: { interval: 1, frequency: "month" } },
|
||
target: "trigger",
|
||
)
|
||
|
||
freeze_time(Time.parse("2023-02-28 07:30:00.141775363 UTC")) { automation.trigger! }
|
||
|
||
pending_automations = automation.reload.pending_automations
|
||
expect(pending_automations.first.execute_at).to eq_time(
|
||
Time.parse("2023-03-28 07:30:00 UTC"),
|
||
)
|
||
end
|
||
|
||
it "preserves the selected day when it is the last day of April" do
|
||
automation.upsert_field!(
|
||
"start_date",
|
||
"date_time",
|
||
{ value: Time.parse("2024-04-30 07:30:00 UTC") },
|
||
target: "trigger",
|
||
)
|
||
automation.upsert_field!(
|
||
"recurrence",
|
||
"period",
|
||
{ value: { interval: 1, frequency: "month" } },
|
||
target: "trigger",
|
||
)
|
||
|
||
freeze_time(Time.parse("2024-04-30 07:30:00.141775363 UTC")) { automation.trigger! }
|
||
|
||
pending_automations = automation.reload.pending_automations
|
||
expect(pending_automations.first.execute_at).to eq_time(
|
||
Time.parse("2024-05-30 07:30:00 UTC"),
|
||
)
|
||
end
|
||
|
||
it "preserves the original day across a real clamped execution" do
|
||
automation.upsert_field!(
|
||
"start_date",
|
||
"date_time",
|
||
{ value: Time.parse("2023-01-31 07:30:00 UTC") },
|
||
target: "trigger",
|
||
)
|
||
automation.upsert_field!(
|
||
"recurrence",
|
||
"period",
|
||
{ value: { interval: 1, frequency: "month" } },
|
||
target: "trigger",
|
||
)
|
||
|
||
freeze_time(Time.parse("2023-01-31 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-02-28 07:30:00 UTC"),
|
||
)
|
||
|
||
freeze_time(Time.parse("2023-02-28 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-03-31 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(
|
||
2.days.from_now,
|
||
)
|
||
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(
|
||
2.hours.from_now,
|
||
)
|
||
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-04 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.days)
|
||
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(
|
||
1.hour.from_now.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(
|
||
1.minute.from_now.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
|
