Skip to content
This repository has been archived by the owner on Feb 12, 2022. It is now read-only.

Scheduler: Weekdays frequency should be 'weekly' instead of 'daily' #1627

Merged
merged 8 commits into from
Dec 4, 2015
Merged

Conversation

nmerchant
Copy link
Contributor

When selecting 'Weekdays' as the repeat option of the scheduler control, the recurrencePattern is as follows:

FREQ=DAILY;BYDAY=MO,TU,WE,TH,FR;INTERVAL=1

when it should be:

FREQ=WEEKLY;BYDAY=MO,TU,WE,TH,FR;INTERVAL=1

This can be verified against the recurrencePattern that gets set if you choose 'Weekly' as the repeat option, and manually select each week day, as these are functionally identical.

@nmerchant nmerchant changed the title Weekdays frequency should be 'weekly' instead of 'daily' Scheduler: Weekdays frequency should be 'weekly' instead of 'daily' Nov 23, 2015
@@ -554,14 +554,10 @@
}

if (recur.FREQ === 'DAILY') {
if (recur.BYDAY === 'MO,TU,WE,TH,FR') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please allow "old API" to work here. We can add a comment:

// Weekdays should be set farther along in this conditional when frequency is `WEEKLY`, but in order to maintain backwards compatibility with an older version of `setValue` the following few lines set the UI to weekdays when frequency is `DAILY` also.

@interactivellama
Copy link
Contributor

Please note this could be a breaking change if you are expecting the old FREQ=DAILY;BYDAY=MO,TU,WE,TH,FR;INTERVAL=1 when the Repeat: Weekdays is selected.

@interactivellama
Copy link
Contributor

@dkilgore Does this change of output RRULE string cause any issue with your team?

@kevinparkerson kevinparkerson self-assigned this Dec 3, 2015
kevinparkerson pushed a commit that referenced this pull request Dec 4, 2015
Scheduler: Weekdays frequency should be 'weekly' instead of 'daily'
@kevinparkerson kevinparkerson merged commit d7de469 into ExactTarget:master Dec 4, 2015
@kevinparkerson
Copy link
Contributor

All checks out! Thanks @nmerchant

interactivellama added a commit to interactivellama/fuelux that referenced this pull request Dec 28, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants