Maintenance: Fix difference function to compare both ways and also make sure that the observer copies hashes to prevent references.

app/assets/javascripts/app/controllers/_application_controller/observer.coffee

@@ -2,6 +2,7 @@ class App.ControllerObserver extends App.Controller
   model: 'Ticket'
   template: 'tba'
   globalRerender: true
+  lastAttributes: undefined
 
   ###
   observe:
@@ -25,7 +26,7 @@ class App.ControllerObserver extends App.Controller
     # rerender, e. g. on language change
     if @globalRerender
       @controllerBind('ui:rerender', =>
-        @lastAttributres = undefined
+        @lastAttributes = undefined
         @maybeRender(App[@model].fullLocal(@object_id))
       )
 
@@ -43,32 +44,39 @@ class App.ControllerObserver extends App.Controller
     if !@subscribeId
       @subscribeId = object.subscribe(@subscribe)
 
-    # remember current attributes
+    return if !@hasChanged(object)
+
+    @render(object)
+
+  hasChanged: (object) =>
     currentAttributes = {}
+
     if @observe
       for key, active of @observe
-        if active
-          currentAttributes[key] = object[key]
+        if active && !_.isFunction(value)
+          currentAttributes[key] = clone(object[key])
+
     if @observeNot
       for key, value of object
-        if key isnt 'cid' && !@observeNot[key] && !_.isFunction(value) && !_.isObject(value)
-          currentAttributes[key] = value
+        if key isnt 'cid' && !@observeNot[key] && !_.isFunction(value)
+          currentAttributes[key] = clone(value)
 
-    if !@lastAttributres
-      @lastAttributres = {}
-    else
-      diff = difference(currentAttributes, @lastAttributres)
-      if _.isEmpty(diff)
-        @log 'debug', 'maybeRender no diff, no rerender'
-        return
+    if !@lastAttributes
+      @lastAttributes = currentAttributes
+      return true
+
+    diff = difference(currentAttributes, @lastAttributes)
+    if _.isEmpty(diff)
+      @log 'debug', 'maybeRender no diff, no rerender'
+      return false
 
     @log 'debug', 'maybeRender.diff', diff, @observe, @model
-    @lastAttributres = currentAttributes
+    @lastAttributes = currentAttributes
 
-    @render(object, diff)
+    true
 
-  render: (object, diff) =>
-    @log 'debug', 'render', @template, object, diff
+  render: (object) =>
+    @log 'debug', 'render', @template, object
     @html App.view(@template)(
       object: object
     )

app/assets/javascripts/app/controllers/organization_profile.coffee

@@ -202,7 +202,7 @@ class Object extends App.ControllerObserver
     value = $(e.target).html()
     org   = App.Organization.find(@object_id)
     if org[name] isnt value
-      @lastAttributres[name] = value
+      @lastAttributes[name] = value
       data = {}
       data[name] = value
       org.updateAttributes(data)

app/assets/javascripts/app/controllers/ticket_zoom/article_action/internal.coffee

@@ -39,7 +39,7 @@ class Internal
     internal = true
     if article.internal == true
       internal = false
-    ui.lastAttributres.internal = internal
+    ui.lastAttributes.internal = internal
     article.updateAttributes(internal: internal)
 
     # runtime update

app/assets/javascripts/app/controllers/ticket_zoom/article_view.coffee

@@ -218,11 +218,11 @@ class ArticleViewItem extends App.ControllerObserver
     )
 
     @articleActions = new App.TicketZoomArticleActions(
-      el:              @$('.js-article-actions')
-      ticket:          @ticket
-      article:         article
-      lastAttributres: @lastAttributres
-      form_id:         @form_id
+      el:             @$('.js-article-actions')
+      ticket:         @ticket
+      article:        article
+      lastAttributes: @lastAttributes
+      form_id:        @form_id
     )
 
     # set see more

app/assets/javascripts/app/controllers/ticket_zoom/title.coffee

