-
Notifications
You must be signed in to change notification settings - Fork 6
Feature 2357 pulse survey report #2432
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
Conversation
| INSERT INTO pulse_response | ||
| (id, submissiondate, teammemberid, internalfeelings, externalfeelings, internal_score, external_score) -- Holly Williams | ||
| VALUES | ||
| ('cda41eed-70ea-4d3f-a9d7-cd0c5158eb5f', '2021-01-29', '8fa673c0-ca19-4271-b759-41cb9db2e83a', PGP_SYM_ENCRYPT('Feeling pretty happy','${aeskey}'), PGP_SYM_ENCRYPT('Feeling really good','${aeskey}'), 1, 3); |
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.
I added some pulse_report records to enable testing the new pulse report page.
| const { expand, ...other } = props; | ||
| return ( | ||
| <IconButton {...other}> | ||
| <IconButton classes={{ root: 'expand-more' }} {...other}> |
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.
I added a CSS class to this so I can apply CSS properties in pages were it is used.
| </div> | ||
| <Button | ||
| color="inherit" | ||
| disabled={checked.size === 0} |
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 now allows deselecting all members.
| //TODO: Skipping the permission check during testing | ||
| // because this permission has not been implemented yet. | ||
| // if (selectHasPulseReportPermission(state)) { | ||
| links.push(['/pulse-reports', 'Pulses']); |
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.
Adds a new page to the menu.
| selectCurrentUser, | ||
| selectCurrentUserSubordinates, | ||
| selectIsAdmin, | ||
| selectMyTeam, |
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 import is not used.
| Avatar, | ||
| Card, | ||
| CardActions, | ||
| CardContent, |
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 import is not used.
| import React, { useCallback, useContext, useEffect, useState } from 'react'; | ||
|
|
||
| import { | ||
| Avatar, |
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 import is not used.
| <Header title="Annual Review Report" /> | ||
| <AnnualReviewReportPage /> | ||
| </Route> | ||
| <Route path="/pulse-reports"> |
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.
New route for new page.
| ) | ||
| ); | ||
|
|
||
| export const selectHasPulseReportPermission = createSelector( |
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.
New permission check.
| const csrf = selectCsrfToken(state); | ||
|
|
||
| const [externalComment, setExternalComment] = useState(''); | ||
| const [externalScore, setExternalScore] = useState(0); |
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.
Updated default internal and external scores.
These are 0-based in the code (not in the database), so 2 is in the middle.
|
|
||
| const submit = async () => { | ||
| const myId = currentUser?.id; | ||
| //TODO: The POST endpoint doesn't currently save the score values, |
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 TODO has been addressed.
| @@ -0,0 +1,402 @@ | |||
| import { format } from 'date-fns'; | |||
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 the new report page.
vhscom
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.
Looking great! One change suggested for consideration.
web-ui/src/pages/PulsePage.jsx
Outdated
| if (res.error) throw new Error(res.error.message); | ||
|
|
||
| // Refresh browser to show that pulses where already submitted today. | ||
| location.reload(); |
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.
I noticed the page reloads nicely using react-router-dom's useHistory hook like:
import {useHistory} from 'react-router-dom';
const PulsePage = () => {
const { state } = useContext(AppContext);
const currentUser = selectCurrentUser(state);
const csrf = selectCsrfToken(state);
const history = useHistory();Then the hook can be used to navigate the same page:
history.push(location.pathname);The user will then see a gentle page update and the thank you message.
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.
I made the change you suggested. Thanks for catching that!
S78901
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.
Looks good, agree with @vhscom on the router update
No description provided.