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.

Sign up for GitHub

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

Jump to bottom

365 - schedule validation #383

Open
wants to merge 33 commits into
base: master
Choose a base branch
from
Open

365 - schedule validation #383

wants to merge 33 commits into from

Conversation

Lathrisk
Copy link

@Lathrisk Lathrisk commented Mar 18, 2021 •
edited
Loading

#365

This PR covers the issues listed in the ticket above.

@Lathrisk Lathrisk self-assigned this Mar 18, 2021
@Lathrisk Lathrisk added the schedule-related Ticket relates to the correct and consistent indication of a time when an Event is occurring. label Mar 18, 2021
@Lathrisk Lathrisk force-pushed the 365/schedule-validation branch from b6f8b70 to ed6cd2d Compare March 18, 2021 16:04
@nickevansuk
Copy link
Contributor

Will take a look @Lathrisk

One quick one: are you able to refactor repeatCount to use minValueInclusive, which should already be in the model files (#378)

@Lathrisk Lathrisk force-pushed the 365/schedule-validation branch from 09bf9de to 00faf5f Compare March 19, 2021 14:22
@Lathrisk Lathrisk force-pushed the 365/schedule-validation branch from 1d31b77 to 8d48385 Compare March 24, 2021 16:50
@Lathrisk Lathrisk marked this pull request as draft March 24, 2021 16:51
@Lathrisk Lathrisk force-pushed the 365/schedule-validation branch 4 times, most recently from c5418ff to 6613cdd Compare March 26, 2021 14:28
nickevansuk
nickevansuk requested changes Mar 30, 2021
View reviewed changes
Copy link
Contributor

@nickevansuk nickevansuk left a comment

Choose a reason for hiding this comment

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

A few comments on the work so far - great progress!

src/rules/format/repeatcount-format-rule.js Outdated Show resolved Hide resolved
src/rules/data-quality/schedule-exceptdates-match-recurrence-dates.js Outdated Show resolved Hide resolved
src/rules/data-quality/schedule-contains-recurrence-data-rule.js Outdated Show resolved Hide resolved
src/rules/data-quality/schedule-contains-recurrence-data-rule.js Show resolved Hide resolved
src/rules/data-quality/schedule-contains-recurrence-data-rule.js Outdated Show resolved Hide resolved
src/rules/data-quality/schedule-contains-recurrence-data-rule.js Outdated Show resolved Hide resolved
src/rules/data-quality/schedule-contains-recurrence-data-rule.js Outdated Show resolved Hide resolved
src/rules/data-quality/scheduletimezone-in-partialschedule-rule.js Outdated Show resolved Hide resolved
@Lathrisk Lathrisk force-pushed the 365/schedule-validation branch 2 times, most recently from 9939ac5 to 1d8088e Compare March 31, 2021 10:57
@Lathrisk Lathrisk linked an issue Apr 1, 2021 that may be closed by this pull request
Schedule validation #365
Open
@Lathrisk Lathrisk marked this pull request as ready for review April 1, 2021 09:56
@Lathrisk Lathrisk requested a review from nickevansuk April 1, 2021 09:58
@Lathrisk Lathrisk marked this pull request as draft April 1, 2021 15:39
Jared Parnell added 15 commits April 1, 2021 16:40
data-quality: schedule-templates: Add ensure {startDate} rule
be6d53d
data-quality: Add rule to check for recurrence data in schedule
b0ff0fb
data-quality: schedule-exception-dates: Add rule that checks if excep…
8d6707a 
…tion dates are outside the recurrence rule
data-helpers: refactor tests to use generateRRuleOptions
c397c08
data-helpers: Remove old logging
876e0f0
helpers: Separate concerns between helper files
5bc8b22
data-quality: repeatCount rule now superseded by minValueInclusive ru…
0402176 
…le (and data type rule)
data-quality: schedules: Update messages and rule tests
76ea071
rebase: Resolve duplicate
e1312f8
data-quality: recurrence rule: Update tests to include more information
7d4b784
data-quality: Update exception date rule test message
d729577
validation types: Correct case for consistency
d9ec6f0
data-quality: schedule: Exception dates error type update
4c7f8a9
data-quality: schedule: Check scheduleEventType is a valid event subC…
4bb1e75 
…lass
data-quality: schedule: Catch errors when the model or subclassgraph …
95c782f 
…is unknown
@Lathrisk Lathrisk force-pushed the 365/schedule-validation branch from 7525bda to 95c782f Compare April 1, 2021 15:40
@Lathrisk Lathrisk marked this pull request as ready for review April 8, 2021 16:05
helpers: Update datetime function
dc3ee61 
Remove the process.env.TZ check - we explicitly set this
Add a comment about setting this value
@Lathrisk Lathrisk force-pushed the 365/schedule-validation branch from 1a14594 to dc3ee61 Compare April 8, 2021 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thill-odi thill-odi Awaiting requested review from thill-odi

@nickevansuk nickevansuk Awaiting requested review from nickevansuk

Assignees

@Lathrisk Lathrisk

Labels
schedule-related Ticket relates to the correct and consistent indication of a time when an Event is occurring.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Schedule validation
2 participants
@Lathrisk @nickevansuk