Feature 2207 split birthday anniversary reports #2214
Conversation
| const birthdayReportUrl = "/services/reports/birthdays"; | ||
| const celebrationsToday = "/services/today"; | ||
|
|
||
| export const getAnniversary = async (month, cookie) => { |
There was a problem hiding this comment.
I had to create the branch for this PR from the feature-2209-birthdays-bug branch because I needed those fixes for the changes to work. So some of the changes here are really from that branch.
| import React, { useContext, useEffect, useRef, useState } from "react"; | ||
|
|
||
| import { postEmployeeHours } from "../../api/hours"; | ||
| import {reportAllMembersCsv} from "../../api/member" |
There was a problem hiding this comment.
Most of the changes here are from Prettier. I'll add more comments to describe all the non-Prettier changes.
| const links = [] | ||
|
|
||
| if (selectHasBirthdayAnniversaryReportPermission(state)) { | ||
| links.push(["/birthday-anniversary-reports", "Birthdays & Anniversaries"]); |
There was a problem hiding this comment.
This link was replaced by two links below, one for Birthdays and one for Anniversaries.
web-ui/src/components/menu/Menu.jsx
Outdated
| links.push(["/checkins-reports", "Check-ins"]); | ||
| } | ||
| if (selectHasBirthdayAnniversaryReportPermission(state)) { | ||
| links.push(["/anniversary-reports", "Anniversaries"]); |
There was a problem hiding this comment.
These are the two new links.
All remaining changes in this file are from Prettier.
|
|
||
| import { AppContext } from "../../context/AppContext"; | ||
|
|
||
| import BirthdayAnniversaryReportPage from "../../pages/BirthdayAnniversaryReportPage"; |
There was a problem hiding this comment.
This route is replaced by two new routes below.
|
|
||
| const SearchBirthdayAnniversaryResults = ({ | ||
| hasSearched, | ||
| searchBirthdayResults, |
There was a problem hiding this comment.
Instead of taking two arrays of results, this now only takes one because we no longer operate on both anniversaries AND birthdays.
| searchBirthdayResults, | ||
| searchAnniversaryResults, | ||
| results, | ||
| anniversary, |
There was a problem hiding this comment.
anniversary and birthday are boolean arguments that indicate which kind of data is described by the results array. I could replace these with a single boolean parameter, but then callers would have to explicitly pass true or false instead of just using those as attributes with no value.
| const bdate = new Date(b.birthDay); | ||
| return adate - bdate; | ||
| }); | ||
| if (anniversary) { |
There was a problem hiding this comment.
We sort the array differently based on whether the items represent anniversaries or birthdays.
| import { Button, TextField } from "@mui/material"; | ||
| import Autocomplete from "@mui/material/Autocomplete"; | ||
|
|
||
| import "./BirthdayAnniversaryReportPage.css"; |
There was a problem hiding this comment.
This file was renamed to indicate that it is only used for anniversaries, not birthdays.
|
|
||
| import "./BirthdayAnniversaryReportPage.css"; | ||
|
|
||
| import { getAnniversary, getBirthday } from "../api/birthdayanniversary"; |
There was a problem hiding this comment.
Everything related to birthdays was removed from this file.
| Run Search | ||
| </Button> | ||
| </div> | ||
| <div className="checkbox-row"> |
There was a problem hiding this comment.
We no longer need these checkboxes since this page only deals with anniversaries.
| @@ -1,16 +1,16 @@ | |||
| import React from "react"; | |||
There was a problem hiding this comment.
This test file was renamed to indicate that it tests the new AnniversaryReportPage component.
| @@ -0,0 +1,110 @@ | |||
| import React, { useContext, useState } from "react"; | |||
There was a problem hiding this comment.
This file is like AnniversaryReportPage, but deals with birthdays instead of anniversaries.
| @@ -0,0 +1,16 @@ | |||
| import React from "react"; | |||
There was a problem hiding this comment.
This new test file tests the new BirthdayReportPage component.
| @@ -138,30 +138,6 @@ exports[`renders correctly 1`] = ` | |||
| /> | |||
| </button> | |||
| </div> | |||
| <div | |||
There was a problem hiding this comment.
This snapshot file was updated to remove the row of checkboxes that we no longer render.
| @@ -0,0 +1,156 @@ | |||
| // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html | |||
There was a problem hiding this comment.
New snapshot file for new BirthdayReportPage component.
vhscom
left a comment
vhscom
left a comment
There was a problem hiding this comment.
LGTM. Left one suggestion.
|
|
||
| const AnniversaryMap = () => { | ||
| if (searchAnniversaryResults.length > 0) { | ||
| if (anniversary && results.length > 0) { |
There was a problem hiding this comment.
Nice to have change suggestion:
results.length > 0To simply:
results.lengthThis works because results.length is truthy when the number of elements in the array is > 0. This will work for all instances where we check .length > 0, is idiomatic and a little more compact.
There was a problem hiding this comment.
I also prefer removing > 0. That was old code, but I'm making those changes everywhere that is done in this file.
No description provided.