-
Notifications
You must be signed in to change notification settings - Fork 1
CMS-1290: Create winter seasons and implement winter season's form #383
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
base: alpha
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for winter seasons in the BC Parks Staff Portal. It separates winter fee date management from regular season operations by introducing a dedicated winter season type that runs parallel to regular seasons.
Key Changes:
- Adds winter season tracking and management alongside regular seasons
- Creates backend script to migrate Winter fee DateRanges from regular to winter seasons
- Updates UI to display and edit winter season dates separately
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
frontend/src/router/pages/EditAndReview.jsx |
Updates to handle separate regular and winter season objects in park data structure |
frontend/src/components/SeasonForms/ParkSeasonForm.jsx |
Adds winter season form section with dedicated date range management |
frontend/src/components/ReadyToPublishBox.jsx |
Extends component to support both regular and winter season types with conditional rendering |
frontend/src/components/FormPanel.jsx |
Integrates winter season data handling in form save/submit logic |
frontend/src/components/EditAndReviewTable.jsx |
Updates table to display winter season dates alongside regular season dates |
backend/tasks/create-winter-seasons/create-winter-seasons.js |
New script to create winter seasons and migrate Winter fee DateRanges |
backend/tasks/create-winter-seasons/README.md |
Documentation for the winter season creation script |
backend/routes/api/seasons.js |
Adds winter season data fetching and refactors save logic into reusable function |
backend/routes/api/publish.js |
Adds seasonType to query results for approved seasons |
backend/routes/api/parks.js |
Updates park data structure to return separate regular and winter seasons |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| // build a current season object | ||
| function buildCurrentSeasonOutput(seasons) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is always called but the value is only conditionally used in the output. The call should be moved inside the condition.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What condition did you mean?
|
The PR title somewhat downplays the magnitude of this change. Can we change it to something like "Implement and enable winter seasons" |
d79d59a to
e6bf070
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
b5878e9 to
cd19e7b
Compare
duncan-oxd
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work! I didn't get a chance to review it all today, but I left a few comments about potential optimizations to consider 👍
| tempId = false, | ||
| seasonType = "regular", | ||
| ) { | ||
| const seasonData = seasonType === "winter" ? winterSeason : season; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's create a constants file for seasonType to have these "winter" and "regular" strings. I could imagine us changing it to a numerical ID in the future (and/or adding more special kinds of seasons for Tier 1/2, etc.) So it would be great to have this as constant variables from the start 👍
| if (publishable?.type === "park") { | ||
| displayOperatingYear = | ||
| season.seasonType === "winter" | ||
| ? `${season.operatingYear} Winter fee` | ||
| : `${season.operatingYear} Tiers and gate`; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might make sense to move this logic into the template. It's fine to leave it in this case too, because it's only used in one place/file, but we'd need it in the template if we ever have to wrap "Tiers and gate" in a <b> tag, for example. It's no problem if we leave it here for this branch though 👍
| // get all date ranges from seasons | ||
| function getAllDateRanges(seasons) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is that comment no longer accurate? We should have some kind of comment to document what the function does
backend/routes/api/parks.js
Outdated
| const regularSeasons = park.seasons.filter( | ||
| (season) => season.seasonType === "regular", | ||
| ); | ||
| const winterSeasons = park.seasons.filter( | ||
| (season) => season.seasonType === "winter", | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you ever need to split a collection into two, you can use lodash's partition function to do that, and then you don't have to write two expressions and loop over the collection twice
| - Finds all parks that have at least one active feature | ||
| - Ensures each park has required `publishableId` and `dateableId` (creates them if missing) | ||
| - Creates a new Season record for the specified operating year | ||
| - Creates blank Tier 2 date ranges for parks that had them in the previous year |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of scope for this ticket, but we should probably eventually move this logic into the "create blank dates" script(s), now that we have dedicated scripts to do that
| // Get the Winter fee DateType | ||
| const winterFeeDateType = await DateType.findOne({ | ||
| where: { | ||
| name: "Winter fee", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some environments still have two date types with the name "Winter fee" - (a park-level one and an area/feature-level one). We can probably clean that up by updating the "parklevel" and "featurelevel" values and deleting the extra record, but this query with no WHERE might cause problems until then 😬
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The extra record (id = 13) should be gone from all environments now.
cd19e7b to
62454b3
Compare
Jira Ticket
CMS-1080
CMS-1290
CMS-1357
CMS-1359
Description
Backend: Season Model and API Enhancements
seasonTypefield to the season model and API responses, allowing seasons to be distinguished as either "regular" or "winter". This enables the system to treat winter operations separately from regular park operations.seasonType, and to provide separate grouped date ranges for winter seasons (winterGroupedDateRanges).Backend: Winter Season Creation Script
create-winter-seasons.jswith documentation) that:Frontend: Display and Prop Types Updates
Backend: Ready-to-Publish API Improvements
seasonTypein its response and to display operating years differently for winter seasons (e.g., "2027 Winter fee").