-
Notifications
You must be signed in to change notification settings - Fork 6
Feature 2285 member selector tenure #2324
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
Changes from all commits
87a5a52
90077a9
d0d3faa
f74a7fb
4f186b8
4638b4c
f2ff45e
b8e17ee
f166347
4fcefee
a4552a9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,7 @@ | ||
| .member-selector-dialog .custom-tenure-picker { | ||
| width: 8rem; | ||
| } | ||
|
|
||
| .member-selector-dialog .toolbar-title-container { | ||
| display: flex; | ||
| flex-direction: row; | ||
|
|
@@ -30,15 +34,15 @@ | |
| } | ||
|
|
||
| .member-selector-dialog .name-search-field { | ||
| width: 350px; | ||
| margin-right: 3rem; | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think there was too much horizontal space between the "Name" input and the "Filter Type" dropdown, so I removed this |
||
| width: 300px; | ||
| } | ||
|
|
||
| .member-selector-dialog .filter-input-container { | ||
| display: flex; | ||
| flex-direction: row; | ||
| gap: 0.5rem; | ||
| gap: 1rem; | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we need a bit more space between each of the form controls. |
||
| flex-wrap: wrap; | ||
| width: 100%; | ||
| } | ||
|
|
||
| .member-selector-dialog .direct-reports-only-checkbox { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,20 @@ | ||
| import { differenceInMonths } from 'date-fns'; | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This function is used to determine if a member has been at OCI for at least the selected tenure value. |
||
| import React, { useCallback, useContext, useEffect, useState } from 'react'; | ||
| import PropTypes from 'prop-types'; | ||
| import { | ||
| AppBar, | ||
| Autocomplete, | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I combined some imports. |
||
| Avatar, | ||
| Button, | ||
| Checkbox, | ||
| Dialog, | ||
| DialogContent, | ||
| Divider, | ||
| FormControl, | ||
| FormControlLabel, | ||
| FormGroup, | ||
| IconButton, | ||
| InputAdornment, | ||
| InputLabel, | ||
| List, | ||
| ListItem, | ||
|
|
@@ -17,16 +23,15 @@ import { | |
| ListItemText, | ||
| MenuItem, | ||
| Select, | ||
| Slide, | ||
| TextField, | ||
| Toolbar, | ||
| Typography | ||
| } from '@mui/material'; | ||
| import Slide from '@mui/material/Slide'; | ||
| import CloseIcon from '@mui/icons-material/Close'; | ||
| import InputAdornment from '@mui/material/InputAdornment'; | ||
| import SearchIcon from '@mui/icons-material/Search'; | ||
| import Autocomplete from '@mui/material/Autocomplete'; | ||
| import FormControl from '@mui/material/FormControl'; | ||
| import { DatePicker } from '@mui/x-date-pickers'; | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is used to specify a custom tenure start date. |
||
|
|
||
| import { getAvatarURL } from '../../../api/api'; | ||
| import { AppContext } from '../../../context/AppContext'; | ||
| import { | ||
|
|
@@ -47,8 +52,6 @@ import { getMembersByGuild } from '../../../api/guild'; | |
| import { getSkillMembers } from '../../../api/memberskill'; | ||
|
|
||
| import './MemberSelectorDialog.css'; | ||
| import FormControlLabel from '@mui/material/FormControlLabel'; | ||
| import Divider from '@mui/material/Divider'; | ||
|
|
||
| const DialogTransition = React.forwardRef((props, ref) => ( | ||
| <Slide direction="up" ref={ref} {...props} /> | ||
|
|
@@ -64,9 +67,29 @@ export const FilterType = Object.freeze({ | |
| MANAGER: 'Manager' | ||
| }); | ||
|
|
||
| export const Tenures = Object.freeze({ | ||
| All: 'All', | ||
| Months6: '6 Months', | ||
| Years1: '1 Year', | ||
| Years5: '5 Years', | ||
| Years10: '10 Years', | ||
| Years20: '20 Years', | ||
| Custom: 'Custom' | ||
| }); | ||
|
|
||
| const tenureToMonths = { | ||
| [Tenures.Months6]: 6, | ||
| [Tenures.Years1]: 12, | ||
| [Tenures.Years5]: 60, | ||
| [Tenures.Years10]: 120, | ||
| [Tenures.Years20]: 240, | ||
| [Tenures.Custom]: 0 | ||
| }; | ||
|
|
||
| const propTypes = { | ||
| initialFilters: PropTypes.arrayOf( | ||
| PropTypes.shape({ | ||
| tenure: PropTypes.oneOf(Object.values(Tenures)), | ||
| type: PropTypes.oneOf(Object.values(FilterType)), | ||
| value: PropTypes.oneOfType([ | ||
| PropTypes.string, | ||
|
|
@@ -108,8 +131,9 @@ const MemberSelectorDialog = ({ | |
| const [filter, setFilter] = useState(null); | ||
| const [filteredMembers, setFilteredMembers] = useState([]); | ||
| const [directReportsOnly, setDirectReportsOnly] = useState(false); | ||
|
|
||
| const [selectableMembers, setSelectableMembers] = useState([]); | ||
| const [tenure, setTenure] = useState(initialFilter?.tenure || Tenures.All); | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The initial value of the |
||
| const [customTenure, setCustomTenure] = useState(new Date()); | ||
|
|
||
| const handleSubmit = useCallback(() => { | ||
| const membersToAdd = members.filter(member => checked.has(member.id)); | ||
|
|
@@ -230,6 +254,22 @@ const MemberSelectorDialog = ({ | |
| member => !selectedMembers.includes(member) | ||
| ); | ||
|
|
||
| // Exclude members that don't have the selected tenure. | ||
| if (tenure === Tenures.Custom) { | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If a custom tenure date was selected, keep only the members whose start date was on or before that date. |
||
| filteredMemberList = members.filter(member => { | ||
| const start = new Date(member.startDate); | ||
| return start <= customTenure; | ||
| }); | ||
| } else if (tenure !== Tenures.All) { | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If a specific tenure value like "5 Years" was selected, keep only the members that have been at OCI at least that long. |
||
| const now = new Date(); | ||
| const requiredMonths = tenureToMonths[tenure]; | ||
| filteredMemberList = members.filter(member => { | ||
| const start = new Date(member.startDate); | ||
| const diffMonths = differenceInMonths(now, start); | ||
| return diffMonths >= requiredMonths; | ||
| }); | ||
| } | ||
|
|
||
| // If a filter is selected, use it to filter the list of selectable members | ||
| if (filter) { | ||
| switch (filterType) { | ||
|
|
@@ -330,15 +370,17 @@ const MemberSelectorDialog = ({ | |
| }); | ||
| } | ||
| }, [ | ||
| state, | ||
| csrf, | ||
| members, | ||
| filterType, | ||
| customTenure, | ||
| directReportsOnly, | ||
| filter, | ||
| filterType, | ||
| members, | ||
| open, | ||
| selectedMembers, | ||
| showError, | ||
| directReportsOnly, | ||
| open | ||
| state, | ||
| tenure | ||
| ]); | ||
|
|
||
| useEffect(() => { | ||
|
|
@@ -438,13 +480,32 @@ const MemberSelectorDialog = ({ | |
| ) | ||
| }} | ||
| /> | ||
| <FormControl className="filter-type-select"> | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This Select is not new. I just relabelled it to make its purpose more clear. |
||
| <InputLabel id="filter-type-label">Filter Type</InputLabel> | ||
| <Select | ||
| labelId="filter-type-label" | ||
| label="Filter Type" | ||
| value={filterType} | ||
| onChange={event => { | ||
| setFilter(null); | ||
| setFilterType(event.target.value); | ||
| }} | ||
| disabled={!!initialFilter} | ||
| > | ||
| {Object.values(FilterType).map(name => ( | ||
| <MenuItem key={name} value={name}> | ||
| {name} | ||
| </MenuItem> | ||
| ))} | ||
| </Select> | ||
| </FormControl> | ||
| <Autocomplete | ||
| className="filter-input" | ||
| renderInput={params => ( | ||
| <TextField | ||
| {...params} | ||
| variant="outlined" | ||
| label="Filter Members" | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This input is for a value of the type specified in the Select above. I think labeling it "Filter Value" instead of "Filter Members" makes this more clear. |
||
| label="Filter Value" | ||
| placeholder={`Search for ${filterType.toLowerCase()}`} | ||
| /> | ||
| )} | ||
|
|
@@ -458,53 +519,71 @@ const MemberSelectorDialog = ({ | |
| value={filter} | ||
| onChange={(_, value) => setFilter(value)} | ||
| /> | ||
| </div> | ||
| <Checkbox | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This checkbox is not new. It was just moved in the code. |
||
| className="toggle-selectable-members-checkbox" | ||
| onChange={event => handleToggleAll(event.target.checked)} | ||
| checked={ | ||
| selectableMembers.length > 0 && | ||
| visibleChecked().length === selectableMembers.length | ||
| } | ||
| indeterminate={ | ||
| visibleChecked().length > 0 && | ||
| visibleChecked().length !== selectableMembers.length | ||
| } | ||
| disabled={selectableMembers.length === 0} | ||
| /> | ||
| </FormGroup> | ||
| <FormGroup className="dialog-form-group"> | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This checkbox is not new. It was just moved in the code. |
||
| <div className="filter-input-container"> | ||
| {filterType === FilterType.MANAGER && ( | ||
| <FormControlLabel | ||
| className="direct-reports-only-checkbox" | ||
| control={ | ||
| <Checkbox | ||
| checked={directReportsOnly} | ||
| onChange={event => | ||
| setDirectReportsOnly(event.target.checked) | ||
| } | ||
| /> | ||
| } | ||
| label="Direct reports only" | ||
| /> | ||
| )} | ||
| <FormControl className="filter-type-select"> | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the new Select for specifying a desired tenure. |
||
| <InputLabel id="member-filter-label">Filter by</InputLabel> | ||
| <InputLabel id="member-filter-label">Required Tenure</InputLabel> | ||
| <Select | ||
| labelId="member-filter-label" | ||
| label="Filter by" | ||
| value={filterType} | ||
| label="Required Tenure" | ||
| value={tenure} | ||
| onChange={event => { | ||
| setFilter(null); | ||
| setFilterType(event.target.value); | ||
| const tenure = event.target.value; | ||
| setTenure(tenure); | ||
| }} | ||
| disabled={!!initialFilter} | ||
| > | ||
| {Object.values(FilterType).map(name => ( | ||
| {Object.values(Tenures).map(name => ( | ||
| <MenuItem key={name} value={name}> | ||
| {name} | ||
| </MenuItem> | ||
| ))} | ||
| </Select> | ||
| </FormControl> | ||
| {filterType === FilterType.MANAGER && ( | ||
| <FormControlLabel | ||
| className="direct-reports-only-checkbox" | ||
| control={ | ||
| <Checkbox | ||
| checked={directReportsOnly} | ||
| onChange={event => | ||
| setDirectReportsOnly(event.target.checked) | ||
| } | ||
| /> | ||
| } | ||
| label="Direct reports only" | ||
| {tenure === Tenures.Custom && ( | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a date picker for selecting a custom start date for tenure. |
||
| <DatePicker | ||
| className="custom-tenure-picker" | ||
| slotProps={{ textField: { className: 'halfWidth' } }} | ||
| label="Custom Tenure Start" | ||
| format="MM/dd/yyyy" | ||
| value={customTenure} | ||
| openTo="year" | ||
| onChange={setCustomTenure} | ||
| KeyboardButtonProps={{ | ||
| 'aria-label': 'Change Date' | ||
| }} | ||
| /> | ||
| )} | ||
| </div> | ||
| <Checkbox | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This checkbox was not deleted. It was just moved up in the code. |
||
| className="toggle-selectable-members-checkbox" | ||
| onChange={event => handleToggleAll(event.target.checked)} | ||
| checked={ | ||
| selectableMembers.length > 0 && | ||
| visibleChecked().length === selectableMembers.length | ||
| } | ||
| indeterminate={ | ||
| visibleChecked().length > 0 && | ||
| visibleChecked().length !== selectableMembers.length | ||
| } | ||
| disabled={selectableMembers.length === 0} | ||
| /> | ||
| </FormGroup> | ||
| <Divider /> | ||
| <List dense role="list" sx={{ height: '85%', overflowY: 'scroll' }}> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -122,10 +122,10 @@ describe('MemberSelectorDialog', () => { | |
| expect(memberList).toHaveLength(initialState.memberProfiles.length); | ||
|
|
||
| const filterField = await screen.findByRole('combobox', { | ||
| name: /filter members/i | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I changed some labels in the code, so these UI tests needed to be updated to find elements based on their new labels. |
||
| name: /filter value/i | ||
| }); | ||
| let filterTypeField = await screen.findByRole('combobox', { | ||
| name: /filter by/i | ||
| name: /filter type/i | ||
| }); | ||
| expect(filterField.innerHTML).toBe(''); | ||
| expect(filterTypeField.innerHTML).toBe('Team'); | ||
|
|
@@ -155,10 +155,10 @@ describe('MemberSelectorDialog', () => { | |
| expect(memberList).toHaveLength(initialState.memberProfiles.length); | ||
|
|
||
| const filterField = await screen.findByRole('combobox', { | ||
| name: /filter members/i | ||
| name: /filter value/i | ||
| }); | ||
| let filterTypeField = await screen.findByRole('combobox', { | ||
| name: /filter by/i | ||
| name: /filter type/i | ||
| }); | ||
| expect(filterField.innerHTML).toBe(''); | ||
| expect(filterTypeField.innerHTML).toBe('Team'); | ||
|
|
||
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 sets the width of a DatePicker that was added for selecting a custom tenure value.