@@ -19,7 +19,7 @@ class App.TicketZoomTitle extends App.ControllerObserver
     title = $(e.target).ceg() || ''
 
     # update title
-    return if title is @lastAttributres.title
+    return if title is @lastAttributes.title
     ticket = App.Ticket.find(@object_id)
     ticket.title = title
 

app/assets/javascripts/app/controllers/user_profile.coffee

@@ -220,7 +220,7 @@ class Object extends App.ControllerObserver
     value = $(e.target).html()
     user  = App.User.find(@object_id)
     if user[name] isnt value
-      @lastAttributres[name] = value
+      @lastAttributes[name] = value
       data = {}
       data[name] = value
       user.updateAttributes(data)

app/assets/javascripts/application.js

@@ -70,9 +70,9 @@ Date.prototype.getWeek = function() {
 
 function difference(object1, object2) {
   var changes = {};
-  for (var name in object1) {
-    if (name in object2) {
-      if (_.isObject(object2[name]) && !_.isArray(object2[name])) {
+  _.uniq(Object.keys(object1).concat(Object.keys(object2))).forEach(function(name) {
+    if (name in object1 && name in object2) {
+      if (_.isObject(object1[name]) && !_.isArray(object1[name]) && _.isObject(object2[name]) && !_.isArray(object2[name])) {
         var diff = difference(object1[name], object2[name]);
         if (!_.isEmpty(diff)) {
             changes[name] = diff;
@@ -80,8 +80,10 @@ function difference(object1, object2) {
       } else if (!_.isEqual(object1[name], object2[name])) {
         changes[name] = object2[name];
       }
+    } else {
+      changes[name] = object2[name]
     }
-  }
+  })
   return changes;
 }
 

public/assets/tests/qunit/controller_observer.js

@@ -0,0 +1,202 @@
+QUnit.test( "controller observer tests - observe", assert => {
+
+  App.Ticket.refresh([{
+    id: 1,
+    title: 'ticket',
+    state_id: 1,
+    customer_id: 33,
+    organization_id: 1,
+    owner_id: 1,
+    preferences: { a: 1, b: 2 },
+  }])
+
+  var observer1 = new App.ControllerObserver({
+    object_id: 1,
+    template: 'version',
+    observe: {
+      title: true,
+      preferences: true,
+    },
+  })
+
+  var ticket = App.Ticket.find(1)
+
+  assert.equal(false, observer1.hasChanged(ticket))
+
+  // track title changes
+  ticket.title = 'title 2'
+
+  assert.equal(true, observer1.hasChanged(ticket))
+  assert.equal(false, observer1.hasChanged(ticket))
+
+  ticket.title = undefined
+
+  assert.equal(true, observer1.hasChanged(ticket))
+  assert.equal(false, observer1.hasChanged(ticket))
+
+  ticket.title = 'title 3'
+
+  assert.equal(true, observer1.hasChanged(ticket))
+  assert.equal(false, observer1.hasChanged(ticket))
+
+  // track no owner_id changes
+  ticket.owner_id = 2
+
+  assert.equal(false, observer1.hasChanged(ticket))
+  assert.equal(false, observer1.hasChanged(ticket))
+
+  // track preferences changes
+  ticket.preferences['a'] = 3
+
+  assert.equal(true, observer1.hasChanged(ticket))
+  assert.equal(false, observer1.hasChanged(ticket))
+
+  ticket.preferences['c'] = 3
+  assert.equal(true, observer1.hasChanged(ticket))
+  assert.equal(false, observer1.hasChanged(ticket))
+
+  // track no new_attribute1 changes
+  ticket.new_attribute1 = 'na 3'
+
+  assert.equal(false, observer1.hasChanged(ticket))
+  assert.equal(false, observer1.hasChanged(ticket))
+
+  ticket.new_attribute2 = function() { console.log(1) }
+
+  assert.equal(false, observer1.hasChanged(ticket))
+  assert.equal(false, observer1.hasChanged(ticket))
+
+  ticket.new_attribute2 = function() { console.log(2) }
+
+  assert.equal(false, observer1.hasChanged(ticket))
+  assert.equal(false, observer1.hasChanged(ticket))
+
+  // track title changes
+  ticket.title = function() { console.log(1) }
+
+  assert.equal(true, observer1.hasChanged(ticket))
+  assert.equal(false, observer1.hasChanged(ticket))
+
+  ticket.title = function() { console.log(2) }
+
+  assert.equal(false, observer1.hasChanged(ticket))
+  assert.equal(false, observer1.hasChanged(ticket))
+
+  ticket.title = 1
+
+  assert.equal(true, observer1.hasChanged(ticket))
+  assert.equal(false, observer1.hasChanged(ticket))
+
+});
+
+QUnit.test( "controller observer tests - observeNot", assert => {
+
+  App.Ticket.refresh([{
+    id: 2,
+    title: 'ticket',
+    state_id: 1,
+    customer_id: 33,
+    organization_id: 1,
+    owner_id: 1,
+    preferences: { a: 1, b: 2 },
+  }])
+
+  var observer1 = new App.ControllerObserver({
+    object_id: 2,
+    template: 'version',
+    observeNot: {
+      title: true,
+      preferences: true,
+    },
+  })
+
+  var ticket = App.Ticket.find(2)
+
+  assert.equal(false, observer1.hasChanged(ticket))
+
+  // track no title changes
+  ticket.title = 'title 2'
+
+  assert.equal(false, observer1.hasChanged(ticket))
+  assert.equal(false, observer1.hasChanged(ticket))
+
+  // track owner_id changes
+  ticket.owner_id = 2
+
+  assert.equal(true, observer1.hasChanged(ticket))
+  assert.equal(false, observer1.hasChanged(ticket))
+
+  ticket.owner_id = undefined
+
+  assert.equal(true, observer1.hasChanged(ticket))
+  assert.equal(false, observer1.hasChanged(ticket))
+
+  ticket.owner_id = 3
+
+  assert.equal(true, observer1.hasChanged(ticket))
+  assert.equal(false, observer1.hasChanged(ticket))
+
+  // track no preferences changes
+  ticket.preferences['a'] = 3
+
+  assert.equal(false, observer1.hasChanged(ticket))
+  assert.equal(false, observer1.hasChanged(ticket))
+
+  ticket.preferences['c'] = 3
+  assert.equal(false, observer1.hasChanged(ticket))
+  assert.equal(false, observer1.hasChanged(ticket))
+
+  // track preferences2 changes
+  ticket.preferences2 = {}
+
+  assert.equal(true, observer1.hasChanged(ticket))
+  assert.equal(false, observer1.hasChanged(ticket))
+
+  ticket.preferences2['a'] = 3
+
+  assert.equal(true, observer1.hasChanged(ticket))
+  assert.equal(false, observer1.hasChanged(ticket))
+
+  ticket.preferences2['a'] = 2
+
+  assert.equal(true, observer1.hasChanged(ticket))
+  assert.equal(false, observer1.hasChanged(ticket))
+
+  ticket.preferences2['c'] = 3
+  assert.equal(true, observer1.hasChanged(ticket))
+  assert.equal(false, observer1.hasChanged(ticket))
+
+  // track new_attribute1 changes
+  ticket.new_attribute1 = 'na 3'
+
+  assert.equal(true, observer1.hasChanged(ticket))
+  assert.equal(false, observer1.hasChanged(ticket))
+
+  // track no new_attribute2 changes (because of function content)
+  ticket.new_attribute2 = function() { console.log(1) }
+
+  assert.equal(false, observer1.hasChanged(ticket))
+  assert.equal(false, observer1.hasChanged(ticket))
+
+  ticket.new_attribute2 = function() { console.log(2) }
+
+  assert.equal(false, observer1.hasChanged(ticket))
+  assert.equal(false, observer1.hasChanged(ticket))
+
+  // track owner_id changes (pnly if content has no function content)
+  ticket.owner_id = function() { console.log(1) }
+
+  assert.equal(true, observer1.hasChanged(ticket))
+  assert.equal(false, observer1.hasChanged(ticket))
+
+  ticket.owner_id = function() { console.log(2) }
+
+  assert.equal(false, observer1.hasChanged(ticket))
+  assert.equal(false, observer1.hasChanged(ticket))
+
+  ticket.owner_id = 1
+
+  assert.equal(true, observer1.hasChanged(ticket))
+  assert.equal(false, observer1.hasChanged(ticket))
+
+});

public/assets/tests/qunit/core.js

@@ -538,7 +538,9 @@ QUnit.test('difference', assert => {
   object2 = {
     key1: 123,
   }
-  result = {}
+  result = {
+    key2: undefined
+  }
   item = difference(object1, object2)
   assert.deepEqual(item, result)
 
@@ -549,38 +551,106 @@ QUnit.test('difference', assert => {
     key1: 123,
     key2: 124
   }
+  result = {
+    key2: 124
+  }
+  item = difference(object1, object2)
+  assert.deepEqual(item, result)
+
+  object1 = {
+    customer_id: 1,
+    organization_id: 2,
+  }
+  object2 = {
+    customer_id: 1,
+    organization_id: null,
+  }
+  result = {
+    organization_id: null,
+  }
+  item = difference(object1, object2)
+  assert.deepEqual(item, result)
+
+  object1 = {
+    customer_id: 1,
+    organization_id: null,
+  }
+  object2 = {
+    customer_id: 1,
+    organization_id: 2,
+  }
+  result = {
+    organization_id: 2,
+  }
+  item = difference(object1, object2)
+  assert.deepEqual(item, result)
+
+  object1 = {
+    customer_id: 1,
+    preferences: { resolved: true },
+  }
+  object2 = {
+    customer_id: 1,
+    preferences: {},
+  }
+  result = {
+    preferences: { resolved: undefined }
+  }
+  item = difference(object1, object2)
+  assert.deepEqual(item, result)
+
+  object1 = {
+    customer_id: 1,
+  }
+  object2 = {
+    customer_id: 1,
+    preferences: { resolved: true },
+  }
+  result = {
+    preferences: { resolved: true }
+  }
+  item = difference(object1, object2)
+  assert.deepEqual(item, result)
+
+  object1 = {
+    customer_id: 1,
+    preferences: {},
+  }
+  object2 = {
+    customer_id: 1,
+    preferences: { resolved: true },
+  }
+  result = {
+    preferences: { resolved: true }
+  }
+  item = difference(object1, object2)
+  assert.deepEqual(item, result)
+
+  object1 = {
+    customer_id: 1,
+    preferences: { resolved: false },
+  }
+  object2 = {
+    customer_id: 1,
+    preferences: { resolved: true },
+  }
+  result = {
+    preferences: { resolved: true }
+  }
+  item = difference(object1, object2)
+  assert.deepEqual(item, result)
+
+  object1 = {
+    customer_id: 1,
+    preferences: { resolved: true },
+  }
+  object2 = {
+    customer_id: 1,
+    preferences: { resolved: true },
+  }
   result = {}
   item = difference(object1, object2)
   assert.deepEqual(item, result)
-
-  object1 = {
-    customer_id: 1,
-    organization_id: 2,
-  }
-  object2 = {
-    customer_id: 1,
-    organization_id: null,
-  }
-  result = {
-    organization_id: null,
-  }
-  item = difference(object1, object2)
-  assert.deepEqual(item, result)
-
-  object1 = {
-    customer_id: 1,
-    organization_id: null,
-  }
-  object2 = {
-    customer_id: 1,
-    organization_id: 2,
-  }
-  result = {
-    organization_id: 2,
-  }
-  item = difference(object1, object2)
-  assert.deepEqual(item, result)
-
 });
 
 QUnit.test('auth - not existing user', assert => {