From 99a3031b474978770c6c0640740ae9c7fc2c68c7 Mon Sep 17 00:00:00 2001 From: wickathou Date: Sat, 1 Jun 2024 19:44:24 +0300 Subject: [PATCH 1/5] Fixed overwrite of projectScores from badly implemented logic Modified projectScore logic to prevent value overwrite from stale data Improved error messages from partner reviewer pages and actions Added notification for score criteria and challenges organizer pages to inform users about risks of making changes when the reviewing is ongoing or past Removed project score from project table Added validation to project grid item to show feedback information only if it is available Fixed issue on organizer projects tab not showing team member information when the user reloads the page Fixed project review submission for partners to prevent passing stale data Improved project review page performance and stability by removing unnecesary useEffects Removed unnused and commented code --- backend/modules/project_score/controller.js | 72 ++++-- backend/modules/project_score/routes.js | 46 ++-- .../layouts/MaterialTabsLayout/index.js | 2 - .../modals/EditProjectModal/index.js | 190 +------------- .../projects/ProjectsGridItem/index.js | 28 ++- .../components/tables/ProjectsTable/index.js | 12 - .../src/components/tables/TeamsTable/index.js | 12 +- .../organiser/edit/challenges/index.js | 17 ++ .../organiser/edit/scoreCriteria/index.js | 27 +- .../organiser/projects/index.js | 2 + .../renderDashboard/partner/projects/index.js | 231 +++++++++--------- .../_projects/slug/challenge/token/index.js | 4 - .../slug/view/projectId/EvaluationForm.js | 3 +- frontend/src/services/projectScores.js | 22 +- 14 files changed, 270 insertions(+), 398 deletions(-) diff --git a/backend/modules/project_score/controller.js b/backend/modules/project_score/controller.js index 07aa8029a..8a89d2224 100644 --- a/backend/modules/project_score/controller.js +++ b/backend/modules/project_score/controller.js @@ -2,19 +2,29 @@ const _ = require('lodash') const Event = require('../event/model') const Project = require('../project/model') const { ProjectScore } = require('./model') -const { NotFoundError } = require('../../common/errors/errors') +const { + NotFoundError, + AlreadyExistsError, +} = require('../../common/errors/errors') const controller = {} -controller.addProjectScore = async score => { - await Project.findById(score.project).orFail( +controller.addProjectScore = async (projectId, projectScoreByReviewer) => { + const projectScoreFound = await ProjectScore.findOne({ project: projectId }) + if (projectScoreFound) { + return controller.updateProjectScoreWithReviewers( + projectScoreFound._id, + projectScoreByReviewer, + ) + } + const projectData = await Project.findById(projectId).orFail( new NotFoundError('The given project does not exist.'), ) - await Event.findById(score.event).orFail( - new NotFoundError('The given event does not exist.'), - ) - const projectScore = new ProjectScore({ ...score }) - projectScore.averageScore = projectScore.score + const projectScore = new ProjectScore({ + reviewers: [projectScoreByReviewer], + project: projectId, + event: projectData.event, + }) if (projectScore.reviewers && projectScore.reviewers.length > 0) { const averageScore = averageScoreCalculation( @@ -22,11 +32,14 @@ controller.addProjectScore = async score => { projectScore.score, ) if (!averageScore) { - throw new Error('Score average calculation failed') + throw new Error( + 'Score average calculation failed, make sure all criteria are filled in', + ) } projectScore.averageScore = averageScore } - return projectScore.save() + projectScore.save() + return projectScore } controller.updateProjectScore = async (id, updatedProjectScore) => { @@ -112,7 +125,9 @@ const limitDecimals = (number, decimalPlaces) => { const averageScoreCalculation = (reviewers, globalScore) => { const allScores = reviewers.map(review => review.score) - allScores.push(globalScore) + if (globalScore) { + allScores.push(globalScore) + } let scoreCount = allScores.length const scoreSum = allScores.reduce((acc, current) => { @@ -132,17 +147,27 @@ const averageScoreCalculation = (reviewers, globalScore) => { } controller.updateProjectScoreWithReviewers = async ( - id, + projectScoreId, updatedProjectScore, ) => { - const projectScore = await ProjectScore.findById(id).orFail( - new NotFoundError('The given ProjectScore does not exist.'), + const projectScore = await ProjectScore.findById(projectScoreId).orFail( + new NotFoundError( + 'The given ProjectScore does not exist, refresh and try again.', + ), ) - projectScore.status = updatedProjectScore.status - projectScore.track = updatedProjectScore.track - projectScore.challenge = updatedProjectScore.challenge - projectScore.reviewers = updatedProjectScore.reviewers - + const reviewerFoundIndex = _.findIndex(projectScore.reviewers, { + userId: updatedProjectScore.userId, + }) + if (reviewerFoundIndex !== -1) { + projectScore.reviewers[reviewerFoundIndex].score = + updatedProjectScore.score + projectScore.reviewers[reviewerFoundIndex].scoreCriteria = + updatedProjectScore.scoreCriteria + projectScore.reviewers[reviewerFoundIndex].message = + updatedProjectScore.message + } else { + projectScore.reviewers.push(updatedProjectScore) + } projectScore.averageScore = projectScore.score if (projectScore.reviewers && projectScore.reviewers.length > 0) { @@ -151,15 +176,12 @@ controller.updateProjectScoreWithReviewers = async ( projectScore.score, ) if (!averageScore) { - throw new Error('Score average calculation failed') + throw new Error( + 'Score average calculation failed, make sure all criteria are filled in', + ) } projectScore.averageScore = averageScore } - - //scores from reviewers + scores from global, divide by total count of scores - - // projectScore.averageScore = projectScore.score + - await projectScore.save() return projectScore } diff --git a/backend/modules/project_score/routes.js b/backend/modules/project_score/routes.js index 25b0635ce..b9b9c8142 100644 --- a/backend/modules/project_score/routes.js +++ b/backend/modules/project_score/routes.js @@ -16,18 +16,20 @@ const { } = require('../../common/middleware/events') const addProjectScore = asyncHandler(async (req, res) => { - console.log('addProjectScore is running') + const { projectId } = req.params try { - if (req.params.track) { - req.body.track = req.params.track._id - } - if (req.params.challenge) { - req.body.challenge = req.params.challenge._id - } - const score = await ProjectScoreController.addProjectScore(req.body) + // if (req.params.track) { + // req.body.track = req.params.track._id + // } + // if (req.params.challenge) { + // req.body.challenge = req.params.challenge._id + // } + const score = await ProjectScoreController.addProjectScore( + projectId, + req.body, + ) return res.status(200).json(score) } catch (e) { - console.error(e) return res.status(500).json({ message: 'Add project score encountered an unknown error. Please check your request and try again.', @@ -86,20 +88,20 @@ const getPublicScores = asyncHandler(async (req, res) => { }) const updateProjectScoreWithReviewers = asyncHandler(async (req, res) => { - console.log('Received on bcknd') + const { slug, projectScoreId } = req.params try { - if (req.params.track) { - req.body.track = req.params.track._id - } - if (req.params.challenge) { - req.body.challenge = req.params.challenge._id - } - const score = + // if (req.params.track) { + // req.body.track = req.params.track._id + // } + // if (req.params.challenge) { + // req.body.challenge = req.params.challenge._id + // } + const projectScore = await ProjectScoreController.updateProjectScoreWithReviewers( - req.params.id, + projectScoreId, req.body, ) - return res.status(200).json(score) + return res.status(200).json(projectScore) } catch (e) { return res.status(500).json({ message: @@ -122,7 +124,6 @@ router.get( '/event/:slug/project/:projectId', hasToken, isOrganiserOrCanSubmitProject, - //isEventOrganiser, getScoreByProjectId, ) @@ -160,15 +161,14 @@ router.get( ) router.post( - '/review/event/:slug/:id', + '/review/event/:slug/:projectId', hasToken, isEventPartner, - // getEventFromParams, addProjectScore, ) router.put( - '/review/event/:slug/:id', + '/review/event/:slug/:projectScoreId', hasToken, isEventPartner, updateProjectScoreWithReviewers, diff --git a/frontend/src/components/layouts/MaterialTabsLayout/index.js b/frontend/src/components/layouts/MaterialTabsLayout/index.js index 1a0103710..a9cd562f2 100644 --- a/frontend/src/components/layouts/MaterialTabsLayout/index.js +++ b/frontend/src/components/layouts/MaterialTabsLayout/index.js @@ -106,7 +106,6 @@ export default ({ tabs, location, baseRoute, transparent = false }) => { {tabs.map(({ key, path, component }, index) => { - return ( { /> ) })} - diff --git a/frontend/src/components/modals/EditProjectModal/index.js b/frontend/src/components/modals/EditProjectModal/index.js index 26a4700c6..4feafbd30 100644 --- a/frontend/src/components/modals/EditProjectModal/index.js +++ b/frontend/src/components/modals/EditProjectModal/index.js @@ -1,9 +1,8 @@ -import { useDispatch, useSelector } from 'react-redux' -import React, { useState, useEffect, useMemo } from 'react' +import { useSelector } from 'react-redux' +import React, { useState, useMemo } from 'react' import * as AuthSelectors from 'redux/auth/selectors' import * as OrganiserSelectors from 'redux/organiser/selectors' -import * as SnackbarActions from 'redux/snackbar/actions' import { Dialog, List, @@ -14,11 +13,6 @@ import { Typography, ExpansionPanelDetails, Box, - TextField, - InputLabel, - FormControl, - Select, - MenuItem, Checkbox, } from '@material-ui/core' import PageWrapper from 'components/layouts/PageWrapper' @@ -27,15 +21,10 @@ import PageHeader from 'components/generic/PageHeader' import ExpandMoreIcon from '@material-ui/icons/ExpandMore' import TeamsTable from 'components/tables/TeamsTable' -import ProjectScoresService from 'services/projectScores' import EventsService from 'services/events' - -import { Formik, Form, Field, ErrorMessage } from 'formik' -import Button from 'components/generic/Button' import { projectURLgenerator } from 'utils/dataModifiers' export default ({ project, onClose = () => {}, onEdited = () => {} }) => { - const dispatch = useDispatch() const idToken = useSelector(AuthSelectors.getIdToken) const event = useSelector(OrganiserSelectors.event) const teams = useSelector(OrganiserSelectors.teams) @@ -44,30 +33,7 @@ export default ({ project, onClose = () => {}, onEdited = () => {} }) => { projectURL = projectURLgenerator(event.slug, project._id) } const [finalistChecked, setFinalistChecked] = useState(false) - const [projectScores, setProjectScores] = useState([ - { - project: '', - event: '', - status: 'submitted', - score: 0, - maxScore: 10, - message: '', - track: 'null', - challenge: 'null', - }, - ]) - useEffect(() => { - if (project && event) { - ProjectScoresService.getScoreByEventSlugAndProjectId( - idToken, - event.slug, - project._id, - ).then(score => { - if (score) setProjectScores(score) - }) - setFinalistChecked(event.finalists.includes(project._id)) - } - }, [event, idToken, project]) + const team = useMemo(() => { if (teams && project) { return [teams.find(team => team._id === project.team)] @@ -173,156 +139,6 @@ export default ({ project, onClose = () => {}, onEdited = () => {} }) => { - - - } - aria-controls="panel3a-content" - id="panel3a-header" - > - Project Score - - - {projectScores.map(projectScore => ( - { - values.project = project._id - values.event = event._id - values.track = - projectScore.track - values.challenge = - projectScore.challenge - try { - if (projectScore._id) { - await ProjectScoresService.updateScoreByEventSlug( - idToken, - event.slug, - values, - ) - } else { - await ProjectScoresService.addScoreByEventSlug( - idToken, - event.slug, - values, - ) - } - dispatch( - SnackbarActions.success( - 'Score saved successfully.', - ), - ) - } catch (e) { - dispatch( - SnackbarActions.error( - `Score could not be saved. Error: ${e.message}`, - ), - ) - } finally { - setSubmitting(false) - } - }} - > - {({ isSubmitting }) => ( -
- - {({ field }) => ( - - - Status{' '} - {projectScore.track - ? 'in track ' + - projectScore.track - : null}{' '} - {projectScore.challenge - ? 'in challenge ' + - projectScore.challenge - : null} - - - - )} - - - - {({ field }) => ( - - )} - - - - {({ field }) => ( - - )} - - - - {({ field }) => ( - - )} - - - - - - )} -
- ))} - - {event.overallReviewMethod === 'finalsManualSelection' ? ( 0) { + const findReviewMessages = () => { + const returnArray = [] + const reviewMap = project?.scoreData?.reviewers?.map( + reviewer => { + if (reviewer?.message?.length > 0) { + returnArray.push(reviewer.message) + } + }, + ) + return returnArray + } + const feedbackMessages = findReviewMessages() + if (feedbackMessages?.length > 0) { + messageCount = feedbackMessages.length + message = _.last(feedbackMessages) + } + } } const statusTag = status => { @@ -220,6 +239,7 @@ const ProjectsGridItem = ({ {message && ( + Latest feedback{' '} {_.truncate(message, { length: 20 })} )} @@ -229,7 +249,7 @@ const ProjectsGridItem = ({ -
+
{onClickMore && ( )}
diff --git a/frontend/src/components/tables/ProjectsTable/index.js b/frontend/src/components/tables/ProjectsTable/index.js index afbb2b1ec..18b8dc12a 100644 --- a/frontend/src/components/tables/ProjectsTable/index.js +++ b/frontend/src/components/tables/ProjectsTable/index.js @@ -101,18 +101,6 @@ const ProjectsTable = ({ projects }) => { setSelected(selectedRows) } - // debugGroup( - // 'Projects to export', - // selected.map(item => { - // const returnObject = { - // ...flattenObject(item.original), - // projectId: item.original._id, - // projectURL: projectURLgenerator(event.slug, item.original._id), - // } - // return returnObject - // }), - // ) - return ( <> { const searchParams = new URLSearchParams(location.search) const query = new URLSearchParams(location.search) - const hasModal = query.has('modal') const activeModal = query.get('modal') - // TODO add expansion - // const [expandedRows, setExpandedRows] = useState({ 1: true }) const openSingleTeamEdit = row => { const search = `?${new URLSearchParams({ @@ -115,8 +111,6 @@ export default ({ loading, teams = [], simplifiedView = false }) => { }, []) }, [teamsFiltered]) - - const columns = useMemo(() => { return [ { @@ -188,7 +182,9 @@ export default ({ loading, teams = [], simplifiedView = false }) => { @@ -310,9 +306,7 @@ export default ({ loading, teams = [], simplifiedView = false }) => { key: 'edit-team', label: 'Edit team', action: rows => { - openSingleTeamEdit(rows[0]) - }, }, ]} diff --git a/frontend/src/pages/_dashboard/renderDashboard/organiser/edit/challenges/index.js b/frontend/src/pages/_dashboard/renderDashboard/organiser/edit/challenges/index.js index b9552897c..152226f14 100644 --- a/frontend/src/pages/_dashboard/renderDashboard/organiser/edit/challenges/index.js +++ b/frontend/src/pages/_dashboard/renderDashboard/organiser/edit/challenges/index.js @@ -7,11 +7,28 @@ import FormControl from 'components/inputs/FormControl' import BooleanInput from 'components/inputs/BooleanInput' import ChallengesForm from './ChallengesForm' +import { useSelector } from 'react-redux' +import * as OrganiserSelectors from 'redux/organiser/selectors' +import { EventHelpers } from '@hackjunction/shared' +import moment from 'moment-timezone' export default () => { + const event = useSelector(OrganiserSelectors.event) + const isReviewingOpen = EventHelpers.isReviewingOpen(event, moment) + const isReviewingPast = EventHelpers.isReviewingPast(event, moment) return ( + {(isReviewingOpen || isReviewingPast) && ( +

+ Warning: Reviewing is now{' '} + {isReviewingOpen + ? 'open' + : isReviewingPast && 'completed'}{' '} + and if you make changes to the challenges, it might + cause unexpected problems. Modify at your own risk! +

+ )} ( diff --git a/frontend/src/pages/_dashboard/renderDashboard/organiser/edit/scoreCriteria/index.js b/frontend/src/pages/_dashboard/renderDashboard/organiser/edit/scoreCriteria/index.js index 1a50194cc..bb45ee4c8 100644 --- a/frontend/src/pages/_dashboard/renderDashboard/organiser/edit/scoreCriteria/index.js +++ b/frontend/src/pages/_dashboard/renderDashboard/organiser/edit/scoreCriteria/index.js @@ -1,17 +1,19 @@ -import React, { useEffect, useMemo, useState } from 'react' -import { FastField, useFormikContext } from 'formik' +import React from 'react' +import { FastField } from 'formik' import { Grid, Typography } from '@material-ui/core' import FormControl from 'components/inputs/FormControl' -import CustomSectionList from '../questions/CustomSectionList' import * as OrganiserSelectors from 'redux/organiser/selectors' import { useSelector } from 'react-redux' import _ from 'lodash' -import { useTranslation } from 'react-i18next' -import Button from 'components/generic/Button' -import Switch from '../../../../../../components/generic/Switch' +import Switch from 'components/generic/Switch' import EditableOptions from '../submission/components/EditableOptions' +import { EventHelpers } from '@hackjunction/shared' +import moment from 'moment-timezone' export default () => { + const event = useSelector(OrganiserSelectors.event) + const isReviewingOpen = EventHelpers.isReviewingOpen(event, moment) + const isReviewingPast = EventHelpers.isReviewingPast(event, moment) return ( {
+ {/* Create a P element that looks like a warning */} + {(isReviewingOpen || isReviewingPast) && ( +

+ Warning: Reviewing is now{' '} + {isReviewingOpen + ? 'open' + : isReviewingPast && 'completed'}{' '} + and if you make changes to the score criteria, it might + cause unexpected problems. Modify at your own risk! +

+ )} ( { if (event.slug) { dispatch(OrganiserActions.updateTeamsForEvent(event.slug)) dispatch(OrganiserActions.updateProjects(event.slug)) + dispatch(OrganiserActions.updateRegistrationsForEvent(event.slug)) + dispatch(OrganiserActions.updateTeamsForEvent(event.slug)) } }, [event, location]) diff --git a/frontend/src/pages/_dashboard/renderDashboard/partner/projects/index.js b/frontend/src/pages/_dashboard/renderDashboard/partner/projects/index.js index 493f8d1f1..0e12e5a5a 100644 --- a/frontend/src/pages/_dashboard/renderDashboard/partner/projects/index.js +++ b/frontend/src/pages/_dashboard/renderDashboard/partner/projects/index.js @@ -11,30 +11,22 @@ import _ from 'lodash' import ProjectDetail from 'components/projects/ProjectDetail' import * as AuthSelectors from 'redux/auth/selectors' -import * as userSelectors from 'redux/user/selectors' import ProjectScoresService from 'services/projectScores' import EvaluationForm from 'pages/_projects/slug/view/projectId/EvaluationForm' import Empty from 'components/generic/Empty' import * as SnackbarActions from 'redux/snackbar/actions' -const projectScoreBase = { - project: '', - event: '', - status: 'submitted', - score: 0, - maxScore: 10, - message: '', - scoreCriteria: [], - reviewers: [], -} - //TODO simplify this component and the reviewer score process //TODO make this and track one into a component export default ({ event }) => { - const scoreCriteriaBase = event.scoreCriteriaSettings?.scoreCriteria + const scoreCriteriaBase = [ + ...event?.scoreCriteriaSettings?.scoreCriteria.map(criteria => ({ + ...criteria, + })), + ] + const idToken = useSelector(AuthSelectors.getIdToken) const userId = useSelector(AuthSelectors.getUserId) - const userProfile = useSelector(userSelectors.userProfile) const dispatch = useDispatch() const { slug } = event const [data, setData] = useState({}) @@ -43,8 +35,7 @@ export default ({ event }) => { const [error, setError] = useState(false) const [selected, setSelected] = useState(null) - const [scoreExists, setScoreExists] = useState(false) - const [projectScore, setProjectScore] = useState(projectScoreBase) + const [projectScore, setProjectScore] = useState({}) const resetProjectData = () => { if (Array.isArray(scoreCriteriaBase) && scoreCriteriaBase.length > 0) { @@ -55,91 +46,90 @@ export default ({ event }) => { }) } setSelected(null) - setScoreExists(false) - setProjectScore(projectScoreBase) + setProjectScore({}) } - if (event.scoreCriteriaSettings === undefined) { + if (event?.scoreCriteriaSettings === undefined) { return } - const fetchProjects = useCallback(async () => { - setLoading(true) - try { - const dataOt = await ProjectsService.getProjectsByEventAsPartner( - idToken, - slug, - ) - const data = { - event, - ...dataOt, + const fetchProjects = useCallback( + async (idToken, slug) => { + setLoading(true) + try { + const dataOt = + await ProjectsService.getProjectsByEventAsPartner( + idToken, + slug, + ) + const data = { + event, + ...dataOt, + } + setData(data) + setProjects(data?.projects) + } catch (err) { + dispatch( + SnackbarActions.error( + `Error found when loading projects: ${err.message}`, + ), + ) + setError(true) + } finally { + setLoading(false) } - setData(data) - setProjects(data?.projects) - } catch (err) { - dispatch(SnackbarActions.error(`Error found: ${err.message}`)) - setError(true) - } finally { - setLoading(false) - } - }, [slug, idToken, projectScore]) + }, + [slug, idToken], + ) const handleSubmit = async (values, { setSubmitting, resetForm }) => { - const submissionValues = { ...values } - submissionValues.project = selected._id - submissionValues.event = event._id - let reviewerData - if (userId) { - reviewerData = _.find( - submissionValues.reviewers, - reviewer => reviewer.userId === userId, + const { projectScoreId, score, scoreCriteria, message } = values + if (score < 1) { + return dispatch( + SnackbarActions.error( + `You must assign a value for all criteria before submitting.`, + ), ) - if (reviewerData) { - if (reviewerData.firstname !== userProfile?.firstName) { - reviewerData.firstname = userProfile.firstName - } - if (reviewerData.avatar !== userProfile?.avatar) { - reviewerData.avatar = userProfile.avatar - } - reviewerData.score = submissionValues.score - reviewerData.scoreCriteria = submissionValues.scoreCriteria - reviewerData.message = submissionValues.message - } else { - reviewerData = { - userId: userId, - avatar: userProfile?.avatar, - firstname: userProfile?.firstName, - score: submissionValues.score, - scoreCriteria: submissionValues.scoreCriteria, - message: submissionValues.message, - } - submissionValues.reviewers.push(reviewerData) - } - delete submissionValues.score - delete submissionValues.scoreCriteria - delete submissionValues.message + } + const reviewData = { + userId, + score, + scoreCriteria, + message, } try { - if (scoreExists) { + if (projectScoreId) { await ProjectScoresService.updateScoreByEventSlugAndProjectIdAndPartnerAccount( idToken, - event.slug, - submissionValues, + slug, + projectScoreId, + reviewData, ) } else { + const projectId = selected._id await ProjectScoresService.addScoreByEventSlugAndProjectIdAndPartnerAccount( idToken, - event.slug, - submissionValues, + slug, + projectId, + reviewData, ) } - setProjectScore(values) - dispatch(SnackbarActions.success(`Score saved.`)) + dispatch( + SnackbarActions.success( + `Score saved${ + selected?.name && + ' for ' + _.truncate(selected?.name, { length: 15 }) + } `, + ), + ) resetForm() + resetProjectData() } catch (e) { dispatch( SnackbarActions.error( - `Score could not be saved. Error: ${e.message}`, + `Score could not be saved. Error: ${ + e.response.data.error || e.message + }`, ), ) } finally { @@ -148,51 +138,62 @@ export default ({ event }) => { } useEffect(() => { - fetchProjects() - }, [selected, projectScore]) - - if (!data) { - return null - } + if (idToken && event) { + if (!selected) { + fetchProjects(idToken, slug) + } else if (selected) { + fetchProjectScore(idToken, slug, selected._id) + } + } + }, [event, idToken, selected]) - //TODO perform this on the backend - useEffect(() => { - if (idToken && selected && event) { + const fetchProjectScore = useCallback( + async (idToken, slug, projectId) => { ProjectScoresService.getScoreByEventSlugAndProjectIdAndPartnerAccount( idToken, - event.slug, - selected._id, - ).then(score => { - if (score[0]) { - const reviewerData = _.find( - score[0].reviewers, - reviewer => reviewer.userId === userId, - ) - if (reviewerData) { - setProjectScore({ - ...score[0], - score: reviewerData.score, - scoreCriteria: reviewerData.scoreCriteria, - message: reviewerData.message, - }) + slug, + projectId, + ) + .then(score => { + if (score[0]) { + const reviewerData = _.find( + score[0].reviewers, + reviewer => reviewer.userId === userId, + ) + if (reviewerData) { + setProjectScore({ + projectScoreId: score[0]._id, + score: reviewerData.score, + scoreCriteria: reviewerData.scoreCriteria, + message: reviewerData.message, + }) + } else { + setProjectScore({ + projectScoreId: score[0]._id, + score: 0, + scoreCriteria: scoreCriteriaBase, + message: '', + }) + } } else { setProjectScore({ - ...score[0], - score: 0, + ...projectScore, scoreCriteria: scoreCriteriaBase, - message: '', }) } - setScoreExists(true) - } else { - setProjectScore({ - ...projectScore, - scoreCriteria: scoreCriteriaBase, - }) - } - }) - } - }, [event, idToken, selected]) + }) + .catch(e => { + dispatch( + SnackbarActions.error( + `Score could not be loaded. Error: ${ + e.response.data.error || e.message + }`, + ), + ) + }) + }, + [selected], + ) const renderProjects = inputData => { return ( diff --git a/frontend/src/pages/_projects/slug/challenge/token/index.js b/frontend/src/pages/_projects/slug/challenge/token/index.js index 2e4440fc6..026d8d99a 100644 --- a/frontend/src/pages/_projects/slug/challenge/token/index.js +++ b/frontend/src/pages/_projects/slug/challenge/token/index.js @@ -56,10 +56,6 @@ export default ({ event }) => { fetchProjects() }, [fetchProjects]) - if (!data) { - return null - } - const projectsToRender = filter => { switch (filter) { case 'draft': diff --git a/frontend/src/pages/_projects/slug/view/projectId/EvaluationForm.js b/frontend/src/pages/_projects/slug/view/projectId/EvaluationForm.js index d0af0a652..30b408e51 100644 --- a/frontend/src/pages/_projects/slug/view/projectId/EvaluationForm.js +++ b/frontend/src/pages/_projects/slug/view/projectId/EvaluationForm.js @@ -27,9 +27,11 @@ const EvaluationForm = ({ submit = () => {}, score, scoreCriteria }) => { return scoreAverageFormatted } + //TODO Update scoreCriteria from backend, when the event scoreCriteria is updated if ( scoreCriteria && scoreCriteria.length > 0 && + score.scoreCriteria && score.scoreCriteria.length > 0 ) { const scoreFiltered = score.scoreCriteria.filter(criteria => @@ -38,7 +40,6 @@ const EvaluationForm = ({ submit = () => {}, score, scoreCriteria }) => { criteria.criteria, ), ) - score.scoreCriteria = scoreFiltered } diff --git a/frontend/src/services/projectScores.js b/frontend/src/services/projectScores.js index 5d6f057c1..28078a4e7 100644 --- a/frontend/src/services/projectScores.js +++ b/frontend/src/services/projectScores.js @@ -25,6 +25,7 @@ ProjectScoresService.getScoreByEventSlugAndProjectId = ( ) } +//TODO delete this endpoint ProjectScoresService.addScoreByEventSlug = ( idToken, eventSlug, @@ -61,6 +62,7 @@ ProjectScoresService.getScoreByEventSlugAndProjectIdAndPartnerToken = ( ) } +//TODO delete this endpoint ProjectScoresService.addScoreByEventSlugAndPartnerToken = ( token, eventSlug, @@ -99,12 +101,13 @@ ProjectScoresService.getScoreByEventSlugAndProjectIdAndPartnerAccount = ( ProjectScoresService.addScoreByEventSlugAndProjectIdAndPartnerAccount = ( idToken, eventSlug, - projectScore, + projectId, + projectScoreByReviewer, ) => { - console.log('Adding new score from partner account', projectScore._id) + console.log('Adding new score from partner account') return _axios.post( - `/project-scores/review/event/${eventSlug}/${projectScore._id}`, - projectScore, + `/project-scores/review/event/${eventSlug}/${projectId}`, + projectScoreByReviewer, config(idToken), ) } @@ -112,17 +115,18 @@ ProjectScoresService.addScoreByEventSlugAndProjectIdAndPartnerAccount = ( ProjectScoresService.updateScoreByEventSlugAndProjectIdAndPartnerAccount = ( idToken, eventSlug, - // projectId, - projectScore, + projectScoreId, + projectScoreByReviewer, ) => { console.log('Updating working', { idToken, eventSlug, - projectScore, + projectScoreId, + projectScoreByReviewer, }) return _axios.put( - `/project-scores/review/event/${eventSlug}/${projectScore._id}`, - projectScore, + `/project-scores/review/event/${eventSlug}/${projectScoreId}`, + projectScoreByReviewer, config(idToken), ) } From 7b590279bb94d27729481b8f0e7323081d05d99c Mon Sep 17 00:00:00 2001 From: wickathou Date: Sat, 1 Jun 2024 19:51:40 +0300 Subject: [PATCH 2/5] Removed copilot comment --- .../renderDashboard/organiser/edit/scoreCriteria/index.js | 1 - 1 file changed, 1 deletion(-) diff --git a/frontend/src/pages/_dashboard/renderDashboard/organiser/edit/scoreCriteria/index.js b/frontend/src/pages/_dashboard/renderDashboard/organiser/edit/scoreCriteria/index.js index bb45ee4c8..91d79b062 100644 --- a/frontend/src/pages/_dashboard/renderDashboard/organiser/edit/scoreCriteria/index.js +++ b/frontend/src/pages/_dashboard/renderDashboard/organiser/edit/scoreCriteria/index.js @@ -126,7 +126,6 @@ export default () => { - {/* Create a P element that looks like a warning */} {(isReviewingOpen || isReviewingPast) && (

Warning: Reviewing is now{' '} From e71cc0ebcc08f21b4cdbcdff5e8b16f592b56ee5 Mon Sep 17 00:00:00 2001 From: wickathou Date: Sat, 1 Jun 2024 20:17:23 +0300 Subject: [PATCH 3/5] Fixed issue showing feedback messages when only the score visibility is enabled --- frontend/src/components/projects/ProjectsGridItem/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/src/components/projects/ProjectsGridItem/index.js b/frontend/src/components/projects/ProjectsGridItem/index.js index 4ebbeeb52..79fc265a4 100644 --- a/frontend/src/components/projects/ProjectsGridItem/index.js +++ b/frontend/src/components/projects/ProjectsGridItem/index.js @@ -237,7 +237,7 @@ const ProjectsGridItem = ({ Score {score} - {message && ( + {showReviewers && message && ( Latest feedback{' '} {_.truncate(message, { length: 20 })} From 8104b0894507a5840038ad32f9411c0039644a01 Mon Sep 17 00:00:00 2001 From: wickathou Date: Wed, 5 Jun 2024 17:27:45 +0300 Subject: [PATCH 4/5] Improved error messaging logic for the backend with standarized responses and logging for errors Moved sumitVote logic from routes to controller Improved the error handling for the submitVote logic Initiated error handling improvements for the frontend when receiving errors from the backend Made userId into a required property when submitting a new winner-vote Removed commented and unnused code Improved session verification useEffect logic in App.js --- backend/common/errors/errorHandler.js | 64 +++++++++---- backend/common/errors/errors.js | 33 ++++--- backend/modules/voting-token/controller.js | 3 +- backend/modules/winner-votes/controller.js | 46 ++++++++- backend/modules/winner-votes/model.js | 3 +- backend/modules/winner-votes/routes.js | 94 +++++-------------- .../organiser/projects/winners/index.js | 13 ++- .../participant/finalist-voting/index.js | 38 ++++---- 8 files changed, 165 insertions(+), 129 deletions(-) diff --git a/backend/common/errors/errorHandler.js b/backend/common/errors/errorHandler.js index 98ce0e689..4b208acb5 100644 --- a/backend/common/errors/errorHandler.js +++ b/backend/common/errors/errorHandler.js @@ -1,6 +1,22 @@ const logger = require('../../misc/logger') +const logError = (error, request) => { + logger.error({ + message: error.message, + error: { + stack: error.stack, + complete: error, + }, + request: { + requestBody: request.body, + requestParams: request.params, + requestQuery: request.query, + }, + }) +} + const errorHandler = (error, request, response, next) => { + logError(error, request) switch (error.name) { case 'UnauthorizedError': { return response.status(401).json({ @@ -8,51 +24,59 @@ const errorHandler = (error, request, response, next) => { status: 401, }) } + case 'NotFoundError': { + return response.status(404).json({ + message: error.message || 'Not found', + status: 404, + }) + } case 'InsufficientPrivilegesError': { return response.status(401).json({ - message: 'Insufficient privileges', + message: error.message || 'Insufficient privileges', status: 401, }) } - case 'ForbiddenError': { + case 'EmailVerificationError': { return response.status(403).json({ - message: error.message || 'Forbidden', + message: error.message || 'Email verification required', status: 403, }) } - case 'NotFoundError': { - return response.status(404).json({ - message: error.message || 'Not found', - status: 404, - }) - } case 'ValidationError': { return response.status(400).json({ - message: error.message, + message: error.message || 'Validation error', errors: error.errors, status: 400, }) } + case 'ForbiddenError': { + return response.status(403).json({ + message: error.message || 'Forbidden', + status: 403, + }) + } + case 'AlreadyExistsError': { + return response.status(400).json({ + message: error.message || 'Already exists', + status: 400, + }) + } case 'MongoError': { if (error.code === 11000) { return response.status(400).json({ + message: 'Report to tech support', type: 'unique-violation', status: 400, }) } - break + return response.status(400).json({ + message: error.message || 'Validation error', + status: 400, + }) } default: - logger.error({ - message: 'Unhandled error', - error: { - message: error.message, - stack: error.stack, - }, - }) - return response.status(500).json({ - message: 'Unexpected error', + message: error.message || 'Unexpected error', status: 500, }) } diff --git a/backend/common/errors/errors.js b/backend/common/errors/errors.js index 8ffe6b640..7dec726c3 100644 --- a/backend/common/errors/errors.js +++ b/backend/common/errors/errors.js @@ -18,26 +18,26 @@ class CustomError extends Error { } class NotFoundError extends CustomError { - constructor(message) { + constructor(message, details) { const name = 'NotFoundError' const code = 1 - super(message, name, code) + super(message, name, code, details) } } class InsufficientPrivilegesError extends CustomError { - constructor(message) { + constructor(message, details) { const name = 'InsufficientPrivilegesError' const code = 2 - super(message, name, code) + super(message, name, code, details) } } class EmailVerificationError extends CustomError { - constructor(message) { + constructor(message, details) { const name = 'EmailVerificationError' const code = 3 - super(message, name, code) + super(message, name, code, details) } } @@ -50,26 +50,34 @@ class ValidationError extends CustomError { } class ForbiddenError extends CustomError { - constructor(message) { + constructor(message, details) { const name = 'ForbiddenError' const code = 5 - super(message, name, code) + super(message, name, code, details) } } class UnauthorizedError extends CustomError { - constructor(message) { + constructor(message, details) { const name = 'UnauthorizedError' const code = 6 - super(message, name, code) + super(message, name, code, details) } } class AlreadyExistsError extends CustomError { - constructor(message) { + constructor(message, details) { const name = 'AlreadyExistsError' const code = 7 - super(message, name, code) + super(message, name, code, details) + } +} + +class MongoError extends CustomError { + constructor(message, details) { + const name = 'MongoError' + const code = 8 + super(message, name, code, details) } } @@ -81,4 +89,5 @@ module.exports = { ForbiddenError, UnauthorizedError, AlreadyExistsError, + MongoError, } diff --git a/backend/modules/voting-token/controller.js b/backend/modules/voting-token/controller.js index 428c3deb6..962541b6f 100644 --- a/backend/modules/voting-token/controller.js +++ b/backend/modules/voting-token/controller.js @@ -35,8 +35,7 @@ controller.getVotesByProject = async event => { }) return result }, []) - const sorted = _.orderBy(results, ['votes'], 'desc') - return sorted + return results } controller.getToken = id => { diff --git a/backend/modules/winner-votes/controller.js b/backend/modules/winner-votes/controller.js index 3ab3712f3..15c23fc16 100644 --- a/backend/modules/winner-votes/controller.js +++ b/backend/modules/winner-votes/controller.js @@ -2,6 +2,7 @@ const _ = require('lodash') const WinnerVote = require('./model') const projectController = require('../project/controller') const tokenVotingController = require('../voting-token/controller') +const { ValidationError, MongoError } = require('../../common/errors/errors') const controller = {} @@ -17,8 +18,7 @@ controller.getFinalistProjectsWithAllVotes = async event => { return projectObject }) const userVotes = await controller.getVotesForEvent(event) - const tokenVotes = await tokenVotingController.getVotesByProject(event._id) - if (userVotes) { + if (userVotes && userVotes.length > 0) { userVotes.map(v => { const projectWithVotes = _.find( finalistProjectsWithVotes, @@ -29,7 +29,8 @@ controller.getFinalistProjectsWithAllVotes = async event => { } }) } - if (tokenVotes) { + const tokenVotes = await tokenVotingController.getVotesByProject(event._id) + if (tokenVotes && tokenVotes.length > 0) { tokenVotes.map(v => { const projectWithVotes = _.find( finalistProjectsWithVotes, @@ -65,4 +66,43 @@ controller.getVotesForEvent = async event => { return results } +controller.submitVote = async (eventId, userId, projectId) => { + if (!eventId || !projectId || !userId) { + throw new ValidationError( + 'Some data is missing, try again. If the problem persists, contact support.', + ) + } + + let vote = await WinnerVote.findOne({ + event: eventId, + user: userId, + }) + + if (vote) { + vote.project = projectId + } else { + vote = new WinnerVote({ + event: eventId, + user: userId, + project: projectId, + }) + } + + if (!vote) { + throw new Error( + 'Unknown error, please try again. If the problem persists, contact support.', + ) + } + + try { + const result = await vote.save() + return result + } catch (err) { + throw new MongoError( + 'Vote could not be saved, reload the page and try again.', + err, + ) + } +} + module.exports = controller diff --git a/backend/modules/winner-votes/model.js b/backend/modules/winner-votes/model.js index 17f23aae6..a24561925 100644 --- a/backend/modules/winner-votes/model.js +++ b/backend/modules/winner-votes/model.js @@ -8,6 +8,7 @@ const WinnerVoteSchema = new mongoose.Schema({ }, user: { type: String, + required: true, }, project: { type: mongoose.Schema.Types.ObjectId, @@ -24,7 +25,7 @@ WinnerVoteSchema.index( }, { unique: true, - } + }, ) const WinnerVote = mongoose.model('WinnerVote', WinnerVoteSchema) diff --git a/backend/modules/winner-votes/routes.js b/backend/modules/winner-votes/routes.js index f7483e10a..89075a358 100644 --- a/backend/modules/winner-votes/routes.js +++ b/backend/modules/winner-votes/routes.js @@ -13,11 +13,16 @@ const { } = require('../../common/middleware/events') const votingController = require('./controller') +const errorHandler = require('../../common/errors/errorHandler') const getProjectsWithVotesForEvent = asyncHandler(async (req, res) => { - const finalistProjectsWithAllVotes = - await votingController.getFinalistProjectsWithAllVotes(req.event) - return res.status(200).json(finalistProjectsWithAllVotes) + try { + const finalistProjectsWithAllVotes = + await votingController.getFinalistProjectsWithAllVotes(req.event) + return res.status(200).json(finalistProjectsWithAllVotes) + } catch (e) { + return errorHandler(e, req, res) + } }) const getVote = asyncHandler(async (req, res) => { @@ -30,79 +35,28 @@ const getVote = asyncHandler(async (req, res) => { }) const submitVote = asyncHandler(async (req, res) => { - const vote = await WinnerVote.findOne({ - event: req.event._id, - user: req.user.sub, - }) - if (vote) { - vote.project = req.body.projectId - const result = await vote.save() + const eventId = req.event._id + const userId = req.user.sub + const projectId = req.body.projectId + try { + const result = await votingController.submitVote( + eventId, + userId, + projectId, + ) return res.status(200).json(result) + } catch (e) { + return errorHandler(e, req, res) } - const newVote = new WinnerVote({ - event: req.event._id, - user: req.user.sub, - project: req.body.projectId, - }) - const result = await newVote.save() - return res.status(200).json(result) }) router .route('/:slug') - .get( - hasToken, - hasRegisteredToEvent, - getVote, - // asyncHandler(async (req, res) => { - // const vote = await WinnerVote.findOne({ - // event: req.event._id, - // user: req.user.sub, - // }) - - // return res.status(200).json(vote) - // }), - ) - .post( - hasToken, - hasRegisteredToEvent, - submitVote, - // asyncHandler(async (req, res) => { - // const vote = await WinnerVote.findOne({ - // event: req.event._id, - // user: req.user.sub, - // }) - // if (vote) { - // vote.project = req.body.projectId - // const result = await vote.save() - // return res.status(200).json(result) - // } - // const newVote = new WinnerVote({ - // event: req.event._id, - // user: req.user.sub, - // project: req.body.projectId, - // }) - // const result = await newVote.save() - // return res.status(200).json(result) - // }), - ) - -router.route('/:slug/results').get( - hasToken, - isEventOrganiser, - getProjectsWithVotesForEvent, - // asyncHandler(async (req, res) => { - // const votes = await WinnerVote.find({ - // event: req.event._id, - // }) - - // const grouped = _.groupBy(votes, 'project') - // console.log('Grouped votes:', grouped) - // getProjectsWithVotesForEvent(req, res) + .get(hasToken, hasRegisteredToEvent, getVote) + .post(hasToken, hasRegisteredToEvent, submitVote) - // return res.status(200).json(grouped) - // } -) -// ) +router + .route('/:slug/results') + .get(hasToken, isEventOrganiser, getProjectsWithVotesForEvent) module.exports = router diff --git a/frontend/src/pages/_dashboard/renderDashboard/organiser/projects/winners/index.js b/frontend/src/pages/_dashboard/renderDashboard/organiser/projects/winners/index.js index c8eb2d7d2..1debed269 100644 --- a/frontend/src/pages/_dashboard/renderDashboard/organiser/projects/winners/index.js +++ b/frontend/src/pages/_dashboard/renderDashboard/organiser/projects/winners/index.js @@ -1,7 +1,7 @@ import React, { useCallback, useEffect, useState } from 'react' import { Grid, Box, Dialog } from '@material-ui/core' -import { useSelector } from 'react-redux' +import { useDispatch, useSelector } from 'react-redux' import PageHeader from 'components/generic/PageHeader' import PageWrapper from 'components/layouts/PageWrapper' @@ -10,6 +10,7 @@ import ProjectDetail from 'components/projects/ProjectDetail' import * as OrganiserSelectors from 'redux/organiser/selectors' import * as AuthSelectors from 'redux/auth/selectors' +import * as SnackbarActions from 'redux/snackbar/actions' import WinnerVoteService from 'services/winnerVote' import _ from 'lodash' @@ -17,6 +18,7 @@ import _ from 'lodash' export default () => { const event = useSelector(OrganiserSelectors.event) const idToken = useSelector(AuthSelectors.getIdToken) + const dispatch = useDispatch() const [loading, setLoading] = useState(true) const [selected, setSelected] = useState(false) @@ -36,10 +38,17 @@ export default () => { } } catch (error) { console.error(error) + dispatch( + SnackbarActions.error( + `Error retrieving the voting data: ${ + error.response.data.message || error.message + }`, + ), + ) } finally { setLoading(false) } - }, [event, idToken]) + }, [event]) useEffect(() => { if (event?.slug) { diff --git a/frontend/src/pages/_dashboard/renderDashboard/participant/finalist-voting/index.js b/frontend/src/pages/_dashboard/renderDashboard/participant/finalist-voting/index.js index 107bfde2d..e8511c60b 100644 --- a/frontend/src/pages/_dashboard/renderDashboard/participant/finalist-voting/index.js +++ b/frontend/src/pages/_dashboard/renderDashboard/participant/finalist-voting/index.js @@ -29,12 +29,11 @@ export default () => { const [vote, setVote] = useState(null) const [hasVoted, setVoted] = useState(false) - const getCurrentVote = useCallback(() => { + const getCurrentVote = useCallback(async () => { return WinnerVoteService.getVote(idToken, event.slug) }, [idToken, event]) const getFinalists = useCallback(async () => { - setLoading(true) EventsService.getFinalists(idToken, event.slug) .then(finalistProjects => { setProjects(finalistProjects) @@ -42,42 +41,40 @@ export default () => { .catch(err => { dispatch( SnackbarActions.error( - 'Something went wrong... Please try again', + 'Something went wrong while fetching finalists, refresh the page to try again.', ), ) - console.error(err) - }) - .finally(() => { - setLoading(false) }) }, [idToken, event]) useEffect(() => { - getFinalists() - update() + setLoading(true) + try { + getFinalists() + update() + } catch (err) { + } finally { + setLoading(false) + } }, []) const update = useCallback(async () => { try { - setLoading(true) const vote = await getCurrentVote() - if (vote) { - setVote(vote.project) + if (vote && vote?.project) { + setVote(vote?.project) setVoted(true) } } catch (err) { dispatch( SnackbarActions.error( - 'Something went wrong... Please try again', + 'Something went wrong while fetching your vote, refresh the page to try again.', ), ) - console.error(err) - } finally { - setLoading(false) } }, [event, idToken]) - const handleSubmit = useCallback(async () => { + const handleSubmit = async () => { try { setLoading(true) const result = await WinnerVoteService.submitVote( @@ -91,13 +88,16 @@ export default () => { } catch (err) { dispatch( SnackbarActions.error( - 'Something went wrong... Please try again', + `Score could not be saved. Error: ${ + err.response.data.message || err.message + }`, ), ) } finally { setLoading(false) } - }, [idToken, event, vote]) + } + return ( Date: Wed, 5 Jun 2024 17:28:22 +0300 Subject: [PATCH 5/5] Improved session verification useEffect logic in App.js --- frontend/src/App.js | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/frontend/src/App.js b/frontend/src/App.js index 20a9c5560..daf478ab1 100755 --- a/frontend/src/App.js +++ b/frontend/src/App.js @@ -34,24 +34,18 @@ export default ({ history, location }) => { }, [location, history]) useEffect(() => { - if (isAuthenticated) { - if (isSessionExpired) { - setLoading(true) - console.log('renewing session now') + setLoading(false) + if (isAuthenticated && isSessionExpired) { + setLoading(true) + console.log('renewing session now') + try { dispatch(AuthActions.renewSession()) - .then(() => { - setLoading(false) - }) - .catch(err => { - console.log(err) - dispatch(SnackbarActions.error('Please, log in again')) - }) - .finally(() => setLoading(false)) - } else { + } catch (err) { + console.log(err) + dispatch(SnackbarActions.error('Please, log in again')) + } finally { setLoading(false) } - } else { - setLoading(false) } }, [dispatch, isAuthenticated, isSessionExpired])