VAF.Extensions.Community icon indicating copy to clipboard operation
VAF.Extensions.Community copied to clipboard

Improvement: ScheduledExecution DayOfMonthTrigger - allow variable day of month (first saturday, second monday, etc.)

Open william-deeken opened this issue 10 months ago • 2 comments

Allow users to set a DayOfMonthTrigger that specifies the first, second, third, etc. day of week (monday, tuesday, etc.) instead of just a static date.

Changes would include:

  1. Updating DayOfMonthTrigger to have a new data member "Date Type" with select choices "Variable Date" or "Specific Date".
  2. The "Specific Date" selection would cause this trigger configuration to behave as is currently does. User has the ability to choose Trigger Days and Trigger Times.
  3. The "Variable Date" choice will hide Trigger Days configuration and instead show "Nth Weekday", a value between 1-5 inclusive, and Weekday (Mon-Sun).
  4. New methods DayOfMonthTrigger.GetVariableDateOfMonth, DayOfMonthTrigger.GetNextMonthWithValidVariableDate to handle getting the current month's next run time and the next valid runtime after the current month.
  5. Update existing methods for new trigger type.

This was needed internally, so the code updates/tests have already been completed, but I figured I'd bring it up as an issue in case the maintainers thought it was something worthy to include in the public release, and then I can initiate a pull request.

repo: https://github.com/william-deeken/VAF.Extensions.Community/tree/Schedule_VariableDayOfMonth

The attached spreadsheet includes all the new test cases I devised for the various methods. These test cases have all been added to the MFiles.VAF.Extensions.Tests project.

VariableDate_TestCases.xlsx

william-deeken avatar Mar 20 '25 20:03 william-deeken

Please submit it as a PR. I really like the concept.

I had a very quick look through the repo and the only thing that caught my eye was ensuring that the day type functionality is backwards-compatible. Whilst the enum for the older approach is zero, so should be backwards compatible, I noticed at least that the ShowWhen value for the trigger days explicitly looks for the specific date value which - I think - would not be set if upgrading... If I'm right then this would mean that pulling this in would effectively hide that value until the user explicitly set the day type back to specific date.

It's worth testing. I think the fix is simply to see whether it's != to VariableDate instead of == "SpecificDate" regardless.

FYI: When you create the PR I will forward it onto others in the team as it's unlikely to be me now who accepts these.

CraigHawker avatar Mar 21 '25 15:03 CraigHawker

Thanks, Craig. You were right about the ShowWhen on the TriggerDays property. I had to replace == "SpecificDate" with (!.value || .value != "VariableDate" ) to allow existing TriggerDays values to appear after an app update. I've made these updates and created a pull request.

william-deeken avatar Mar 24 '25 21:03 william-deeken