Bug 1142515: add unit test coverage for the editing of context data inside the Loop conversation view. r=Standard8

browser/components/loop/content/shared/js/mixins.js

@@ -24,7 +24,7 @@ loop.shared.mixins = (function() {
    * @param {Object}
    */
   function setRootObject(obj) {
-    console.log("loop.shared.mixins: rootObject set to " + obj);
+    // console.log("loop.shared.mixins: rootObject set to " + obj);
     rootObject = obj;
   }
 

browser/components/loop/test/desktop-local/roomStore_test.js

@@ -597,37 +597,45 @@ describe("loop.store.RoomStore", function () {
     });
   });
 
-  describe("#renameRoom", function() {
+  describe("#updateRoomContext", function() {
     var store, fakeMozLoop;
 
     beforeEach(function() {
       fakeMozLoop = {
         rooms: {
-          rename: null
+          get: sinon.stub().callsArgWith(1, null, {
+            roomToken: "42abc",
+            decryptedContext: {
+              roomName: "sillier name"
+            }
+          }),
+          update: null
         }
       };
       store = new loop.store.RoomStore(dispatcher, {mozLoop: fakeMozLoop});
     });
 
     it("should rename the room via mozLoop", function() {
-      fakeMozLoop.rooms.rename = sinon.spy();
+      fakeMozLoop.rooms.update = sinon.spy();
-      dispatcher.dispatch(new sharedActions.RenameRoom({
+      dispatcher.dispatch(new sharedActions.UpdateRoomContext({
         roomToken: "42abc",
         newRoomName: "silly name"
       }));
 
-      sinon.assert.calledOnce(fakeMozLoop.rooms.rename);
+      sinon.assert.calledOnce(fakeMozLoop.rooms.get);
-      sinon.assert.calledWith(fakeMozLoop.rooms.rename, "42abc",
+      sinon.assert.calledOnce(fakeMozLoop.rooms.update);
-        "silly name");
+      sinon.assert.calledWith(fakeMozLoop.rooms.update, "42abc", {
+        roomName: "silly name"
+      });
     });
 
-    it("should store any rename-encountered error", function() {
+    it("should store any update-encountered error", function() {
       var err = new Error("fake");
-      sandbox.stub(fakeMozLoop.rooms, "rename", function(roomToken, roomName, cb) {
+      sandbox.stub(fakeMozLoop.rooms, "update", function(roomToken, roomData, cb) {
         cb(err);
       });
 
-      dispatcher.dispatch(new sharedActions.RenameRoom({
+      dispatcher.dispatch(new sharedActions.UpdateRoomContext({
         roomToken: "42abc",
         newRoomName: "silly name"
       }));
@@ -636,14 +644,70 @@ describe("loop.store.RoomStore", function () {
     });
 
     it("should ensure only submitting a non-empty room name", function() {
-      fakeMozLoop.rooms.rename = sinon.spy();
+      fakeMozLoop.rooms.update = sinon.spy();
 
-      dispatcher.dispatch(new sharedActions.RenameRoom({
+      dispatcher.dispatch(new sharedActions.UpdateRoomContext({
         roomToken: "42abc",
         newRoomName: " \t  \t "
       }));
 
-      sinon.assert.notCalled(fakeMozLoop.rooms.rename);
+      sinon.assert.notCalled(fakeMozLoop.rooms.update);
     });
+
+    it("should save updated context information", function() {
+      fakeMozLoop.rooms.update = sinon.spy();
+
+      dispatcher.dispatch(new sharedActions.UpdateRoomContext({
+        roomToken: "42abc",
+        // Room name doesn't need to change.
+        newRoomName: "sillier name",
+        newRoomDescription: "Hello, is it me you're looking for?",
+        newRoomThumbnail: "http://example.com/empty.gif",
+        newRoomURL: "http://example.com"
+      }));
+
+      sinon.assert.calledOnce(fakeMozLoop.rooms.update);
+      sinon.assert.calledWith(fakeMozLoop.rooms.update, "42abc", {
+        urls: [{
+          description: "Hello, is it me you're looking for?",
+          location: "http://example.com",
+          thumbnail: "http://example.com/empty.gif"
+        }]
+      });
+    });
+
+    it("should not save context information with an invalid URL", function() {
+      fakeMozLoop.rooms.update = sinon.spy();
+
+      dispatcher.dispatch(new sharedActions.UpdateRoomContext({
+        roomToken: "42abc",
+        // Room name doesn't need to change.
+        newRoomName: "sillier name",
+        newRoomDescription: "Hello, is it me you're looking for?",
+        newRoomThumbnail: "http://example.com/empty.gif",
+        // NOTE: there are many variation we could test here, but the URL object
+        // constructor also fails on empty strings and is using the Gecko URL
+        // parser. Therefore we ought to rely on it working properly.
+        newRoomURL: "http/example.com"
+      }));
+
+      sinon.assert.notCalled(fakeMozLoop.rooms.update);
+    });
+
+    it("should not save context information when no context information is provided",
+      function() {
+        fakeMozLoop.rooms.update = sinon.spy();
+
+        dispatcher.dispatch(new sharedActions.UpdateRoomContext({
+          roomToken: "42abc",
+          // Room name doesn't need to change.
+          newRoomName: "sillier name",
+          newRoomDescription: "",
+          newRoomThumbnail: "",
+          newRoomURL: ""
+        }));
+
+        sinon.assert.notCalled(fakeMozLoop.rooms.update);
+      });
   });
 });

browser/components/loop/test/desktop-local/roomViews_test.js

@@ -19,6 +19,10 @@ describe("loop.roomViews", function () {
     fakeMozLoop = {
       getAudioBlob: sinon.stub(),
       getLoopPref: sinon.stub(),
+      getSelectedTabMetadata: sinon.stub().callsArgWith(0, {
+        previews: [],
+        title: ""
+      }),
       isSocialShareButtonAvailable: sinon.stub()
     };
 
@@ -112,6 +116,7 @@ describe("loop.roomViews", function () {
     function mountTestComponent(props) {
       props = _.extend({
         dispatcher: dispatcher,
+        mozLoop: fakeMozLoop,
         roomData: {},
         show: true,
         showContext: false
@@ -135,53 +140,6 @@ describe("loop.roomViews", function () {
           new sharedActions.EmailRoomUrl({roomUrl: "http://invalid"}));
       });
 
-    describe("Rename Room", function() {
-      var roomNameBox;
-
-      beforeEach(function() {
-        view = mountTestComponent({
-          roomData: {
-            roomToken: "fakeToken",
-            roomName: "fakeName"
-          }
-        });
-
-        roomNameBox = view.getDOMNode().querySelector(".input-room-name");
-      });
-
-      it("should dispatch a RenameRoom action when the focus is lost",
-        function() {
-          React.addons.TestUtils.Simulate.change(roomNameBox, { target: {
-            value: "reallyFake"
-          }});
-
-          React.addons.TestUtils.Simulate.blur(roomNameBox);
-
-          sinon.assert.calledOnce(dispatcher.dispatch);
-          sinon.assert.calledWithExactly(dispatcher.dispatch,
-            new sharedActions.RenameRoom({
-              roomToken: "fakeToken",
-              newRoomName: "reallyFake"
-            }));
-        });
-
-      it("should dispatch a RenameRoom action when Enter key is pressed",
-        function() {
-          React.addons.TestUtils.Simulate.change(roomNameBox, { target: {
-            value: "reallyFake"
-          }});
-
-          TestUtils.Simulate.keyDown(roomNameBox, {key: "Enter", which: 13});
-
-          sinon.assert.calledOnce(dispatcher.dispatch);
-          sinon.assert.calledWithExactly(dispatcher.dispatch,
-            new sharedActions.RenameRoom({
-              roomToken: "fakeToken",
-              newRoomName: "reallyFake"
-            }));
-        });
-    });
-
     describe("Copy Button", function() {
       beforeEach(function() {
         view = mountTestComponent({
@@ -243,6 +201,29 @@ describe("loop.roomViews", function () {
         expect(view.getDOMNode().querySelector(".room-context")).to.not.eql(null);
       });
 
+      it("should render the context in editMode when the pencil is clicked", function() {
+        view = mountTestComponent({
+          showContext: true,
+          roomData: {
+            roomContextUrls: [fakeContextURL]
+          }
+        });
+
+        var pencil = view.getDOMNode().querySelector(".room-context-btn-edit");
+        expect(pencil).to.not.eql(null);
+
+        React.addons.TestUtils.Simulate.click(pencil);
+
+        expect(view.state.editMode).to.eql(true);
+        var node = view.getDOMNode();
+        expect(node.querySelector("form")).to.not.eql(null);
+        // No text paragraphs should be visible in editMode.
+        var visiblePs = Array.slice(node.querySelector("p")).filter(function(p) {
+          return p.classList.contains("hide") || p.classList.contains("error");
+        });
+        expect(visiblePs.length).to.eql(0);
+      });
+
       it("should format the context url for display", function() {
         sandbox.stub(sharedUtils, "formatURL").returns({
           location: "location",
@@ -629,7 +610,12 @@ describe("loop.roomViews", function () {
 
     function mountTestComponent(props) {
       props = _.extend({
-        show: true
+        dispatcher: dispatcher,
+        mozLoop: fakeMozLoop,
+        show: true,
+        roomData: {
+          roomToken: "fakeToken"
+        }
       }, props);
       return TestUtils.renderIntoDocument(
         React.createElement(loop.roomViews.DesktopRoomContextView, props));
@@ -680,6 +666,134 @@ describe("loop.roomViews", function () {
         React.addons.TestUtils.Simulate.click(closeBtn);
         expect(view.getDOMNode()).to.eql(null);
       });
+
+      it("should render the view in editMode when appropriate", function() {
+        var roomName = "Hello, is it me you're looking for?";
+        view = mountTestComponent({
+          editMode: true,
+          roomData: {
+            roomName: roomName,
+            roomContextUrls: [fakeContextURL]
+          }
+        });
+
+        var node = view.getDOMNode();
+        expect(node.querySelector("form")).to.not.eql(null);
+        // Check the contents of the form fields.
+        expect(node.querySelector(".room-context-name").value).to.eql(roomName);
+        expect(node.querySelector(".room-context-url").value).to.eql(fakeContextURL.location);
+        expect(node.querySelector(".room-context-comments").value).to.eql(fakeContextURL.description);
+      });
+
+      it("should show the checkbox as disabled when no context is available", function() {
+        view = mountTestComponent({
+          editMode: true,
+          roomData: {
+            roomToken: "fakeToken",
+            roomName: "fakeName"
+          }
+        });
+
+        var checkbox = view.getDOMNode().querySelector(".checkbox");
+        expect(checkbox.classList.contains("disabled")).to.eql(true);
+      });
+    });
+
+    describe("Update Room", function() {
+      var roomNameBox;
+
+      beforeEach(function() {
+        sandbox.stub(dispatcher, "dispatch");
+
+        view = mountTestComponent({
+          editMode: true,
+          roomData: {
+            roomToken: "fakeToken",
+            roomName: "fakeName",
+            roomContextUrls: [fakeContextURL]
+          }
+        });
+
+        roomNameBox = view.getDOMNode().querySelector(".room-context-name");
+      });
+
+      it("should dispatch a UpdateRoomContext action when the focus is lost",
+        function() {
+          React.addons.TestUtils.Simulate.change(roomNameBox, { target: {
+            value: "reallyFake"
+          }});
+
+          React.addons.TestUtils.Simulate.blur(roomNameBox);
+
+          sinon.assert.calledOnce(dispatcher.dispatch);
+          sinon.assert.calledWithExactly(dispatcher.dispatch,
+            new sharedActions.UpdateRoomContext({
+              roomToken: "fakeToken",
+              newRoomName: "reallyFake",
+              newRoomDescription: fakeContextURL.description,
+              newRoomURL: fakeContextURL.location,
+              newRoomThumbnail: fakeContextURL.thumbnail
+            }));
+        });
+
+      it("should dispatch a UpdateRoomContext action when Enter key is pressed",
+        function() {
+          React.addons.TestUtils.Simulate.change(roomNameBox, { target: {
+            value: "reallyFake"
+          }});
+
+          TestUtils.Simulate.keyDown(roomNameBox, {key: "Enter", which: 13});
+
+          sinon.assert.calledOnce(dispatcher.dispatch);
+          sinon.assert.calledWithExactly(dispatcher.dispatch,
+            new sharedActions.UpdateRoomContext({
+              roomToken: "fakeToken",
+              newRoomName: "reallyFake",
+              newRoomDescription: fakeContextURL.description,
+              newRoomURL: fakeContextURL.location,
+              newRoomThumbnail: fakeContextURL.thumbnail
+            }));
+        });
+    });
+
+    describe("#handleCheckboxChange", function() {
+      var node, checkbox;
+
+      beforeEach(function() {
+        view = mountTestComponent({
+          availableContext: {
+            description: fakeContextURL.description,
+            previewImage: fakeContextURL.thumbnail,
+            url: fakeContextURL.location
+          },
+          editMode: true,
+          roomData: {
+            roomToken: "fakeToken",
+            roomName: "fakeName"
+          }
+        });
+
+        node = view.getDOMNode();
+        checkbox = node.querySelector(".checkbox");
+      });
+
+      it("should prefill the form with available context data when clicked", function() {
+        React.addons.TestUtils.Simulate.click(checkbox);
+
+        expect(node.querySelector(".room-context-name").value).to.eql("fakeName");
+        expect(node.querySelector(".room-context-url").value).to.eql(fakeContextURL.location);
+        expect(node.querySelector(".room-context-comments").value).to.eql(fakeContextURL.description);
+      });
+
+      it("should undo prefill when clicking the checkbox again", function() {
+        React.addons.TestUtils.Simulate.click(checkbox);
+        // Twice.
+        React.addons.TestUtils.Simulate.click(checkbox);
+
+        expect(node.querySelector(".room-context-name").value).to.eql("fakeName");
+        expect(node.querySelector(".room-context-url").value).to.eql("");
+        expect(node.querySelector(".room-context-comments").value).to.eql("");
+      });
     });
   });
 });

browser/components/loop/test/xpcshell/test_looprooms.js

@@ -98,7 +98,7 @@ const kExpectedRooms = new Map([
   }]
 ]);
 
-let roomDetail = {
+const kRoomDetail = {
   decryptedContext: {
     roomName: "First Room Name"
   },
@@ -186,12 +186,12 @@ const normalizeRoom = function(room) {
   let newRoom = extend({}, room);
   let name = newRoom.decryptedContext.roomName;
 
-  for (let key of Object.getOwnPropertyNames(roomDetail)) {
+  for (let key of Object.getOwnPropertyNames(kRoomDetail)) {
     // Handle sub-objects if necessary (e.g. context, decryptedContext).
-    if (typeof roomDetail[key] == "object") {
+    if (typeof kRoomDetail[key] == "object") {
-      newRoom[key] = extend({}, roomDetail[key]);
+      newRoom[key] = extend({}, kRoomDetail[key]);
     } else {
-      newRoom[key] = roomDetail[key];
+      newRoom[key] = kRoomDetail[key];
     }
   }
 
@@ -317,8 +317,11 @@ add_task(function* setup_server() {
     res.finish();
   });
 
-  function returnRoomDetails(res, roomName) {
+  function returnRoomDetails(res, roomDetail, roomName) {
     roomDetail.roomName = roomName;
+    // The decrypted context and roomKey are never part of the server response.
+    delete roomDetail.decryptedContext;
+    delete roomDetail.roomKey;
     res.setStatusLine(null, 200, "OK");
     res.write(JSON.stringify(roomDetail));
     res.processAsync();
@@ -332,6 +335,7 @@ add_task(function* setup_server() {
   // Add a request handler for each room in the list.
   [...kRoomsResponses.values()].forEach(function(room) {
     loopServer.registerPathHandler("/rooms/" + encodeURIComponent(room.roomToken), (req, res) => {
+      let roomDetail = extend({}, kRoomDetail);
       if (req.method == "POST") {
         let data = getJSONData(req.bodyInputStream);
         res.setStatusLine(null, 200, "OK");
@@ -344,7 +348,7 @@ add_task(function* setup_server() {
         Assert.ok("context" in data, "should have encrypted context");
         // We return a fake encrypted name here as the context is
         // encrypted.
-        returnRoomDetails(res, "fakeEncrypted");
+        returnRoomDetails(res, roomDetail, "fakeEncrypted");
       } else {
         roomDetail.context = room.context;
         res.setStatusLine(null, 200, "OK");
@@ -574,11 +578,34 @@ add_task(function* test_sendConnectionStatus() {
   Assert.deepEqual(statusData, extraData);
 });
 
-// Test if renaming a room works as expected.
+// Test if updating a room works as expected.
-add_task(function* test_renameRoom() {
+add_task(function* test_updateRoom() {
   let roomToken = "_nxD4V4FflQ";
-  let renameData = yield LoopRooms.promise("rename", roomToken, "fakeName");
+  let fakeContext = {
-  Assert.equal(renameData.roomName, "fakeEncrypted", "should have set the new name");
+    description: "Hello, is it me you're looking for?",
+    location: "https://example.com",
+    thumbnail: "https://example.com/empty.gif"
+  };
+  let updateData = yield LoopRooms.promise("update", roomToken, {
+    roomName: "fakeEncrypted",
+    urls: [fakeContext]
+  });
+  Assert.equal(updateData.roomName, "fakeEncrypted", "should have set the new name");
+  let contextURL = updateData.decryptedContext.urls[0];
+  Assert.equal(contextURL.description, contextURL.description,
+    "should have set the new context URL description");
+  Assert.equal(contextURL.location, contextURL.location,
+    "should have set the new context URL location");
+  Assert.equal(contextURL.thumbnail, contextURL.thumbnail,
+    "should have set the new context URL thumbnail");
+});
+
+add_task(function* test_updateRoom_nameOnly() {
+  let roomToken = "_nxD4V4FflQ";
+  let updateData = yield LoopRooms.promise("update", roomToken, {
+    roomName: "fakeEncrypted"
+  });
+  Assert.equal(updateData.roomName, "fakeEncrypted", "should have set the new name");
 });
 
 add_task(function* test_roomDeleteNotifications() {