Skip to content

Commit

Permalink
fix: Feedback visibility update based on text content, not id
Browse files Browse the repository at this point in the history
  • Loading branch information
Veikkosuhonen committed Jan 30, 2024
1 parent e138eb2 commit 489a5ae
Show file tree
Hide file tree
Showing 8 changed files with 91 additions and 71 deletions.
2 changes: 2 additions & 0 deletions public/locales/en/translation.json
Original file line number Diff line number Diff line change
Expand Up @@ -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.",
Expand Down
2 changes: 2 additions & 0 deletions public/locales/fi/translation.json
Original file line number Diff line number Diff line change
Expand Up @@ -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.",
Expand Down
2 changes: 1 addition & 1 deletion src/client/pages/App.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ const App = () => {
<ThemeProvider theme={theme}>
<CssBaseline />
<Suspense fallback={null}>
<SnackbarProvider maxSnack={3} preventDuplicate autoHideDuration={10_000}>
<SnackbarProvider maxSnack={3} preventDuplicate autoHideDuration={15_000}>
<Switch>
<Route path="/noad">
<GuestUser />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)])
},
})

Expand Down
9 changes: 9 additions & 0 deletions src/server/routes/feedbackTargets/feedbackTargetController.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ const {
remindStudentsOnFeedback,
getFeedbackTargetsForCourseUnit,
getFeedbackTargetsForOrganisation,
hideFeedback,
} = require('../../services/feedbackTargets')
const { getFeedbackErrorViewDetails } = require('../../services/feedbackTargets/getErrorViewDetails')

Expand Down Expand Up @@ -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 = {
Expand Down
53 changes: 1 addition & 52 deletions src/server/routes/feedbacks/feedbacksController.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down
63 changes: 63 additions & 0 deletions src/server/services/feedbackTargets/hideFeedback.js
Original file line number Diff line number Diff line change
@@ -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,
}
2 changes: 2 additions & 0 deletions src/server/services/feedbackTargets/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -32,6 +33,7 @@ module.exports = {
remindStudentsOnFeedback,
getFeedbackTargetsForCourseUnit: getForCourseUnit,
getFeedbackTargetsForOrganisation: getByOrganisation,
hideFeedback,
getFeedbackTargetContext,
feedbackTargetCache: cache,
}

0 comments on commit 489a5ae

Please sign in to comment.