From af1d13d3a2214674d81595a158fb31b8045a84ef Mon Sep 17 00:00:00 2001
From: Robert Whitaker <__github__@robwhitaker.com>
Date: Fri, 3 Jul 2020 10:48:45 -0400
Subject: [PATCH] Fix bug where updated webhook options failed to save (fixes
 #12336) (#12342)

* Fix bug where updated webhook options failed to save

This bug was caused by Mongoose not creating getters/setters for array
elements (https://mongoosejs.com/docs/faq.html#array-changes-not-saved).
So, although the webhook was being updated properly, Mongoose was not
actually committing it to the database. Telling Mongoose that the array
of webhooks has changed via `markModified` fixes the issue.

Additionally, the relevant API test case was only checking whether or
not the webhook returned from the PUT endpoint matched the expected
update. Since the endpoint was returning the updated webhook without
querying the database again, this test case would pass. It has been
updated to check both the returned webhook as well as the version of the
webhook that is saved to the database against the expected. In other
words:

`assert returned === saved === expected`

Fixes #12336

* Call markModified on webhook.options instead of user.webhooks

This tells Mongoose that only the modified webhook's options changed
instead of telling it that the entire user.webhooks array changed,
saving a costly DB update.
---
 .../webhook/PUT-user_update_webhook.test.js   | 25 ++++++++++++-------
 website/server/controllers/api-v3/webhook.js  |  4 +++
 2 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/test/api/v3/integration/webhook/PUT-user_update_webhook.test.js b/test/api/v3/integration/webhook/PUT-user_update_webhook.test.js
index 7745872984..f0fcaf9ad2 100644
--- a/test/api/v3/integration/webhook/PUT-user_update_webhook.test.js
+++ b/test/api/v3/integration/webhook/PUT-user_update_webhook.test.js
@@ -127,19 +127,26 @@ describe('PUT /user/webhook/:id', () => {
   it('can update taskActivity options', async () => {
     const type = 'taskActivity';
     const options = {
+      checklistScored: true,
       updated: false,
-      deleted: true,
+      scored: false,
     };
-
-    const webhook = await user.put(`/user/webhook/${webhookToUpdate.id}`, { type, options });
-
-    expect(webhook.options).to.eql({
-      checklistScored: false, // starting value
+    const expected = {
+      checklistScored: true,
       created: true, // starting value
       updated: false,
-      deleted: true,
-      scored: true, // default value
-    });
+      deleted: false, // starting value
+      scored: false,
+    };
+
+    const returnedWebhook = await user.put(`/user/webhook/${webhookToUpdate.id}`, { type, options });
+
+    await user.sync();
+
+    const savedWebhook = user.webhooks.find(hook => webhookToUpdate.id === hook.id);
+
+    expect(returnedWebhook.options).to.eql(expected);
+    expect(savedWebhook.options).to.eql(expected);
   });
 
   it('errors if taskActivity option is not a boolean', async () => {
diff --git a/website/server/controllers/api-v3/webhook.js b/website/server/controllers/api-v3/webhook.js
index 3336f0eaec..00b5bf54cb 100644
--- a/website/server/controllers/api-v3/webhook.js
+++ b/website/server/controllers/api-v3/webhook.js
@@ -223,6 +223,10 @@ api.updateWebhook = {
 
     webhook.formatOptions(res);
 
+    // Tell Mongoose that the webhook's options have been modified
+    // so it actually commits the options changes to the database
+    webhook.markModified('options');
+
     await user.save();
     res.respond(200, webhook);
   },