From 489a5ae3c859fae1deb40ee176dbc7440280771f Mon Sep 17 00:00:00 2001 From: Veikkosuhonen Date: Tue, 30 Jan 2024 18:15:59 +0200 Subject: [PATCH] fix: Feedback visibility update based on text content, not id --- public/locales/en/translation.json | 2 + public/locales/fi/translation.json | 2 + src/client/pages/App.js | 2 +- .../useUpdateOpenFeedbackVisibility.js | 29 ++++----- .../feedbackTargetController.js | 9 +++ .../routes/feedbacks/feedbacksController.js | 53 +--------------- .../services/feedbackTargets/hideFeedback.js | 63 +++++++++++++++++++ src/server/services/feedbackTargets/index.js | 2 + 8 files changed, 91 insertions(+), 71 deletions(-) create mode 100644 src/server/services/feedbackTargets/hideFeedback.js diff --git a/public/locales/en/translation.json b/public/locales/en/translation.json index 8f3db852f..74f7d74c5 100644 --- a/public/locales/en/translation.json +++ b/public/locales/en/translation.json @@ -279,6 +279,8 @@ "setHidden": "Hide this answer", "setVisible": "Unhide", "hiddenInfo": "This answer is hidden and is visible only to responsible teachers and programme administrators", + "hideSuccess": "Ok. Answers to this question with the content \"{{content}}...\" ({{count}}) have been hidden.", + "showSuccess": "Ok. Answers to this question with the content \"{{content}}...\" ({{count}}) have been made visible.", "hidingFeatureInfoTitle": "Info about hiding answers", "publishingInfo": "The answers to this questions are not shown to students. You can publish the answers to enrolled students from the button on the left titled \"Not public\".", "unpublishingInfo": "The answers to this questions are shown to students.", diff --git a/public/locales/fi/translation.json b/public/locales/fi/translation.json index ebf59425d..daff08776 100644 --- a/public/locales/fi/translation.json +++ b/public/locales/fi/translation.json @@ -278,6 +278,8 @@ "setHidden": "Piilota näkyvistä", "setVisible": "Poista piilotus", "hiddenInfo": "Tämä vastaus on piilotettu, ja se näkyy vain vastuuopettajille tai koulutusohjelman hallintohenkilöille", + "hideSuccess": "Ok. Kysymyksen vastaukset sisällöllä \"{{content}}...\" on nyt piilotettu. ({{count}} kpl)", + "showSuccess": "Ok. Kysymyksen vastaukset sisällöllä \"{{content}}...\" ovat nyt näkyviä. ({{count}} kpl)", "hidingFeatureInfoTitle": "Tietoa vastausten näkyvyydestä", "publishingInfo": "Tämän kysymyksen vastaukset eivät näy kurssin opiskelijoille. Halutessasi voit julkistaa ne kurssin opiskelijoiden nähtäväksi klikkaamalla vasemmanpuolista nappia \"Ei julkinen\".", "unpublishingInfo": "Tämän kysymyksen vastaukset näkyvät kurssin opiskelijoille.", diff --git a/src/client/pages/App.js b/src/client/pages/App.js index f219c7704..ff29aba93 100644 --- a/src/client/pages/App.js +++ b/src/client/pages/App.js @@ -22,7 +22,7 @@ const App = () => { - + diff --git a/src/client/pages/FeedbackTarget/tabs/Results/QuestionResults/useUpdateOpenFeedbackVisibility.js b/src/client/pages/FeedbackTarget/tabs/Results/QuestionResults/useUpdateOpenFeedbackVisibility.js index 5da643cd3..fc36f894e 100644 --- a/src/client/pages/FeedbackTarget/tabs/Results/QuestionResults/useUpdateOpenFeedbackVisibility.js +++ b/src/client/pages/FeedbackTarget/tabs/Results/QuestionResults/useUpdateOpenFeedbackVisibility.js @@ -18,24 +18,17 @@ const useUpdateOpenFeedbackVisibility = () => { apiClient.put(`/feedback-targets/${feedbackTargetId}/hide-feedback`, { hidden, feedbackContent, questionId }) const mutation = useMutation(mutationFn, { - onSuccess: (response, { feedbackId, questionId }) => { - const { hidden } = response.data - - queryClient.setQueriesData(['feedbackTargetFeedbacks', String(feedbackTarget.id)], data => { - const feedbacks = data?.feedbacks - if (!feedbacks) return data - - const updatedFeedbacks = feedbacks.map(f => - f.id === feedbackId - ? { - ...f, - data: f.data.map(q => (q.questionId === questionId ? { ...q, hidden } : q)), - } - : f - ) - - return { ...data, feedbacks: updatedFeedbacks } - }) + onSuccess: (response, { feedbackContent }) => { + const { hidden, count } = response.data + + enqueueSnackbar( + hidden + ? t('feedbackTargetResults:hideSuccess', { count, content: feedbackContent.slice(0, 20) }) + : t('feedbackTargetResults:showSuccess', { count, content: feedbackContent.slice(0, 20) }), + { variant: 'info' } + ) + + queryClient.invalidateQueries(['feedbackTargetFeedbacks', String(feedbackTarget.id)]) }, }) diff --git a/src/server/routes/feedbackTargets/feedbackTargetController.js b/src/server/routes/feedbackTargets/feedbackTargetController.js index 32117d506..d2b1ef49d 100644 --- a/src/server/routes/feedbackTargets/feedbackTargetController.js +++ b/src/server/routes/feedbackTargets/feedbackTargetController.js @@ -18,6 +18,7 @@ const { remindStudentsOnFeedback, getFeedbackTargetsForCourseUnit, getFeedbackTargetsForOrganisation, + hideFeedback, } = require('../../services/feedbackTargets') const { getFeedbackErrorViewDetails } = require('../../services/feedbackTargets/getErrorViewDetails') @@ -230,6 +231,14 @@ adRouter.get('/:id/logs', async (req, res) => { return res.send(logs) }) +adRouter.put('/:id/hide-feedback', async (req, res) => { + const { user } = req + const { id: feedbackTargetId } = req.params + const { questionId, feedbackContent, hidden } = req.body + const count = await hideFeedback({ user, feedbackTargetId, questionId, feedbackContent, hidden }) + res.send({ hidden, count }) +}) + adRouter.use('/', interimFeedbackController) module.exports = { diff --git a/src/server/routes/feedbacks/feedbacksController.js b/src/server/routes/feedbacks/feedbacksController.js index 9128839b2..ebac1af81 100644 --- a/src/server/routes/feedbacks/feedbacksController.js +++ b/src/server/routes/feedbacks/feedbacksController.js @@ -21,10 +21,7 @@ const create = async (req, res) => { if (!access?.canGiveFeedback() || !userFeedbackTarget) ApplicationError.Forbidden('Not an enrolled student') if (userFeedbackTarget.feedbackId) - throw new ApplicationError( - 'Attempt to create new feedback where one already exists. Use PUT to update the old', - 400 - ) + throw new ApplicationError('Attempt to create new feedback where one already exists. Use PUT to update the old') if (!(await validateFeedback(data, feedbackTarget))) throw new ApplicationError('Form data not valid', 400) @@ -115,53 +112,6 @@ const destroy = async (req, res) => { return res.sendStatus(200) } -const updateAnswerHidden = async (req, res) => { - const { user } = req - const { id: feedbackId, questionId } = req.params - const { hidden } = req.body - if (typeof hidden !== 'boolean') { - throw new ApplicationError('Invalid value for hidden', 400) - } - - // find feedback - const feedback = await Feedback.findByPk(feedbackId, { - include: { - model: UserFeedbackTarget, - as: 'userFeedbackTarget', - attributes: ['feedbackTargetId'], - }, - }) - - if (!feedback) ApplicationError.NotFound('Feedback not found') - const { userFeedbackTarget } = feedback - - // check access - const { feedbackTarget, access } = await getFeedbackTargetContext({ - feedbackTargetId: userFeedbackTarget.feedbackTargetId, - user, - }) - if (!access?.canHideFeedback()) ApplicationError.Forbidden('Must be responsible teacher, organisation admin or admin') - - // find and update question - let updated = false - feedback.data = feedback.data.map(answer => { - if (answer.questionId === Number(questionId)) { - updated = true - return { ...answer, hidden } - } - return answer - }) - - if (!updated) { - throw new ApplicationError('Question not found on feedback', 404) - } - - await feedbackTarget.increment({ hiddenCount: hidden ? 1 : -1 }) - await feedback.save() - - return res.send({ hidden }) -} - const adminDeleteAnswer = async (req, res) => { const { user } = req const { id: feedbackId, questionId } = req.params @@ -212,7 +162,6 @@ adRouter.post('/', create) adRouter.get('/:id', getOne) adRouter.put('/:id', update) adRouter.delete('/:id', destroy) -adRouter.put('/:id/question/:questionId', updateAnswerHidden) adRouter.delete('/:id/question/:questionId', adminAccess, adminDeleteAnswer) const noadRouter = Router() diff --git a/src/server/services/feedbackTargets/hideFeedback.js b/src/server/services/feedbackTargets/hideFeedback.js new file mode 100644 index 000000000..8dfdcc223 --- /dev/null +++ b/src/server/services/feedbackTargets/hideFeedback.js @@ -0,0 +1,63 @@ +const { sequelize } = require('../../db/dbConnection') +const { Feedback, UserFeedbackTarget } = require('../../models') +const { ApplicationError } = require('../../util/customErrors') +const { getFeedbackTargetContext } = require('./getFeedbackTargetContext') + +const hideFeedback = async ({ feedbackTargetId, questionId, hidden, user, feedbackContent }) => { + if (typeof hidden !== 'boolean') { + throw new ApplicationError('Invalid value for hidden', 400) + } + + // check access + const { feedbackTarget, access } = await getFeedbackTargetContext({ + feedbackTargetId, + user, + }) + if (!access?.canHideFeedback()) ApplicationError.Forbidden('Must be responsible teacher, organisation admin or admin') + + const allFeedbacks = await Feedback.findAll({ + include: { + model: UserFeedbackTarget, + as: 'userFeedbackTarget', + where: { + feedbackTargetId, + }, + }, + }) + + const feedbacksToUpdate = allFeedbacks + .map(feedback => { + if (!Array.isArray(feedback.data)) return { updated: false } + + let updated = false + feedback.data = feedback.data.map(answer => { + if (answer.data === feedbackContent && answer.questionId === questionId) { + updated = true + return { ...answer, hidden } + } + return answer + }) + + return { updated, feedback } + }) + .filter(({ updated }) => updated) + .map(({ feedback }) => feedback) + + if (feedbacksToUpdate.length === 0) return ApplicationError.BadRequest('Matching feedback not found') + + await sequelize.transaction(async transaction => { + await feedbackTarget.increment( + { hiddenCount: hidden ? feedbacksToUpdate.length : -feedbacksToUpdate.length }, + { transaction } + ) + for (const feedback of feedbacksToUpdate) { + feedback.save() + } + }) + + return feedbacksToUpdate.length +} + +module.exports = { + hideFeedback, +} diff --git a/src/server/services/feedbackTargets/index.js b/src/server/services/feedbackTargets/index.js index 39d025831..44f845a79 100644 --- a/src/server/services/feedbackTargets/index.js +++ b/src/server/services/feedbackTargets/index.js @@ -15,6 +15,7 @@ const { remindStudentsOnFeedback } = require('./remindStudentsOnFeedback') const { getForCourseUnit } = require('./getForCourseUnit') const { getByOrganisation } = require('./getByOrganisation') const { getFeedbackTargetContext } = require('./getFeedbackTargetContext') +const { hideFeedback } = require('./hideFeedback') const cache = require('./cache') module.exports = { @@ -32,6 +33,7 @@ module.exports = { remindStudentsOnFeedback, getFeedbackTargetsForCourseUnit: getForCourseUnit, getFeedbackTargetsForOrganisation: getByOrganisation, + hideFeedback, getFeedbackTargetContext, feedbackTargetCache: cache, }