Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Schedule "required options" is incorrect regarding byDay/byMonth/byMonthDay/repeatFrequency #34

Open
lukehesluke opened this issue Feb 19, 2021 · 3 comments

Comments

@lukehesluke
Copy link

lukehesluke commented Feb 19, 2021

https://developer.openactive.io/data-model/types/schedule#required-options states that

a data publisher must provide either a byDay, byMonth, byMonthDay or repeatFrequency for a schedule

This is vague and incorrect in most cases. Should the docs instead exactly describe the behaviour currently being implemented in validator in this PR (openactive/data-model-validator#362)

i.e.

Options:

  • Daily repeatFrequency (e.g. P1D)
  • Weekly repeatFrequency (e.g. P2W). byDay is required
  • ...etc

This implies that it is possible to have a repeatFrequency but no byDay, byMonth, byMonthDay. Whereas this PR (openactive/data-model-validator#362) is explicit making that invalid.

The Modelling spec also states that at least one of these other values is needed (https://openactive.io/modelling-opportunity-data/#h-note14:~:text=While%20marked%20as%20optional%2C%20publishers%20also,the%20schema%3AbyDay%2C%20schema%3AbyMonth%20or%20schema%3AbyMonthDay%20properties).

@nickevansuk
Copy link
Contributor

Yes good spot! See openactive/modelling-opportunity-data#260

Also note that the description of the PR openactive/data-model-validator#362 does not match the code behind it. The code in the PR actually allows for the "daily" case (my comment openactive/data-model-validator#362 (comment))

@lukehesluke
Copy link
Author

Thanks @nickevansuk. So perhaps the ideal would be documentation that exactly maps onto the behaviour currently being implemented in validator in this PR (openactive/data-model-validator#362).

i.e.

Options:

  • Daily repeatFrequency (e.g. P1D)
  • Weekly repeatFrequency (e.g. P2W). byDay is required
  • ...etc

@lukehesluke
Copy link
Author

I have updated the initial comment to say as much

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants