From 841cfda863b391e926a1421e84a4b1ce267fd633 Mon Sep 17 00:00:00 2001
From: Mike Murray <hellomikemurray@gmail.com>
Date: Wed, 5 Oct 2016 14:47:15 -0700
Subject: [PATCH 1/3] Updated publish controls UX and fixing issues

- Fix bug causing error in console when publishing nothing.
- Disabled the publish button if there is nothing to publish.
- Added alert toasts for successful and unsuccessful publish.
- Publish controls now properly finds relevant revisions.
---
 .../client/components/publishControls.js      | 53 ++++++++++++++++---
 .../client/containers/publishContainer.js     | 36 +++++++++++--
 .../plugins/core/revisions/server/methods.js  |  4 +-
 private/data/i18n/en.json                     |  5 +-
 4 files changed, 86 insertions(+), 12 deletions(-)

diff --git a/imports/plugins/core/revisions/client/components/publishControls.js b/imports/plugins/core/revisions/client/components/publishControls.js
index 3b0e7497726..e7713f87aff 100644
--- a/imports/plugins/core/revisions/client/components/publishControls.js
+++ b/imports/plugins/core/revisions/client/components/publishControls.js
@@ -43,6 +43,10 @@ class PublishControls extends Component {
     return "app.hideChanges";
   }
 
+  get hasRevisions() {
+    return Array.isArray(this.props.revisions) && this.props.revisions.length;
+  }
+
   get diffs() {
     return this.props.revisions;
   }
@@ -51,6 +55,36 @@ class PublishControls extends Component {
     return this.diffs && this.state.showDiffs;
   }
 
