Correctly update feedback count on my teaching page

UniversityOfHelsinkiCS · Jan 8, 2025 · f33044f · f33044f
1 parent f01ad0f
commit f33044f
Show file tree

Hide file tree

Showing 5 changed files with 48 additions and 16 deletions.
diff --git a/cypress/integration/feedbackCount.spec.js b/cypress/integration/feedbackCount.spec.js
@@ -10,13 +10,24 @@ describe('Feedback count', () => {
     cy.getTestFbtId().as('fbtId')
   })
 
-  it('Feedback count increases on feedback page when a student gives feedback', () => {
-    // Initial feedback count
+  it('Feedback count increases everywhere when a student gives feedback', () => {
     cy.loginAs(teacher)
+    // Initial feedback count on my teaching page
+    cy.visit('/courses')
+    cy.get('@fbtId')
+      .then(id => cy.get(`[data-cy="my-teaching-feedback-target-secondary-text-${id}"]`))
+      .invoke('text')
+      .then(text => {
+        cy.wrap(text).as('myTeachingInitialCount')
+      })
+
+    // Initial feedback count on feedback page
     cy.get('@fbtId').then(id => cy.visit(`/targets/${id}`))
-    cy.get('[data-cy=feedback-target-feedback-count-percentage]').invoke('text').then(text => {
-      cy.wrap(text).as('initialCount')
-    })
+    cy.get('[data-cy=feedback-target-feedback-count-percentage]')
+      .invoke('text')
+      .then(text => {
+        cy.wrap(text).as('fbtPageInitialCount')
+      })
 
     // Student gives feedback
     cy.loginAs(student)
@@ -33,17 +44,28 @@ describe('Feedback count', () => {
     cy.get('[data-cy=feedback-view-give-feedback]').click()
     cy.contains('Feedback has been given. Thank you for your feedback!')
 
-    // Verify feedback count increased
+    // Verify feedback count increased on feedback page
     cy.loginAs(teacher)
     cy.get('@fbtId').then(id => cy.visit(`/targets/${id}`))
     cy.get('[data-cy=feedback-target-feedback-count-percentage]').invoke('text').then(newCount => {
-      cy.get('@initialCount').then(initialCount => {
+      cy.get('@fbtPageInitialCount').then(initialCount => {
         const newCountInt = parseInt(newCount.split("/")[0], 10)
-        console.log(newCountInt)
         const initialCountInt = parseInt(initialCount.split("/")[0], 10)
-        console.log(initialCountInt)
         expect(newCountInt).to.eq(initialCountInt + 1)
       })
     })
+
+    // Verify feedback count increased on my teaching page
+    cy.visit('/courses')
+    cy.get('@fbtId')
+      .then(id => cy.get(`[data-cy="my-teaching-feedback-target-secondary-text-${id}"]`))
+      .invoke('text')
+      .then(newCount => {
+        cy.get('@myTeachingInitialCount').then(initialCount => {
+          const newCountInt = parseInt(newCount.split("/")[0], 10)
+          const initialCountInt = parseInt(initialCount.split("/")[0], 10)
+          expect(newCountInt).to.eq(initialCountInt + 1)
+        })
+      })
   })
 })
diff --git a/src/client/pages/MyTeaching/V2/FeedbackTargetList/FeedbackTargetListItem.jsx b/src/client/pages/MyTeaching/V2/FeedbackTargetList/FeedbackTargetListItem.jsx
@@ -93,7 +93,12 @@ const FeedbackTargetSecondaryText = ({ feedbackTarget }) => {
   const feedbackPercentage = getFeedbackPercentageString(feedbackTarget)
 
   return (
-    <Typography variant="body2" color="textSecondary" component="span">
+    <Typography
+      variant="body2"
+      color="textSecondary"
+      component="span"
+      data-cy={`my-teaching-feedback-target-secondary-text-${feedbackTarget.id}`}
+    >
       {parseISO(feedbackTarget.opensAt) < new Date() ? (
         <>
           {t('teacherView:feedbackCount', {

diff --git a/src/server/db/dbConnection.js b/src/server/db/dbConnection.js
@@ -45,6 +45,7 @@ const connectToDatabase = async (attempt = 0) => {
       return process.exit(1)
     }
     logger.info(`Connection to database failed! Attempt ${attempt} of ${DB_CONNECTION_RETRY_LIMIT}`)
+    // eslint-disable-next-line no-console
     console.log(err)
     await sleep(5000)
     return connectToDatabase(attempt + 1)

diff --git a/src/server/models/hooks.ts b/src/server/models/hooks.ts
@@ -47,6 +47,8 @@ Feedback.afterCreate(async feedback => {
   //@ts-expect-error fbt is not yet typescripted
   await feedbackTargetCache.set(fbt.id, cachedFbt)
 
+  // Because sequelize is soo "optimized", we need to manually mark the field as changed
+  summary.changed('data', true)
   await summary.save()
 })
 
@@ -85,6 +87,7 @@ Feedback.afterDestroy(async feedback => {
   //@ts-expect-error fbt is not yet typescripted
   await feedbackTargetCache.set(fbt.id, cachedFbt)
 
+  summary.changed('data', true)
   await summary.save()
 })
 

diff --git a/src/server/routes/courseUnits/courseUnitController.js b/src/server/routes/courseUnits/courseUnitController.js
@@ -102,11 +102,11 @@ const getCourseUnitsForTeacher = async (req, res) => {
                 },
               ],
             },
-            include: {
-              model: Summary,
-              required: false,
-              as: 'summary',
-            },
+          },
+          {
+            model: Summary,
+            required: false,
+            as: 'summary',
           },
           {
             model: CourseUnit,
@@ -181,7 +181,8 @@ const getCourseUnitsForTeacher = async (req, res) => {
             ...feedbackTarget.courseRealisation.toJSON(),
             feedbackResponseGiven: feedbackTarget.dataValues.feedbackResponseGiven,
             feedbackResponseSent: feedbackTarget.dataValues.feedbackResponseSent,
-            feedbackCount: Number(feedbackTarget.courseRealisation.summary?.feedbackCount ?? 0),
+            feedbackCount: feedbackTarget.summary?.feedbackCount ?? 0,
+            studentCount: feedbackTarget.summary?.studentCount ?? 0,
             feedbackTarget: _.pick(feedbackTarget, targetFields),
           }
         : null