Skip to content

CMS-1848: Add team approval CSV columns - #702

Open
duncan-oxd wants to merge 1 commit into
mainfrom
CMS-1848-dual-approval-CSV
Open

CMS-1848: Add team approval CSV columns#702
duncan-oxd wants to merge 1 commit into
mainfrom
CMS-1848-dual-approval-CSV

Conversation

@duncan-oxd

@duncan-oxd duncan-oxd commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Jira Ticket

CMS-1848

Description

This ticket adds two columns to the CSV export: "Information Services team approved" & "Reservation Services team approved".
If the date row's season is approved by that team, the column displays "Yes"
If the date row's season is not approved by the team, but the team's approval is required, it will be blank
If the date row's season is not approved by the team and the team's approval is not required, it displays "N/A"

I added some helper functions to help figure out if each team's approval is required for a given season.

While working on this and #692 we also noticed a gap in the existing logic that checks if hasGate changed from true to false. I moved that to #704 so the fix can be included in v2.5.0

I also updated the logic for the "In BC Parks Reservation system" column to also check the inReservationSystem flag on all the area features, to be consistent with the logic for getSeasonReservationCoverage. UX confirmed this just now 👍

@duncan-oxd
duncan-oxd requested review from ayumi-oxd and molund and a lite review from Copilot August 7, 2026 22:14
@duncan-oxd duncan-oxd self-assigned this Aug 7, 2026
@duncan-oxd duncan-oxd added the v2.5.0 DOOT validation changes before the ACT release label Aug 7, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Updates season approval determination to better reflect reservation-system coverage (including mixed ParkArea feature coverage) and historical gate removals, and extends the CSV export to show team-approval status per season/date row.

Changes:

  • Refactors reservation-system checks into a new getSeasonReservationCoverage helper and makes approval-resolution helpers async to support historical checks.
  • Adds hasGateRemoved(seasonId) to detect any prior hasGate: true -> false transitions via SeasonChangeLog.
  • Adds “Information Services team approved” and “Reservation Services team approved” columns to the CSV export, with “Yes” / blank / “N/A” formatting.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
backend/utils/seasonApprovalHelpers.js Adds reservation coverage helper and historical gate-removal lookup; makes approval resolution async.
backend/routes/api/seasons.js Awaits async approval resolution when saving seasons.
backend/routes/api/export.js Adds CSV columns and formatting helpers for IS/RS approval status; loads gate values in changelogs for export logic.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread backend/routes/api/export.js
Comment thread backend/routes/api/export.js
@duncan-oxd

duncan-oxd commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator Author

Notes for testing, which I'll repost in the Jira ticket for QA:

Verifying the logic in a generated CSV:

  • If the "In BC Parks Reservation system" column is "Yes", the RS approval column should either be "Yes" or blank, never "N/A" because it's required.
  • If "In BC Parks Reservation system" is false, the IS approval column should be "Yes" or blank.
  • If it's a winter date, RS should be "Yes" or blank
  • If "Has gate" is "Yes" then IS should be "Yes" or blank.
  • If "Has gate" is "No" then we don't know from that column alone if the gate was always No, or if it was changed from Yes to No, so we can't conclude anything. (The code checks the changelogs to figure this out, but the CSV doesn't have a definite indicator for this)

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (1)

backend/utils/seasonApprovalHelpers.js:38

  • For ParkArea seasons, anyNotInReservationSystem doesn’t consider the ParkArea-level inReservationSystem === false when at least one feature is true. In that mixed case, anyInReservationSystem becomes true and the current !anyInReservationSystem check becomes false, so anyNotInReservationSystem can incorrectly be false even though the ParkArea itself is explicitly not in the reservation system. This can under-require IS approval for ParkArea seasons with mixed feature coverage.
    const anyNotInReservationSystem =
      !anyInReservationSystem ||
      features.some((f) => f.inReservationSystem === false);

@duncan-oxd
duncan-oxd marked this pull request as ready for review August 8, 2026 00:08
@duncan-oxd duncan-oxd added doot-september24 DO NOT MERGE and removed v2.5.0 DOOT validation changes before the ACT release labels Aug 10, 2026
@duncan-oxd

Copy link
Copy Markdown
Collaborator Author

Added the "DO NOT MERGE" label for now, as this is for SP v2.6.0

Once it's approved, I will grab the bugfix parts and merge those into main, leaving the CSV stuff for later in Sept.

@duncan-oxd
duncan-oxd changed the base branch from main to CMS-1935-logic-fixes August 10, 2026 17:23
@duncan-oxd
duncan-oxd force-pushed the CMS-1848-dual-approval-CSV branch from 6c08518 to 1042319 Compare August 10, 2026 17:26
@duncan-oxd duncan-oxd changed the title CMS-1848: Update team approval logic, add CSV columns CMS-1848: Add team approval CSV columns Aug 10, 2026
@duncan-oxd
duncan-oxd requested a lite review from Copilot August 10, 2026 17:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (1)

backend/routes/api/export.js:106

  • gateRemoved treats a changelog as a “gate removed” whenever the old value is true and the new value is anything other than true (including null/missing). However, SeasonChangeLogs can legitimately have gateDetailNewValue: null when gateDetail isn’t supplied (e.g., backend/utils/saveSeasonData.js sets gateDetailNewValue to gateDetailToSave, which stays null when gateDetail isn’t included). This can incorrectly mark IS approval as required. To match the existing hasGateRemoved() semantics in backend/utils/seasonApprovalHelpers.js (old=true AND new=false), only treat it as removal when the new value is explicitly false.
    const oldHasGate = changeLog.gateDetailOldValue?.hasGate === true;
    const newHasGate = changeLog.gateDetailNewValue?.hasGate === true;

    return oldHasGate && !newHasGate;
  });

@duncan-oxd
duncan-oxd force-pushed the CMS-1848-dual-approval-CSV branch from 1042319 to a139700 Compare August 10, 2026 18:45
@duncan-oxd
duncan-oxd force-pushed the CMS-1848-dual-approval-CSV branch from a139700 to 8d841b6 Compare August 10, 2026 19:57
Base automatically changed from CMS-1935-logic-fixes to main August 10, 2026 20:01
CMS-1848: Add IS/RS approval status to CSV export

CMS-1848: Fix "gate removed" logic in season approval helpers

CMS-1848: Include missing area.feature.inReservationSystem

CMS-1848: Update Area-level "inReservationSystem" logic
@duncan-oxd
duncan-oxd force-pushed the CMS-1848-dual-approval-CSV branch from 8d841b6 to bca87ec Compare August 10, 2026 20:01

@ayumi-oxd ayumi-oxd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good, I didn't know the relation with a gate and IS team 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants