From 86f52d2eb7f8cfaf864f8abaa01304e251a15ce5 Mon Sep 17 00:00:00 2001 From: Brent Hoover Date: Fri, 11 Nov 2016 05:44:34 +0800 Subject: [PATCH] Revision control for images (#1555) * Add media hook. Rename file to "hooks'" * Only return published images for non-admin users * Only set product images as unpublished * Capture revision record and show Publish button when image changes are made * Adding images deferred until published * Removing images deferred until published * Capturing and deferring other changes to metadata (priority really) * Combine revisions when making multiple changes before publishing * Properly handle non-product revisions in applyProductRevision so that product doesn't get wiped. * Find the product correctly based on revision type * Fix regression where images weren't being published because revision data was wrong * Setting priceRange should be in an `else` statement * If you add an image and then delete it before publishing it, just remove it. * Update revisions with media * Show visual indicator on image if there are pending revisions * Discard drafts of image revisions * Sometimes rendering the "edited" marker * Reset revisions so that removals take effect * Set priority to revision priority * Fix issue where changing order wouldn't stick * Remove logging * Change lost in merge * Another change from merge --- .../client/components/publishControls.js | 4 +- .../client/containers/publishContainer.js | 13 +- .../revisions/server/{startup.js => hooks.js} | 141 +++++++++++++++++- .../plugins/core/revisions/server/index.js | 2 +- .../plugins/core/revisions/server/methods.js | 70 +++++++-- .../core/ui/client/components/media/media.js | 16 ++ .../client/components/media/mediaGallery.js | 2 + .../containers/mediaGalleryContainer.js | 49 +++++- .../containers/productDetailContainer.js | 5 +- lib/api/products.js | 14 +- lib/collections/schemas/revisions.js | 18 +++ server/publications/collections/media.js | 48 +++++- server/publications/collections/product.js | 16 +- server/publications/collections/products.js | 31 +++- 14 files changed, 389 insertions(+), 40 deletions(-) rename imports/plugins/core/revisions/server/{startup.js => hooks.js} (62%) diff --git a/imports/plugins/core/revisions/client/components/publishControls.js b/imports/plugins/core/revisions/client/components/publishControls.js index 1f6a35b15c5..cc3142e5967 100644 --- a/imports/plugins/core/revisions/client/components/publishControls.js +++ b/imports/plugins/core/revisions/client/components/publishControls.js @@ -114,12 +114,12 @@ class PublishControls extends Component { get hasChanges() { // Verify we even have any revision at all if (this.hasRevisions) { - // Loop through all revisions to determin if they have changes + // Loop through all revisions to determine 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) { + if (Array.isArray(revision.diff) && revision.diff.length || revision.documentType !== "product") { return true; } diff --git a/imports/plugins/core/revisions/client/containers/publishContainer.js b/imports/plugins/core/revisions/client/containers/publishContainer.js index 098d1c6d48f..0aa22ac0dec 100644 --- a/imports/plugins/core/revisions/client/containers/publishContainer.js +++ b/imports/plugins/core/revisions/client/containers/publishContainer.js @@ -13,10 +13,15 @@ import { i18next } from "/client/api"; class PublishContainer extends Component { handlePublishClick = (revisions) => { if (Array.isArray(revisions)) { - const documentIds = revisions.map((revision) => { + let documentIds = revisions.map((revision) => { + if (revision.parentDocument && revision.documentType !== "product") { + return revision.parentDocument; + } return revision.documentId; }); + const documentIdsSet = new Set(documentIds); // ensures they are unique + documentIds = Array.from(documentIdsSet); Meteor.call("revisions/publish", documentIds, (error, result) => { if (result === true) { const message = i18next.t("revisions.changedPublished", { @@ -105,6 +110,11 @@ function composer(props, onData) { "documentData.ancestors": { $in: props.documentIds } + }, + { + parentDocument: { + $in: props.documentIds + } } ], "workflow.status": { @@ -113,7 +123,6 @@ function composer(props, onData) { ] } }).fetch(); - onData(null, { isEnabled: isRevisionControlEnabled(), documentIds: props.documentIds, diff --git a/imports/plugins/core/revisions/server/startup.js b/imports/plugins/core/revisions/server/hooks.js similarity index 62% rename from imports/plugins/core/revisions/server/startup.js rename to imports/plugins/core/revisions/server/hooks.js index 8ac85ab990b..1d57e694dba 100644 --- a/imports/plugins/core/revisions/server/startup.js +++ b/imports/plugins/core/revisions/server/hooks.js @@ -1,8 +1,127 @@ -import { Products, Revisions, Tags } from "/lib/collections"; -import { Logger } from "/server/api"; +import _ from "lodash"; import { diff } from "deep-diff"; +import { Products, Revisions, Tags, Media } from "/lib/collections"; +import { Logger } from "/server/api"; import { RevisionApi } from "../lib/api"; + +function convertMetadata(modifierObject) { + const metadata = {}; + for (const prop in modifierObject) { + if (modifierObject.hasOwnProperty(prop)) { + if (prop.indexOf("metadata") !== -1) { + const splitName = _.split(prop, ".")[1]; + metadata[splitName] = modifierObject[prop]; + } + } + } + return metadata; +} + +Media.files.before.insert((userid, media) => { + if (RevisionApi.isRevisionControlEnabled() === false) { + return true; + } + if (media.metadata.productId) { + const revisionMetadata = Object.assign({}, media.metadata); + revisionMetadata.workflow = "published"; + Revisions.insert({ + documentId: media._id, + documentData: revisionMetadata, + documentType: "image", + parentDocument: media.metadata.productId, + changeType: "insert", + workflow: { + status: "revision/update" + } + }); + media.metadata.workflow = "unpublished"; + } else { + media.metadata.workflow = "published"; + } + return true; +}); + +Media.files.before.update((userId, media, fieldNames, modifier) => { + if (RevisionApi.isRevisionControlEnabled() === false) { + return true; + } + // if it's not metadata ignore it, as LOTS of othing things change on this record + if (!_.includes(fieldNames, "metadata")) { + return true; + } + + if (media.metadata.productId) { + const convertedModifier = convertMetadata(modifier.$set); + const convertedMetadata = Object.assign({}, media.metadata, convertedModifier); + const existingRevision = Revisions.findOne({ + "documentId": media._id, + "workflow.status": { + $nin: [ + "revision/published" + ] + } + }); + if (existingRevision) { + const updatedMetadata = Object.assign({}, existingRevision.documentData, convertedMetadata); + // Special case where if we have both added and reordered images before publishing we don't want to overwrite + // the workflow status since it would be "unpublished" + if (existingRevision.documentData.workflow === "published" || existingRevision.changeType === "insert") { + updatedMetadata.workflow = "published"; + } + Revisions.update({_id: existingRevision._id}, { + $set: { + documentData: updatedMetadata + } + }); + } else { + Revisions.insert({ + documentId: media._id, + documentData: convertedMetadata, + documentType: "image", + parentDocument: media.metadata.productId, + changeType: "update", + workflow: { + status: "revision/update" + } + }); + } + + return false; // prevent actual update of image. This also stops other hooks from running :/ + } + // for non-product images, just ignore and keep on moving + return true; +}); + +Media.files.before.remove((userId, media) => { + if (RevisionApi.isRevisionControlEnabled() === false) { + return true; + } + + // if the media is unpublished, then go ahead and just delete it + if (media.metadata.workflow && media.metadata.workflow === "unpublished") { + Revisions.remove({ + documentId: media._id + }); + return true; + } + if (media.metadata.productId) { + Revisions.insert({ + documentId: media._id, + documentData: media.metadata, + documentType: "image", + parentDocument: media.metadata.productId, + changeType: "remove", + workflow: { + status: "revision/update" + } + }); + return false; // prevent actual deletion of image. This also stops other hooks from running :/ + } + return true; +}); + + Products.before.insert((userId, product) => { if (RevisionApi.isRevisionControlEnabled() === false) { return true; @@ -243,13 +362,21 @@ Revisions.after.update(function (userId, revision) { if (RevisionApi.isRevisionControlEnabled() === false) { return true; } + let differences; - // Make diff - const product = Products.findOne({ - _id: revision.documentId - }); - const differences = diff(product, revision.documentData); + if (!revision.documentType || revision.documentType === "product") { + // Make diff + const product = Products.findOne({ + _id: revision.documentId + }); + differences = diff(product, revision.documentData); + } + + if (revision.documentType && revision.documentType === "image") { + const image = Media.findOne(revision.documentId); + differences = diff(image.metadata, revision.documentData); + } Revisions.direct.update({ _id: revision._id diff --git a/imports/plugins/core/revisions/server/index.js b/imports/plugins/core/revisions/server/index.js index 3ff30cc5278..347bebd457d 100644 --- a/imports/plugins/core/revisions/server/index.js +++ b/imports/plugins/core/revisions/server/index.js @@ -1,2 +1,2 @@ -import "./startup"; +import "./hooks"; import "./methods"; diff --git a/imports/plugins/core/revisions/server/methods.js b/imports/plugins/core/revisions/server/methods.js index 8619a1cf040..9fabae0925e 100644 --- a/imports/plugins/core/revisions/server/methods.js +++ b/imports/plugins/core/revisions/server/methods.js @@ -1,6 +1,7 @@ -import { Products, Revisions, Packages } from "/lib/collections"; import { Meteor } from "meteor/meteor"; import { check, Match } from "meteor/check"; +import { Products, Media, Revisions, Packages } from "/lib/collections"; +import { Logger } from "/server/api"; export function updateSettings(settings) { check(settings, Object); @@ -41,6 +42,11 @@ export function discardDrafts(documentIds) { "documentData.ancestors": { $in: documentIdArray } + }, + { + parentDocument: { + $in: documentIds + } } ] }; @@ -76,6 +82,11 @@ Meteor.methods({ "documentData.ancestors": { $in: documentIds } + }, + { + parentDocument: { + $in: documentIds + } } ] }).fetch(); @@ -101,15 +112,54 @@ Meteor.methods({ if (revisions) { for (const revision of revisions) { - const res = Products.update({ - _id: revision.documentId - }, { - $set: revision.documentData - }, { - publish: true - }); - - updatedDocuments += res; + if (!revision.documentType || revision.documentType === "product") { + const res = Products.update({ + _id: revision.documentId + }, { + $set: revision.documentData + }, { + publish: true + }); + updatedDocuments += res; + } else if (revision.documentType === "image") { + if (revision.changeType === "insert") { + const res = Media.files.direct.update({ + _id: revision.documentId + }, { + $set: { + metadata: revision.documentData + } + }); + updatedDocuments += res; + } else if (revision.changeType === "remove") { + const res = Media.files.direct.update({ + _id: revision.documentId + }, { + $set: { + "metadata.workflow": "archived" + } + }); + updatedDocuments += res; + } else if (revision.changeType === "update") { + const res = Media.files.direct.update({ + _id: revision.documentId + }, { + $set: { + metadata: revision.documentData + } + }); + updatedDocuments += res; + Logger.debug(`setting metadata for ${revision.documentId} to ${JSON.stringify(revision.documentData, null, 4)}`); + } + // mark revision published whether we are publishing the image or not + Revisions.direct.update({ + _id: revision._id + }, { + $set: { + "workflow.status": "revision/published" + } + }); + } } } diff --git a/imports/plugins/core/ui/client/components/media/media.js b/imports/plugins/core/ui/client/components/media/media.js index a2a6b3b1442..655a33ca5d9 100644 --- a/imports/plugins/core/ui/client/components/media/media.js +++ b/imports/plugins/core/ui/client/components/media/media.js @@ -25,6 +25,19 @@ class MediaItem extends Component { } } + renderRevision() { + if (this.props.revision) { + if (this.props.revision.changeType === "remove") { + return ( + + ); + } + return ( + + ); + } + } + renderControls() { if (this.props.editable) { return ( @@ -33,6 +46,7 @@ class MediaItem extends Component { icon="fa fa-times" onClick={this.handleRemoveMedia} /> + {this.renderRevision()} ); } @@ -93,9 +107,11 @@ MediaItem.propTypes = { connectDropTarget: PropTypes.func, defaultSource: PropTypes.string, editable: PropTypes.bool, + metadata: PropTypes.object, onMouseEnter: PropTypes.func, onMouseLeave: PropTypes.func, onRemoveMedia: PropTypes.func, + revision: PropTypes.object, source: PropTypes.oneOfType([PropTypes.string, PropTypes.object]) }; diff --git a/imports/plugins/core/ui/client/components/media/mediaGallery.js b/imports/plugins/core/ui/client/components/media/mediaGallery.js index 76be7307020..680a5c041ed 100644 --- a/imports/plugins/core/ui/client/components/media/mediaGallery.js +++ b/imports/plugins/core/ui/client/components/media/mediaGallery.js @@ -51,6 +51,7 @@ class MediaGallery extends Component { editable={this.props.editable} index={index} key={index} + revision={this.featuredMedia.revision} metadata={this.featuredMedia.metadata} onMouseEnter={this.props.onMouseEnterMedia} onMouseLeave={this.props.onMouseLeaveMedia} @@ -66,6 +67,7 @@ class MediaGallery extends Component { editable={this.props.editable} index={index} key={index} + revision={media.revision} metadata={media.metadata} onMouseEnter={this.props.onMouseEnterMedia} onMouseLeave={this.props.onMouseLeaveMedia} diff --git a/imports/plugins/core/ui/client/containers/mediaGalleryContainer.js b/imports/plugins/core/ui/client/containers/mediaGalleryContainer.js index f0c1d735a49..bdfd58db1d2 100644 --- a/imports/plugins/core/ui/client/containers/mediaGalleryContainer.js +++ b/imports/plugins/core/ui/client/containers/mediaGalleryContainer.js @@ -4,7 +4,7 @@ import { composeWithTracker } from "react-komposer"; import { MediaGallery } from "../components"; import { Reaction } from "/client/api"; import { ReactionProduct } from "/lib/api"; -import { Media } from "/lib/collections"; +import { Media, Revisions } from "/lib/collections"; function uploadHandler(files) { // TODO: It would be cool to move this logic to common ValidatedMethod, but @@ -81,6 +81,7 @@ class MediaGalleryContainer extends Component { // updateImagePriorities(); }); } + // show media as removed (since it will not disappear until changes are published }); } @@ -88,6 +89,12 @@ class MediaGalleryContainer extends Component { return (this.state && this.state.media) || this.props.media; } + componentWillReceiveProps(nextProps) { + this.setState({ + media: nextProps.media + }); + } + handleMouseEnterMedia = (event, media) => { this.setState({ featuredMedia: media @@ -146,6 +153,44 @@ class MediaGalleryContainer extends Component { } } +function fetchMediaRevisions() { + const productId = ReactionProduct.selectedProductId(); + const mediaRevisions = Revisions.find({ + parentDocument: productId, + documentType: "image", + "workflow.status": { + $nin: ["revision/published"] + } + }).fetch(); + return mediaRevisions; +} + +// resort the media in +function sortMedia(media) { + const sortedMedia = _.sortBy(media, function(m) { return m.metadata.priority}); + return sortedMedia; +} + +// Search through revisions and if we find one for the image, stick it on the object +function appendRevisionsToMedia(props, media) { + if (!Reaction.hasPermission(props.permission || ["createProduct"])) { + return media; + } + const mediaRevisions = fetchMediaRevisions(); + const newMedia = []; + for (const image of media) { + image.revision = undefined; + for (const revision of mediaRevisions) { + if (revision.documentId === image._id) { + image.revision = revision; + image.metadata.priority = revision.documentData.priority; + } + } + newMedia.push(image); + } + return sortMedia(newMedia); +} + function composer(props, onData) { let media; let editable; @@ -154,7 +199,7 @@ function composer(props, onData) { if (!props.media) { // Fetch media based on props } else { - media = props.media; + media = appendRevisionsToMedia(props, props.media); } if (viewAs === "customer") { diff --git a/imports/plugins/included/product-detail-simple/client/containers/productDetailContainer.js b/imports/plugins/included/product-detail-simple/client/containers/productDetailContainer.js index d530786302b..d706c38049d 100644 --- a/imports/plugins/included/product-detail-simple/client/containers/productDetailContainer.js +++ b/imports/plugins/included/product-detail-simple/client/containers/productDetailContainer.js @@ -228,9 +228,10 @@ function composer(props, onData) { // when top variant has no child variants we display only its price if (childVariants.length === 0) { priceRange = selectedVariant.price; + } else { + // otherwise we want to show child variants price range + priceRange = ReactionProduct.getVariantPriceRange(); } - // otherwise we want to show child variants price range - priceRange = ReactionProduct.getVariantPriceRange(); } let productRevision; diff --git a/lib/api/products.js b/lib/api/products.js index bcd3a7fd431..c83694d07d3 100644 --- a/lib/api/products.js +++ b/lib/api/products.js @@ -24,16 +24,26 @@ export function applyProductRevision(product) { if (product.__revisions && product.__revisions.length) { const cleanProduct = Object.assign({}, product); delete cleanProduct.__revisions; + let revisedProduct; + // check for product revisions and set that as the current product + for (const revision of product.__revisions) { + if (!revision.parentDocument) { + revisedProduct = product.__revisions[0].documentData; + } + } + // if there are no revision to product (image and/or tag only) just set the original product as the product + if (!revisedProduct) { + revisedProduct = cleanProduct; + } return Object.assign({}, - product.__revisions[0].documentData, + revisedProduct, { __published: cleanProduct, __draft: product.__revisions[0] } ); } - return product; } diff --git a/lib/collections/schemas/revisions.js b/lib/collections/schemas/revisions.js index 68843cb758a..525845e660c 100644 --- a/lib/collections/schemas/revisions.js +++ b/lib/collections/schemas/revisions.js @@ -22,11 +22,29 @@ export const Revisions = new SimpleSchema({ label: "Reference Document Id" }, + documentType: { + type: String, + label: "Document Type", + defaultValue: "product", + allowedValues: ["product", "image", "tag"] + }, + + parentDocument: { + type: String, + optional: true + }, + documentData: { type: "object", blackbox: true }, + changeType: { + type: String, + optional: true, + allowedValues: ["insert", "update", "remove"] + }, + diff: { type: [Object], blackbox: true, diff --git a/server/publications/collections/media.js b/server/publications/collections/media.js index f08811700b8..d45b9981c40 100644 --- a/server/publications/collections/media.js +++ b/server/publications/collections/media.js @@ -1,5 +1,7 @@ -import { Media } from "/lib/collections"; +import { Media, Revisions } from "/lib/collections"; import { Reaction } from "/server/api"; +import { RevisionApi } from "/imports/plugins/core/revisions/lib/api/revisions"; + /** * CollectionFS - Image/Video Publication @@ -24,6 +26,50 @@ Meteor.publish("Media", function (shops) { } }; } + + // Product editors can see both published and unpublished images + if (!Reaction.hasPermission(["createProduct"], this.userId)) { + selector["metadata.workflow"] = { + $in: [null, "published"] + }; + } else { + // but no one gets to see archived images + selector["metadata.workflow"] = { + $nin: ["archived"] + }; + } + + if (RevisionApi.isRevisionControlEnabled()) { + const revisionHandle = Revisions.find({ + "documentType": "image", + "workflow.status": {$nin: [ "revision/published"]} + }).observe({ + added: (revision) => { + const media = Media.findOne(revision.documentId); + if (media) { + this.added("Media", media._id, media); + this.added("Revisions", revision._id, revision); + } + }, + changed: (revision) => { + const media = Media.findOne(revision.documentId); + this.changed("Media", media._id, media); + this.changed("Revisions", revision._id, revision); + }, + removed: (revision) => { + if (revision) { + const media = Media.findOne(revision.documentId); + this.changed("Media", media._id, media); + this.removed("Revisions", revision._id, revision); + } + } + }); + + this.onStop(() => { + revisionHandle.stop(); + }); + } + return Media.find(selector, { sort: { "metadata.priority": 1 diff --git a/server/publications/collections/product.js b/server/publications/collections/product.js index a728a848a86..26016ec3583 100644 --- a/server/publications/collections/product.js +++ b/server/publications/collections/product.js @@ -113,16 +113,24 @@ Meteor.publish("Product", function (productId) { this.added("Revisions", revision._id, revision); }, changed: (revision) => { - const product = Products.findOne(revision.documentId); - + let product; + if (!revision.parentDocument) { + product = Products.findOne(revision.documentId); + } else { + product = Products.findOne(revision.parentDocument); + } product.__revisions = [revision]; this.changed("Products", product._id, product); this.changed("Revisions", revision._id, revision); }, removed: (revision) => { - const product = Products.findOne(revision.documentId); - + let product; + if (!revision.parentDocument) { + product = Products.findOne(revision.documentId); + } else { + product = Products.findOne(revision.parentDocument); + } product.__revisions = []; this.changed("Products", product._id, product); diff --git a/server/publications/collections/products.js b/server/publications/collections/products.js index 5c9b99c12da..bcdacabc0aa 100644 --- a/server/publications/collections/products.js +++ b/server/publications/collections/products.js @@ -223,7 +223,7 @@ Meteor.publish("Products", function (productScrollLimit = 24, productFilters, so } // Authorized content curators fo the shop get special publication of the product - // all all relevant revisions all is one package + // with all relevant revisions all is one package if (Roles.userIsInRole(this.userId, ["owner", "admin", "createProduct"], shop._id)) { selector.isVisible = { @@ -237,7 +237,10 @@ Meteor.publish("Products", function (productScrollLimit = 24, productFilters, so }).observeChanges({ added: (id, fields) => { const revisions = Revisions.find({ - "documentId": id, + $or: [ + {"documentId": id }, + {"parentDocument": id} + ], "workflow.status": { $nin: [ "revision/published" @@ -250,7 +253,10 @@ Meteor.publish("Products", function (productScrollLimit = 24, productFilters, so }, changed: (id, fields) => { const revisions = Revisions.find({ - "documentId": id, + $or: [ + {"documentId": id }, + {"parentDocument": id} + ], "workflow.status": { $nin: [ "revision/published" @@ -277,15 +283,26 @@ Meteor.publish("Products", function (productScrollLimit = 24, productFilters, so this.added("Revisions", revision._id, revision); }, changed: (revision) => { - const product = Products.findOne(revision.documentId); - + let product; + if (!revision.documentType || revision.documentType === "product") { + product = Products.findOne(revision.documentId); + } else if (revision.documentType === "image" || revision.documentType === "tag") { + product = Products.findOne(revision.parentDocument); + } product.__revisions = [revision]; this.changed("Products", product._id, product); this.changed("Revisions", revision._id, revision); }, removed: (revision) => { - const product = Products.findOne(revision.documentId); + let product; + + if (!revision.documentType || revision.documentType === "product") { + product = Products.findOne(revision.documentId); + } else if (revision.docuentType === "image" || revision.documentType === "tag") { + product = Products.findOne(revision.parentDocument); + } + product.__revisions = []; this.changed("Products", product._id, product); @@ -308,7 +325,7 @@ Meteor.publish("Products", function (productScrollLimit = 24, productFilters, so }); } - // Everyone else gets the standard, visibile products + // Everyone else gets the standard, visible products selector.isVisible = true; return Products.find(selector, {