Merge pull request #1662 from botpress/ya-fix-attributes

fix(users): attributes were overwritten when updating

modules/channel-web/src/backend/api.ts

@@ -116,10 +116,8 @@ export default async (bp: typeof sdk, db: Database) => {
       }
 
       await bp.users.getOrCreateUser('web', userId) // Create the user if it doesn't exist
-
       const payload = req.body || {}
-      const { timezone } = payload
-      const isValidTimezone = _.isNumber(timezone) && timezone >= -12 && timezone <= 14 && timezone % 0.5 === 0
+
       let { conversationId = undefined } = req.query || {}
       conversationId = conversationId && parseInt(conversationId)
 
@@ -133,8 +131,17 @@ export default async (bp: typeof sdk, db: Database) => {
         return res.status(400).send(ERR_MSG_TYPE)
       }
 
-      if (timezone && isValidTimezone) {
-        await bp.users.updateAttributes('web', userId, { timezone })
+      if (payload.type === 'visit') {
+        const { timezone } = payload
+        const isValidTimezone = _.isNumber(timezone) && timezone >= -12 && timezone <= 14 && timezone % 0.5 === 0
+
+        const newAttributes = {
+          ...(isValidTimezone && { timezone })
+        }
+
+        if (Object.getOwnPropertyNames(newAttributes).length) {
+          await bp.users.updateAttributes('web', userId, newAttributes)
+        }
       }
 
       if (!conversationId) {

src/bp/core/api.ts

@@ -117,6 +117,7 @@ const users = (userRepo: UserRepository): typeof sdk.users => {
   return {
     getOrCreateUser: userRepo.getOrCreate.bind(userRepo),
     updateAttributes: userRepo.updateAttributes.bind(userRepo),
+    setAttributes: userRepo.setAttributes.bind(userRepo),
     getAllUsers: userRepo.getAllUsers.bind(userRepo),
     getUserCount: userRepo.getUserCount.bind(userRepo)
   }

src/bp/core/repositories/users.ts

@@ -8,6 +8,7 @@ import { TYPES } from '../types'
 export interface UserRepository {
   getOrCreate(channel: string, id: string): Knex.GetOrCreateResult<User>
   updateAttributes(channel: string, id: string, attributes: any): Promise<void>
+  setAttributes(channel: string, id: string, attributes: any): Promise<void>
   getAllUsers(paging?: Paging): Promise<any>
   getUserCount(): Promise<any>
 }
@@ -81,20 +82,35 @@ export class KnexUserRepository implements UserRepository {
     return this.database.knex.json.get(user.attributes)
   }
 
+  async setAttributes(channel: string, user_id: string, attributes: any): Promise<void> {
+    channel = channel.toLowerCase()
+    await this._dataRetentionUpdate(channel, user_id, attributes)
+
+    await this.database
+      .knex(this.tableName)
+      .update({ attributes: this.database.knex.json.set(attributes) })
+      .where({ channel, user_id })
+  }
+
   async updateAttributes(channel: string, user_id: string, attributes: any): Promise<void> {
     channel = channel.toLowerCase()
+    await this._dataRetentionUpdate(channel, user_id, attributes)
 
-    if (this.dataRetentionService.hasPolicy()) {
-      const originalAttributes = await this.getAttributes(channel, user_id)
-      await this.dataRetentionService.updateExpirationForChangedFields(channel, user_id, originalAttributes, attributes)
-    }
+    const originalAttributes = await this.getAttributes(channel, user_id)
 
     await this.database
       .knex(this.tableName)
-      .update({ attributes: this.database.knex.json.set(attributes) })
+      .update({ attributes: this.database.knex.json.set({ ...originalAttributes, ...attributes }) })
       .where({ channel, user_id })
   }
 
+  private async _dataRetentionUpdate(channel: string, user_id: string, attributes: any) {
+    if (this.dataRetentionService.hasPolicy()) {
+      const originalAttributes = await this.getAttributes(channel, user_id)
+      await this.dataRetentionService.updateExpirationForChangedFields(channel, user_id, originalAttributes, attributes)
+    }
+  }
+
   async getAllUsers(paging?: Paging) {
     let query = this.database
       .knex(this.tableName)

src/bp/core/services/middleware/state-manager.ts

@@ -46,7 +46,7 @@ export class StateManager {
     const { user, context, session, temp } = event.state
     const sessionId = SessionIdFactory.createIdFromEvent(event)
 
-    await this.userRepo.updateAttributes(event.channel, event.target, _.omitBy(user, _.isNil))
+    await this.userRepo.setAttributes(event.channel, event.target, _.omitBy(user, _.isNil))
 
     // Take last 5 messages only
     if (session && session.lastMessages) {

src/bp/core/services/retention/janitor.ts

@@ -43,7 +43,7 @@ export class DataRetentionJanitor extends Janitor {
       await Promise.mapSeries(expired, async ({ channel, user_id, field_path }) => {
         const { result: user } = await this.userRepo.getOrCreate(channel, user_id)
 
-        await this.userRepo.updateAttributes(channel, user.id, _.omit(user.attributes, field_path))
+        await this.userRepo.setAttributes(channel, user.id, _.omit(user.attributes, field_path))
         await this.dataRetentionService.delete(channel, user_id, field_path)
       })
 

src/bp/sdk/botpress.d.ts

@@ -1016,9 +1016,14 @@ declare module 'botpress/sdk' {
     export function getOrCreateUser(channel: string, userId: string): GetOrCreateResult<User>
 
     /**
-     * Update attributes of a specific user
+     * Merge the specified attributes to the existing attributes of the user
      */
     export function updateAttributes(channel: string, userId: string, attributes: any): Promise<void>
+
+    /**
+     * Overwrite all the attributes of the user with the specified payload
+     */
+    export function setAttributes(channel: string, userId: string, attributes: any): Promise<void>
     export function getAllUsers(paging?: Paging): Promise<any>
     export function getUserCount(): Promise<any>
   }