From e514b521cf20f3243db5c41822f6096e890c4447 Mon Sep 17 00:00:00 2001
From: brontolosone <177225737+brontolosone@users.noreply.github.com>
Date: Wed, 18 Sep 2024 13:02:31 +0000
Subject: [PATCH] fix storing jsonb-null propertyValue

---
 lib/model/query/user-preferences.js      | 10 ++++++++--
 test/integration/api/user-preferences.js | 21 +++++++++++++++++++++
 2 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/lib/model/query/user-preferences.js b/lib/model/query/user-preferences.js
index 9dcd7efde..234662a21 100644
--- a/lib/model/query/user-preferences.js
+++ b/lib/model/query/user-preferences.js
@@ -65,7 +65,13 @@ const _writeProperty = (tablename, subject, userId, propertyName, propertyValue)
     .concat((subject === null) ? [] : ['projectId'])
     .map(el => sql.identifier([el]));
 
-  const values = [userId, propertyName, sql.json(propertyValue)]
+  // Work around null confusion (potential Slonik bug?).
+  // sql.json(null) doesn't produce what we need, it results in an exception
+  // "Error: Required parameter propertyValue missing."
+  // Yet the string 'null' (as distinct from the *jsonb* string '"null"' one would get with sql.json('null') !)
+  // gets properly casted by PostgreSQL to a jsonb null (as distinct from an SQL NULL), so we use that in this case.
+  const preparedPropertyValue = (propertyValue === null) ? 'null': sql.json(propertyValue);
+  const values = [userId, propertyName, preparedPropertyValue]
     .concat((subject === null) ? [] : [subject]);
 
   return one(sql`
@@ -75,7 +81,7 @@ const _writeProperty = (tablename, subject, userId, propertyName, propertyValue)
       (${sql.join(values, `, `)})
     ON CONFLICT ON CONSTRAINT ${sql.identifier([`${tablename}_primary_key`])}
       DO UPDATE
-        SET "propertyValue" = ${sql.json(propertyValue)}
+        SET "propertyValue" = ${preparedPropertyValue}
     RETURNING
       1 AS "modified_count"
   `);
diff --git a/test/integration/api/user-preferences.js b/test/integration/api/user-preferences.js
index 64fee5223..816fbe965 100644
--- a/test/integration/api/user-preferences.js
+++ b/test/integration/api/user-preferences.js
@@ -2,6 +2,27 @@ const { testService } = require('../setup');
 
 describe('api: user-preferences', () => {
 
+  it('can store a JS null propertyValue', testService(async (service) => {
+    const asAlice = await service.login('alice');
+
+    await asAlice.put('/v1/user-preferences/site/let-us-store-a-null')
+      .send({ propertyValue: null })
+      .expect(200);
+
+    await asAlice.get('/v1/users/current')
+      .set('X-Extended-Metadata', 'true')
+      .expect(200)
+      .then(({ body }) => {
+        body.preferences.should.eql({
+          site: {
+            'let-us-store-a-null': null,
+          },
+          projects: {
+          },
+        });
+      });
+  }));
+
   it('should not allow storing preferences for nonexistent projects', testService(async (service) => {
     const asAlice = await service.login('alice');