search

openactive / developer-documentation

Developer documentation synced with GitBook and the Data Models
https://developer.openactive.io/
MIT License
0 stars 3 forks source link

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

Open lukehesluke opened 3 years ago

lukehesluke commented 3 years ago

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 (https://github.com/openactive/data-model-validator/pull/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 (https://github.com/openactive/data-model-validator/pull/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 commented 3 years ago

Yes good spot! See https://github.com/openactive/modelling-opportunity-data/issues/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 https://github.com/openactive/data-model-validator/pull/362#issuecomment-780506121)

lukehesluke commented 3 years ago

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

i.e.

Options:

  • Daily repeatFrequency (e.g. P1D)
  • Weekly repeatFrequency (e.g. P2W). byDay is required
  • ...etc
lukehesluke commented 3 years ago

I have updated the initial comment to say as much