+  /**
+   * Getter hasChanges
+   * @return {Boolean} one or more revision has changes
+   */
+  get hasChanges() {
+    // Verify we even have any revision at all
+    if (this.hasRevisions) {
+      // Loop through all revisions to determin if they have changes
+      const diffHasActualChanges = this.props.revisions.map((revision) => {
+        // We probably do have chnages to publish
+        // Note: Sometimes "updatedAt" will cause false positives, but just incase, lets
+        // enable the publish button anyway.
+        if (Array.isArray(revision.diff) && revision.diff.length) {
+          return true;
+        }
+
+        // If all else fails, we will disable the button
+        return false;
+      });
+
+      // If even one revision as changes we should enable the publish button
+      return diffHasActualChanges.some((element) => {
+        return element === true;
+      });
+    }
+
+    // No revisions, no publishing
+    return false;
+  }
+
   renderChanges() {
     if (this.showDiffs) {
       const diffs = this.props.revisions.map((revision) => {
@@ -66,6 +100,18 @@ class PublishControls extends Component {
     return null;
   }
 
+  renderPublishButton() {
+    return (
+      <Button
+        disabled={this.hasChanges === false}
+        i18nKeyLabel="app.publishChanges"
+        label="Publish Changes"
+        onClick={this.handlePublishClick}
+        status="success"
+      />
+    );
+  }
+
   render() {
     if (this.props.isEnabled) {
       return (
@@ -76,12 +122,7 @@ class PublishControls extends Component {
             onClick={this.handleToggleShowChanges}
             status="link"
           />
-          <Button
-            i18nKeyLabel="app.publishChanges"
-            label="Publish Changes"
-            onClick={this.handlePublishClick}
-            primary={true}
-          />
+          {this.renderPublishButton()}
           {this.showDiffs && <hr />}
           {this.renderChanges()}
         </div>
diff --git a/imports/plugins/core/revisions/client/containers/publishContainer.js b/imports/plugins/core/revisions/client/containers/publishContainer.js
index bb91b08488b..3bc2956797e 100644
--- a/imports/plugins/core/revisions/client/containers/publishContainer.js
+++ b/imports/plugins/core/revisions/client/containers/publishContainer.js
@@ -5,6 +5,7 @@ import { Revisions } from "/lib/collections";
 import { Meteor } from "meteor/meteor";
 import TranslationProvider from "/imports/plugins/core/ui/client/providers/translationProvider";
 import { isRevisionControlEnabled } from "../../lib/api";
+import { i18next } from "/client/api";
 
 /**
  * Publish container is a stateless container component connected to Meteor data source.
@@ -35,7 +36,22 @@ export function handlePublishClick(revisions) {
     const documentIds = revisions.map((revision) => {
       return revision.documentId;
     });
-    Meteor.call("revisions/publish", documentIds);
+
+    Meteor.call("revisions/publish", documentIds, (error, result) => {
+      if (result === true) {
+        const message = i18next.t("revisions.changedPublished", {
+          defaultValue: "Changes published successfuly"
+        });
+
+        Alerts.toast(message, "success");
+      } else {
+        const message = i18next.t("revisions.noChangesPublished", {
+          defaultValue: "There are no changes to publish"
+        });
+
+        Alerts.toast(message, "warning");
+      }
+    });
   }
 }
 
@@ -45,8 +61,22 @@ function composer(props, onData) {
 
     if (subscription.ready()) {
       const revisions = Revisions.find({
-        documentId: {
-          $in: props.documentIds
+        "$or": [
+          {
+            documentId: {
+              $in: props.documentIds
+            }
+          },
+          {
+            "documentData.ancestors": {
+              $in: props.documentIds
+            }
+          }
+        ],
+        "workflow.status": {
+          $nin: [
+            "revision/published"
+          ]
         }
       }).fetch();
 
diff --git a/imports/plugins/core/revisions/server/methods.js b/imports/plugins/core/revisions/server/methods.js
index 57157408e17..0875fbf70c9 100644
--- a/imports/plugins/core/revisions/server/methods.js
+++ b/imports/plugins/core/revisions/server/methods.js
@@ -64,7 +64,7 @@ Meteor.methods({
     let updatedDocuments = 0;
 
     if (revisions) {
-      for (let revision of revisions) {
+      for (const revision of revisions) {
         const res = Products.update({
           _id: revision.documentId
         }, {
@@ -81,6 +81,6 @@ Meteor.methods({
       return true;
     }
 
-    throw new Meteor.Error(403, "Forbidden", "Could not publish product revision");
+    return false;
   }
 });
diff --git a/private/data/i18n/en.json b/private/data/i18n/en.json
index 0336b5d1112..013a35d63ef 100644
--- a/private/data/i18n/en.json
+++ b/private/data/i18n/en.json
@@ -568,7 +568,10 @@
         "unpublishedChanges": "Unpublished Changes",
         "publishChanges": "Publish Changes",
         "showChanges": "Show Changes",
-        "hideChanges": "Hide Changes"
+        "hideChanges": "Hide Changes",
+        "changedPublished": "Changes published successfuly",
+        "noChanges": "No Changes",
+        "noChangesPublished": "There are no changes to publish"
       },
       "address": {
         "country": "Country",

From c75fadbf1a1e561bdf756fd100a660ba09cc06c9 Mon Sep 17 00:00:00 2001
From: Mike Murray <hellomikemurray@gmail.com>
Date: Wed, 5 Oct 2016 17:08:04 -0700
Subject: [PATCH 2/3] Fixed typo

---
 .../core/revisions/client/containers/publishContainer.js        | 2 +-
 private/data/i18n/en.json                                       | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/imports/plugins/core/revisions/client/containers/publishContainer.js b/imports/plugins/core/revisions/client/containers/publishContainer.js
index 3bc2956797e..3ea1db8b09c 100644
--- a/imports/plugins/core/revisions/client/containers/publishContainer.js
+++ b/imports/plugins/core/revisions/client/containers/publishContainer.js
@@ -40,7 +40,7 @@ export function handlePublishClick(revisions) {
     Meteor.call("revisions/publish", documentIds, (error, result) => {
       if (result === true) {
         const message = i18next.t("revisions.changedPublished", {
-          defaultValue: "Changes published successfuly"
+          defaultValue: "Changes published successfully"
         });
 
         Alerts.toast(message, "success");
diff --git a/private/data/i18n/en.json b/private/data/i18n/en.json
index 013a35d63ef..ca779186af1 100644
--- a/private/data/i18n/en.json
+++ b/private/data/i18n/en.json
@@ -569,7 +569,7 @@
         "publishChanges": "Publish Changes",
         "showChanges": "Show Changes",
         "hideChanges": "Hide Changes",
-        "changedPublished": "Changes published successfuly",
+        "changedPublished": "Changes published successfully",
         "noChanges": "No Changes",
         "noChangesPublished": "There are no changes to publish"
       },

From 9e6ea9a4b7c97bc1346d854542a026a8cc9fdc6c Mon Sep 17 00:00:00 2001
From: Mike Murray <hellomikemurray@gmail.com>
Date: Wed, 5 Oct 2016 17:13:04 -0700
Subject: [PATCH 3/3] Fixed typo in comment

---
 .../plugins/core/revisions/client/components/publishControls.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/imports/plugins/core/revisions/client/components/publishControls.js b/imports/plugins/core/revisions/client/components/publishControls.js
index e7713f87aff..57c2ed44e47 100644
--- a/imports/plugins/core/revisions/client/components/publishControls.js
+++ b/imports/plugins/core/revisions/client/components/publishControls.js
@@ -75,7 +75,7 @@ class PublishControls extends Component {
         return false;
       });
 
-      // If even one revision as changes we should enable the publish button
+      // If even one revision has changes we should enable the publish button
       return diffHasActualChanges.some((element) => {
         return element === true;
       });