Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -50,16 +50,18 @@ describe('form_feedback.server.model', () => {
)
})

it('should throw validation error when rating param is missing', async () => {
it('should save successfully when rating param is missing (now optional; new rows use csat)', async () => {
// Arrange
const paramsWithoutRating = omit(DEFAULT_PARAMS, 'rating')
// Act
const actualPromise = new FeedbackModel(paramsWithoutRating).save()
const actual = await FeedbackModel.create({
...paramsWithoutRating,
csat: 4,
})

// Assert
await expect(actualPromise).rejects.toThrow(
mongoose.Error.ValidationError,
)
expect(actual.rating).toBeUndefined()
expect(actual.csat).toEqual(4)
})

it('should save successfully with triggerSource and formId', async () => {
Expand Down Expand Up @@ -137,6 +139,38 @@ describe('form_feedback.server.model', () => {
expect(actual.rating).toEqual(0)
})

it('should accept csat values 1 through 5', async () => {
for (const csat of [1, 2, 3, 4, 5] as const) {
const actual = await FeedbackModel.create({
...omit(DEFAULT_PARAMS, 'rating'),
csat,
})
expect(actual.csat).toEqual(csat)
}
})

it('should reject csat value of 6', async () => {
const actualPromise = new FeedbackModel({
...omit(DEFAULT_PARAMS, 'rating'),
csat: 6,
}).save()

await expect(actualPromise).rejects.toThrow(
mongoose.Error.ValidationError,
)
})

it('should reject csat value of 0 (csat is 1-5, unlike legacy rating)', async () => {
const actualPromise = new FeedbackModel({
...omit(DEFAULT_PARAMS, 'rating'),
csat: 0,
}).save()

await expect(actualPromise).rejects.toThrow(
mongoose.Error.ValidationError,
)
})

it('should default ratingChanged to false', async () => {
const actual = await FeedbackModel.create(DEFAULT_PARAMS)
expect(actual.ratingChanged).toEqual(false)
Expand Down
11 changes: 10 additions & 1 deletion apps/backend/src/app/models/admin_feedback.server.model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,20 @@ const AdminFeedbackSchema = new Schema<
ref: USER_SCHEMA_ID,
required: true,
},
// Legacy thumbs (0/1) key. Optional so new rows can omit it in favour of `csat`.
rating: {
type: Number,
min: 0,
max: 5,
required: true,
required: false,
},
// CSAT: the new 1-5 star satisfaction score. Own key to keep it separate from
// legacy `rating` data.
csat: {
type: Number,
min: 1,
max: 5,
required: false,
},
comment: {
type: String,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ describe('feedback.service', () => {
const MOCK_COMMENT = 'mock comment'
const MOCK_ADMIN_FEEDBACK = new AdminFeedbackModel({
userId: MOCK_USER_ID,
rating: MOCK_RATING,
csat: MOCK_RATING,
comment: MOCK_COMMENT,
})

Expand All @@ -41,7 +41,7 @@ describe('feedback.service', () => {
// Act
const actualResult = await AdminFeedbackService.insertAdminFeedback({
userId: MOCK_USER_ID,
rating: MOCK_RATING,
csat: MOCK_RATING,
comment: MOCK_COMMENT,
})

Expand All @@ -54,7 +54,7 @@ describe('feedback.service', () => {
it('should return Admin Feedback document on successful insertion without comment', async () => {
const mock_feedback_no_comment = new AdminFeedbackModel({
userId: MOCK_USER_ID,
rating: MOCK_RATING,
csat: MOCK_RATING,
})
// Arrange
// Mock success.
Expand All @@ -66,7 +66,7 @@ describe('feedback.service', () => {
// Act
const actualResult = await AdminFeedbackService.insertAdminFeedback({
userId: MOCK_USER_ID,
rating: MOCK_RATING,
csat: MOCK_RATING,
})

// Assert
Expand All @@ -80,7 +80,7 @@ describe('feedback.service', () => {
const mockFormId = new ObjectId().toHexString()
const mockFeedbackWithTrigger = new AdminFeedbackModel({
userId: MOCK_USER_ID,
rating: MOCK_RATING,
csat: MOCK_RATING,
comment: MOCK_COMMENT,
triggerSource: mockTriggerSource,
formId: mockFormId,
Expand All @@ -93,7 +93,7 @@ describe('feedback.service', () => {

const actualResult = await AdminFeedbackService.insertAdminFeedback({
userId: MOCK_USER_ID,
rating: MOCK_RATING,
csat: MOCK_RATING,
comment: MOCK_COMMENT,
triggerSource: mockTriggerSource,
formId: mockFormId,
Expand All @@ -102,7 +102,7 @@ describe('feedback.service', () => {
expect(insertSpy).toHaveBeenCalledTimes(1)
expect(insertSpy).toHaveBeenCalledWith({
userId: MOCK_USER_ID,
rating: MOCK_RATING,
csat: MOCK_RATING,
comment: MOCK_COMMENT,
triggerSource: mockTriggerSource,
formId: mockFormId,
Expand All @@ -122,7 +122,7 @@ describe('feedback.service', () => {
// Act
const actualResult = await AdminFeedbackService.insertAdminFeedback({
userId: MOCK_USER_ID,
rating: MOCK_RATING,
csat: MOCK_RATING,
})

// Assert
Expand All @@ -143,7 +143,7 @@ describe('feedback.service', () => {
await AdminFeedbackModel.create({
_id: MOCK_FEEDBACK_ID,
userId: MOCK_USER_ID,
rating: MOCK_RATING,
csat: MOCK_RATING,
comment: MOCK_COMMENT,
})
})
Expand All @@ -157,7 +157,7 @@ describe('feedback.service', () => {
const actualResult = await AdminFeedbackService.updateAdminFeedback({
feedbackId: MOCK_FEEDBACK_ID,
userId: MOCK_USER_ID,
rating: newRating,
csat: newRating,
comment: newComment,
})

Expand All @@ -166,7 +166,7 @@ describe('feedback.service', () => {
// Assert
expect(actualResult.isOk()).toEqual(true)
expect(newFeedback?.comment).toEqual(newComment)
expect(newFeedback?.rating).toEqual(newRating)
expect(newFeedback?.csat).toEqual(newRating)
})

it('should set ratingChanged to true when rating is updated', async () => {
Expand All @@ -175,13 +175,13 @@ describe('feedback.service', () => {
await AdminFeedbackService.updateAdminFeedback({
feedbackId: MOCK_FEEDBACK_ID,
userId: MOCK_USER_ID,
rating: newRating,
csat: newRating,
})

const newFeedback = await AdminFeedbackModel.findById(MOCK_FEEDBACK_ID)

expect(newFeedback?.ratingChanged).toEqual(true)
expect(newFeedback?.rating).toEqual(newRating)
expect(newFeedback?.csat).toEqual(newRating)
})

it('should not set ratingChanged when only comment is updated', async () => {
Expand All @@ -200,30 +200,30 @@ describe('feedback.service', () => {
const newRating = 3
const expectedResult = new AdminFeedbackModel({
userId: MOCK_USER_ID,
rating: newRating,
csat: newRating,
comment: MOCK_COMMENT,
})

// Act
const actualResult = await AdminFeedbackService.updateAdminFeedback({
feedbackId: MOCK_FEEDBACK_ID,
userId: MOCK_USER_ID,
rating: newRating,
csat: newRating,
})

const newFeedback = await AdminFeedbackModel.findById(MOCK_FEEDBACK_ID)

// Assert
expect(actualResult.isOk()).toEqual(true)
expect(newFeedback?.comment).toEqual(expectedResult.comment)
expect(newFeedback?.rating).toEqual(expectedResult.rating)
expect(newFeedback?.csat).toEqual(expectedResult.csat)
})

it('should update feedback successfully with only comment changes', async () => {
const newComment = 'new comment'
const expectedResult = new AdminFeedbackModel({
userId: MOCK_USER_ID,
rating: MOCK_RATING,
csat: MOCK_RATING,
comment: newComment,
})

Expand All @@ -239,7 +239,7 @@ describe('feedback.service', () => {
// Assert
expect(actualResult.isOk()).toEqual(true)
expect(newFeedback?.comment).toEqual(expectedResult.comment)
expect(newFeedback?.rating).toEqual(expectedResult.rating)
expect(newFeedback?.csat).toEqual(expectedResult.csat)
})

it('should return ok if there are no changes to be made', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,20 +45,22 @@ const submitAdminFeedback: ControllerHandler<
const { rating, comment, triggerSource, formId } = req.body

// Dual-emit: old metric kept so existing dashboards don't break during
// migration. New metric (.v2) for the 1-5 scale. Remove the old metric
// once report card has migrated to .v2.
// migration. New `.csat` metric is the 1-5 CSAT measure. Remove the old
// metric once the report card has migrated to `.csat`.
statsdClient.distribution('formsg.users.feedback.rating', rating, 1, {
rating: `${rating}`,
...(triggerSource ? { triggerSource } : {}),
})
statsdClient.distribution('formsg.users.feedback.rating.v2', rating, 1, {
rating: `${rating}`,
statsdClient.distribution('formsg.users.feedback.csat', rating, 1, {
csat: `${rating}`,
...(triggerSource ? { triggerSource } : {}),
})

// The wire keeps sending `rating`; store it under its own `csat` key so the
// new measure never mixes with legacy thumbs data.
return AdminFeedbackService.insertAdminFeedback({
userId: sessionUserId,
rating,
csat: rating,
comment,
triggerSource,
formId,
Expand Down Expand Up @@ -109,11 +111,12 @@ const updateAdminFeedback: ControllerHandler<

const { rating, comment } = req.body

// The wire keeps sending `rating`; store it under its own `csat` key.
return AdminFeedbackService.updateAdminFeedback({
feedbackId,
userId: sessionUserId,
comment,
rating,
csat: rating,
})
.map(() =>
res
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,28 +14,28 @@ const logger = createLoggerWithLabel(module)
/**
* Inserts given admin feedback to the database.
* @param userId the userId of the admin that provided the feedback
* @param rating the feedback rating to insert (0 for thumbs down, 1 for thumbs up)
* @param csat the 1-5 star satisfaction score to insert
* @param comment the feedback comment to insert if available
* @returns ok(IAdminFeedbackSchema) if successfully inserted
* @returns err(DatabaseError) on database error
*/
export const insertAdminFeedback = ({
userId,
rating,
csat,
comment,
triggerSource,
formId,
}: {
userId: string
rating: number
csat: number
comment?: string
triggerSource?: string
formId?: string
}) => {
return ResultAsync.fromPromise(
AdminFeedbackModel.create({
userId,
rating,
csat,
comment,
triggerSource,
formId,
Expand All @@ -57,11 +57,11 @@ export const insertAdminFeedback = ({

/**
* Updates admin feedback in the database.
* Will not update previous value if comment or rating is undefined
* Will not update previous value if comment or csat is undefined
* @param feedbackId the id of the admin feedback to update
* @param userId the id of the admin
* @param comment the feedback comment to insert
* @param rating the feedback rating to insert (0 for thumbs down, 1 for thumbs up)
* @param csat the 1-5 star satisfaction score to insert
* @returns ok(IAdminFeedbackSchema) if successfully inserted
* @returns err(MissingAdminFeedbackError) if feedback document with the same feedbackId and userId is not found
* @returns err(DatabaseError) on database error
Expand All @@ -70,17 +70,18 @@ export const updateAdminFeedback = ({
feedbackId,
userId,
comment,
rating,
csat,
}: {
feedbackId: string
userId: string
comment?: string
rating?: number
csat?: number
}) => {
const updateObj: Record<string, unknown> = {}
if (comment !== undefined) updateObj.comment = comment
if (rating !== undefined) {
updateObj.rating = rating
if (csat !== undefined) {
updateObj.csat = csat
// `ratingChanged` is a pre-existing flag; it now reflects a `csat` edit.
updateObj.ratingChanged = true
}

Expand Down
Loading
Loading