cc: add cron tab trigger

[SoggySaussages]

This PR adds a cron tab trigger to CCs. It differs from intervals by having all the more customizability of your cron tab, for instance: run on the hour and quarter to: 0,45 * * * * or simply to allow scheduling your executions without timing a run
now, for instance run every weekday at 4:30 PM: 30 16 * * 1,2,3,4,5.

Unlike your traditional cron tab, this trigger works almost identically to the interval triggers of today, scheduling the next run
time based off the given interval criteria factoring in excluded hours and days of the week. Subsequently, there is little added
overhead to the bot.

There is a deliberate check to ensure a cron cannot be set to execute at an interval of 10 minutes or less. The following are examples of cron settings which will not save:

• * * * * *
• */1 * * * *
• */10 * * * *
• 0,1,2,3 * * * *
• 0,10 * * * *
• 0,50 * * * *
• 0,15,30,5,45 * * * *

Apart from that, functions as any other interval trigger, but without the run now button. Still must set a channel to execute in, still all the same context and behaviours of an interval trigger, excluding days and hours still works.

Thank you @l-zeuch for the head start and for the challenge :)

Signed-off-by: SoggySaussages [email protected]

[SoggySaussages]

Luca suggested a random smear so that if a lot of CCs across the shard overlap they dont overload the server. Thoughts on necessity?

[SoggySaussages]

While trying to search for the reason the interval tests are failing, I fixed a few frontend issues. I failed to find why the tests
are failing, and manual testing rules out the issues that the failed tests would indicate are present. I believe I am done
fiddling now so please feel free to review without risk that I'll push more changes. I would appreciate input as to why the
tests are failing but the behaviour is as expected.

[SoggySaussages]

Scratch that, want to make one more change for optimization

[SoggySaussages]

Scratch that, want to make one more change for optimization

Nevermind, it's perfect just the way it is

[jo3-l]

Perhaps it'd be more straightforward and/or reliable to reuse the parser from robfig/cron instead of ad-hoc string splitting. What do you think about something along the lines of (untested):

sched, err := cron.NewParser(cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow).Parse(dbModel.TextTrigger)
if err != nil {
	return templateData, nil // invalid cron expression; should not get here
}
spec, ok := sched.(*cron.SpecSchedule)
if !ok {
	return templateData, nil // we don't enable constant delay schedules in parser; should not get here
}

var mins []int
for min := range 60 {
	if spec.Minute&(1<<min)) != 0 {
		mins = append(mins, min)
	}
}
if len(mins) == 0 {
	return templateData, nil
}

// ...check mins as usual?

[SoggySaussages]

I got ChatGPT to explain this to me and still don’t understand it, so I’m not sure “straightforward” is the word I’d use. If Des agrees this is the best solution, please PR against my branch or wait until this PR is merged and open a new one. What’s here now has worked in all my testing, if you can provide an instance where it would not work that would be helpful.

[SoggySaussages]

I've implemented your strategy instead of the string parsing. Thank you for the suggestion and snippet.

[jo3-l]

Do we need to support these options for cron triggers? The same behavior can be achieved by directly adjusting the cron expression. It seems as though it complicates the implementation just a bit here and duplicates functionality, so I'm wondering how easy it'd be to, say, simply hide these options from the user interface.

[SoggySaussages]

Deliberate effort was made to show these options so as not to deviate too far from the user experience of interval triggers. If anyone is just using them for instance to run every 15 minutes starting on the hour, but wants to exclude 8 hours and weekends, it’s still easier to use a 0,15,30,45 * * * * and exclude the hours and days with the dropdowns. Open to further opinions.

[ashishjh-bst]

This fails the existing test cases for interval CCs

--- FAIL: TestNextRunTimeBasic (0.00s)
    interval_test.go:21: next run should be now:  2024-11-24 13:50:28.396418594 +0000 UTC , not:  0001-01-01 00:00:00 +0000 UTC
    interval_test.go:30: incorrect next run, should be:  0001-01-01 00:05:00 +0000 UTC , got:  0001-01-01 00:00:00 +0000 UTC
--- FAIL: TestNextRunTimeExcludingHours (0.00s)
    interval_test.go:78: next run should be now:  0001-01-01 02:05:00 +0000 UTC , got:  0001-01-01 00:00:00 +0000 UTC
--- FAIL: TestNextRunTimeExcludingDays (0.00s)
    interval_test.go:94: next run should be now:  0001-01-02 00:00:00 +0000 UTC , got:  0001-01-01 00:00:00 +0000 UTC Monday
--- FAIL: TestNextRunTimeExcludingDaysHours (0.00s)
    interval_test.go:113: next run should be: 0001-01-03 01:00:00 +0000 UTC (w:3) got 0001-01-01 00:00:00 +0000 UTC (w:1 - 0)
FAIL
FAIL	github.com/botlabs-gg/yagpdb/v2/customcommands	0.296s

[SoggySaussages]

I failed to find why the tests
are failing, and manual testing rules out the issues that the failed tests would indicate are present. I would appreciate input as to why the
tests are failing but the behaviour is as expected.

[ashishjh-bst]

Alright, I'll check into this tomorrow.

…

On Sun, 24 Nov, 2024, 8:52 pm Veda Maharaj, ***@***.***> wrote: I failed to find why the tests are failing, and manual testing rules out the issues that the failed tests would indicate are present. I would appreciate input as to why the tests are failing but the behaviour is as expected. — Reply to this email directly, view it on GitHub <#1766 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AMJ55RWDVCBDQQF2YDAHG3L2CHVM5AVCNFSM6AAAAABR43VXCWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIOJWGA3DEMZXGI> . You are receiving this because you commented.Message ID: ***@***.***>

[ashishjh-bst]

I failed to find why the tests
are failing, and manual testing rules out the issues that the failed tests would indicate are present. I would appreciate input as to why the
tests are failing but the behaviour is as expected.

@SoggySaussages these are failing because the test cases don't specify a trigger type in the CC object, which then defaults to 0 and returns the same value as now for tNext, setting trigger type explicitly in the cc object will run the test cases successfully,

Additionally please add test cases for cron too.

[SoggySaussages]

I'm trying to add cron test cases but it is much harder since cron does not calculate off last run, it calculates off current time.

[SoggySaussages]

Tests are now functional and boasting a 99.583% accuracy.