From 1a6e831b9edcc964d73dd6757994549ec2745019 Mon Sep 17 00:00:00 2001 From: Paulo Reis Date: Thu, 7 Dec 2017 14:14:42 +0000 Subject: [PATCH 01/56] Basic PDF viewer colors for v2. --- services/web/public/stylesheets/app/editor/pdf.less | 10 +++++++--- .../web/public/stylesheets/app/editor/toolbar.less | 4 ++-- .../web/public/stylesheets/core/_common-variables.less | 9 +++++++++ services/web/public/stylesheets/core/ol-variables.less | 6 ++++++ 4 files changed, 24 insertions(+), 5 deletions(-) diff --git a/services/web/public/stylesheets/app/editor/pdf.less b/services/web/public/stylesheets/app/editor/pdf.less index a873cd5e8f..aab748d17f 100644 --- a/services/web/public/stylesheets/app/editor/pdf.less +++ b/services/web/public/stylesheets/app/editor/pdf.less @@ -10,9 +10,13 @@ padding: 0 (@line-height-computed / 2); } +.pdf §{ + background-color: @pdf-bg; +} + .pdf-viewer, .pdf-logs, .pdf-errors, .pdf-uncompiled { .full-size; - top: 58px; + top: @pdf-top-offset; } .pdf-logs, .pdf-errors, .pdf-uncompiled, .pdf-validation-problems{ @@ -69,11 +73,11 @@ } .pdfjs-viewer { .full-size; - background-color: @gray-lighter; + background-color: @pdfjs-bg; overflow: scroll; canvas, div.pdf-canvas { background: white; - box-shadow: black 0px 0px 10px; + box-shadow: @pdf-page-shadow-color 0px 0px 10px; } div.pdf-canvas.pdfng-empty { background-color: white; diff --git a/services/web/public/stylesheets/app/editor/toolbar.less b/services/web/public/stylesheets/app/editor/toolbar.less index 5c00f00567..1243c07634 100644 --- a/services/web/public/stylesheets/app/editor/toolbar.less +++ b/services/web/public/stylesheets/app/editor/toolbar.less @@ -129,11 +129,11 @@ } .toolbar-small-mixin() { - height: 32px; + height: @toolbar-small-height; } .toolbar-tall-mixin() { - height: 58px; + height: @toolbar-tall-height; padding-top: 10px; } .toolbar-alt-mixin() { diff --git a/services/web/public/stylesheets/core/_common-variables.less b/services/web/public/stylesheets/core/_common-variables.less index acf6bcc91f..a0342c5534 100644 --- a/services/web/public/stylesheets/core/_common-variables.less +++ b/services/web/public/stylesheets/core/_common-variables.less @@ -904,6 +904,8 @@ @toolbar-icon-btn-hover-shadow : 0 1px 0 rgba(0, 0, 0, 0.25); @toolbar-icon-btn-hover-boxshadow : inset 0 3px 5px rgba(0, 0, 0, 0.225); @toolbar-border-bottom : 1px solid @toolbar-border-color; +@toolbar-small-height : 32px; +@toolbar-tall-height : 58px; // Editor file-tree @file-tree-bg : transparent; @@ -917,6 +919,13 @@ @file-tree-item-selected-bg : transparent; @file-tree-multiselect-bg : lighten(@brand-info, 40%); @file-tree-multiselect-hover-bg : lighten(@brand-info, 30%); + +// PDF +@pdf-top-offset : @toolbar-tall-height; +@pdf-bg : transparent; +@pdfjs-bg : @gray-lighter; +@pdf-page-shadow-color : #000; + // Tags @tag-border-radius : 0.25em; @tag-bg-color : @label-default-bg; diff --git a/services/web/public/stylesheets/core/ol-variables.less b/services/web/public/stylesheets/core/ol-variables.less index 5c15ad2d3d..8116c79a6a 100644 --- a/services/web/public/stylesheets/core/ol-variables.less +++ b/services/web/public/stylesheets/core/ol-variables.less @@ -186,6 +186,12 @@ @file-tree-multiselect-bg : @ol-blue; @file-tree-multiselect-hover-bg : @ol-dark-blue; @file-tree-droppable-bg-color : tint(@ol-green, 5%); + +// PDF +@pdf-top-offset : @toolbar-small-height; +@pdf-bg : transparent; +@pdfjs-bg : @ol-blue-gray-1; +@pdf-page-shadow-color : rgba(0, 0, 0, 0.5); //== Colors // //## Gray and brand colors for use across Bootstrap. From 5bea196b8faceadd5e6eb4f9289b88d217881d5d Mon Sep 17 00:00:00 2001 From: Paulo Reis Date: Thu, 7 Dec 2017 16:35:39 +0000 Subject: [PATCH 02/56] Style logs, log hints and alerts in general. --- .../public/stylesheets/app/editor/pdf.less | 22 ++++----- .../public/stylesheets/components/alerts.less | 2 +- .../public/stylesheets/components/card.less | 1 - .../stylesheets/core/_common-variables.less | 5 +- .../public/stylesheets/core/ol-variables.less | 46 +++++++++++++++++-- 5 files changed, 58 insertions(+), 18 deletions(-) diff --git a/services/web/public/stylesheets/app/editor/pdf.less b/services/web/public/stylesheets/app/editor/pdf.less index aab748d17f..eadf39cb9e 100644 --- a/services/web/public/stylesheets/app/editor/pdf.less +++ b/services/web/public/stylesheets/app/editor/pdf.less @@ -10,7 +10,7 @@ padding: 0 (@line-height-computed / 2); } -.pdf §{ +.pdf { background-color: @pdf-bg; } @@ -183,7 +183,7 @@ cursor: pointer; .line-no { float: right; - color: @gray; + color: @log-line-no-color; font-weight: 700; .fa { @@ -303,22 +303,22 @@ } .alert-danger & { - color: @alert-danger-border; + color: @state-danger-border; } .alert-warning & { - color: @alert-warning-border; + color: @state-warning-border; } .alert-info & { - color: @alert-info-border; + color: @state-info-border; } } &-text, &-feedback-label { - color: @gray-dark; + color: @log-hints-color; font-size: 0.9rem; margin-bottom: 20px; } @@ -343,25 +343,25 @@ &-actions a, &-text a { .alert-danger & { - color: @alert-danger-text; + color: @state-danger-text; } .alert-warning & { - color: @alert-warning-text; + color: @state-warning-text; } .alert-info & { - color: @alert-info-text; + color: @state-info-text; } } &-feedback { - color: @gray-dark; + color: @log-hints-color; float: right; } &-extra-feedback { - color: @gray-dark; + color: @log-hints-color; font-size: 0.8rem; margin-top: 10px; padding-bottom: 5px; diff --git a/services/web/public/stylesheets/components/alerts.less b/services/web/public/stylesheets/components/alerts.less index 527c97b151..6ed93d29d4 100755 --- a/services/web/public/stylesheets/components/alerts.less +++ b/services/web/public/stylesheets/components/alerts.less @@ -10,7 +10,7 @@ padding: @alert-padding; margin-bottom: @line-height-computed; border-left: 3px solid transparent; - // border-radius: @alert-border-radius; + border-radius: @alert-border-radius; // Headings for larger alerts h4 { diff --git a/services/web/public/stylesheets/components/card.less b/services/web/public/stylesheets/components/card.less index 1e06fbe3b4..ac43c038b7 100644 --- a/services/web/public/stylesheets/components/card.less +++ b/services/web/public/stylesheets/components/card.less @@ -1,7 +1,6 @@ .card { background-color: white; border-radius: @border-radius-base; - -webkit-box-shadow: @card-box-shadow; box-shadow: @card-box-shadow; padding: @line-height-computed; .page-header { diff --git a/services/web/public/stylesheets/core/_common-variables.less b/services/web/public/stylesheets/core/_common-variables.less index a0342c5534..23db1a202b 100644 --- a/services/web/public/stylesheets/core/_common-variables.less +++ b/services/web/public/stylesheets/core/_common-variables.less @@ -550,7 +550,7 @@ //## Define alert colors, border radius, and padding. @alert-padding: 15px; -@alert-border-radius: @border-radius-base; +@alert-border-radius: 0; @alert-link-font-weight: bold; @alert-success-bg: @state-success-bg; @@ -925,7 +925,8 @@ @pdf-bg : transparent; @pdfjs-bg : @gray-lighter; @pdf-page-shadow-color : #000; - +@log-line-no-color : @gray; +@log-hints-color : @gray-dark; // Tags @tag-border-radius : 0.25em; @tag-bg-color : @label-default-bg; diff --git a/services/web/public/stylesheets/core/ol-variables.less b/services/web/public/stylesheets/core/ol-variables.less index 8116c79a6a..bb3ba436b7 100644 --- a/services/web/public/stylesheets/core/ol-variables.less +++ b/services/web/public/stylesheets/core/ol-variables.less @@ -64,6 +64,44 @@ @btn-info-bg : @ol-blue; @btn-info-border : transparent; +// Alerts +@alert-padding : 15px; +@alert-border-radius : @border-radius-base; +@alert-link-font-weight : bold; + +@alert-success-bg : @brand-success; +@alert-success-text : #FFF; +@alert-success-border: transparent; + +@alert-info-bg : @brand-info; +@alert-info-text : #FFF; +@alert-info-border : transparent; + +@alert-warning-bg : @brand-warning; +@alert-warning-text : #FFF; +@alert-warning-border: transparent; + +@alert-danger-bg : @brand-danger; +@alert-danger-text : #FFF; +@alert-danger-border : transparent; + + +// @state-success-text: darken(@brand-success, 20%); +// @state-success-bg: lighten(@brand-success, 50%); +// @state-success-border: darken(@brand-success, 5%); + +// @state-info-text: darken(@brand-info, 20%); +// @state-info-bg: lighten(@brand-info, 47%); +// @state-info-border: darken(@brand-info, 7%); + +// @state-warning-text: darken(@brand-warning, 10%); +// @state-warning-bg: lighten(@brand-warning, 45%); +// @state-warning-border: @brand-warning; + +// @state-danger-text: darken(@brand-danger, 10%); +// @state-danger-bg: lighten(@brand-danger, 50%); +// @state-danger-border: darken(@brand-danger, 5%); + // Tags @tag-border-radius : 9999px; @tag-bg-color : @ol-green; @@ -189,9 +227,11 @@ // PDF @pdf-top-offset : @toolbar-small-height; -@pdf-bg : transparent; -@pdfjs-bg : @ol-blue-gray-1; +@pdf-bg : @ol-blue-gray-1; +@pdfjs-bg : transparent; @pdf-page-shadow-color : rgba(0, 0, 0, 0.5); +@log-line-no-color : #FFF; +@log-hints-color : @ol-blue-gray-4; //== Colors // //## Gray and brand colors for use across Bootstrap. @@ -213,7 +253,7 @@ @brand-primary: @ol-green; @brand-success: @green; -@brand-info: @ol-dark-green; +@brand-info: @ol-blue; @brand-warning: @orange; @brand-danger: #E03A06; From f5c914ed6cbd345f984cf4ba946ad536485f6ea1 Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Mon, 11 Dec 2017 18:03:09 +0000 Subject: [PATCH 03/56] When upgrading track changes on V2, redirect to V1 trial page --- services/web/app/views/layout.pug | 4 ++++ .../web/public/coffee/main/account-upgrade.coffee | 11 +++++++---- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/services/web/app/views/layout.pug b/services/web/app/views/layout.pug index e12d7d6f81..fc14c2030b 100644 --- a/services/web/app/views/layout.pug +++ b/services/web/app/views/layout.pug @@ -95,6 +95,10 @@ html(itemscope, itemtype='http://schema.org/Product') cdnDomain : '!{settings.templates.cdnDomain}', indexName : '!{settings.templates.indexName}' } + + - if (settings.overleaf && settings.overleaf.host) + script. + window.v1BaseUrl = '!{settings.overleaf.host}' body if(settings.recaptcha) diff --git a/services/web/public/coffee/main/account-upgrade.coffee b/services/web/public/coffee/main/account-upgrade.coffee index 6144bea9ef..29bc472505 100644 --- a/services/web/public/coffee/main/account-upgrade.coffee +++ b/services/web/public/coffee/main/account-upgrade.coffee @@ -11,9 +11,12 @@ define [ w = window.open() go = () -> ga?('send', 'event', 'subscription-funnel', 'upgraded-free-trial', source) - url = "/user/subscription/new?planCode=#{plan}&ssp=true" - if couponCode? - url = "#{url}&cc=#{couponCode}" + if window.v1BaseUrl? + url = "#{window.v1BaseUrl}/users/trial" + else + url = "/user/subscription/new?planCode=#{plan}&ssp=true" + if couponCode? + url = "#{url}&cc=#{couponCode}" $scope.startedFreeTrial = true switch source @@ -27,7 +30,7 @@ define [ else event_tracking.sendMB "subscription-start-trial", { source, plan } - + w.location = url if $scope.shouldABTestPlans From 603252f3b4c6d216fa657ec59d52796e8f943130 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Tue, 12 Dec 2017 16:23:24 +0000 Subject: [PATCH 04/56] Prevent user from trying to delete archived project they don't own This change disables the select-project checkbox if the user is on the 'archived' project pane and they don't own the project. The request to delete would fail anyway, but this prevents UI confusion --- services/web/app/views/project/list/item.pug | 1 + .../web/public/coffee/main/project-list/project-list.coffee | 3 +++ 2 files changed, 4 insertions(+) diff --git a/services/web/app/views/project/list/item.pug b/services/web/app/views/project/list/item.pug index f246ad34d4..a4bb240c4b 100644 --- a/services/web/app/views/project/list/item.pug +++ b/services/web/app/views/project/list/item.pug @@ -2,6 +2,7 @@ input.select-item( select-individual, type="checkbox", + ng-disabled="shouldDisableCheckbox(project)", ng-model="project.selected" stop-propagation="click" aria-label=translate('select_project') + " '{{ project.name }}'" diff --git a/services/web/public/coffee/main/project-list/project-list.coffee b/services/web/public/coffee/main/project-list/project-list.coffee index f4a6951e4a..75d4767ec0 100644 --- a/services/web/public/coffee/main/project-list/project-list.coffee +++ b/services/web/public/coffee/main/project-list/project-list.coffee @@ -462,6 +462,9 @@ define [ App.controller "ProjectListItemController", ($scope) -> + $scope.shouldDisableCheckbox = (project) -> + $scope.filter == 'archived' && project.accessLevel != 'owner' + $scope.projectLink = (project) -> if project.accessLevel == 'readAndWrite' and project.source == 'token' "/#{project.tokens.readAndWrite}" From 80e6a660be2f3fb938b938541206c94ba160b994 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Tue, 12 Dec 2017 17:21:01 +0000 Subject: [PATCH 05/56] wip, this does not work minified yet for some reason lib.js is not being requested no, console errors or network errors --- services/web/.gitignore | 9 ++++++++ .../infrastructure/ExpressLocals.coffee | 22 ++++++++++++++++++- services/web/app/views/layout.pug | 4 ++-- services/web/app/views/project/editor.pug | 4 ++-- 4 files changed, 34 insertions(+), 5 deletions(-) diff --git a/services/web/.gitignore b/services/web/.gitignore index a48481690a..8df4ab2fa6 100644 --- a/services/web/.gitignore +++ b/services/web/.gitignore @@ -67,6 +67,15 @@ public/minjs/ Gemfile.lock +public/js/libs/fineuploader-5.15.4.*.js +public/js/libs/fineuploader-*.js +public/js/libs/pdfjs-1.7.225/compatibility.*.js +public/js/libs/pdfjs-1.7.225/pdf.*.js +public/js/libs/pdfjs-1.7.225/pdf.worker.*.js +public/js/libs/require.*.js +public/stylesheets/ol-style.*.css +public/stylesheets/style.*.css + *.swp .DS_Store diff --git a/services/web/app/coffee/infrastructure/ExpressLocals.coffee b/services/web/app/coffee/infrastructure/ExpressLocals.coffee index b126819f56..d2aeae655c 100644 --- a/services/web/app/coffee/infrastructure/ExpressLocals.coffee +++ b/services/web/app/coffee/infrastructure/ExpressLocals.coffee @@ -13,6 +13,7 @@ Url = require "url" PackageVersions = require "./PackageVersions" htmlEncoder = new require("node-html-encoder").Encoder("numerical") fingerprints = {} +hashedFiles = {} Path = require 'path' Features = require "./Features" @@ -59,6 +60,19 @@ for paths in pathList logger.log "#{filePath}: #{hash}" fingerprints[filePath] = hash + if paths.length == 1 + #todo deal with ace multi file hash + path = paths[0] + splitPath = path.split("/") + filenameSplit = splitPath.pop().split(".") + filenameSplit.splice(filenameSplit.length-1, 0, hash) + splitPath.push(filenameSplit.join(".")) + hashPath = splitPath.join("/") + fsHashPath = Path.join __dirname, "../../../", "public#{hashPath}" + fs.writeFileSync(fsHashPath, content) + hashedFiles[paths] = hashPath + + getFingerprint = (path) -> if fingerprints[path]? return fingerprints[path] @@ -123,6 +137,9 @@ module.exports = (app, webRouter, privateApiRouter, publicApiRouter)-> res.locals.buildJsPath = (jsFile, opts = {})-> path = Path.join(jsPath, jsFile) + if opts.hashedPath + return hashedFiles[path] + doFingerPrint = opts.fingerprint != false if !opts.qs? @@ -140,8 +157,11 @@ module.exports = (app, webRouter, privateApiRouter, publicApiRouter)-> path = path + "?" + qs return path - res.locals.buildCssPath = (cssFile)-> + res.locals.buildCssPath = (cssFile, opts)-> path = Path.join("/stylesheets/", cssFile) + if opts.hashedPath + hashedPath = hashedFiles[path] + return Url.resolve(staticFilesBase, hashedPath) return Url.resolve(staticFilesBase, path) + "?fingerprint=" + getFingerprint(path) res.locals.buildImgPath = (imgFile)-> diff --git a/services/web/app/views/layout.pug b/services/web/app/views/layout.pug index e12d7d6f81..537417dce3 100644 --- a/services/web/app/views/layout.pug +++ b/services/web/app/views/layout.pug @@ -23,7 +23,7 @@ html(itemscope, itemtype='http://schema.org/Product') link(rel="apple-touch-icon-precomposed", href="/" + settings.brandPrefix + "apple-touch-icon-precomposed.png") link(rel="mask-icon", href="/" + settings.brandPrefix + "mask-favicon.svg", color="#a93529") - link(rel='stylesheet', href=buildCssPath("/" + settings.brandPrefix + "style.css")) + link(rel='stylesheet', href=buildCssPath("/" + settings.brandPrefix + "style.css", {hashedPath:true})) block _headLinks @@ -152,7 +152,7 @@ html(itemscope, itemtype='http://schema.org/Product') } }; script( - data-main=buildJsPath('main.js', {fingerprint:false}), + data-main=buildJsPath('main.js', {hashedPath:true}), baseurl=fullJsPath, src=buildJsPath('libs/require.js') ) diff --git a/services/web/app/views/project/editor.pug b/services/web/app/views/project/editor.pug index ec1d129714..c49afee0b2 100644 --- a/services/web/app/views/project/editor.pug +++ b/services/web/app/views/project/editor.pug @@ -161,8 +161,8 @@ block requirejs window.uiConfig = JSON.parse('!{JSON.stringify(uiConfig).replace(/\//g, "\\/")}'); script( - data-main=buildJsPath("ide.js", {fingerprint:false}), + data-main=buildJsPath("ide.js", {hashedPath:true}), baseurl=fullJsPath, data-ace-base=buildJsPath(lib('ace'), {fingerprint:false}), - src=buildJsPath('libs/require.js') + src=buildJsPath('libs/require.js', {hashedPath:true}) ) From 4c81cd874fb81d8c3581217f1efe48779d869a67 Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Wed, 13 Dec 2017 11:09:43 +0000 Subject: [PATCH 06/56] Remove unnecessary method - handled by FreeTrialModalController --- .../controllers/TrackChangesUpgradeModalController.coffee | 5 ----- 1 file changed, 5 deletions(-) diff --git a/services/web/public/coffee/ide/review-panel/controllers/TrackChangesUpgradeModalController.coffee b/services/web/public/coffee/ide/review-panel/controllers/TrackChangesUpgradeModalController.coffee index ae8c049f69..3fcc5e416f 100644 --- a/services/web/public/coffee/ide/review-panel/controllers/TrackChangesUpgradeModalController.coffee +++ b/services/web/public/coffee/ide/review-panel/controllers/TrackChangesUpgradeModalController.coffee @@ -4,8 +4,3 @@ define [ App.controller "TrackChangesUpgradeModalController", ($scope, $modalInstance) -> $scope.cancel = () -> $modalInstance.dismiss() - - $scope.startFreeTrial = (source) -> - ga?('send', 'event', 'subscription-funnel', 'upgraded-free-trial', source) - window.open("/user/subscription/new?planCode=student_free_trial_7_days") - $scope.startedFreeTrial = true \ No newline at end of file From 65efbbce537bfe4a5428b85848c06a71c8ee1ca0 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Wed, 13 Dec 2017 13:06:38 +0000 Subject: [PATCH 07/56] seems to work now. --- .../web/app/coffee/infrastructure/ExpressLocals.coffee | 8 ++++++-- services/web/app/views/layout.pug | 6 +++--- services/web/app/views/project/editor.pug | 6 ++++-- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/services/web/app/coffee/infrastructure/ExpressLocals.coffee b/services/web/app/coffee/infrastructure/ExpressLocals.coffee index d2aeae655c..7aef6e5424 100644 --- a/services/web/app/coffee/infrastructure/ExpressLocals.coffee +++ b/services/web/app/coffee/infrastructure/ExpressLocals.coffee @@ -134,11 +134,15 @@ module.exports = (app, webRouter, privateApiRouter, publicApiRouter)-> res.locals.fullJsPath = Url.resolve(staticFilesBase, jsPath) res.locals.lib = PackageVersions.lib + + res.locals.buildJsPath = (jsFile, opts = {})-> path = Path.join(jsPath, jsFile) + + # TODO CDN? if opts.hashedPath - return hashedFiles[path] + path = hashedFiles[path] doFingerPrint = opts.fingerprint != false @@ -159,7 +163,7 @@ module.exports = (app, webRouter, privateApiRouter, publicApiRouter)-> res.locals.buildCssPath = (cssFile, opts)-> path = Path.join("/stylesheets/", cssFile) - if opts.hashedPath + if opts?.hashedPath hashedPath = hashedFiles[path] return Url.resolve(staticFilesBase, hashedPath) return Url.resolve(staticFilesBase, path) + "?fingerprint=" + getFingerprint(path) diff --git a/services/web/app/views/layout.pug b/services/web/app/views/layout.pug index 537417dce3..d9bba35f0b 100644 --- a/services/web/app/views/layout.pug +++ b/services/web/app/views/layout.pug @@ -142,9 +142,9 @@ html(itemscope, itemtype='http://schema.org/Product') window.requirejs = { "paths" : { "moment": "libs/#{lib('moment')}", - "fineuploader": "libs/#{lib('fineuploader')}" + "fineuploader": "libs/#{lib('fineuploader')}", + "main": "#{buildJsPath('main.js', {hashedPath:true, fingerprint:false}).slice(0,-3)}", }, - "urlArgs": "fingerprint=#{fingerprint(jsPath + 'main.js')}-#{fingerprint(jsPath + 'libs.js')}", "config":{ "moment":{ "noGlobal": true @@ -152,7 +152,7 @@ html(itemscope, itemtype='http://schema.org/Product') } }; script( - data-main=buildJsPath('main.js', {hashedPath:true}), + data-main=buildJsPath('main.js', {hashedPath:false, fingerprint:false}), baseurl=fullJsPath, src=buildJsPath('libs/require.js') ) diff --git a/services/web/app/views/project/editor.pug b/services/web/app/views/project/editor.pug index c49afee0b2..a37ac6e6d5 100644 --- a/services/web/app/views/project/editor.pug +++ b/services/web/app/views/project/editor.pug @@ -131,7 +131,9 @@ block requirejs "pdfjs-dist/build/pdf": "libs/#{lib('pdfjs')}/pdf", "pdfjs-dist/build/pdf.worker": "#{pdfWorkerPath}", "ace": "#{lib('ace')}", - "fineuploader": "libs/#{lib('fineuploader')}" + "fineuploader": "libs/#{lib('fineuploader')}", + "ide": "#{buildJsPath('ide.js', {hashedPath:true, fingerprint:false}).slice(0,-3)}", + }, "urlArgs" : "fingerprint=#{fingerprint(jsPath + 'ide.js')}-#{fingerprint(jsPath + 'libs.js')}", "waitSeconds": 0, @@ -161,7 +163,7 @@ block requirejs window.uiConfig = JSON.parse('!{JSON.stringify(uiConfig).replace(/\//g, "\\/")}'); script( - data-main=buildJsPath("ide.js", {hashedPath:true}), + data-main=buildJsPath("ide.js", {hashedPath:false}), baseurl=fullJsPath, data-ace-base=buildJsPath(lib('ace'), {fingerprint:false}), src=buildJsPath('libs/require.js', {hashedPath:true}) From 88334959d708c2cef12b7fc5556d9a39823b8300 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Wed, 13 Dec 2017 14:13:45 +0000 Subject: [PATCH 08/56] added remove extension --- services/web/app/coffee/infrastructure/ExpressLocals.coffee | 3 +++ services/web/app/views/layout.pug | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/services/web/app/coffee/infrastructure/ExpressLocals.coffee b/services/web/app/coffee/infrastructure/ExpressLocals.coffee index 7aef6e5424..33dbe2f34a 100644 --- a/services/web/app/coffee/infrastructure/ExpressLocals.coffee +++ b/services/web/app/coffee/infrastructure/ExpressLocals.coffee @@ -157,6 +157,9 @@ module.exports = (app, webRouter, privateApiRouter, publicApiRouter)-> qs = querystring.stringify(opts.qs) + if opts.removeExtension == true + path = path.slice(0,-3) + if qs? and qs.length > 0 path = path + "?" + qs return path diff --git a/services/web/app/views/layout.pug b/services/web/app/views/layout.pug index d9bba35f0b..0220a5af71 100644 --- a/services/web/app/views/layout.pug +++ b/services/web/app/views/layout.pug @@ -143,7 +143,7 @@ html(itemscope, itemtype='http://schema.org/Product') "paths" : { "moment": "libs/#{lib('moment')}", "fineuploader": "libs/#{lib('fineuploader')}", - "main": "#{buildJsPath('main.js', {hashedPath:true, fingerprint:false}).slice(0,-3)}", + "main": "#{buildJsPath('main.js', {hashedPath:true, fingerprint:false, removeExtension:true})}" }, "config":{ "moment":{ From 357617d952f15dfb679470fa6e6d19a416899601 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Wed, 13 Dec 2017 14:19:19 +0000 Subject: [PATCH 09/56] works with libs compiled into it --- services/web/Gruntfile.coffee | 5 ++--- services/web/app/views/project/editor.pug | 4 ++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/services/web/Gruntfile.coffee b/services/web/Gruntfile.coffee index c49add4a91..374177e7d2 100644 --- a/services/web/Gruntfile.coffee +++ b/services/web/Gruntfile.coffee @@ -205,11 +205,10 @@ module.exports = (grunt) -> modules: [ { name: "main", - exclude: ["libs"] }, { name: "ide", - exclude: ["libs", "pdfjs-dist/build/pdf"] - }, { + exclude: ["pdfjs-dist/build/pdf"] + },{ name: "libs" },{ name: "ace/mode-latex" diff --git a/services/web/app/views/project/editor.pug b/services/web/app/views/project/editor.pug index a37ac6e6d5..19b2478f48 100644 --- a/services/web/app/views/project/editor.pug +++ b/services/web/app/views/project/editor.pug @@ -132,7 +132,7 @@ block requirejs "pdfjs-dist/build/pdf.worker": "#{pdfWorkerPath}", "ace": "#{lib('ace')}", "fineuploader": "libs/#{lib('fineuploader')}", - "ide": "#{buildJsPath('ide.js', {hashedPath:true, fingerprint:false}).slice(0,-3)}", + "ide": "#{buildJsPath('ide.js', {hashedPath:true, fingerprint:false, removeExtension:true})}" }, "urlArgs" : "fingerprint=#{fingerprint(jsPath + 'ide.js')}-#{fingerprint(jsPath + 'libs.js')}", @@ -163,7 +163,7 @@ block requirejs window.uiConfig = JSON.parse('!{JSON.stringify(uiConfig).replace(/\//g, "\\/")}'); script( - data-main=buildJsPath("ide.js", {hashedPath:false}), + data-main=buildJsPath("ide.js", {hashedPath:false, fingerprint:false}), baseurl=fullJsPath, data-ace-base=buildJsPath(lib('ace'), {fingerprint:false}), src=buildJsPath('libs/require.js', {hashedPath:true}) From ccbb15c82ef038bc8a3a23eeb7c6402fa47dd8e0 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Wed, 13 Dec 2017 14:54:08 +0000 Subject: [PATCH 10/56] remove fingerprint from ide --- services/web/app/views/project/editor.pug | 2 -- 1 file changed, 2 deletions(-) diff --git a/services/web/app/views/project/editor.pug b/services/web/app/views/project/editor.pug index 19b2478f48..38ffdd3402 100644 --- a/services/web/app/views/project/editor.pug +++ b/services/web/app/views/project/editor.pug @@ -133,9 +133,7 @@ block requirejs "ace": "#{lib('ace')}", "fineuploader": "libs/#{lib('fineuploader')}", "ide": "#{buildJsPath('ide.js', {hashedPath:true, fingerprint:false, removeExtension:true})}" - }, - "urlArgs" : "fingerprint=#{fingerprint(jsPath + 'ide.js')}-#{fingerprint(jsPath + 'libs.js')}", "waitSeconds": 0, "shim": { "pdfjs-dist/build/pdf": { From b7a43d95e0e54035c55c8cd35eb49a661da4874e Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Wed, 13 Dec 2017 15:55:32 +0000 Subject: [PATCH 11/56] Remove unused method, now handled by FreeTrialModalController --- .../coffee/ide/pdf/controllers/PdfController.coffee | 8 -------- 1 file changed, 8 deletions(-) diff --git a/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee b/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee index 17afd8bbcb..90e5eca145 100644 --- a/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee +++ b/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee @@ -533,14 +533,6 @@ define [ else $scope.switchToSideBySideLayout() - $scope.startFreeTrial = (source) -> - ga?('send', 'event', 'subscription-funnel', 'compile-timeout', source) - - event_tracking.sendMB "subscription-start-trial", { source } - - window.open("/user/subscription/new?planCode=#{$scope.startTrialPlanCode}") - $scope.startedFreeTrial = true - App.factory "synctex", ["ide", "$http", "$q", (ide, $http, $q) -> # enable per-user containers by default perUserCompile = true From f021f21f11c3c5049a555fc84edb70f71a3733ad Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Wed, 13 Dec 2017 16:37:51 +0000 Subject: [PATCH 12/56] only hash when minified --- services/web/app/views/layout.pug | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/app/views/layout.pug b/services/web/app/views/layout.pug index 0220a5af71..f32e90a6ed 100644 --- a/services/web/app/views/layout.pug +++ b/services/web/app/views/layout.pug @@ -143,7 +143,7 @@ html(itemscope, itemtype='http://schema.org/Product') "paths" : { "moment": "libs/#{lib('moment')}", "fineuploader": "libs/#{lib('fineuploader')}", - "main": "#{buildJsPath('main.js', {hashedPath:true, fingerprint:false, removeExtension:true})}" + "main": "#{buildJsPath('main.js', {hashedPath:settings.useMinifiedJs, fingerprint:false, removeExtension:true})}", }, "config":{ "moment":{ From 716b3092088ee98e5891b36d437d7a468c7eddf2 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Wed, 13 Dec 2017 16:50:18 +0000 Subject: [PATCH 13/56] remove todo statments --- services/web/app/coffee/infrastructure/ExpressLocals.coffee | 3 --- 1 file changed, 3 deletions(-) diff --git a/services/web/app/coffee/infrastructure/ExpressLocals.coffee b/services/web/app/coffee/infrastructure/ExpressLocals.coffee index 33dbe2f34a..7cac6743e5 100644 --- a/services/web/app/coffee/infrastructure/ExpressLocals.coffee +++ b/services/web/app/coffee/infrastructure/ExpressLocals.coffee @@ -61,7 +61,6 @@ for paths in pathList fingerprints[filePath] = hash if paths.length == 1 - #todo deal with ace multi file hash path = paths[0] splitPath = path.split("/") filenameSplit = splitPath.pop().split(".") @@ -139,8 +138,6 @@ module.exports = (app, webRouter, privateApiRouter, publicApiRouter)-> res.locals.buildJsPath = (jsFile, opts = {})-> path = Path.join(jsPath, jsFile) - - # TODO CDN? if opts.hashedPath path = hashedFiles[path] From 490c56c692cae2e856f86fca659bbf724f6a1cc2 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Wed, 13 Dec 2017 17:07:58 +0000 Subject: [PATCH 14/56] libs -> libraries don't use hash in non minjs version --- services/web/Gruntfile.coffee | 5 +++-- services/web/app/coffee/infrastructure/ExpressLocals.coffee | 2 +- services/web/app/views/layout.pug | 1 + services/web/app/views/project/editor.pug | 3 ++- services/web/public/coffee/base.coffee | 2 +- services/web/public/coffee/{libs.coffee => libraries.coffee} | 0 services/web/public/coffee/utils/underscore.coffee | 2 +- 7 files changed, 9 insertions(+), 6 deletions(-) rename services/web/public/coffee/{libs.coffee => libraries.coffee} (100%) diff --git a/services/web/Gruntfile.coffee b/services/web/Gruntfile.coffee index 374177e7d2..a0000360b3 100644 --- a/services/web/Gruntfile.coffee +++ b/services/web/Gruntfile.coffee @@ -205,11 +205,12 @@ module.exports = (grunt) -> modules: [ { name: "main", + exclude: ["libraries"] }, { name: "ide", - exclude: ["pdfjs-dist/build/pdf"] + exclude: ["pdfjs-dist/build/pdf", "libraries"] },{ - name: "libs" + name: "libraries" },{ name: "ace/mode-latex" },{ diff --git a/services/web/app/coffee/infrastructure/ExpressLocals.coffee b/services/web/app/coffee/infrastructure/ExpressLocals.coffee index 7cac6743e5..46f9eefb86 100644 --- a/services/web/app/coffee/infrastructure/ExpressLocals.coffee +++ b/services/web/app/coffee/infrastructure/ExpressLocals.coffee @@ -43,7 +43,7 @@ pathList = [ ["#{jsPath}libs/require.js"] ["#{jsPath}ide.js"] ["#{jsPath}main.js"] - ["#{jsPath}libs.js"] + ["#{jsPath}libraries.js"] ["#{jsPath}#{ace}/ace.js","#{jsPath}#{ace}/mode-latex.js","#{jsPath}#{ace}/worker-latex.js","#{jsPath}#{ace}/snippets/latex.js"] ["#{jsPath}libs/#{pdfjs}/pdf.js"] ["#{jsPath}libs/#{pdfjs}/pdf.worker.js"] diff --git a/services/web/app/views/layout.pug b/services/web/app/views/layout.pug index f32e90a6ed..5aa41c56c7 100644 --- a/services/web/app/views/layout.pug +++ b/services/web/app/views/layout.pug @@ -144,6 +144,7 @@ html(itemscope, itemtype='http://schema.org/Product') "moment": "libs/#{lib('moment')}", "fineuploader": "libs/#{lib('fineuploader')}", "main": "#{buildJsPath('main.js', {hashedPath:settings.useMinifiedJs, fingerprint:false, removeExtension:true})}", + "libraries": "#{buildJsPath('libraries.js', {hashedPath:settings.useMinifiedJs, fingerprint:false, removeExtension:true})}", }, "config":{ "moment":{ diff --git a/services/web/app/views/project/editor.pug b/services/web/app/views/project/editor.pug index 38ffdd3402..c3fd408e10 100644 --- a/services/web/app/views/project/editor.pug +++ b/services/web/app/views/project/editor.pug @@ -132,7 +132,8 @@ block requirejs "pdfjs-dist/build/pdf.worker": "#{pdfWorkerPath}", "ace": "#{lib('ace')}", "fineuploader": "libs/#{lib('fineuploader')}", - "ide": "#{buildJsPath('ide.js', {hashedPath:true, fingerprint:false, removeExtension:true})}" + "ide": "#{buildJsPath('ide.js', {hashedPath:settings.useMinifiedJs, fingerprint:false, removeExtension:true})}", + "libraries": "#{buildJsPath('libraries.js', {hashedPath:settings.useMinifiedJs, fingerprint:false, removeExtension:true})}", }, "waitSeconds": 0, "shim": { diff --git a/services/web/public/coffee/base.coffee b/services/web/public/coffee/base.coffee index 03d0ada365..c3847c9342 100644 --- a/services/web/public/coffee/base.coffee +++ b/services/web/public/coffee/base.coffee @@ -1,5 +1,5 @@ define [ - "libs" + "libraries" "modules/recursionHelper" "modules/errorCatcher" "modules/localStorage" diff --git a/services/web/public/coffee/libs.coffee b/services/web/public/coffee/libraries.coffee similarity index 100% rename from services/web/public/coffee/libs.coffee rename to services/web/public/coffee/libraries.coffee diff --git a/services/web/public/coffee/utils/underscore.coffee b/services/web/public/coffee/utils/underscore.coffee index 7a1551f90b..944abccb5d 100644 --- a/services/web/public/coffee/utils/underscore.coffee +++ b/services/web/public/coffee/utils/underscore.coffee @@ -1,5 +1,5 @@ define [ - "libs" + "libraries" ], () -> angular.module('underscore', []).factory '_', -> return window._ From bbbba701b019682b026634ab20ff6b3a827c99b7 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Thu, 14 Dec 2017 10:21:53 +0000 Subject: [PATCH 15/56] Update editor.pug --- services/web/app/views/project/editor.pug | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/app/views/project/editor.pug b/services/web/app/views/project/editor.pug index c3fd408e10..582e0f3541 100644 --- a/services/web/app/views/project/editor.pug +++ b/services/web/app/views/project/editor.pug @@ -165,5 +165,5 @@ block requirejs data-main=buildJsPath("ide.js", {hashedPath:false, fingerprint:false}), baseurl=fullJsPath, data-ace-base=buildJsPath(lib('ace'), {fingerprint:false}), - src=buildJsPath('libs/require.js', {hashedPath:true}) + src=buildJsPath('libs/require.js', {hashedPath:false}) ) From 288af6772afcc2dcf3398ec927e5263965fdff50 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Thu, 14 Dec 2017 10:59:46 +0000 Subject: [PATCH 16/56] write hashes for all files we fingerprint --- services/web/.gitignore | 10 +++++++--- .../web/app/coffee/infrastructure/ExpressLocals.coffee | 3 +-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/services/web/.gitignore b/services/web/.gitignore index 8df4ab2fa6..d0053e47c8 100644 --- a/services/web/.gitignore +++ b/services/web/.gitignore @@ -69,12 +69,16 @@ Gemfile.lock public/js/libs/fineuploader-5.15.4.*.js public/js/libs/fineuploader-*.js -public/js/libs/pdfjs-1.7.225/compatibility.*.js -public/js/libs/pdfjs-1.7.225/pdf.*.js -public/js/libs/pdfjs-1.7.225/pdf.worker.*.js +public/js/libs/pdfjs-*/compatibility.*.js +public/js/libs/pdfjs-*/pdf.*.js +public/js/libs/pdfjs-*/pdf.worker.*.js public/js/libs/require.*.js public/stylesheets/ol-style.*.css public/stylesheets/style.*.css +public/js/ace-*/ace.*.js +public/js/ace-*/mode-latex.*.js +public/js/ace-*/snippets/latex.*.js +public/js/ace-*/worker-latex.*.js *.swp .DS_Store diff --git a/services/web/app/coffee/infrastructure/ExpressLocals.coffee b/services/web/app/coffee/infrastructure/ExpressLocals.coffee index 46f9eefb86..51b44a8996 100644 --- a/services/web/app/coffee/infrastructure/ExpressLocals.coffee +++ b/services/web/app/coffee/infrastructure/ExpressLocals.coffee @@ -60,8 +60,7 @@ for paths in pathList logger.log "#{filePath}: #{hash}" fingerprints[filePath] = hash - if paths.length == 1 - path = paths[0] + for path in paths splitPath = path.split("/") filenameSplit = splitPath.pop().split(".") filenameSplit.splice(filenameSplit.length-1, 0, hash) From cbf656518f33ac6df64f488c7aec90d0b2a2f484 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Thu, 14 Dec 2017 11:24:47 +0000 Subject: [PATCH 17/56] remove versioned files from fingerprinting --- services/web/app/coffee/infrastructure/ExpressLocals.coffee | 5 ----- services/web/app/views/project/editor.pug | 1 - .../web/public/coffee/ide/editor/directives/aceEditor.coffee | 2 +- 3 files changed, 1 insertion(+), 7 deletions(-) diff --git a/services/web/app/coffee/infrastructure/ExpressLocals.coffee b/services/web/app/coffee/infrastructure/ExpressLocals.coffee index 51b44a8996..74c4b242fc 100644 --- a/services/web/app/coffee/infrastructure/ExpressLocals.coffee +++ b/services/web/app/coffee/infrastructure/ExpressLocals.coffee @@ -39,15 +39,10 @@ getFileContent = (filePath)-> logger.log "Generating file fingerprints..." pathList = [ - ["#{jsPath}libs/#{fineuploader}.js"] ["#{jsPath}libs/require.js"] ["#{jsPath}ide.js"] ["#{jsPath}main.js"] ["#{jsPath}libraries.js"] - ["#{jsPath}#{ace}/ace.js","#{jsPath}#{ace}/mode-latex.js","#{jsPath}#{ace}/worker-latex.js","#{jsPath}#{ace}/snippets/latex.js"] - ["#{jsPath}libs/#{pdfjs}/pdf.js"] - ["#{jsPath}libs/#{pdfjs}/pdf.worker.js"] - ["#{jsPath}libs/#{pdfjs}/compatibility.js"] ["/stylesheets/style.css"] ["/stylesheets/ol-style.css"] ] diff --git a/services/web/app/views/project/editor.pug b/services/web/app/views/project/editor.pug index 582e0f3541..38ade1a6b2 100644 --- a/services/web/app/views/project/editor.pug +++ b/services/web/app/views/project/editor.pug @@ -156,7 +156,6 @@ block requirejs } } }; - window.aceFingerprint = "#{fingerprint(jsPath + lib('ace') + '/ace.js')}" window.aceWorkerPath = "#{aceWorkerPath}"; window.pdfCMapsPath = "#{pdfCMapsPath}" window.uiConfig = JSON.parse('!{JSON.stringify(uiConfig).replace(/\//g, "\\/")}'); diff --git a/services/web/public/coffee/ide/editor/directives/aceEditor.coffee b/services/web/public/coffee/ide/editor/directives/aceEditor.coffee index 6908b25bce..ff5d1554e2 100644 --- a/services/web/public/coffee/ide/editor/directives/aceEditor.coffee +++ b/services/web/public/coffee/ide/editor/directives/aceEditor.coffee @@ -33,7 +33,7 @@ define [ if !ace.config._moduleUrl? ace.config._moduleUrl = ace.config.moduleUrl ace.config.moduleUrl = (args...) -> - url = ace.config._moduleUrl(args...) + "?fingerprint=#{window.aceFingerprint}" + url = ace.config._moduleUrl(args...) return url App.directive "aceEditor", ($timeout, $compile, $rootScope, event_tracking, localStorage, $cacheFactory, metadata, graphics, preamble, files, $http, $q) -> From 507503fe89ecae00f6976cac1fc0471d0c943c91 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Thu, 14 Dec 2017 11:36:33 +0000 Subject: [PATCH 18/56] remove old .gitignore paths --- services/web/.gitignore | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/services/web/.gitignore b/services/web/.gitignore index d0053e47c8..3830036f24 100644 --- a/services/web/.gitignore +++ b/services/web/.gitignore @@ -67,18 +67,9 @@ public/minjs/ Gemfile.lock -public/js/libs/fineuploader-5.15.4.*.js -public/js/libs/fineuploader-*.js -public/js/libs/pdfjs-*/compatibility.*.js -public/js/libs/pdfjs-*/pdf.*.js -public/js/libs/pdfjs-*/pdf.worker.*.js -public/js/libs/require.*.js public/stylesheets/ol-style.*.css public/stylesheets/style.*.css -public/js/ace-*/ace.*.js -public/js/ace-*/mode-latex.*.js -public/js/ace-*/snippets/latex.*.js -public/js/ace-*/worker-latex.*.js + *.swp .DS_Store From d89aa0ca02f7d3a6358466d6d430a602b090bf36 Mon Sep 17 00:00:00 2001 From: Paulo Reis Date: Thu, 14 Dec 2017 11:52:41 +0000 Subject: [PATCH 19/56] Remove commented out code. --- .../public/stylesheets/core/ol-variables.less | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/services/web/public/stylesheets/core/ol-variables.less b/services/web/public/stylesheets/core/ol-variables.less index a1a7738102..aee331f3d2 100644 --- a/services/web/public/stylesheets/core/ol-variables.less +++ b/services/web/public/stylesheets/core/ol-variables.less @@ -86,22 +86,6 @@ @alert-danger-border : transparent; -// @state-success-text: darken(@brand-success, 20%); -// @state-success-bg: lighten(@brand-success, 50%); -// @state-success-border: darken(@brand-success, 5%); - -// @state-info-text: darken(@brand-info, 20%); -// @state-info-bg: lighten(@brand-info, 47%); -// @state-info-border: darken(@brand-info, 7%); - -// @state-warning-text: darken(@brand-warning, 10%); -// @state-warning-bg: lighten(@brand-warning, 45%); -// @state-warning-border: @brand-warning; - -// @state-danger-text: darken(@brand-danger, 10%); -// @state-danger-bg: lighten(@brand-danger, 50%); -// @state-danger-border: darken(@brand-danger, 5%); - // Tags @tag-border-radius : 9999px; @tag-bg-color : @ol-green; From bf276b08142ed3fbb752bee8ea3c9d2d4f26729f Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Thu, 14 Dec 2017 12:11:13 +0000 Subject: [PATCH 20/56] ripped file fingerprinting out --- .../infrastructure/ExpressLocals.coffee | 66 +++++++------------ services/web/app/views/layout.pug | 12 ++-- services/web/app/views/project/editor.pug | 18 ++--- 3 files changed, 38 insertions(+), 58 deletions(-) diff --git a/services/web/app/coffee/infrastructure/ExpressLocals.coffee b/services/web/app/coffee/infrastructure/ExpressLocals.coffee index 74c4b242fc..002a10ca89 100644 --- a/services/web/app/coffee/infrastructure/ExpressLocals.coffee +++ b/services/web/app/coffee/infrastructure/ExpressLocals.coffee @@ -12,7 +12,6 @@ Modules = require "./Modules" Url = require "url" PackageVersions = require "./PackageVersions" htmlEncoder = new require("node-html-encoder").Encoder("numerical") -fingerprints = {} hashedFiles = {} Path = require 'path' Features = require "./Features" @@ -31,49 +30,39 @@ getFileContent = (filePath)-> filePath = Path.join __dirname, "../../../", "public#{filePath}" exists = fs.existsSync filePath if exists - content = fs.readFileSync filePath + content = fs.readFileSync filePath, "UTF-8" return content else - logger.log filePath:filePath, "file does not exist for fingerprints" + logger.log filePath:filePath, "file does not exist for hashing" return "" -logger.log "Generating file fingerprints..." +logger.log "Generating file hashes..." pathList = [ - ["#{jsPath}libs/require.js"] - ["#{jsPath}ide.js"] - ["#{jsPath}main.js"] - ["#{jsPath}libraries.js"] - ["/stylesheets/style.css"] - ["/stylesheets/ol-style.css"] + "#{jsPath}libs/require.js" + "#{jsPath}ide.js" + "#{jsPath}main.js" + "#{jsPath}libraries.js" + "/stylesheets/style.css" + "/stylesheets/ol-style.css" ] -for paths in pathList - contentList = _.map(paths, getFileContent) - content = contentList.join("") +for path in pathList + content = getFileContent(path) hash = crypto.createHash("md5").update(content).digest("hex") - _.each paths, (filePath)-> - logger.log "#{filePath}: #{hash}" - fingerprints[filePath] = hash + + splitPath = path.split("/") + filenameSplit = splitPath.pop().split(".") + filenameSplit.splice(filenameSplit.length-1, 0, hash) + splitPath.push(filenameSplit.join(".")) - for path in paths - splitPath = path.split("/") - filenameSplit = splitPath.pop().split(".") - filenameSplit.splice(filenameSplit.length-1, 0, hash) - splitPath.push(filenameSplit.join(".")) - hashPath = splitPath.join("/") - fsHashPath = Path.join __dirname, "../../../", "public#{hashPath}" - fs.writeFileSync(fsHashPath, content) - hashedFiles[paths] = hashPath + hashPath = splitPath.join("/") + hashedFiles[path] = hashPath + + fsHashPath = Path.join __dirname, "../../../", "public#{hashPath}" + fs.writeFileSync(fsHashPath, content) -getFingerprint = (path) -> - if fingerprints[path]? - return fingerprints[path] - else - logger.err "No fingerprint for file: #{path}" - return "" - -logger.log "Finished generating file fingerprints" +logger.log "Finished hashing static content" cdnAvailable = Settings.cdn?.web?.host? darkCdnAvailable = Settings.cdn?.web?.darkHost? @@ -135,14 +124,9 @@ module.exports = (app, webRouter, privateApiRouter, publicApiRouter)-> if opts.hashedPath path = hashedFiles[path] - doFingerPrint = opts.fingerprint != false - if !opts.qs? opts.qs = {} - if !opts.qs?.fingerprint? and doFingerPrint - opts.qs.fingerprint = getFingerprint(path) - if opts.cdn != false path = Url.resolve(staticFilesBase, path) @@ -160,7 +144,7 @@ module.exports = (app, webRouter, privateApiRouter, publicApiRouter)-> if opts?.hashedPath hashedPath = hashedFiles[path] return Url.resolve(staticFilesBase, hashedPath) - return Url.resolve(staticFilesBase, path) + "?fingerprint=" + getFingerprint(path) + return Url.resolve(staticFilesBase, path) res.locals.buildImgPath = (imgFile)-> path = Path.join("/img/", imgFile) @@ -245,10 +229,6 @@ module.exports = (app, webRouter, privateApiRouter, publicApiRouter)-> return req.query?[field] next() - webRouter.use (req, res, next)-> - res.locals.fingerprint = getFingerprint - next() - webRouter.use (req, res, next)-> res.locals.formatPrice = SubscriptionFormatters.formatPrice next() diff --git a/services/web/app/views/layout.pug b/services/web/app/views/layout.pug index 5aa41c56c7..f203d29357 100644 --- a/services/web/app/views/layout.pug +++ b/services/web/app/views/layout.pug @@ -57,7 +57,7 @@ html(itemscope, itemtype='http://schema.org/Product') script(type="text/javascript"). window.csrfToken = "#{csrfToken}"; - script(src=buildJsPath("libs/jquery-1.11.1.min.js", {fingerprint:false})) + script(src=buildJsPath("libs/jquery-1.11.1.min.js")) script(type="text/javascript"). var noCdnKey = "nocdn=true" var cdnBlocked = typeof jQuery === 'undefined' @@ -68,7 +68,7 @@ html(itemscope, itemtype='http://schema.org/Product') block scripts - script(src=buildJsPath("libs/angular-1.6.4.min.js", {fingerprint:false})) + script(src=buildJsPath("libs/angular-1.6.4.min.js")) script. window.sharelatex = { @@ -143,8 +143,8 @@ html(itemscope, itemtype='http://schema.org/Product') "paths" : { "moment": "libs/#{lib('moment')}", "fineuploader": "libs/#{lib('fineuploader')}", - "main": "#{buildJsPath('main.js', {hashedPath:settings.useMinifiedJs, fingerprint:false, removeExtension:true})}", - "libraries": "#{buildJsPath('libraries.js', {hashedPath:settings.useMinifiedJs, fingerprint:false, removeExtension:true})}", + "main": "#{buildJsPath('main.js', {hashedPath:settings.useMinifiedJs, removeExtension:true})}", + "libraries": "#{buildJsPath('libraries.js', {hashedPath:settings.useMinifiedJs, removeExtension:true})}", }, "config":{ "moment":{ @@ -153,9 +153,9 @@ html(itemscope, itemtype='http://schema.org/Product') } }; script( - data-main=buildJsPath('main.js', {hashedPath:false, fingerprint:false}), + data-main=buildJsPath('main.js', {hashedPath:false}), baseurl=fullJsPath, - src=buildJsPath('libs/require.js') + src=buildJsPath('libs/require.js', {hashedPath:true}) ) include contact-us-modal diff --git a/services/web/app/views/project/editor.pug b/services/web/app/views/project/editor.pug index 38ade1a6b2..d1d0d94cf3 100644 --- a/services/web/app/views/project/editor.pug +++ b/services/web/app/views/project/editor.pug @@ -98,9 +98,9 @@ block requirejs script(type="text/javascript" src='/socket.io/socket.io.js') //- don't use cdn for workers - - var aceWorkerPath = buildJsPath(lib('ace'), {cdn:false,fingerprint:false}) - - var pdfWorkerPath = buildJsPath('/libs/' + lib('pdfjs') + '/pdf.worker', {cdn:false,fingerprint:false}) - - var pdfCMapsPath = buildJsPath('/libs/' + lib('pdfjs') + '/bcmaps/', {cdn:false,fingerprint:false}) + - var aceWorkerPath = buildJsPath(lib('ace'), {cdn:false}) + - var pdfWorkerPath = buildJsPath('/libs/' + lib('pdfjs') + '/pdf.worker', {cdn:false}) + - var pdfCMapsPath = buildJsPath('/libs/' + lib('pdfjs') + '/bcmaps/', {cdn:false}) //- We need to do .replace(/\//g, '\\/') do that '' -> '<\/script>' //- and doesn't prematurely end the script tag. @@ -126,14 +126,14 @@ block requirejs window.wikiEnabled = #{!!(settings.apis.wiki && settings.apis.wiki.url)}; window.requirejs = { "paths" : { - "mathjax": "#{buildJsPath('/libs/mathjax/MathJax.js', {cdn:false, fingerprint:false, qs:{config:'TeX-AMS_HTML'}})}", + "mathjax": "#{buildJsPath('/libs/mathjax/MathJax.js', {cdn:false, qs:{config:'TeX-AMS_HTML'}})}", "moment": "libs/#{lib('moment')}", "pdfjs-dist/build/pdf": "libs/#{lib('pdfjs')}/pdf", "pdfjs-dist/build/pdf.worker": "#{pdfWorkerPath}", "ace": "#{lib('ace')}", "fineuploader": "libs/#{lib('fineuploader')}", - "ide": "#{buildJsPath('ide.js', {hashedPath:settings.useMinifiedJs, fingerprint:false, removeExtension:true})}", - "libraries": "#{buildJsPath('libraries.js', {hashedPath:settings.useMinifiedJs, fingerprint:false, removeExtension:true})}", + "ide": "#{buildJsPath('ide.js', {hashedPath:settings.useMinifiedJs, removeExtension:true})}", + "libraries": "#{buildJsPath('libraries.js', {hashedPath:settings.useMinifiedJs, removeExtension:true})}", }, "waitSeconds": 0, "shim": { @@ -161,8 +161,8 @@ block requirejs window.uiConfig = JSON.parse('!{JSON.stringify(uiConfig).replace(/\//g, "\\/")}'); script( - data-main=buildJsPath("ide.js", {hashedPath:false, fingerprint:false}), + data-main=buildJsPath("ide.js", {hashedPath:false}), baseurl=fullJsPath, - data-ace-base=buildJsPath(lib('ace'), {fingerprint:false}), - src=buildJsPath('libs/require.js', {hashedPath:false}) + data-ace-base=buildJsPath(lib('ace')), + src=buildJsPath('libs/require.js', {hashedPath:true}) ) From b6d6ac8304cb1de36503342be95e09cd0b6f2afe Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Thu, 14 Dec 2017 16:18:01 +0000 Subject: [PATCH 21/56] Ignore select-all events for disabled check-boxes --- services/web/public/coffee/directives/selectAll.coffee | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/services/web/public/coffee/directives/selectAll.coffee b/services/web/public/coffee/directives/selectAll.coffee index 4194d050e8..eb0bd05e86 100644 --- a/services/web/public/coffee/directives/selectAll.coffee +++ b/services/web/public/coffee/directives/selectAll.coffee @@ -49,18 +49,21 @@ define [ selectAllListController.clearSelectAllState() scope.$on "select-all:select", () -> + return if element.prop('disabled') ignoreChanges = true scope.$apply () -> scope.ngModel = true ignoreChanges = false scope.$on "select-all:deselect", () -> + return if element.prop('disabled') ignoreChanges = true scope.$apply () -> scope.ngModel = false ignoreChanges = false scope.$on "select-all:row-clicked", () -> + return if element.prop('disabled') ignoreChanges = true scope.$apply () -> scope.ngModel = !scope.ngModel @@ -75,4 +78,4 @@ define [ link: (scope, element, attrs) -> element.on "click", (e) -> scope.$broadcast "select-all:row-clicked" - } \ No newline at end of file + } From 8d91cd274870829f8ed167afdcd5847552f548e0 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Thu, 14 Dec 2017 16:19:43 +0000 Subject: [PATCH 22/56] allow autocompile without code check --- .../public/coffee/ide/pdf/controllers/PdfController.coffee | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee b/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee index 17afd8bbcb..4ac48c20d0 100644 --- a/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee +++ b/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee @@ -90,7 +90,9 @@ define [ # to block auto compiles. It also causes problems where server-provided # linting errors aren't cleared after typing if (ide.$scope.settings.syntaxValidation and !ide.$scope.hasLintingError) - $scope.recompile(isAutoCompileOnChange: true) + $scope.recompile(isAutoCompileOnChange: true) # compile if no linting errors + else if !ide.$scope.settings.syntaxValidation + $scope.recompile(isAutoCompileOnChange: true) # always recompile else # Extend remainder of timeout autoCompileTimeout = setTimeout () -> From 73225223236b25567bc574875786b09ffc3e3cc2 Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Thu, 14 Dec 2017 17:05:12 +0000 Subject: [PATCH 23/56] Use freeTrialBaseUrl instead of potentially fragile v1BaseUrl --- services/web/app/views/layout.pug | 5 ++++- services/web/public/coffee/main/account-upgrade.coffee | 9 +++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/services/web/app/views/layout.pug b/services/web/app/views/layout.pug index fc14c2030b..dd93b0b0fa 100644 --- a/services/web/app/views/layout.pug +++ b/services/web/app/views/layout.pug @@ -98,7 +98,10 @@ html(itemscope, itemtype='http://schema.org/Product') - if (settings.overleaf && settings.overleaf.host) script. - window.v1BaseUrl = '!{settings.overleaf.host}' + window.freeTrialBaseUrl = '!{settings.overleaf.host}/users/trial' + - else + script. + window.freeTrialBaseUrl = '/user/subscription/new' body if(settings.recaptcha) diff --git a/services/web/public/coffee/main/account-upgrade.coffee b/services/web/public/coffee/main/account-upgrade.coffee index 29bc472505..8356458ef9 100644 --- a/services/web/public/coffee/main/account-upgrade.coffee +++ b/services/web/public/coffee/main/account-upgrade.coffee @@ -11,12 +11,9 @@ define [ w = window.open() go = () -> ga?('send', 'event', 'subscription-funnel', 'upgraded-free-trial', source) - if window.v1BaseUrl? - url = "#{window.v1BaseUrl}/users/trial" - else - url = "/user/subscription/new?planCode=#{plan}&ssp=true" - if couponCode? - url = "#{url}&cc=#{couponCode}" + url = "#{window.freeTrialBaseUrl}?planCode=#{plan}&ssp=true" + if couponCode? + url = "#{url}&cc=#{couponCode}" $scope.startedFreeTrial = true switch source From da2c0b3fead5de68e96b5e2cbf39ec577ecff004 Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Thu, 14 Dec 2017 17:09:11 +0000 Subject: [PATCH 24/56] Switch redirectToOLFreeTrialUrl to calculate trial url Sending unnecessary query params to OL seems like a footgun, so switch to compromise that is more explicit but doesn't send unnecessary query params --- services/web/app/views/layout.pug | 7 ++----- services/web/public/coffee/main/account-upgrade.coffee | 9 ++++++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/services/web/app/views/layout.pug b/services/web/app/views/layout.pug index dd93b0b0fa..14ebc3c576 100644 --- a/services/web/app/views/layout.pug +++ b/services/web/app/views/layout.pug @@ -98,11 +98,8 @@ html(itemscope, itemtype='http://schema.org/Product') - if (settings.overleaf && settings.overleaf.host) script. - window.freeTrialBaseUrl = '!{settings.overleaf.host}/users/trial' - - else - script. - window.freeTrialBaseUrl = '/user/subscription/new' - + window.redirectToOLFreeTrialUrl = '!{settings.overleaf.host}/users/trial' + body if(settings.recaptcha) script(src="https://www.google.com/recaptcha/api.js?render=explicit") diff --git a/services/web/public/coffee/main/account-upgrade.coffee b/services/web/public/coffee/main/account-upgrade.coffee index 8356458ef9..0cabc224a0 100644 --- a/services/web/public/coffee/main/account-upgrade.coffee +++ b/services/web/public/coffee/main/account-upgrade.coffee @@ -11,9 +11,12 @@ define [ w = window.open() go = () -> ga?('send', 'event', 'subscription-funnel', 'upgraded-free-trial', source) - url = "#{window.freeTrialBaseUrl}?planCode=#{plan}&ssp=true" - if couponCode? - url = "#{url}&cc=#{couponCode}" + if window.redirectToOLFreeTrialUrl? + url = window.redirectToOLFreeTrialUrl + else + url = "/user/subscription/new?planCode=#{plan}&ssp=true" + if couponCode? + url = "#{url}&cc=#{couponCode}" $scope.startedFreeTrial = true switch source From 4955fb373bd342792f86b63d61f37b65c11d9083 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Fri, 15 Dec 2017 10:10:13 +0000 Subject: [PATCH 25/56] ignore hashed requirejs path --- services/web/.gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/services/web/.gitignore b/services/web/.gitignore index 3830036f24..83d4d28b65 100644 --- a/services/web/.gitignore +++ b/services/web/.gitignore @@ -69,6 +69,7 @@ Gemfile.lock public/stylesheets/ol-style.*.css public/stylesheets/style.*.css +public/js/libs/require.js *.swp From 7aaf08da48f986dd75f6ecd807ed3692df4b62b6 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Fri, 15 Dec 2017 13:38:34 +0000 Subject: [PATCH 26/56] added no-cache endpoint --- services/web/app/coffee/router.coffee | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/services/web/app/coffee/router.coffee b/services/web/app/coffee/router.coffee index f8ec65ac53..c89d9cdf5e 100644 --- a/services/web/app/coffee/router.coffee +++ b/services/web/app/coffee/router.coffee @@ -324,6 +324,10 @@ module.exports = class Router headers: req.headers }) + webRouter.get "/no-cache", (req, res, next)-> + res.header("Cache-Control", "max-age=0") + res.sendStatus(404) + webRouter.get '/oops-express', (req, res, next) -> next(new Error("Test error")) webRouter.get '/oops-internal', (req, res, next) -> throw new Error("Test error") webRouter.get '/oops-mongo', (req, res, next) -> From e5e75a8ccb0685fa7a36b3c43cd9d36823c15705 Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Thu, 14 Dec 2017 13:06:27 +0000 Subject: [PATCH 27/56] update DocumentUpdaterHandler.updateProjectStructure to support entity deletions --- .../DocumentUpdater/DocumentUpdaterHandler.coffee | 15 ++++++++++++--- .../DocumentUpdaterHandlerTests.coffee | 13 ++++++++++--- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee index f7dd9f3e17..f1336a7f77 100644 --- a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee +++ b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee @@ -207,8 +207,8 @@ module.exports = DocumentUpdaterHandler = updateProjectStructure : (project_id, userId, changes, callback = (error) ->)-> return callback() if !settings.apis.project_history?.enabled - docUpdates = DocumentUpdaterHandler._getRenameUpdates('doc', changes.oldDocs, changes.newDocs) - fileUpdates = DocumentUpdaterHandler._getRenameUpdates('file', changes.oldFiles, changes.newFiles) + docUpdates = DocumentUpdaterHandler._getUpdates('doc', changes.oldDocs, changes.newDocs) + fileUpdates = DocumentUpdaterHandler._getUpdates('file', changes.oldFiles, changes.newFiles) timer = new metrics.Timer("set-document") url = "#{settings.apis.documentupdater.url}/project/#{project_id}" @@ -230,7 +230,7 @@ module.exports = DocumentUpdaterHandler = logger.error {project_id, url}, "doc updater returned a non-success status code: #{res.statusCode}" callback new Error("doc updater returned a non-success status code: #{res.statusCode}") - _getRenameUpdates: (entityType, oldEntities, newEntities) -> + _getUpdates: (entityType, oldEntities, newEntities) -> oldEntities ||= [] newEntities ||= [] updates = [] @@ -255,6 +255,15 @@ module.exports = DocumentUpdaterHandler = pathname: oldEntity.path newPathname: newEntity.path + for id, oldEntity of oldEntitiesHash + newEntity = newEntitiesHash[id] + + if !newEntity? + # entity deleted + updates.push + id: id + pathname: oldEntity.path + updates PENDINGUPDATESKEY = "PendingUpdates" diff --git a/services/web/test/unit/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee b/services/web/test/unit/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee index 14ccaa3a33..266de9bfb7 100644 --- a/services/web/test/unit/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee +++ b/services/web/test/unit/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee @@ -478,14 +478,21 @@ describe 'DocumentUpdaterHandler', -> .should.equal true done() - describe "when a doc has been deleted", -> - it 'should do nothing', (done) -> + describe "when an entity has been deleted", -> + it 'should end the structure update to the document updater', (done) -> @docId = new ObjectId() @changes = oldDocs: [ { path: '/foo', docLines: 'a\nb', doc: _id: @docId } ] + docUpdates = [ + id: @docId.toString(), + pathname: "/foo", + ] + @handler.updateProjectStructure @project_id, @user_id, @changes, () => - @request.post.called.should.equal false + @request.post + .calledWith(url: @url, json: {docUpdates, fileUpdates: [], userId: @user_id}) + .should.equal true done() From 81c061c6a7ce3236213d67ca491f5b925742602d Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Thu, 14 Dec 2017 14:39:27 +0000 Subject: [PATCH 28/56] acceptance test moving entities --- .../DocumentUpdaterHandler.coffee | 1 + .../coffee/ProjectStructureTests.coffee | 89 +++++++++++++++++++ .../DocumentUpdaterHandlerTests.coffee | 3 +- 3 files changed, 92 insertions(+), 1 deletion(-) diff --git a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee index f1336a7f77..bbd7ca4919 100644 --- a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee +++ b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee @@ -263,6 +263,7 @@ module.exports = DocumentUpdaterHandler = updates.push id: id pathname: oldEntity.path + newPathname: '' updates diff --git a/services/web/test/acceptance/coffee/ProjectStructureTests.coffee b/services/web/test/acceptance/coffee/ProjectStructureTests.coffee index e54d4fac9d..92fcd2e34e 100644 --- a/services/web/test/acceptance/coffee/ProjectStructureTests.coffee +++ b/services/web/test/acceptance/coffee/ProjectStructureTests.coffee @@ -91,6 +91,7 @@ describe "ProjectStructureChanges", -> throw error if error? if res.statusCode < 200 || res.statusCode >= 300 throw new Error("failed to add doc #{res.statusCode}") + @example_doc_id = body._id done() it "should version the doc added", -> @@ -162,6 +163,8 @@ describe "ProjectStructureChanges", -> if res.statusCode < 200 || res.statusCode >= 300 throw new Error("failed to upload file #{res.statusCode}") + @example_file_id = JSON.parse(body).entity_id + updates = MockDocUpdaterApi.getProjectStructureUpdates(@example_project_id).fileUpdates expect(updates.length).to.equal(1) update = updates[0] @@ -199,6 +202,92 @@ describe "ProjectStructureChanges", -> done() + describe "moving entities", -> + before (done) -> + @owner.request.post { + uri: "project/#{@example_project_id}/folder", + formData: + name: 'foo' + }, (error, res, body) => + throw error if error? + @example_folder_id_1 = JSON.parse(body)._id + done() + + beforeEach () -> + MockDocUpdaterApi.clearProjectStructureUpdates() + + it "should version moving a doc", (done) -> + @owner.request.post { + uri: "project/#{@example_project_id}/Doc/#{@example_doc_id}/move", + json: + folder_id: @example_folder_id_1 + }, (error, res, body) => + throw error if error? + if res.statusCode < 200 || res.statusCode >= 300 + throw new Error("failed to move doc #{res.statusCode}") + + updates = MockDocUpdaterApi.getProjectStructureUpdates(@example_project_id).docUpdates + expect(updates.length).to.equal(1) + update = updates[0] + expect(update.userId).to.equal(@owner._id) + expect(update.pathname).to.equal("/new.tex") + expect(update.newPathname).to.equal("/foo/new.tex") + + done() + + it "should version moving a file", (done) -> + @owner.request.post { + uri: "project/#{@example_project_id}/File/#{@example_file_id}/move", + json: + folder_id: @example_folder_id_1 + }, (error, res, body) => + throw error if error? + if res.statusCode < 200 || res.statusCode >= 300 + throw new Error("failed to move file #{res.statusCode}") + + updates = MockDocUpdaterApi.getProjectStructureUpdates(@example_project_id).fileUpdates + expect(updates.length).to.equal(1) + update = updates[0] + expect(update.userId).to.equal(@owner._id) + expect(update.pathname).to.equal("/1pixel.png") + expect(update.newPathname).to.equal("/foo/1pixel.png") + + done() + + it "should version moving a folder", (done) -> + @owner.request.post { + uri: "project/#{@example_project_id}/folder", + formData: + name: 'bar' + }, (error, res, body) => + throw error if error? + example_folder_id_2 = JSON.parse(body)._id + + @owner.request.post { + uri: "project/#{@example_project_id}/Folder/#{@example_folder_id_1}/move", + json: + folder_id: example_folder_id_2 + }, (error, res, body) => + throw error if error? + if res.statusCode < 200 || res.statusCode >= 300 + throw new Error("failed to move folder #{res.statusCode}") + + updates = MockDocUpdaterApi.getProjectStructureUpdates(@example_project_id).docUpdates + expect(updates.length).to.equal(1) + update = updates[0] + expect(update.userId).to.equal(@owner._id) + expect(update.pathname).to.equal("/foo/new.tex") + expect(update.newPathname).to.equal("/bar/foo/new.tex") + + updates = MockDocUpdaterApi.getProjectStructureUpdates(@example_project_id).fileUpdates + expect(updates.length).to.equal(1) + update = updates[0] + expect(update.userId).to.equal(@owner._id) + expect(update.pathname).to.equal("/foo/1pixel.png") + expect(update.newPathname).to.equal("/bar/foo/1pixel.png") + + done() + describe "tpds", -> before (done) -> @tpds_project_name = "tpds-project-#{new ObjectId().toString()}" diff --git a/services/web/test/unit/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee b/services/web/test/unit/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee index 266de9bfb7..111d2ace68 100644 --- a/services/web/test/unit/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee +++ b/services/web/test/unit/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee @@ -487,7 +487,8 @@ describe 'DocumentUpdaterHandler', -> docUpdates = [ id: @docId.toString(), - pathname: "/foo", + pathname: '/foo', + newPathname: '' ] @handler.updateProjectStructure @project_id, @user_id, @changes, () => From 475e84b0390cc8c1b8872abf6c9fef9443aef6de Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Thu, 14 Dec 2017 16:03:43 +0000 Subject: [PATCH 29/56] version entity deletions in ProjectEntityHandler --- .../Project/ProjectEntityHandler.coffee | 33 ++++++----- .../Project/ProjectEntityHandlerTests.coffee | 57 ++++++++++++------- 2 files changed, 58 insertions(+), 32 deletions(-) diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index 446786fa57..4ff3818516 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -423,7 +423,7 @@ module.exports = ProjectEntityHandler = return callback(error) if error? projectLocator.findElement {project: project, element_id: entity_id, type: entityType}, (error, entity, path)=> return callback(error) if error? - ProjectEntityHandler._cleanUpEntity project, entity, entityType, (error) -> + ProjectEntityHandler._cleanUpEntity project, entity, entityType, path.fileSystem, (error) -> return callback(error) if error? tpdsUpdateSender.deleteEntity project_id:project_id, path:path.fileSystem, project_name:project.name, (error) -> return callback(error) if error? @@ -456,17 +456,17 @@ module.exports = ProjectEntityHandler = return callback(error) if error? DocumentUpdaterHandler.updateProjectStructure project_id, userId, {oldDocs, newDocs, oldFiles, newFiles}, callback - _cleanUpEntity: (project, entity, entityType, callback = (error) ->) -> + _cleanUpEntity: (project, entity, entityType, path, callback = (error) ->) -> if(entityType.indexOf("file") != -1) - ProjectEntityHandler._cleanUpFile project, entity, callback + ProjectEntityHandler._cleanUpFile project, entity, path, callback else if (entityType.indexOf("doc") != -1) - ProjectEntityHandler._cleanUpDoc project, entity, callback + ProjectEntityHandler._cleanUpDoc project, entity, path, callback else if (entityType.indexOf("folder") != -1) - ProjectEntityHandler._cleanUpFolder project, entity, callback + ProjectEntityHandler._cleanUpFolder project, entity, path, callback else callback() - _cleanUpDoc: (project, doc, callback = (error) ->) -> + _cleanUpDoc: (project, doc, path, callback = (error) ->) -> project_id = project._id.toString() doc_id = doc._id.toString() unsetRootDocIfRequired = (callback) => @@ -483,26 +483,33 @@ module.exports = ProjectEntityHandler = return callback(error) if error? DocstoreManager.deleteDoc project_id, doc_id, (error) -> return callback(error) if error? - callback() + changes = oldDocs: [ {doc, path} ] + DocumentUpdaterHandler.updateProjectStructure project_id, null, changes, callback - _cleanUpFile: (project, file, callback = (error) ->) -> + _cleanUpFile: (project, file, path, callback = (error) ->) -> project_id = project._id.toString() file_id = file._id.toString() - FileStoreHandler.deleteFile project_id, file_id, callback + FileStoreHandler.deleteFile project_id, file_id, (error) -> + return callback(error) if error? + changes = oldFiles: [ {file, path} ] + DocumentUpdaterHandler.updateProjectStructure project_id, null, changes, callback - _cleanUpFolder: (project, folder, callback = (error) ->) -> + _cleanUpFolder: (project, folder, folderPath, callback = (error) ->) -> jobs = [] for doc in folder.docs do (doc) -> - jobs.push (callback) -> ProjectEntityHandler._cleanUpDoc project, doc, callback + docPath = path.join(folderPath, doc.name) + jobs.push (callback) -> ProjectEntityHandler._cleanUpDoc project, doc, docPath, callback for file in folder.fileRefs do (file) -> - jobs.push (callback) -> ProjectEntityHandler._cleanUpFile project, file, callback + filePath = path.join(folderPath, file.name) + jobs.push (callback) -> ProjectEntityHandler._cleanUpFile project, file, filePath, callback for childFolder in folder.folders do (childFolder) -> - jobs.push (callback) -> ProjectEntityHandler._cleanUpFolder project, childFolder, callback + folderPath = path.join(folderPath, childFolder.name) + jobs.push (callback) -> ProjectEntityHandler._cleanUpFolder project, childFolder, folderPath, callback async.series jobs, callback diff --git a/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee b/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee index f62690e226..faaaa146da 100644 --- a/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee +++ b/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee @@ -157,7 +157,7 @@ describe 'ProjectEntityHandler', -> @ProjectGetter.getProject.callsArgWith(2, null, @project) @tpdsUpdateSender.deleteEntity = sinon.stub().callsArg(1) @ProjectEntityHandler._removeElementFromMongoArray = sinon.stub().callsArg(3) - @ProjectEntityHandler._cleanUpEntity = sinon.stub().callsArg(3) + @ProjectEntityHandler._cleanUpEntity = sinon.stub().callsArg(4) @path = mongo: "mongo.path", fileSystem: "/file/system/path" @projectLocator.findElement = sinon.stub().callsArgWith(1, null, @entity = { _id: entity_id }, @path) @@ -182,7 +182,7 @@ describe 'ProjectEntityHandler', -> it "should clean up the entity from the rest of the system", -> @ProjectEntityHandler._cleanUpEntity - .calledWith(@project, @entity, @type) + .calledWith(@project, @entity, @type, @path.fileSystem) .should.equal true describe "_cleanUpEntity", -> @@ -193,7 +193,9 @@ describe 'ProjectEntityHandler', -> describe "a file", -> beforeEach (done) -> - @ProjectEntityHandler._cleanUpEntity @project, _id: @entity_id, 'file', done + @path = "/file/system/path.png" + @entity = _id: @entity_id + @ProjectEntityHandler._cleanUpEntity @project, @entity, 'file', @path, done it "should delete the file from FileStoreHandler", -> @FileStoreHandler.deleteFile.calledWith(project_id, @entity_id).should.equal true @@ -201,38 +203,48 @@ describe 'ProjectEntityHandler', -> it "should not attempt to delete from the document updater", -> @documentUpdaterHandler.deleteDoc.called.should.equal false + it "should should send the update to the doc updater", -> + oldFiles = [ file: @entity, path: @path ] + @documentUpdaterHandler.updateProjectStructure + .calledWith(project_id, null, {oldFiles}) + .should.equal true + describe "a doc", -> beforeEach (done) -> - @ProjectEntityHandler._cleanUpDoc = sinon.stub().callsArg(2) - @ProjectEntityHandler._cleanUpEntity @project, @entity = {_id: @entity_id}, 'doc', done + @path = "/file/system/path.tex" + @ProjectEntityHandler._cleanUpDoc = sinon.stub().callsArg(3) + @entity = {_id: @entity_id} + @ProjectEntityHandler._cleanUpEntity @project, @entity, 'doc', @path, done it "should clean up the doc", -> @ProjectEntityHandler._cleanUpDoc - .calledWith(@project, @entity) + .calledWith(@project, @entity, @path) .should.equal true describe "a folder", -> beforeEach (done) -> @folder = folders: [ - fileRefs: [ @file1 = {_id: "file-id-1" } ] - docs: [ @doc1 = { _id: "doc-id-1" } ] + name: "subfolder" + fileRefs: [ @file1 = { _id: "file-id-1", name: "file-name-1"} ] + docs: [ @doc1 = { _id: "doc-id-1", name: "doc-name-1" } ] folders: [] ] - fileRefs: [ @file2 = { _id: "file-id-2" } ] - docs: [ @doc2 = { _id: "doc-id-2" } ] + fileRefs: [ @file2 = { _id: "file-id-2", name: "file-name-2" } ] + docs: [ @doc2 = { _id: "doc-id-2", name: "doc-name-2" } ] - @ProjectEntityHandler._cleanUpDoc = sinon.stub().callsArg(2) - @ProjectEntityHandler._cleanUpFile = sinon.stub().callsArg(2) - @ProjectEntityHandler._cleanUpEntity @project, @folder, "folder", done + @ProjectEntityHandler._cleanUpDoc = sinon.stub().callsArg(3) + @ProjectEntityHandler._cleanUpFile = sinon.stub().callsArg(3) + path = "/folder" + @ProjectEntityHandler._cleanUpEntity @project, @folder, "folder", path, done it "should clean up all sub files", -> - @ProjectEntityHandler._cleanUpFile.calledWith(@project, @file1).should.equal true - @ProjectEntityHandler._cleanUpFile.calledWith(@project, @file2).should.equal true + @ProjectEntityHandler._cleanUpFile.calledWith(@project, @file1, "/folder/subfolder/file-name-1").should.equal true + @ProjectEntityHandler._cleanUpFile.calledWith(@project, @file2, "/folder/file-name-2").should.equal true it "should clean up all sub docs", -> - @ProjectEntityHandler._cleanUpDoc.calledWith(@project, @doc1).should.equal true - @ProjectEntityHandler._cleanUpDoc.calledWith(@project, @doc2).should.equal true + @ProjectEntityHandler._cleanUpDoc.calledWith(@project, @doc1, "/folder/subfolder/doc-name-1").should.equal true + @ProjectEntityHandler._cleanUpDoc.calledWith(@project, @doc2, "/folder/doc-name-2").should.equal true describe 'moveEntity', -> beforeEach -> @@ -1116,6 +1128,7 @@ describe 'ProjectEntityHandler', -> @doc = _id: ObjectId() name: "test.tex" + @path = "/path/to/doc" @ProjectEntityHandler.unsetRootDoc = sinon.stub().callsArg(1) @ProjectEntityHandler._insertDeletedDocReference = sinon.stub().callsArg(2) @documentUpdaterHandler.deleteDoc = sinon.stub().callsArg(2) @@ -1125,7 +1138,7 @@ describe 'ProjectEntityHandler', -> describe "when the doc is the root doc", -> beforeEach -> @project.rootDoc_id = @doc._id - @ProjectEntityHandler._cleanUpDoc @project, @doc, @callback + @ProjectEntityHandler._cleanUpDoc @project, @doc, @path, @callback it "should unset the root doc", -> @ProjectEntityHandler.unsetRootDoc @@ -1146,13 +1159,19 @@ describe 'ProjectEntityHandler', -> .calledWith(project_id, @doc._id.toString()) .should.equal true + it "should should send the update to the doc updater", -> + oldDocs = [ doc: @doc, path: @path ] + @documentUpdaterHandler.updateProjectStructure + .calledWith(project_id, null, {oldDocs}) + .should.equal true + it "should call the callback", -> @callback.called.should.equal true describe "when the doc is not the root doc", -> beforeEach -> @project.rootDoc_id = ObjectId() - @ProjectEntityHandler._cleanUpDoc @project, @doc, @callback + @ProjectEntityHandler._cleanUpDoc @project, @doc, @path, @callback it "should not unset the root doc", -> @ProjectEntityHandler.unsetRootDoc.called.should.equal false From 99a52d48c8bc32c1685b2a411e98c5c929493f28 Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Thu, 14 Dec 2017 16:03:57 +0000 Subject: [PATCH 30/56] acceptance test versioning entity deletions --- .../coffee/ProjectStructureTests.coffee | 32 +++++++++++++++++-- .../coffee/helpers/MockDocUpdaterApi.coffee | 3 ++ .../coffee/helpers/MockDocstoreApi.coffee | 10 ++++++ 3 files changed, 43 insertions(+), 2 deletions(-) diff --git a/services/web/test/acceptance/coffee/ProjectStructureTests.coffee b/services/web/test/acceptance/coffee/ProjectStructureTests.coffee index 92fcd2e34e..0f8086647f 100644 --- a/services/web/test/acceptance/coffee/ProjectStructureTests.coffee +++ b/services/web/test/acceptance/coffee/ProjectStructureTests.coffee @@ -261,12 +261,12 @@ describe "ProjectStructureChanges", -> name: 'bar' }, (error, res, body) => throw error if error? - example_folder_id_2 = JSON.parse(body)._id + @example_folder_id_2 = JSON.parse(body)._id @owner.request.post { uri: "project/#{@example_project_id}/Folder/#{@example_folder_id_1}/move", json: - folder_id: example_folder_id_2 + folder_id: @example_folder_id_2 }, (error, res, body) => throw error if error? if res.statusCode < 200 || res.statusCode >= 300 @@ -288,6 +288,34 @@ describe "ProjectStructureChanges", -> done() + describe "deleting entities", -> + beforeEach () -> + MockDocUpdaterApi.clearProjectStructureUpdates() + + it "should version deleting a folder", (done) -> + @owner.request.delete { + uri: "project/#{@example_project_id}/Folder/#{@example_folder_id_2}", + }, (error, res, body) => + throw error if error? + if res.statusCode < 200 || res.statusCode >= 300 + throw new Error("failed to delete folder #{res.statusCode}") + + updates = MockDocUpdaterApi.getProjectStructureUpdates(@example_project_id).docUpdates + expect(updates.length).to.equal(1) + update = updates[0] + #expect(update.userId).to.equal(@owner._id) + expect(update.pathname).to.equal("/bar/foo/new.tex") + expect(update.newPathname).to.equal("") + + updates = MockDocUpdaterApi.getProjectStructureUpdates(@example_project_id).fileUpdates + expect(updates.length).to.equal(1) + update = updates[0] + #expect(update.userId).to.equal(@owner._id) + expect(update.pathname).to.equal("/bar/foo/1pixel.png") + expect(update.newPathname).to.equal("") + + done() + describe "tpds", -> before (done) -> @tpds_project_name = "tpds-project-#{new ObjectId().toString()}" diff --git a/services/web/test/acceptance/coffee/helpers/MockDocUpdaterApi.coffee b/services/web/test/acceptance/coffee/helpers/MockDocUpdaterApi.coffee index b00cd6b173..b21fb1adab 100644 --- a/services/web/test/acceptance/coffee/helpers/MockDocUpdaterApi.coffee +++ b/services/web/test/acceptance/coffee/helpers/MockDocUpdaterApi.coffee @@ -33,6 +33,9 @@ module.exports = MockDocUpdaterApi = @addProjectStructureUpdates(project_id, userId, docUpdates, fileUpdates) res.sendStatus 200 + app.delete "/project/:project_id/doc/:doc_id", (req, res, next) => + res.send 204 + app.listen 3003, (error) -> throw error if error? .on "error", (error) -> diff --git a/services/web/test/acceptance/coffee/helpers/MockDocstoreApi.coffee b/services/web/test/acceptance/coffee/helpers/MockDocstoreApi.coffee index c5b003ac75..631538fd89 100644 --- a/services/web/test/acceptance/coffee/helpers/MockDocstoreApi.coffee +++ b/services/web/test/acceptance/coffee/helpers/MockDocstoreApi.coffee @@ -23,6 +23,16 @@ module.exports = MockDocStoreApi = docs = (doc for doc_id, doc of @docs[req.params.project_id]) res.send JSON.stringify docs + app.delete "/project/:project_id/doc/:doc_id", (req, res, next) => + {project_id, doc_id} = req.params + if !@docs[project_id]? + res.send 404 + else if !@docs[project_id][doc_id]? + res.send 404 + else + @docs[project_id][doc_id] = undefined + res.send 204 + app.listen 3016, (error) -> throw error if error? .on "error", (error) -> From 2ac74b9adcadb3bd9fb166346b6f43c376cc60e8 Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Fri, 15 Dec 2017 10:14:01 +0000 Subject: [PATCH 31/56] pass userId into _clean methods in ProjectEntityHandler --- .../Project/ProjectEntityHandler.coffee | 26 ++++++------ .../Project/ProjectEntityHandlerTests.coffee | 42 +++++++++++-------- 2 files changed, 38 insertions(+), 30 deletions(-) diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index 4ff3818516..644f60349a 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -423,7 +423,7 @@ module.exports = ProjectEntityHandler = return callback(error) if error? projectLocator.findElement {project: project, element_id: entity_id, type: entityType}, (error, entity, path)=> return callback(error) if error? - ProjectEntityHandler._cleanUpEntity project, entity, entityType, path.fileSystem, (error) -> + ProjectEntityHandler._cleanUpEntity project, null, entity, entityType, path.fileSystem, (error) -> return callback(error) if error? tpdsUpdateSender.deleteEntity project_id:project_id, path:path.fileSystem, project_name:project.name, (error) -> return callback(error) if error? @@ -456,17 +456,17 @@ module.exports = ProjectEntityHandler = return callback(error) if error? DocumentUpdaterHandler.updateProjectStructure project_id, userId, {oldDocs, newDocs, oldFiles, newFiles}, callback - _cleanUpEntity: (project, entity, entityType, path, callback = (error) ->) -> + _cleanUpEntity: (project, userId, entity, entityType, path, callback = (error) ->) -> if(entityType.indexOf("file") != -1) - ProjectEntityHandler._cleanUpFile project, entity, path, callback + ProjectEntityHandler._cleanUpFile project, userId, entity, path, callback else if (entityType.indexOf("doc") != -1) - ProjectEntityHandler._cleanUpDoc project, entity, path, callback + ProjectEntityHandler._cleanUpDoc project, userId, entity, path, callback else if (entityType.indexOf("folder") != -1) - ProjectEntityHandler._cleanUpFolder project, entity, path, callback + ProjectEntityHandler._cleanUpFolder project, userId, entity, path, callback else callback() - _cleanUpDoc: (project, doc, path, callback = (error) ->) -> + _cleanUpDoc: (project, userId, doc, path, callback = (error) ->) -> project_id = project._id.toString() doc_id = doc._id.toString() unsetRootDocIfRequired = (callback) => @@ -484,32 +484,32 @@ module.exports = ProjectEntityHandler = DocstoreManager.deleteDoc project_id, doc_id, (error) -> return callback(error) if error? changes = oldDocs: [ {doc, path} ] - DocumentUpdaterHandler.updateProjectStructure project_id, null, changes, callback + DocumentUpdaterHandler.updateProjectStructure project_id, userId, changes, callback - _cleanUpFile: (project, file, path, callback = (error) ->) -> + _cleanUpFile: (project, userId, file, path, callback = (error) ->) -> project_id = project._id.toString() file_id = file._id.toString() FileStoreHandler.deleteFile project_id, file_id, (error) -> return callback(error) if error? changes = oldFiles: [ {file, path} ] - DocumentUpdaterHandler.updateProjectStructure project_id, null, changes, callback + DocumentUpdaterHandler.updateProjectStructure project_id, userId, changes, callback - _cleanUpFolder: (project, folder, folderPath, callback = (error) ->) -> + _cleanUpFolder: (project, userId, folder, folderPath, callback = (error) ->) -> jobs = [] for doc in folder.docs do (doc) -> docPath = path.join(folderPath, doc.name) - jobs.push (callback) -> ProjectEntityHandler._cleanUpDoc project, doc, docPath, callback + jobs.push (callback) -> ProjectEntityHandler._cleanUpDoc project, userId, doc, docPath, callback for file in folder.fileRefs do (file) -> filePath = path.join(folderPath, file.name) - jobs.push (callback) -> ProjectEntityHandler._cleanUpFile project, file, filePath, callback + jobs.push (callback) -> ProjectEntityHandler._cleanUpFile project, userId, file, filePath, callback for childFolder in folder.folders do (childFolder) -> folderPath = path.join(folderPath, childFolder.name) - jobs.push (callback) -> ProjectEntityHandler._cleanUpFolder project, childFolder, folderPath, callback + jobs.push (callback) -> ProjectEntityHandler._cleanUpFolder project, userId, childFolder, folderPath, callback async.series jobs, callback diff --git a/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee b/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee index faaaa146da..63240f670e 100644 --- a/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee +++ b/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee @@ -157,7 +157,7 @@ describe 'ProjectEntityHandler', -> @ProjectGetter.getProject.callsArgWith(2, null, @project) @tpdsUpdateSender.deleteEntity = sinon.stub().callsArg(1) @ProjectEntityHandler._removeElementFromMongoArray = sinon.stub().callsArg(3) - @ProjectEntityHandler._cleanUpEntity = sinon.stub().callsArg(4) + @ProjectEntityHandler._cleanUpEntity = sinon.stub().callsArg(5) @path = mongo: "mongo.path", fileSystem: "/file/system/path" @projectLocator.findElement = sinon.stub().callsArgWith(1, null, @entity = { _id: entity_id }, @path) @@ -182,7 +182,7 @@ describe 'ProjectEntityHandler', -> it "should clean up the entity from the rest of the system", -> @ProjectEntityHandler._cleanUpEntity - .calledWith(@project, @entity, @type, @path.fileSystem) + .calledWith(@project, null, @entity, @type, @path.fileSystem) .should.equal true describe "_cleanUpEntity", -> @@ -195,7 +195,7 @@ describe 'ProjectEntityHandler', -> beforeEach (done) -> @path = "/file/system/path.png" @entity = _id: @entity_id - @ProjectEntityHandler._cleanUpEntity @project, @entity, 'file', @path, done + @ProjectEntityHandler._cleanUpEntity @project, userId, @entity, 'file', @path, done it "should delete the file from FileStoreHandler", -> @FileStoreHandler.deleteFile.calledWith(project_id, @entity_id).should.equal true @@ -206,19 +206,19 @@ describe 'ProjectEntityHandler', -> it "should should send the update to the doc updater", -> oldFiles = [ file: @entity, path: @path ] @documentUpdaterHandler.updateProjectStructure - .calledWith(project_id, null, {oldFiles}) + .calledWith(project_id, userId, {oldFiles}) .should.equal true describe "a doc", -> beforeEach (done) -> @path = "/file/system/path.tex" - @ProjectEntityHandler._cleanUpDoc = sinon.stub().callsArg(3) + @ProjectEntityHandler._cleanUpDoc = sinon.stub().callsArg(4) @entity = {_id: @entity_id} - @ProjectEntityHandler._cleanUpEntity @project, @entity, 'doc', @path, done + @ProjectEntityHandler._cleanUpEntity @project, userId, @entity, 'doc', @path, done it "should clean up the doc", -> @ProjectEntityHandler._cleanUpDoc - .calledWith(@project, @entity, @path) + .calledWith(@project, userId, @entity, @path) .should.equal true describe "a folder", -> @@ -233,18 +233,26 @@ describe 'ProjectEntityHandler', -> fileRefs: [ @file2 = { _id: "file-id-2", name: "file-name-2" } ] docs: [ @doc2 = { _id: "doc-id-2", name: "doc-name-2" } ] - @ProjectEntityHandler._cleanUpDoc = sinon.stub().callsArg(3) - @ProjectEntityHandler._cleanUpFile = sinon.stub().callsArg(3) + @ProjectEntityHandler._cleanUpDoc = sinon.stub().callsArg(4) + @ProjectEntityHandler._cleanUpFile = sinon.stub().callsArg(4) path = "/folder" - @ProjectEntityHandler._cleanUpEntity @project, @folder, "folder", path, done + @ProjectEntityHandler._cleanUpEntity @project, userId, @folder, "folder", path, done it "should clean up all sub files", -> - @ProjectEntityHandler._cleanUpFile.calledWith(@project, @file1, "/folder/subfolder/file-name-1").should.equal true - @ProjectEntityHandler._cleanUpFile.calledWith(@project, @file2, "/folder/file-name-2").should.equal true + @ProjectEntityHandler._cleanUpFile + .calledWith(@project, userId, @file1, "/folder/subfolder/file-name-1") + .should.equal true + @ProjectEntityHandler._cleanUpFile + .calledWith(@project, userId, @file2, "/folder/file-name-2") + .should.equal true it "should clean up all sub docs", -> - @ProjectEntityHandler._cleanUpDoc.calledWith(@project, @doc1, "/folder/subfolder/doc-name-1").should.equal true - @ProjectEntityHandler._cleanUpDoc.calledWith(@project, @doc2, "/folder/doc-name-2").should.equal true + @ProjectEntityHandler._cleanUpDoc + .calledWith(@project, userId, @doc1, "/folder/subfolder/doc-name-1") + .should.equal true + @ProjectEntityHandler._cleanUpDoc + .calledWith(@project, userId, @doc2, "/folder/doc-name-2") + .should.equal true describe 'moveEntity', -> beforeEach -> @@ -1138,7 +1146,7 @@ describe 'ProjectEntityHandler', -> describe "when the doc is the root doc", -> beforeEach -> @project.rootDoc_id = @doc._id - @ProjectEntityHandler._cleanUpDoc @project, @doc, @path, @callback + @ProjectEntityHandler._cleanUpDoc @project, userId, @doc, @path, @callback it "should unset the root doc", -> @ProjectEntityHandler.unsetRootDoc @@ -1162,7 +1170,7 @@ describe 'ProjectEntityHandler', -> it "should should send the update to the doc updater", -> oldDocs = [ doc: @doc, path: @path ] @documentUpdaterHandler.updateProjectStructure - .calledWith(project_id, null, {oldDocs}) + .calledWith(project_id, userId, {oldDocs}) .should.equal true it "should call the callback", -> @@ -1171,7 +1179,7 @@ describe 'ProjectEntityHandler', -> describe "when the doc is not the root doc", -> beforeEach -> @project.rootDoc_id = ObjectId() - @ProjectEntityHandler._cleanUpDoc @project, @doc, @path, @callback + @ProjectEntityHandler._cleanUpDoc @project, userId, @doc, @path, @callback it "should not unset the root doc", -> @ProjectEntityHandler.unsetRootDoc.called.should.equal false From 5f6686ed3b5da84b4a67205bb8d2e9046084a64d Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Fri, 15 Dec 2017 10:23:10 +0000 Subject: [PATCH 32/56] pass userId to ProjectEntityHandler.deleteEntity --- .../Features/Editor/EditorController.coffee | 6 ++-- .../Project/ProjectEntityHandler.coffee | 4 +-- .../Editor/EditorControllerTests.coffee | 35 +++++++++---------- .../Project/ProjectEntityHandlerTests.coffee | 4 +-- 4 files changed, 23 insertions(+), 26 deletions(-) diff --git a/services/web/app/coffee/Features/Editor/EditorController.coffee b/services/web/app/coffee/Features/Editor/EditorController.coffee index f66094f9e8..75741ceef2 100644 --- a/services/web/app/coffee/Features/Editor/EditorController.coffee +++ b/services/web/app/coffee/Features/Editor/EditorController.coffee @@ -110,14 +110,14 @@ module.exports = EditorController = if err? logger.err err:err, project_id:project_id, "could not get lock to deleteEntity" return callback(err) - EditorController.deleteEntityWithoutLock project_id, entity_id, entityType, source, (err)-> + EditorController.deleteEntityWithoutLock project_id, entity_id, entityType, source, null, (err)-> LockManager.releaseLock project_id, ()-> callback(err) - deleteEntityWithoutLock: (project_id, entity_id, entityType, source, callback)-> + deleteEntityWithoutLock: (project_id, entity_id, entityType, source, userId, callback)-> logger.log {project_id, entity_id, entityType, source}, "start delete process of entity" Metrics.inc "editor.delete-entity" - ProjectEntityHandler.deleteEntity project_id, entity_id, entityType, (err)-> + ProjectEntityHandler.deleteEntity project_id, entity_id, entityType, userId, (err)-> if err? logger.err err:err, project_id:project_id, entity_id:entity_id, entityType:entityType, "error deleting entity" return callback(err) diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index 644f60349a..0dec3f191b 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -412,7 +412,7 @@ module.exports = ProjectEntityHandler = callback() - deleteEntity: (project_id, entity_id, entityType, callback = (error) ->)-> + deleteEntity: (project_id, entity_id, entityType, userId, callback = (error) ->)-> self = @ logger.log entity_id:entity_id, entityType:entityType, project_id:project_id, "deleting project entity" if !entityType? @@ -423,7 +423,7 @@ module.exports = ProjectEntityHandler = return callback(error) if error? projectLocator.findElement {project: project, element_id: entity_id, type: entityType}, (error, entity, path)=> return callback(error) if error? - ProjectEntityHandler._cleanUpEntity project, null, entity, entityType, path.fileSystem, (error) -> + ProjectEntityHandler._cleanUpEntity project, userId, entity, entityType, path.fileSystem, (error) -> return callback(error) if error? tpdsUpdateSender.deleteEntity project_id:project_id, path:path.fileSystem, project_name:project.name, (error) -> return callback(error) if error? diff --git a/services/web/test/unit/coffee/Editor/EditorControllerTests.coffee b/services/web/test/unit/coffee/Editor/EditorControllerTests.coffee index 4e1af79b46..1e84352d38 100644 --- a/services/web/test/unit/coffee/Editor/EditorControllerTests.coffee +++ b/services/web/test/unit/coffee/Editor/EditorControllerTests.coffee @@ -382,11 +382,13 @@ describe "EditorController", -> beforeEach -> @LockManager.getLock.callsArgWith(1) @LockManager.releaseLock.callsArgWith(1) - @EditorController.deleteEntityWithoutLock = sinon.stub().callsArgWith(4) + @EditorController.deleteEntityWithoutLock = sinon.stub().callsArgWith(5) it "should call deleteEntityWithoutLock", (done)-> @EditorController.deleteEntity @project_id, @entity_id, @type, @source, => - @EditorController.deleteEntityWithoutLock.calledWith(@project_id, @entity_id, @type, @source).should.equal true + @EditorController.deleteEntityWithoutLock + .calledWith(@project_id, @entity_id, @type, @source, null) + .should.equal true done() it "should take the lock", (done)-> @@ -404,30 +406,25 @@ describe "EditorController", -> @EditorController.deleteEntity @project_id, @entity_id, @type, @source, (err)=> expect(err).to.exist err.should.equal "timed out" - done() - - + done() describe 'deleteEntityWithoutLock', -> - beforeEach -> - @ProjectEntityHandler.deleteEntity = (project_id, entity_id, type, callback)-> callback() + beforeEach (done) -> @entity_id = "entity_id_here" @type = "doc" @EditorRealTimeController.emitToRoom = sinon.stub() + @ProjectEntityHandler.deleteEntity = sinon.stub().callsArg(4) + @EditorController.deleteEntityWithoutLock @project_id, @entity_id, @type, @source, @user_id, done - it 'should delete the folder using the project entity handler', (done)-> - mock = sinon.mock(@ProjectEntityHandler).expects("deleteEntity").withArgs(@project_id, @entity_id, @type).callsArg(3) + it 'should delete the folder using the project entity handler', -> + @ProjectEntityHandler.deleteEntity + .calledWith(@project_id, @entity_id, @type, @user_id) + .should.equal.true - @EditorController.deleteEntityWithoutLock @project_id, @entity_id, @type, @source, -> - mock.verify() - done() - - it 'notify users an entity has been deleted', (done)-> - @EditorController.deleteEntityWithoutLock @project_id, @entity_id, @type, @source, => - @EditorRealTimeController.emitToRoom - .calledWith(@project_id, "removeEntity", @entity_id, @source) - .should.equal true - done() + it 'notify users an entity has been deleted', -> + @EditorRealTimeController.emitToRoom + .calledWith(@project_id, "removeEntity", @entity_id, @source) + .should.equal true describe "getting a list of project paths", -> diff --git a/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee b/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee index 63240f670e..7a6b62d99f 100644 --- a/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee +++ b/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee @@ -163,7 +163,7 @@ describe 'ProjectEntityHandler', -> describe "deleting from Mongo", -> beforeEach (done) -> - @ProjectEntityHandler.deleteEntity project_id, entity_id, @type = 'file', done + @ProjectEntityHandler.deleteEntity project_id, entity_id, @type = 'file', userId, done it "should retreive the path", -> @projectLocator.findElement.called.should.equal true @@ -182,7 +182,7 @@ describe 'ProjectEntityHandler', -> it "should clean up the entity from the rest of the system", -> @ProjectEntityHandler._cleanUpEntity - .calledWith(@project, null, @entity, @type, @path.fileSystem) + .calledWith(@project, userId, @entity, @type, @path.fileSystem) .should.equal true describe "_cleanUpEntity", -> From ca15fdb6eb74f6873ad3c409bcb44282884b82fe Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Fri, 15 Dec 2017 10:53:38 +0000 Subject: [PATCH 33/56] pass userId to EditorHttpController.deleteEntity --- .../Features/Editor/EditorController.coffee | 4 ++-- .../Features/Editor/EditorHttpController.coffee | 3 ++- .../coffee/ProjectStructureTests.coffee | 4 ++-- .../coffee/Editor/EditorControllerTests.coffee | 16 +++++++--------- .../Editor/EditorHttpControllerTests.coffee | 4 ++-- 5 files changed, 15 insertions(+), 16 deletions(-) diff --git a/services/web/app/coffee/Features/Editor/EditorController.coffee b/services/web/app/coffee/Features/Editor/EditorController.coffee index 75741ceef2..532f515a10 100644 --- a/services/web/app/coffee/Features/Editor/EditorController.coffee +++ b/services/web/app/coffee/Features/Editor/EditorController.coffee @@ -105,12 +105,12 @@ module.exports = EditorController = async.series jobs, (err)-> callback err, newFolders, lastFolder - deleteEntity : (project_id, entity_id, entityType, source, callback)-> + deleteEntity : (project_id, entity_id, entityType, source, userId, callback)-> LockManager.getLock project_id, (err)-> if err? logger.err err:err, project_id:project_id, "could not get lock to deleteEntity" return callback(err) - EditorController.deleteEntityWithoutLock project_id, entity_id, entityType, source, null, (err)-> + EditorController.deleteEntityWithoutLock project_id, entity_id, entityType, source, userId, (err)-> LockManager.releaseLock project_id, ()-> callback(err) diff --git a/services/web/app/coffee/Features/Editor/EditorHttpController.coffee b/services/web/app/coffee/Features/Editor/EditorHttpController.coffee index cbc296b69f..16b1a79e31 100644 --- a/services/web/app/coffee/Features/Editor/EditorHttpController.coffee +++ b/services/web/app/coffee/Features/Editor/EditorHttpController.coffee @@ -147,6 +147,7 @@ module.exports = EditorHttpController = project_id = req.params.Project_id entity_id = req.params.entity_id entity_type = req.params.entity_type - EditorController.deleteEntity project_id, entity_id, entity_type, "editor", (error) -> + user_id = AuthenticationController.getLoggedInUserId(req) + EditorController.deleteEntity project_id, entity_id, entity_type, "editor", user_id, (error) -> return next(error) if error? res.sendStatus 204 diff --git a/services/web/test/acceptance/coffee/ProjectStructureTests.coffee b/services/web/test/acceptance/coffee/ProjectStructureTests.coffee index 0f8086647f..fa91f7aeff 100644 --- a/services/web/test/acceptance/coffee/ProjectStructureTests.coffee +++ b/services/web/test/acceptance/coffee/ProjectStructureTests.coffee @@ -303,14 +303,14 @@ describe "ProjectStructureChanges", -> updates = MockDocUpdaterApi.getProjectStructureUpdates(@example_project_id).docUpdates expect(updates.length).to.equal(1) update = updates[0] - #expect(update.userId).to.equal(@owner._id) + expect(update.userId).to.equal(@owner._id) expect(update.pathname).to.equal("/bar/foo/new.tex") expect(update.newPathname).to.equal("") updates = MockDocUpdaterApi.getProjectStructureUpdates(@example_project_id).fileUpdates expect(updates.length).to.equal(1) update = updates[0] - #expect(update.userId).to.equal(@owner._id) + expect(update.userId).to.equal(@owner._id) expect(update.pathname).to.equal("/bar/foo/1pixel.png") expect(update.newPathname).to.equal("") diff --git a/services/web/test/unit/coffee/Editor/EditorControllerTests.coffee b/services/web/test/unit/coffee/Editor/EditorControllerTests.coffee index 1e84352d38..85760f510d 100644 --- a/services/web/test/unit/coffee/Editor/EditorControllerTests.coffee +++ b/services/web/test/unit/coffee/Editor/EditorControllerTests.coffee @@ -376,36 +376,34 @@ describe "EditorController", -> err.should.equal "timed out" done() - describe "deleteEntity", -> - beforeEach -> @LockManager.getLock.callsArgWith(1) @LockManager.releaseLock.callsArgWith(1) @EditorController.deleteEntityWithoutLock = sinon.stub().callsArgWith(5) it "should call deleteEntityWithoutLock", (done)-> - @EditorController.deleteEntity @project_id, @entity_id, @type, @source, => + @EditorController.deleteEntity @project_id, @entity_id, @type, @source, @user_id, => @EditorController.deleteEntityWithoutLock - .calledWith(@project_id, @entity_id, @type, @source, null) + .calledWith(@project_id, @entity_id, @type, @source, @user_id) .should.equal true done() it "should take the lock", (done)-> - @EditorController.deleteEntity @project_id, @entity_id, @type, @source, => + @EditorController.deleteEntity @project_id, @entity_id, @type, @source, @user_id, => @LockManager.getLock.calledWith(@project_id).should.equal true done() it "should release the lock", (done)-> - @EditorController.deleteEntity @project_id, @entity_id, @type, @source, (error)=> + @EditorController.deleteEntity @project_id, @entity_id, @type, @source, @user_id, (error) => @LockManager.releaseLock.calledWith(@project_id).should.equal true done() it "should error if it can't cat the lock", (done)-> @LockManager.getLock = sinon.stub().callsArgWith(1, "timed out") - @EditorController.deleteEntity @project_id, @entity_id, @type, @source, (err)=> - expect(err).to.exist - err.should.equal "timed out" + @EditorController.deleteEntity @project_id, @entity_id, @type, @source, @user_id, (error) => + expect(error).to.exist + error.should.equal "timed out" done() describe 'deleteEntityWithoutLock', -> diff --git a/services/web/test/unit/coffee/Editor/EditorHttpControllerTests.coffee b/services/web/test/unit/coffee/Editor/EditorHttpControllerTests.coffee index e05846c5d5..38419e6b46 100644 --- a/services/web/test/unit/coffee/Editor/EditorHttpControllerTests.coffee +++ b/services/web/test/unit/coffee/Editor/EditorHttpControllerTests.coffee @@ -331,12 +331,12 @@ describe "EditorHttpController", -> Project_id: @project_id entity_id: @entity_id = "entity-id-123" entity_type: @entity_type = "entity-type" - @EditorController.deleteEntity = sinon.stub().callsArg(4) + @EditorController.deleteEntity = sinon.stub().callsArg(5) @EditorHttpController.deleteEntity @req, @res it "should call EditorController.deleteEntity", -> @EditorController.deleteEntity - .calledWith(@project_id, @entity_id, @entity_type, "editor") + .calledWith(@project_id, @entity_id, @entity_type, "editor", @userId) .should.equal true it "should send back a success response", -> From 938caed4f72dd17d83bac61cab28e6895926f60c Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Fri, 15 Dec 2017 11:32:43 +0000 Subject: [PATCH 34/56] set userId when deleting entities via the tpds --- .../TpdsUpdateHandler.coffee | 2 +- .../ThirdPartyDataStore/UpdateMerger.coffee | 4 ++-- .../coffee/ProjectStructureTests.coffee | 22 +++++++++++++++++++ .../TpdsUpdateHandlerTests.coffee | 8 ++++--- .../UpdateMergerTests.coffee | 6 ++--- 5 files changed, 33 insertions(+), 9 deletions(-) diff --git a/services/web/app/coffee/Features/ThirdPartyDataStore/TpdsUpdateHandler.coffee b/services/web/app/coffee/Features/ThirdPartyDataStore/TpdsUpdateHandler.coffee index c78b588e8a..78e3f12ed6 100644 --- a/services/web/app/coffee/Features/ThirdPartyDataStore/TpdsUpdateHandler.coffee +++ b/services/web/app/coffee/Features/ThirdPartyDataStore/TpdsUpdateHandler.coffee @@ -47,7 +47,7 @@ module.exports = logger.log user_id:user_id, filePath:path, projectName:projectName, project_id:project._id, "project found for delete update, path is root so marking project as deleted" return projectDeleter.markAsDeletedByExternalSource project._id, callback else - updateMerger.deleteUpdate project._id, path, source, (err)-> + updateMerger.deleteUpdate user_id, project._id, path, source, (err)-> callback(err) diff --git a/services/web/app/coffee/Features/ThirdPartyDataStore/UpdateMerger.coffee b/services/web/app/coffee/Features/ThirdPartyDataStore/UpdateMerger.coffee index 010594d16a..e49c52aab2 100644 --- a/services/web/app/coffee/Features/ThirdPartyDataStore/UpdateMerger.coffee +++ b/services/web/app/coffee/Features/ThirdPartyDataStore/UpdateMerger.coffee @@ -32,13 +32,13 @@ module.exports = else self.p.processDoc project_id, elementId, user_id, fsPath, path, source, callback - deleteUpdate: (project_id, path, source, callback)-> + deleteUpdate: (user_id, project_id, path, source, callback)-> projectLocator.findElementByPath project_id, path, (err, element, type)-> if err? || !element? logger.log element:element, project_id:project_id, path:path, "could not find entity for deleting, assuming it was already deleted" return callback() logger.log project_id:project_id, path:path, type:type, element:element, "processing update to delete entity from tpds" - editorController.deleteEntity project_id, element._id, type, source, (err)-> + editorController.deleteEntity project_id, element._id, type, source, user_id, (err)-> logger.log project_id:project_id, path:path, "finished processing update to delete entity from tpds" callback() diff --git a/services/web/test/acceptance/coffee/ProjectStructureTests.coffee b/services/web/test/acceptance/coffee/ProjectStructureTests.coffee index fa91f7aeff..cb59efe1df 100644 --- a/services/web/test/acceptance/coffee/ProjectStructureTests.coffee +++ b/services/web/test/acceptance/coffee/ProjectStructureTests.coffee @@ -422,3 +422,25 @@ describe "ProjectStructureChanges", -> done() image_file.pipe(req) + + it "should version deleting a doc", (done) -> + req = @owner.request.delete { + uri: "/user/#{@owner._id}/update/#{@tpds_project_name}/test.tex", + auth: + user: _.keys(Settings.httpAuthUsers)[0] + pass: _.values(Settings.httpAuthUsers)[0] + sendImmediately: true + }, (error, res, body) => + throw error if error? + if res.statusCode < 200 || res.statusCode >= 300 + throw new Error("failed to delete doc #{res.statusCode}") + + updates = MockDocUpdaterApi.getProjectStructureUpdates(@tpds_project_id).docUpdates + expect(updates.length).to.equal(1) + update = updates[0] + expect(update.userId).to.equal(@owner._id) + expect(update.pathname).to.equal("/test.tex") + expect(update.newPathname).to.equal("") + + done() + diff --git a/services/web/test/unit/coffee/ThirdPartyDataStore/TpdsUpdateHandlerTests.coffee b/services/web/test/unit/coffee/ThirdPartyDataStore/TpdsUpdateHandlerTests.coffee index dedd3ea7c8..3984d6341f 100644 --- a/services/web/test/unit/coffee/ThirdPartyDataStore/TpdsUpdateHandlerTests.coffee +++ b/services/web/test/unit/coffee/ThirdPartyDataStore/TpdsUpdateHandlerTests.coffee @@ -8,7 +8,7 @@ describe 'TpdsUpdateHandler', -> beforeEach -> @requestQueuer = {} @updateMerger = - deleteUpdate: (user_id, path, source, cb)->cb() + deleteUpdate: (user_id, project_id, path, source, cb)->cb() mergeUpdate:(user_id, project_id, path, update, source, cb)->cb() @editorController = {} @project_id = "dsjajilknaksdn" @@ -107,11 +107,13 @@ describe 'TpdsUpdateHandler', -> it 'should call deleteEntity in the collaberation manager', (done)-> path = "/delete/this" update = {} - @updateMerger.deleteUpdate = sinon.stub().callsArg(3) + @updateMerger.deleteUpdate = sinon.stub().callsArg(4) @handler.deleteUpdate @user_id, @project.name, path, @source, => @projectDeleter.markAsDeletedByExternalSource.calledWith(@project._id).should.equal false - @updateMerger.deleteUpdate.calledWith(@project_id, path, @source).should.equal true + @updateMerger.deleteUpdate + .calledWith(@user_id, @project_id, path, @source) + .should.equal true done() it 'should mark the project as deleted by external source if path is a single slash', (done)-> diff --git a/services/web/test/unit/coffee/ThirdPartyDataStore/UpdateMergerTests.coffee b/services/web/test/unit/coffee/ThirdPartyDataStore/UpdateMergerTests.coffee index e20419765f..cb2aa059ea 100644 --- a/services/web/test/unit/coffee/ThirdPartyDataStore/UpdateMergerTests.coffee +++ b/services/web/test/unit/coffee/ThirdPartyDataStore/UpdateMergerTests.coffee @@ -145,13 +145,13 @@ describe 'UpdateMerger :', -> it 'should get the element id', -> @projectLocator.findElementByPath = sinon.spy() - @updateMerger.deleteUpdate @project_id, @path, @source, -> + @updateMerger.deleteUpdate @user_id, @project_id, @path, @source, -> @projectLocator.findElementByPath.calledWith(@project_id, @path).should.equal true it 'should delete the entity in the editor controller with the correct type', (done)-> @entity.lines = [] - mock = sinon.mock(@editorController).expects("deleteEntity").withArgs(@project_id, @entity_id, @type, @source).callsArg(4) - @updateMerger.deleteUpdate @project_id, @path, @source, -> + mock = sinon.mock(@editorController).expects("deleteEntity").withArgs(@project_id, @entity_id, @type, @source, @user_id).callsArg(5) + @updateMerger.deleteUpdate @user_id, @project_id, @path, @source, -> mock.verify() done() From ac36de962924e09f98008178b5911161adad7bed Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Fri, 15 Dec 2017 13:17:26 +0000 Subject: [PATCH 35/56] make ProjectEntityHandler._clean* argument signatures consistent --- .../Project/ProjectEntityHandler.coffee | 22 +++++++++---------- .../Project/ProjectEntityHandlerTests.coffee | 22 +++++++++---------- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index 0dec3f191b..a9d7c708ee 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -423,7 +423,7 @@ module.exports = ProjectEntityHandler = return callback(error) if error? projectLocator.findElement {project: project, element_id: entity_id, type: entityType}, (error, entity, path)=> return callback(error) if error? - ProjectEntityHandler._cleanUpEntity project, userId, entity, entityType, path.fileSystem, (error) -> + ProjectEntityHandler._cleanUpEntity project, entity, entityType, path.fileSystem, userId, (error) -> return callback(error) if error? tpdsUpdateSender.deleteEntity project_id:project_id, path:path.fileSystem, project_name:project.name, (error) -> return callback(error) if error? @@ -456,17 +456,17 @@ module.exports = ProjectEntityHandler = return callback(error) if error? DocumentUpdaterHandler.updateProjectStructure project_id, userId, {oldDocs, newDocs, oldFiles, newFiles}, callback - _cleanUpEntity: (project, userId, entity, entityType, path, callback = (error) ->) -> + _cleanUpEntity: (project, entity, entityType, path, userId, callback = (error) ->) -> if(entityType.indexOf("file") != -1) - ProjectEntityHandler._cleanUpFile project, userId, entity, path, callback + ProjectEntityHandler._cleanUpFile project, entity, path, userId, callback else if (entityType.indexOf("doc") != -1) - ProjectEntityHandler._cleanUpDoc project, userId, entity, path, callback + ProjectEntityHandler._cleanUpDoc project, entity, path, userId, callback else if (entityType.indexOf("folder") != -1) - ProjectEntityHandler._cleanUpFolder project, userId, entity, path, callback + ProjectEntityHandler._cleanUpFolder project, entity, path, userId, callback else callback() - _cleanUpDoc: (project, userId, doc, path, callback = (error) ->) -> + _cleanUpDoc: (project, doc, path, userId, callback = (error) ->) -> project_id = project._id.toString() doc_id = doc._id.toString() unsetRootDocIfRequired = (callback) => @@ -486,7 +486,7 @@ module.exports = ProjectEntityHandler = changes = oldDocs: [ {doc, path} ] DocumentUpdaterHandler.updateProjectStructure project_id, userId, changes, callback - _cleanUpFile: (project, userId, file, path, callback = (error) ->) -> + _cleanUpFile: (project, file, path, userId, callback = (error) ->) -> project_id = project._id.toString() file_id = file._id.toString() FileStoreHandler.deleteFile project_id, file_id, (error) -> @@ -494,22 +494,22 @@ module.exports = ProjectEntityHandler = changes = oldFiles: [ {file, path} ] DocumentUpdaterHandler.updateProjectStructure project_id, userId, changes, callback - _cleanUpFolder: (project, userId, folder, folderPath, callback = (error) ->) -> + _cleanUpFolder: (project, folder, folderPath, userId, callback = (error) ->) -> jobs = [] for doc in folder.docs do (doc) -> docPath = path.join(folderPath, doc.name) - jobs.push (callback) -> ProjectEntityHandler._cleanUpDoc project, userId, doc, docPath, callback + jobs.push (callback) -> ProjectEntityHandler._cleanUpDoc project, doc, docPath, userId, callback for file in folder.fileRefs do (file) -> filePath = path.join(folderPath, file.name) - jobs.push (callback) -> ProjectEntityHandler._cleanUpFile project, userId, file, filePath, callback + jobs.push (callback) -> ProjectEntityHandler._cleanUpFile project, file, filePath, userId, callback for childFolder in folder.folders do (childFolder) -> folderPath = path.join(folderPath, childFolder.name) - jobs.push (callback) -> ProjectEntityHandler._cleanUpFolder project, userId, childFolder, folderPath, callback + jobs.push (callback) -> ProjectEntityHandler._cleanUpFolder project, childFolder, folderPath, userId, callback async.series jobs, callback diff --git a/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee b/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee index 7a6b62d99f..7c4188052f 100644 --- a/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee +++ b/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee @@ -182,7 +182,7 @@ describe 'ProjectEntityHandler', -> it "should clean up the entity from the rest of the system", -> @ProjectEntityHandler._cleanUpEntity - .calledWith(@project, userId, @entity, @type, @path.fileSystem) + .calledWith(@project, @entity, @type, @path.fileSystem, userId) .should.equal true describe "_cleanUpEntity", -> @@ -195,7 +195,7 @@ describe 'ProjectEntityHandler', -> beforeEach (done) -> @path = "/file/system/path.png" @entity = _id: @entity_id - @ProjectEntityHandler._cleanUpEntity @project, userId, @entity, 'file', @path, done + @ProjectEntityHandler._cleanUpEntity @project, @entity, 'file', @path, userId, done it "should delete the file from FileStoreHandler", -> @FileStoreHandler.deleteFile.calledWith(project_id, @entity_id).should.equal true @@ -214,11 +214,11 @@ describe 'ProjectEntityHandler', -> @path = "/file/system/path.tex" @ProjectEntityHandler._cleanUpDoc = sinon.stub().callsArg(4) @entity = {_id: @entity_id} - @ProjectEntityHandler._cleanUpEntity @project, userId, @entity, 'doc', @path, done + @ProjectEntityHandler._cleanUpEntity @project, @entity, 'doc', @path, userId, done it "should clean up the doc", -> @ProjectEntityHandler._cleanUpDoc - .calledWith(@project, userId, @entity, @path) + .calledWith(@project, @entity, @path, userId) .should.equal true describe "a folder", -> @@ -236,22 +236,22 @@ describe 'ProjectEntityHandler', -> @ProjectEntityHandler._cleanUpDoc = sinon.stub().callsArg(4) @ProjectEntityHandler._cleanUpFile = sinon.stub().callsArg(4) path = "/folder" - @ProjectEntityHandler._cleanUpEntity @project, userId, @folder, "folder", path, done + @ProjectEntityHandler._cleanUpEntity @project, @folder, "folder", path, userId, done it "should clean up all sub files", -> @ProjectEntityHandler._cleanUpFile - .calledWith(@project, userId, @file1, "/folder/subfolder/file-name-1") + .calledWith(@project, @file1, "/folder/subfolder/file-name-1", userId) .should.equal true @ProjectEntityHandler._cleanUpFile - .calledWith(@project, userId, @file2, "/folder/file-name-2") + .calledWith(@project, @file2, "/folder/file-name-2", userId) .should.equal true it "should clean up all sub docs", -> @ProjectEntityHandler._cleanUpDoc - .calledWith(@project, userId, @doc1, "/folder/subfolder/doc-name-1") + .calledWith(@project, @doc1, "/folder/subfolder/doc-name-1", userId) .should.equal true @ProjectEntityHandler._cleanUpDoc - .calledWith(@project, userId, @doc2, "/folder/doc-name-2") + .calledWith(@project, @doc2, "/folder/doc-name-2", userId) .should.equal true describe 'moveEntity', -> @@ -1146,7 +1146,7 @@ describe 'ProjectEntityHandler', -> describe "when the doc is the root doc", -> beforeEach -> @project.rootDoc_id = @doc._id - @ProjectEntityHandler._cleanUpDoc @project, userId, @doc, @path, @callback + @ProjectEntityHandler._cleanUpDoc @project, @doc, @path, userId, @callback it "should unset the root doc", -> @ProjectEntityHandler.unsetRootDoc @@ -1179,7 +1179,7 @@ describe 'ProjectEntityHandler', -> describe "when the doc is not the root doc", -> beforeEach -> @project.rootDoc_id = ObjectId() - @ProjectEntityHandler._cleanUpDoc @project, userId, @doc, @path, @callback + @ProjectEntityHandler._cleanUpDoc @project, @doc, @path, userId, @callback it "should not unset the root doc", -> @ProjectEntityHandler.unsetRootDoc.called.should.equal false From dc2ddf7e0938a5891d79ca751e8f58c1855f301e Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Mon, 18 Dec 2017 16:53:58 +0000 Subject: [PATCH 36/56] Check for OL free trial instead of host as it may be fragile indicator --- services/web/app/views/layout.pug | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/app/views/layout.pug b/services/web/app/views/layout.pug index 14ebc3c576..2c6a836e8f 100644 --- a/services/web/app/views/layout.pug +++ b/services/web/app/views/layout.pug @@ -96,7 +96,7 @@ html(itemscope, itemtype='http://schema.org/Product') indexName : '!{settings.templates.indexName}' } - - if (settings.overleaf && settings.overleaf.host) + - if (settings.overleaf && settings.overleaf.useOLFreeTrial) script. window.redirectToOLFreeTrialUrl = '!{settings.overleaf.host}/users/trial' From fdd861e82499dd95c6cc21e4a23b8edb7377d0f6 Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Tue, 19 Dec 2017 10:02:15 +0000 Subject: [PATCH 37/56] Ignore content-addressible compiled require js --- services/web/.gitignore | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/.gitignore b/services/web/.gitignore index 83d4d28b65..f4ee8dcbe0 100644 --- a/services/web/.gitignore +++ b/services/web/.gitignore @@ -69,7 +69,7 @@ Gemfile.lock public/stylesheets/ol-style.*.css public/stylesheets/style.*.css -public/js/libs/require.js +public/js/libs/require*.js *.swp From 547c0cb79f2821ce33ab33f6180ec87d31d79cba Mon Sep 17 00:00:00 2001 From: Paulo Reis Date: Tue, 19 Dec 2017 13:10:12 +0000 Subject: [PATCH 38/56] Use new theme red shade as the danger red; tone down compile log alerts. --- .../public/stylesheets/app/editor/pdf.less | 21 +++++++++++++------ .../public/stylesheets/core/ol-variables.less | 2 +- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/services/web/public/stylesheets/app/editor/pdf.less b/services/web/public/stylesheets/app/editor/pdf.less index f5ff31177f..d59ca93069 100644 --- a/services/web/public/stylesheets/app/editor/pdf.less +++ b/services/web/public/stylesheets/app/editor/pdf.less @@ -207,16 +207,25 @@ } } - &.alert-danger:hover { - background-color: darken(@alert-danger-bg, 5%); + &.alert-danger { + background-color: tint(@alert-danger-bg, 15%); + &:hover { + background-color: @alert-danger-bg; + } } - &.alert-warning:hover { - background-color: darken(@alert-warning-bg, 5%); + &.alert-warning { + background-color: tint(@alert-warning-bg, 15%); + &:hover { + background-color: @alert-warning-bg; + } } - &.alert-info:hover { - background-color: darken(@alert-info-bg, 5%); + &.alert-info { + background-color: tint(@alert-info-bg, 15%); + &:hover { + background-color: @alert-info-bg; + } } } diff --git a/services/web/public/stylesheets/core/ol-variables.less b/services/web/public/stylesheets/core/ol-variables.less index aee331f3d2..b89e476567 100644 --- a/services/web/public/stylesheets/core/ol-variables.less +++ b/services/web/public/stylesheets/core/ol-variables.less @@ -249,7 +249,7 @@ @brand-success: @green; @brand-info: @ol-blue; @brand-warning: @orange; -@brand-danger: #E03A06; +@brand-danger: @ol-red; @editor-loading-logo-padding-top: 115.44%; @editor-loading-logo-background-url: url(/img/ol-brand/overleaf-o-grey.svg); From 70298ba65c6bce0a2325c426c9035c1de9158495 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Tue, 19 Dec 2017 13:13:31 +0000 Subject: [PATCH 39/56] only hash the static content when minified is on --- .../infrastructure/ExpressLocals.coffee | 35 ++++++++++--------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/services/web/app/coffee/infrastructure/ExpressLocals.coffee b/services/web/app/coffee/infrastructure/ExpressLocals.coffee index 002a10ca89..a820a8500b 100644 --- a/services/web/app/coffee/infrastructure/ExpressLocals.coffee +++ b/services/web/app/coffee/infrastructure/ExpressLocals.coffee @@ -36,7 +36,6 @@ getFileContent = (filePath)-> logger.log filePath:filePath, "file does not exist for hashing" return "" -logger.log "Generating file hashes..." pathList = [ "#{jsPath}libs/require.js" "#{jsPath}ide.js" @@ -46,23 +45,27 @@ pathList = [ "/stylesheets/ol-style.css" ] -for path in pathList - content = getFileContent(path) - hash = crypto.createHash("md5").update(content).digest("hex") - - splitPath = path.split("/") - filenameSplit = splitPath.pop().split(".") - filenameSplit.splice(filenameSplit.length-1, 0, hash) - splitPath.push(filenameSplit.join(".")) +if !Settings.useMinifiedJs + logger.log "not using minified JS, not hashing static files" +else + logger.log "Generating file hashes..." + for path in pathList + content = getFileContent(path) + hash = crypto.createHash("md5").update(content).digest("hex") + + splitPath = path.split("/") + filenameSplit = splitPath.pop().split(".") + filenameSplit.splice(filenameSplit.length-1, 0, hash) + splitPath.push(filenameSplit.join(".")) - hashPath = splitPath.join("/") - hashedFiles[path] = hashPath + hashPath = splitPath.join("/") + hashedFiles[path] = hashPath - fsHashPath = Path.join __dirname, "../../../", "public#{hashPath}" - fs.writeFileSync(fsHashPath, content) + fsHashPath = Path.join __dirname, "../../../", "public#{hashPath}" + fs.writeFileSync(fsHashPath, content) -logger.log "Finished hashing static content" + logger.log "Finished hashing static content" cdnAvailable = Settings.cdn?.web?.host? darkCdnAvailable = Settings.cdn?.web?.darkHost? @@ -121,7 +124,7 @@ module.exports = (app, webRouter, privateApiRouter, publicApiRouter)-> res.locals.buildJsPath = (jsFile, opts = {})-> path = Path.join(jsPath, jsFile) - if opts.hashedPath + if opts.hashedPath && hashedFiles[path]? path = hashedFiles[path] if !opts.qs? @@ -141,7 +144,7 @@ module.exports = (app, webRouter, privateApiRouter, publicApiRouter)-> res.locals.buildCssPath = (cssFile, opts)-> path = Path.join("/stylesheets/", cssFile) - if opts?.hashedPath + if opts?.hashedPath && hashedFiles[path]? hashedPath = hashedFiles[path] return Url.resolve(staticFilesBase, hashedPath) return Url.resolve(staticFilesBase, path) From a1615e6d846c7471dfc527bac79ebf1559b95f3f Mon Sep 17 00:00:00 2001 From: James Allen Date: Tue, 28 Nov 2017 18:18:15 +0000 Subject: [PATCH 40/56] Prototype of requesting history information by doc path, while tracking renames --- .../web/app/views/project/editor/history.pug | 6 +- .../coffee/ide/history/HistoryManager.coffee | 129 ++++++++++++------ 2 files changed, 92 insertions(+), 43 deletions(-) diff --git a/services/web/app/views/project/editor/history.pug b/services/web/app/views/project/editor/history.pug index 4806b0a9b8..f62dcebf15 100644 --- a/services/web/app/views/project/editor/history.pug +++ b/services/web/app/views/project/editor/history.pug @@ -134,8 +134,10 @@ div#history(ng-show="ui.view == 'history'") div.description(ng-click="select()") div.time {{ update.meta.end_ts | formatDate:'h:mm a' }} - div.docs(ng-repeat="(doc_id, doc) in update.docs") - span.doc {{ doc.entity.name }} + div.docs(ng-repeat="(doc_path, doc) in update.docs", ng-if="update.docs") + span.doc {{ doc_path }} + div.docs(ng-repeat="rename in update.renames", ng-if="update.renames") + span Renamed {{ rename.pathname }} to {{ rename.newPathname }} div.users div.user(ng-repeat="update_user in update.meta.users") .color-square(ng-if="update_user != null", ng-style="{'background-color': 'hsl({{ update_user.hue }}, 70%, 50%)'}") diff --git a/services/web/public/coffee/ide/history/HistoryManager.coffee b/services/web/public/coffee/ide/history/HistoryManager.coffee index 6b42714e79..a892077b90 100644 --- a/services/web/public/coffee/ide/history/HistoryManager.coffee +++ b/services/web/public/coffee/ide/history/HistoryManager.coffee @@ -22,7 +22,8 @@ define [ @$scope.$on "entity:selected", (event, entity) => if (@$scope.ui.view == "history") and (entity.type == "doc") - @$scope.history.selection.doc = entity + # TODO: Set selection.doc_path to entity path name + # @$scope.history.selection.doc = entity @reloadDiff() show: () -> @@ -83,30 +84,36 @@ define [ reloadDiff: () -> diff = @$scope.history.diff - {updates, doc} = @$scope.history.selection - {fromV, toV, start_ts, end_ts} = @_calculateRangeFromSelection() + {updates} = @$scope.history.selection + {fromV, toV, start_ts, end_ts, original_path} = @_calculateDiffDataFromSelection() + console.log "[reloadDiff] current diff", diff + console.log "[reloadDiff] new diff data", {fromV, toV, start_ts, end_ts, original_path} - return if !doc? + return if !original_path? return if diff? and - diff.doc == doc and - diff.fromV == fromV and - diff.toV == toV + diff.doc_path == original_path and + diff.fromV == fromV and + diff.toV == toV @$scope.history.diff = diff = { fromV: fromV toV: toV start_ts: start_ts end_ts: end_ts - doc: doc + doc_path: original_path error: false } - if !doc.deleted + # TODO: How do we track deleted files now? We can probably show the diffs easily + # with the new system! + if true # !doc.deleted diff.loading = true - url = "/project/#{@$scope.project_id}/doc/#{diff.doc.id}/diff" + url = "/project/#{@$scope.project_id}/doc/by_path/diff" + query = ["path=#{encodeURIComponent(original_path)}"] if diff.fromV? and diff.toV? - url += "?from=#{diff.fromV}&to=#{diff.toV}" + query.push "from=#{diff.fromV}", "to=#{diff.toV}" + url += "?" + query.join("&") @ide.$http .get(url) @@ -190,8 +197,9 @@ define [ previousUpdate = @$scope.history.updates[@$scope.history.updates.length - 1] for update in updates - for doc_id, doc of update.docs or {} - doc.entity = @ide.fileTreeManager.findEntityById(doc_id, includeDeleted: true) + for doc_path, doc of update.docs or {} + doc.path = doc_path + doc.entity = @ide.fileTreeManager.findEntityByPath(doc_path, includeDeleted: true) for user in update.meta.users or [] if user? @@ -210,50 +218,89 @@ define [ @$scope.history.updates = @$scope.history.updates.concat(updates) + console.log "[_loadUpdates] updates", @$scope.history.updates @autoSelectRecentUpdates() if firstLoad - _calculateRangeFromSelection: () -> - fromV = toV = start_ts = end_ts = null + _perDocSummaryOfUpdates: (updates) -> + current_paths = {} + docs_summary = {} - selected_doc_id = @$scope.history.selection.doc?.id - - for update in @$scope.history.selection.updates or [] - for doc_id, doc of update.docs - if doc_id == selected_doc_id - if fromV? and toV? - fromV = Math.min(fromV, doc.fromV) - toV = Math.max(toV, doc.toV) - start_ts = Math.min(start_ts, update.meta.start_ts) - end_ts = Math.max(end_ts, update.meta.end_ts) + for update in updates # Updates are reverse chronologically ordered + console.log "[_perDocSummaryOfUpdates] update", update + if update.docs? + for doc_path, doc of update.docs + # doc_path may not be the latest doc path that this doc has had + if !current_paths[doc_path]? + current_paths[doc_path] = doc_path + current_path = current_paths[doc_path] + console.log "[_perDocSummaryOfUpdates] doc", doc, current_path + if !docs_summary[current_path]? + # todo start_ts and end_ts + docs_summary[current_path] = { + fromV: doc.fromV, toV: doc.toV, + original_path: doc_path + } else - fromV = doc.fromV - toV = doc.toV - start_ts = update.meta.start_ts - end_ts = update.meta.end_ts - break + docs_summary[current_path] = { + fromV: Math.min(docs_summary[current_path].fromV, doc.fromV), + toV: Math.max(docs_summary[current_path].toV, doc.toV), + original_path: doc_path + } + else if update.renames? + for rename in update.renames + console.log "[_perDocSummaryOfUpdates] rename", rename + if !current_paths[rename.newPathname]? + current_paths[rename.newPathname] = rename.newPathname + current_paths[rename.pathname] = current_paths[rename.newPathname] + delete current_paths[rename.newPathname] - return {fromV, toV, start_ts, end_ts} + console.log "[_perDocSummaryOfUpdates] docs_summary", docs_summary + console.log "[_perDocSummaryOfUpdates] current_paths", current_paths + + return docs_summary + + _calculateDiffDataFromSelection: () -> + fromV = toV = start_ts = end_ts = original_path = null + + selected_doc_path = @$scope.history.selection.doc_path + console.log "[_calculateDiffDataFromSelection] selected_doc_path", selected_doc_path + + for doc_path, doc of @_perDocSummaryOfUpdates(@$scope.history.selection.updates) + if doc_path == selected_doc_path + fromV = doc.fromV + toV = doc.toV + start_ts = doc.start_ts + end_ts = doc.end_ts + original_path = doc.original_path + break + + return {fromV, toV, start_ts, end_ts, original_path} # Set the track changes selected doc to one of the docs in the range # of currently selected updates. If we already have a selected doc # then prefer this one if present. _selectDocFromUpdates: () -> - affected_docs = {} - for update in @$scope.history.selection.updates - for doc_id, doc of update.docs - affected_docs[doc_id] = doc.entity + affected_docs = @_perDocSummaryOfUpdates(@$scope.history.selection.updates) + console.log "[_selectDocFromUpdates] affected_docs", affected_docs - selected_doc = @$scope.history.selection.doc - if selected_doc? and affected_docs[selected_doc.id]? + selected_doc_path = @$scope.history.selection.doc_path + console.log "[_selectDocFromUpdates] current selected_doc_path", selected_doc_path + if selected_doc_path? and affected_docs[selected_doc_path] # Selected doc is already open else - for doc_id, doc of affected_docs - selected_doc = doc + # Set to first possible candidate + for doc_path, doc of affected_docs + selected_doc_path = doc_path break - @$scope.history.selection.doc = selected_doc - @ide.fileTreeManager.selectEntity(selected_doc) + console.log "[_selectDocFromUpdates] new selected_doc_path", selected_doc_path + + @$scope.history.selection.doc_path = selected_doc_path + if selected_doc_path? + entity = @ide.fileTreeManager.findEntityByPath(selected_doc_path) + if entity? + @ide.fileTreeManager.selectEntity(entity) _updateContainsUserId: (update, user_id) -> for user in update.meta.users From 4691a6e85c8b43c6ac1068d75f6aeda3df4fb51a Mon Sep 17 00:00:00 2001 From: James Allen Date: Thu, 7 Dec 2017 10:13:46 +0000 Subject: [PATCH 41/56] Get diffs showing in client --- services/web/app/coffee/router.coffee | 1 + .../web/app/views/project/editor/history.pug | 9 +- .../coffee/ide/history/HistoryManager.coffee | 121 +++++++++--------- 3 files changed, 64 insertions(+), 67 deletions(-) diff --git a/services/web/app/coffee/router.coffee b/services/web/app/coffee/router.coffee index c89d9cdf5e..2199bdd08e 100644 --- a/services/web/app/coffee/router.coffee +++ b/services/web/app/coffee/router.coffee @@ -197,6 +197,7 @@ module.exports = class Router webRouter.get "/project/:Project_id/updates", AuthorizationMiddlewear.ensureUserCanReadProject, HistoryController.selectHistoryApi, HistoryController.proxyToHistoryApi webRouter.get "/project/:Project_id/doc/:doc_id/diff", AuthorizationMiddlewear.ensureUserCanReadProject, HistoryController.selectHistoryApi, HistoryController.proxyToHistoryApi + webRouter.get "/project/:Project_id/diff", AuthorizationMiddlewear.ensureUserCanReadProject, HistoryController.selectHistoryApi, HistoryController.proxyToHistoryApi webRouter.post "/project/:Project_id/doc/:doc_id/version/:version_id/restore", AuthorizationMiddlewear.ensureUserCanReadProject, HistoryController.selectHistoryApi, HistoryController.proxyToHistoryApi webRouter.get '/Project/:Project_id/download/zip', AuthorizationMiddlewear.ensureUserCanReadProject, ProjectDownloadsController.downloadProject diff --git a/services/web/app/views/project/editor/history.pug b/services/web/app/views/project/editor/history.pug index f62dcebf15..102d3e05f8 100644 --- a/services/web/app/views/project/editor/history.pug +++ b/services/web/app/views/project/editor/history.pug @@ -134,10 +134,11 @@ div#history(ng-show="ui.view == 'history'") div.description(ng-click="select()") div.time {{ update.meta.end_ts | formatDate:'h:mm a' }} - div.docs(ng-repeat="(doc_path, doc) in update.docs", ng-if="update.docs") - span.doc {{ doc_path }} - div.docs(ng-repeat="rename in update.renames", ng-if="update.renames") - span Renamed {{ rename.pathname }} to {{ rename.newPathname }} + div.docs(ng-repeat="pathname in update.docs", ng-if="update.docs") + span.doc {{ pathname }} + div.docs(ng-repeat="project_op in update.project_ops", ng-if="update.project_ops") + span(ng-if="project_op.rename") + | Renamed {{ project_op.rename.pathname }} to {{ project_op.rename.newPathname }} div.users div.user(ng-repeat="update_user in update.meta.users") .color-square(ng-if="update_user != null", ng-style="{'background-color': 'hsl({{ update_user.hue }}, 70%, 50%)'}") diff --git a/services/web/public/coffee/ide/history/HistoryManager.coffee b/services/web/public/coffee/ide/history/HistoryManager.coffee index a892077b90..80a39a3614 100644 --- a/services/web/public/coffee/ide/history/HistoryManager.coffee +++ b/services/web/public/coffee/ide/history/HistoryManager.coffee @@ -22,7 +22,7 @@ define [ @$scope.$on "entity:selected", (event, entity) => if (@$scope.ui.view == "history") and (entity.type == "doc") - # TODO: Set selection.doc_path to entity path name + # TODO: Set selection.pathname to entity path name # @$scope.history.selection.doc = entity @reloadDiff() @@ -46,8 +46,6 @@ define [ range: { fromV: null toV: null - start_ts: null - end_ts: null } } diff: null @@ -76,6 +74,7 @@ define [ .get(url) .then (response) => { data } = response + console.log "fetchNextBatchOfUpdates", data.updates @_loadUpdates(data.updates) @$scope.history.nextBeforeTimestamp = data.nextBeforeTimestamp if !data.nextBeforeTimestamp? @@ -85,23 +84,21 @@ define [ reloadDiff: () -> diff = @$scope.history.diff {updates} = @$scope.history.selection - {fromV, toV, start_ts, end_ts, original_path} = @_calculateDiffDataFromSelection() + {fromV, toV, pathname} = @_calculateDiffDataFromSelection() console.log "[reloadDiff] current diff", diff - console.log "[reloadDiff] new diff data", {fromV, toV, start_ts, end_ts, original_path} + console.log "[reloadDiff] new diff data", {fromV, toV, pathname} - return if !original_path? + return if !pathname? return if diff? and - diff.doc_path == original_path and + diff.pathname == pathname and diff.fromV == fromV and diff.toV == toV @$scope.history.diff = diff = { fromV: fromV toV: toV - start_ts: start_ts - end_ts: end_ts - doc_path: original_path + pathname: pathname error: false } @@ -109,8 +106,8 @@ define [ # with the new system! if true # !doc.deleted diff.loading = true - url = "/project/#{@$scope.project_id}/doc/by_path/diff" - query = ["path=#{encodeURIComponent(original_path)}"] + url = "/project/#{@$scope.project_id}/diff" + query = ["pathname=#{encodeURIComponent(pathname)}"] if diff.fromV? and diff.toV? query.push "from=#{diff.fromV}", "to=#{diff.toV}" url += "?" + query.join("&") @@ -194,12 +191,12 @@ define [ return {text, highlights} _loadUpdates: (updates = []) -> + console.log "FOO" previousUpdate = @$scope.history.updates[@$scope.history.updates.length - 1] + console.log "BAR", updates - for update in updates - for doc_path, doc of update.docs or {} - doc.path = doc_path - doc.entity = @ide.fileTreeManager.findEntityByPath(doc_path, includeDeleted: true) + for update in updates or [] + console.log "_loadUpdates, loading", update for user in update.meta.users or [] if user? @@ -214,6 +211,7 @@ define [ previousUpdate = update + console.log("BAZ") firstLoad = @$scope.history.updates.length == 0 @$scope.history.updates = @@ -223,59 +221,56 @@ define [ @autoSelectRecentUpdates() if firstLoad _perDocSummaryOfUpdates: (updates) -> - current_paths = {} + current_pathnames = {} docs_summary = {} for update in updates # Updates are reverse chronologically ordered console.log "[_perDocSummaryOfUpdates] update", update - if update.docs? - for doc_path, doc of update.docs - # doc_path may not be the latest doc path that this doc has had - if !current_paths[doc_path]? - current_paths[doc_path] = doc_path - current_path = current_paths[doc_path] - console.log "[_perDocSummaryOfUpdates] doc", doc, current_path - if !docs_summary[current_path]? - # todo start_ts and end_ts - docs_summary[current_path] = { - fromV: doc.fromV, toV: doc.toV, - original_path: doc_path - } - else - docs_summary[current_path] = { - fromV: Math.min(docs_summary[current_path].fromV, doc.fromV), - toV: Math.max(docs_summary[current_path].toV, doc.toV), - original_path: doc_path - } - else if update.renames? - for rename in update.renames + for pathname in update.docs or [] + # current_pathname may not be the latest doc path that this doc has had + if !current_pathnames[pathname]? + current_pathnames[pathname] = pathname + current_pathname = current_pathnames[pathname] + if !docs_summary[current_pathname]? + docs_summary[current_pathname] = { + fromV: update.fromV, toV: update.toV, + pathname: pathname + } + console.log "[_perDocSummaryOfUpdates] creating summary", current_pathname, docs_summary[current_pathname] + else + console.log "[_perDocSummaryOfUpdates] updating summary", docs_summary[current_pathname], update + docs_summary[current_pathname] = { + fromV: Math.min(docs_summary[current_pathname].fromV, update.fromV), + toV: Math.max(docs_summary[current_pathname].toV, update.toV), + pathname: pathname + } + for project_op in update.project_ops or [] + if project_op.rename? + rename = project_op.rename console.log "[_perDocSummaryOfUpdates] rename", rename - if !current_paths[rename.newPathname]? - current_paths[rename.newPathname] = rename.newPathname - current_paths[rename.pathname] = current_paths[rename.newPathname] - delete current_paths[rename.newPathname] + if !current_pathnames[rename.newPathname]? + current_pathnames[rename.newPathname] = rename.newPathname + current_pathnames[rename.current_pathname] = current_pathnames[rename.newPathname] + delete current_pathnames[rename.newPathname] console.log "[_perDocSummaryOfUpdates] docs_summary", docs_summary - console.log "[_perDocSummaryOfUpdates] current_paths", current_paths + console.log "[_perDocSummaryOfUpdates] current_pathnames", current_pathnames return docs_summary _calculateDiffDataFromSelection: () -> - fromV = toV = start_ts = end_ts = original_path = null + fromV = toV = pathname = null - selected_doc_path = @$scope.history.selection.doc_path - console.log "[_calculateDiffDataFromSelection] selected_doc_path", selected_doc_path + selected_pathname = @$scope.history.selection.pathname + console.log "[_calculateDiffDataFromSelection] selected_pathname", selected_pathname - for doc_path, doc of @_perDocSummaryOfUpdates(@$scope.history.selection.updates) - if doc_path == selected_doc_path - fromV = doc.fromV - toV = doc.toV - start_ts = doc.start_ts - end_ts = doc.end_ts - original_path = doc.original_path + for pathname, doc of @_perDocSummaryOfUpdates(@$scope.history.selection.updates) + console.log "[_calculateDiffDataFromSelection] pathname, doc", pathname, doc + if pathname == selected_pathname + {fromV, toV, pathname} = doc break - return {fromV, toV, start_ts, end_ts, original_path} + return {fromV, toV, pathname} # Set the track changes selected doc to one of the docs in the range # of currently selected updates. If we already have a selected doc @@ -284,21 +279,21 @@ define [ affected_docs = @_perDocSummaryOfUpdates(@$scope.history.selection.updates) console.log "[_selectDocFromUpdates] affected_docs", affected_docs - selected_doc_path = @$scope.history.selection.doc_path - console.log "[_selectDocFromUpdates] current selected_doc_path", selected_doc_path - if selected_doc_path? and affected_docs[selected_doc_path] + selected_pathname = @$scope.history.selection.pathname + console.log "[_selectDocFromUpdates] current selected_pathname", selected_pathname + if selected_pathname? and affected_docs[selected_pathname] # Selected doc is already open else # Set to first possible candidate - for doc_path, doc of affected_docs - selected_doc_path = doc_path + for pathname, doc of affected_docs + selected_pathname = pathname break - console.log "[_selectDocFromUpdates] new selected_doc_path", selected_doc_path + console.log "[_selectDocFromUpdates] new selected_pathname", selected_pathname - @$scope.history.selection.doc_path = selected_doc_path - if selected_doc_path? - entity = @ide.fileTreeManager.findEntityByPath(selected_doc_path) + @$scope.history.selection.pathname = selected_pathname + if selected_pathname? + entity = @ide.fileTreeManager.findEntityByPath(selected_pathname) if entity? @ide.fileTreeManager.selectEntity(entity) From 8ea779af58ebbbc3c6a16ac98e7f158e46fd2b6d Mon Sep 17 00:00:00 2001 From: James Allen Date: Thu, 7 Dec 2017 11:21:49 +0000 Subject: [PATCH 42/56] Add some front end tests for HistoryManager --- services/web/.gitignore | 1 + .../web/bin/{compile_app => compile_backend} | 0 services/web/bin/compile_frontend | 5 ++ services/web/bin/compile_frontend_unit_tests | 5 ++ services/web/bin/frontend_unit_test | 5 ++ services/web/package.json | 10 ++- .../coffee/ide/history/HistoryManager.coffee | 55 ++++-------- .../coffee/HistoryManagerTests.coffee | 89 +++++++++++++++++++ 8 files changed, 130 insertions(+), 40 deletions(-) rename services/web/bin/{compile_app => compile_backend} (100%) create mode 100755 services/web/bin/compile_frontend create mode 100755 services/web/bin/compile_frontend_unit_tests create mode 100755 services/web/bin/frontend_unit_test create mode 100644 services/web/test/unit_frontend/coffee/HistoryManagerTests.coffee diff --git a/services/web/.gitignore b/services/web/.gitignore index 83d4d28b65..d725b0b837 100644 --- a/services/web/.gitignore +++ b/services/web/.gitignore @@ -39,6 +39,7 @@ data/* app.js app/js/* test/unit/js/* +test/unit_frontend/js/* test/smoke/js/* test/acceptance/js/* cookies.txt diff --git a/services/web/bin/compile_app b/services/web/bin/compile_backend similarity index 100% rename from services/web/bin/compile_app rename to services/web/bin/compile_backend diff --git a/services/web/bin/compile_frontend b/services/web/bin/compile_frontend new file mode 100755 index 0000000000..bb1dde7dbb --- /dev/null +++ b/services/web/bin/compile_frontend @@ -0,0 +1,5 @@ +#!/bin/bash +set -e; +COFFEE=node_modules/.bin/coffee +echo Compiling public/coffee; +$COFFEE -o public/js -c public/coffee; diff --git a/services/web/bin/compile_frontend_unit_tests b/services/web/bin/compile_frontend_unit_tests new file mode 100755 index 0000000000..0351ad70cd --- /dev/null +++ b/services/web/bin/compile_frontend_unit_tests @@ -0,0 +1,5 @@ +#!/bin/bash +set -e; +COFFEE=node_modules/.bin/coffee +echo Compiling test/unit_frontend/coffee; +$COFFEE -o test/unit_frontend/js -c test/unit_frontend/coffee; diff --git a/services/web/bin/frontend_unit_test b/services/web/bin/frontend_unit_test new file mode 100755 index 0000000000..599055803a --- /dev/null +++ b/services/web/bin/frontend_unit_test @@ -0,0 +1,5 @@ +#!/bin/bash +set -e; +MOCHA="node_modules/.bin/mocha --recursive --reporter spec" +$MOCHA "$@" test/unit_frontend/js + diff --git a/services/web/package.json b/services/web/package.json index a1cf0fd6e0..b5e843a371 100644 --- a/services/web/package.json +++ b/services/web/package.json @@ -14,11 +14,15 @@ "test:acceptance:run": "bin/acceptance_test $@", "test:acceptance:dir": "npm -q run compile:acceptance_tests && npm -q run test:acceptance:wait_for_app && npm -q run test:acceptance:run -- $@", "test:acceptance": "npm -q run test:acceptance:dir -- $@ test/acceptance/js", - "test:unit": "npm -q run compile:app && npm -q run compile:unit_tests && bin/unit_test $@", + "test:unit": "npm -q run compile:backend && npm -q run compile:unit_tests && bin/unit_test $@", + "test:frontend_unit": "npm -q run compile:frontend && npm -q run compile:frontend_unit_tests && bin/frontend_unit_test $@", "compile:unit_tests": "bin/compile_unit_tests", + "compile:frontend_unit_tests": "bin/compile_frontend_unit_tests", "compile:acceptance_tests": "bin/compile_acceptance_tests", - "compile:app": "bin/compile_app", - "start": "npm -q run compile:app && node app.js" + "compile:frontend": "bin/compile_frontend", + "compile:backend": "bin/compile_backend", + "compile": "npm -q run compile:backend && npm -q run compile:frontend", + "start": "npm -q run compile && node app.js" }, "dependencies": { "archiver": "0.9.0", diff --git a/services/web/public/coffee/ide/history/HistoryManager.coffee b/services/web/public/coffee/ide/history/HistoryManager.coffee index 80a39a3614..5528c1153d 100644 --- a/services/web/public/coffee/ide/history/HistoryManager.coffee +++ b/services/web/public/coffee/ide/history/HistoryManager.coffee @@ -191,13 +191,9 @@ define [ return {text, highlights} _loadUpdates: (updates = []) -> - console.log "FOO" previousUpdate = @$scope.history.updates[@$scope.history.updates.length - 1] - console.log "BAR", updates for update in updates or [] - console.log "_loadUpdates, loading", update - for user in update.meta.users or [] if user? user.hue = ColorManager.getHueForUserId(user.id) @@ -211,50 +207,41 @@ define [ previousUpdate = update - console.log("BAZ") firstLoad = @$scope.history.updates.length == 0 @$scope.history.updates = @$scope.history.updates.concat(updates) - console.log "[_loadUpdates] updates", @$scope.history.updates @autoSelectRecentUpdates() if firstLoad _perDocSummaryOfUpdates: (updates) -> - current_pathnames = {} + # Track current_pathname -> original_pathname + original_pathnames = {} docs_summary = {} - for update in updates # Updates are reverse chronologically ordered - console.log "[_perDocSummaryOfUpdates] update", update + # Put updates in ascending chronological order + updates = updates.slice().reverse() + for update in updates for pathname in update.docs or [] - # current_pathname may not be the latest doc path that this doc has had - if !current_pathnames[pathname]? - current_pathnames[pathname] = pathname - current_pathname = current_pathnames[pathname] - if !docs_summary[current_pathname]? - docs_summary[current_pathname] = { + if !original_pathnames[pathname]? + original_pathnames[pathname] = pathname + original_pathname = original_pathnames[pathname] + if !docs_summary[original_pathname]? + docs_summary[original_pathname] = { fromV: update.fromV, toV: update.toV, - pathname: pathname } - console.log "[_perDocSummaryOfUpdates] creating summary", current_pathname, docs_summary[current_pathname] else - console.log "[_perDocSummaryOfUpdates] updating summary", docs_summary[current_pathname], update - docs_summary[current_pathname] = { - fromV: Math.min(docs_summary[current_pathname].fromV, update.fromV), - toV: Math.max(docs_summary[current_pathname].toV, update.toV), - pathname: pathname + docs_summary[original_pathname] = { + fromV: Math.min(docs_summary[original_pathname].fromV, update.fromV), + toV: Math.max(docs_summary[original_pathname].toV, update.toV), } for project_op in update.project_ops or [] if project_op.rename? rename = project_op.rename - console.log "[_perDocSummaryOfUpdates] rename", rename - if !current_pathnames[rename.newPathname]? - current_pathnames[rename.newPathname] = rename.newPathname - current_pathnames[rename.current_pathname] = current_pathnames[rename.newPathname] - delete current_pathnames[rename.newPathname] - - console.log "[_perDocSummaryOfUpdates] docs_summary", docs_summary - console.log "[_perDocSummaryOfUpdates] current_pathnames", current_pathnames + if !original_pathnames[rename.pathname]? + original_pathnames[rename.pathname] = rename.pathname + original_pathnames[rename.newPathname] = original_pathnames[rename.pathname] + delete original_pathnames[rename.pathname] return docs_summary @@ -262,12 +249,10 @@ define [ fromV = toV = pathname = null selected_pathname = @$scope.history.selection.pathname - console.log "[_calculateDiffDataFromSelection] selected_pathname", selected_pathname for pathname, doc of @_perDocSummaryOfUpdates(@$scope.history.selection.updates) - console.log "[_calculateDiffDataFromSelection] pathname, doc", pathname, doc if pathname == selected_pathname - {fromV, toV, pathname} = doc + {fromV, toV} = doc break return {fromV, toV, pathname} @@ -277,10 +262,8 @@ define [ # then prefer this one if present. _selectDocFromUpdates: () -> affected_docs = @_perDocSummaryOfUpdates(@$scope.history.selection.updates) - console.log "[_selectDocFromUpdates] affected_docs", affected_docs selected_pathname = @$scope.history.selection.pathname - console.log "[_selectDocFromUpdates] current selected_pathname", selected_pathname if selected_pathname? and affected_docs[selected_pathname] # Selected doc is already open else @@ -289,8 +272,6 @@ define [ selected_pathname = pathname break - console.log "[_selectDocFromUpdates] new selected_pathname", selected_pathname - @$scope.history.selection.pathname = selected_pathname if selected_pathname? entity = @ide.fileTreeManager.findEntityByPath(selected_pathname) diff --git a/services/web/test/unit_frontend/coffee/HistoryManagerTests.coffee b/services/web/test/unit_frontend/coffee/HistoryManagerTests.coffee new file mode 100644 index 0000000000..8806cad66c --- /dev/null +++ b/services/web/test/unit_frontend/coffee/HistoryManagerTests.coffee @@ -0,0 +1,89 @@ +Path = require 'path' +SandboxedModule = require "sandboxed-module" +modulePath = Path.join __dirname, '../../../public/js/ide/history/HistoryManager' +sinon = require("sinon") +expect = require("chai").expect + +describe "HistoryManager", -> + beforeEach -> + @moment = {} + @ColorManager = {} + SandboxedModule.require modulePath, globals: + "define": (dependencies, builder) => + @HistoryManager = builder(@moment, @ColorManager) + + @scope = + $watch: sinon.stub() + $on: sinon.stub() + @ide = {} + + @historyManager = new @HistoryManager(@ide, @scope) + + it "should setup the history scope on intialization", -> + expect(@scope.history).to.deep.equal({ + updates: [] + nextBeforeTimestamp: null + atEnd: false + selection: { + updates: [] + doc: null + range: { + fromV: null + toV: null + } + } + diff: null + }) + + describe "_perDocSummaryOfUpdates", -> + it "should return the range of updates for the docs", -> + result = @historyManager._perDocSummaryOfUpdates([{ + docs: ["main.tex"] + fromV: 7, toV: 9 + },{ + docs: ["main.tex", "foo.tex"] + fromV: 4, toV: 6 + },{ + docs: ["main.tex"] + fromV: 3, toV: 3 + },{ + docs: ["foo.tex"] + fromV: 0, toV: 2 + }]) + + expect(result).to.deep.equal({ + "main.tex": { fromV: 3, toV: 9 }, + "foo.tex": { fromV: 0, toV: 6 } + }) + + it "should track renames", -> + result = @historyManager._perDocSummaryOfUpdates([{ + docs: ["main2.tex"] + fromV: 5, toV: 9 + },{ + project_ops: [{ + rename: { + pathname: "main1.tex", + newPathname: "main2.tex" + } + }], + fromV: 4, toV: 4 + },{ + docs: ["main1.tex"] + fromV: 3, toV: 3 + },{ + project_ops: [{ + rename: { + pathname: "main0.tex", + newPathname: "main1.tex" + } + }], + fromV: 2, toV: 2 + },{ + docs: ["main0.tex"] + fromV: 0, toV: 1 + }]) + + expect(result).to.deep.equal({ + "main0.tex": { fromV: 0, toV: 9 } + }) From 50b12e88a25f856fa281a47a9176dff167530c69 Mon Sep 17 00:00:00 2001 From: James Allen Date: Wed, 13 Dec 2017 16:11:12 +0000 Subject: [PATCH 43/56] Add HistoryV2Manager alongside existing HistoryManager --- .../Features/Project/ProjectController.coffee | 3 +- services/web/app/views/project/editor.pug | 2 +- .../web/app/views/project/editor/history.pug | 6 +- services/web/public/coffee/ide.coffee | 7 +- .../coffee/ide/history/HistoryManager.coffee | 119 +++---- .../ide/history/HistoryV2Manager.coffee | 298 ++++++++++++++++++ ...ts.coffee => HistoryManagerV2Tests.coffee} | 66 +++- 7 files changed, 415 insertions(+), 86 deletions(-) create mode 100644 services/web/public/coffee/ide/history/HistoryV2Manager.coffee rename services/web/test/unit_frontend/coffee/{HistoryManagerTests.coffee => HistoryManagerV2Tests.coffee} (55%) diff --git a/services/web/app/coffee/Features/Project/ProjectController.coffee b/services/web/app/coffee/Features/Project/ProjectController.coffee index da22373870..fb27611bca 100644 --- a/services/web/app/coffee/Features/Project/ProjectController.coffee +++ b/services/web/app/coffee/Features/Project/ProjectController.coffee @@ -216,7 +216,7 @@ module.exports = ProjectController = project: (cb)-> ProjectGetter.getProject( project_id, - { name: 1, lastUpdated: 1, track_changes: 1, owner_ref: 1 }, + { name: 1, lastUpdated: 1, track_changes: 1, owner_ref: 1, 'overleaf.history.display': 1 }, cb ) user: (cb)-> @@ -351,6 +351,7 @@ module.exports = ProjectController = themes: THEME_LIST maxDocLength: Settings.max_doc_length showLinkSharingOnboarding: !!results.couldShowLinkSharingOnboarding + useV2History: !!project.overleaf?.history?.display timer.done() _buildProjectList: (allProjects, v1Projects = [])-> diff --git a/services/web/app/views/project/editor.pug b/services/web/app/views/project/editor.pug index d1d0d94cf3..4a35b930ff 100644 --- a/services/web/app/views/project/editor.pug +++ b/services/web/app/views/project/editor.pug @@ -105,7 +105,7 @@ block requirejs //- We need to do .replace(/\//g, '\\/') do that '' -> '<\/script>' //- and doesn't prematurely end the script tag. script#data(type="application/json"). - !{JSON.stringify({userSettings: userSettings, user: user, trackChangesState: trackChangesState}).replace(/\//g, '\\/')} + !{JSON.stringify({userSettings: userSettings, user: user, trackChangesState: trackChangesState, useV2History: useV2History}).replace(/\//g, '\\/')} script(type="text/javascript"). window.data = JSON.parse($("#data").text()); diff --git a/services/web/app/views/project/editor/history.pug b/services/web/app/views/project/editor/history.pug index 102d3e05f8..bdc834352f 100644 --- a/services/web/app/views/project/editor/history.pug +++ b/services/web/app/views/project/editor/history.pug @@ -134,11 +134,13 @@ div#history(ng-show="ui.view == 'history'") div.description(ng-click="select()") div.time {{ update.meta.end_ts | formatDate:'h:mm a' }} - div.docs(ng-repeat="pathname in update.docs", ng-if="update.docs") + div.docs(ng-repeat="pathname in update.pathnames") span.doc {{ pathname }} - div.docs(ng-repeat="project_op in update.project_ops", ng-if="update.project_ops") + div.docs(ng-repeat="project_op in update.project_ops") span(ng-if="project_op.rename") | Renamed {{ project_op.rename.pathname }} to {{ project_op.rename.newPathname }} + span(ng-if="project_op.add") + | Added {{ project_op.add.pathname }} div.users div.user(ng-repeat="update_user in update.meta.users") .color-square(ng-if="update_user != null", ng-style="{'background-color': 'hsl({{ update_user.hue }}, 70%, 50%)'}") diff --git a/services/web/public/coffee/ide.coffee b/services/web/public/coffee/ide.coffee index dbf6a2724e..3d514d093c 100644 --- a/services/web/public/coffee/ide.coffee +++ b/services/web/public/coffee/ide.coffee @@ -5,6 +5,7 @@ define [ "ide/editor/EditorManager" "ide/online-users/OnlineUsersManager" "ide/history/HistoryManager" + "ide/history/HistoryV2Manager" "ide/permissions/PermissionsManager" "ide/pdf/PdfManager" "ide/binary-files/BinaryFilesManager" @@ -44,6 +45,7 @@ define [ EditorManager OnlineUsersManager HistoryManager + HistoryV2Manager PermissionsManager PdfManager BinaryFilesManager @@ -137,7 +139,10 @@ define [ ide.fileTreeManager = new FileTreeManager(ide, $scope) ide.editorManager = new EditorManager(ide, $scope) ide.onlineUsersManager = new OnlineUsersManager(ide, $scope) - ide.historyManager = new HistoryManager(ide, $scope) + if window.data.useV2History + ide.historyManager = new HistoryV2Manager(ide, $scope) + else + ide.historyManager = new HistoryManager(ide, $scope) ide.pdfManager = new PdfManager(ide, $scope) ide.permissionsManager = new PermissionsManager(ide, $scope) ide.binaryFilesManager = new BinaryFilesManager(ide, $scope) diff --git a/services/web/public/coffee/ide/history/HistoryManager.coffee b/services/web/public/coffee/ide/history/HistoryManager.coffee index 5528c1153d..b2ecc8c8ef 100644 --- a/services/web/public/coffee/ide/history/HistoryManager.coffee +++ b/services/web/public/coffee/ide/history/HistoryManager.coffee @@ -22,8 +22,7 @@ define [ @$scope.$on "entity:selected", (event, entity) => if (@$scope.ui.view == "history") and (entity.type == "doc") - # TODO: Set selection.pathname to entity path name - # @$scope.history.selection.doc = entity + @$scope.history.selection.doc = entity @reloadDiff() show: () -> @@ -46,6 +45,8 @@ define [ range: { fromV: null toV: null + start_ts: null + end_ts: null } } diff: null @@ -74,7 +75,6 @@ define [ .get(url) .then (response) => { data } = response - console.log "fetchNextBatchOfUpdates", data.updates @_loadUpdates(data.updates) @$scope.history.nextBeforeTimestamp = data.nextBeforeTimestamp if !data.nextBeforeTimestamp? @@ -83,34 +83,30 @@ define [ reloadDiff: () -> diff = @$scope.history.diff - {updates} = @$scope.history.selection - {fromV, toV, pathname} = @_calculateDiffDataFromSelection() - console.log "[reloadDiff] current diff", diff - console.log "[reloadDiff] new diff data", {fromV, toV, pathname} + {updates, doc} = @$scope.history.selection + {fromV, toV, start_ts, end_ts} = @_calculateRangeFromSelection() - return if !pathname? + return if !doc? return if diff? and - diff.pathname == pathname and - diff.fromV == fromV and - diff.toV == toV + diff.doc == doc and + diff.fromV == fromV and + diff.toV == toV @$scope.history.diff = diff = { fromV: fromV toV: toV - pathname: pathname + start_ts: start_ts + end_ts: end_ts + doc: doc error: false } - # TODO: How do we track deleted files now? We can probably show the diffs easily - # with the new system! - if true # !doc.deleted + if !doc.deleted diff.loading = true - url = "/project/#{@$scope.project_id}/diff" - query = ["pathname=#{encodeURIComponent(pathname)}"] + url = "/project/#{@$scope.project_id}/doc/#{diff.doc.id}/diff" if diff.fromV? and diff.toV? - query.push "from=#{diff.fromV}", "to=#{diff.toV}" - url += "?" + query.join("&") + url += "?from=#{diff.fromV}&to=#{diff.toV}" @ide.$http .get(url) @@ -193,7 +189,12 @@ define [ _loadUpdates: (updates = []) -> previousUpdate = @$scope.history.updates[@$scope.history.updates.length - 1] - for update in updates or [] + for update in updates + update.pathnames = [] # Used for display + for doc_id, doc of update.docs or {} + doc.entity = @ide.fileTreeManager.findEntityById(doc_id, includeDeleted: true) + update.pathnames.push doc.entity.name + for user in update.meta.users or [] if user? user.hue = ColorManager.getHueForUserId(user.id) @@ -214,69 +215,47 @@ define [ @autoSelectRecentUpdates() if firstLoad - _perDocSummaryOfUpdates: (updates) -> - # Track current_pathname -> original_pathname - original_pathnames = {} - docs_summary = {} + _calculateRangeFromSelection: () -> + fromV = toV = start_ts = end_ts = null - # Put updates in ascending chronological order - updates = updates.slice().reverse() - for update in updates - for pathname in update.docs or [] - if !original_pathnames[pathname]? - original_pathnames[pathname] = pathname - original_pathname = original_pathnames[pathname] - if !docs_summary[original_pathname]? - docs_summary[original_pathname] = { - fromV: update.fromV, toV: update.toV, - } - else - docs_summary[original_pathname] = { - fromV: Math.min(docs_summary[original_pathname].fromV, update.fromV), - toV: Math.max(docs_summary[original_pathname].toV, update.toV), - } - for project_op in update.project_ops or [] - if project_op.rename? - rename = project_op.rename - if !original_pathnames[rename.pathname]? - original_pathnames[rename.pathname] = rename.pathname - original_pathnames[rename.newPathname] = original_pathnames[rename.pathname] - delete original_pathnames[rename.pathname] + selected_doc_id = @$scope.history.selection.doc?.id - return docs_summary + for update in @$scope.history.selection.updates or [] + for doc_id, doc of update.docs + if doc_id == selected_doc_id + if fromV? and toV? + fromV = Math.min(fromV, doc.fromV) + toV = Math.max(toV, doc.toV) + start_ts = Math.min(start_ts, update.meta.start_ts) + end_ts = Math.max(end_ts, update.meta.end_ts) + else + fromV = doc.fromV + toV = doc.toV + start_ts = update.meta.start_ts + end_ts = update.meta.end_ts + break - _calculateDiffDataFromSelection: () -> - fromV = toV = pathname = null - - selected_pathname = @$scope.history.selection.pathname - - for pathname, doc of @_perDocSummaryOfUpdates(@$scope.history.selection.updates) - if pathname == selected_pathname - {fromV, toV} = doc - break - - return {fromV, toV, pathname} + return {fromV, toV, start_ts, end_ts} # Set the track changes selected doc to one of the docs in the range # of currently selected updates. If we already have a selected doc # then prefer this one if present. _selectDocFromUpdates: () -> - affected_docs = @_perDocSummaryOfUpdates(@$scope.history.selection.updates) + affected_docs = {} + for update in @$scope.history.selection.updates + for doc_id, doc of update.docs + affected_docs[doc_id] = doc.entity - selected_pathname = @$scope.history.selection.pathname - if selected_pathname? and affected_docs[selected_pathname] + selected_doc = @$scope.history.selection.doc + if selected_doc? and affected_docs[selected_doc.id]? # Selected doc is already open else - # Set to first possible candidate - for pathname, doc of affected_docs - selected_pathname = pathname + for doc_id, doc of affected_docs + selected_doc = doc break - @$scope.history.selection.pathname = selected_pathname - if selected_pathname? - entity = @ide.fileTreeManager.findEntityByPath(selected_pathname) - if entity? - @ide.fileTreeManager.selectEntity(entity) + @$scope.history.selection.doc = selected_doc + @ide.fileTreeManager.selectEntity(selected_doc) _updateContainsUserId: (update, user_id) -> for user in update.meta.users diff --git a/services/web/public/coffee/ide/history/HistoryV2Manager.coffee b/services/web/public/coffee/ide/history/HistoryV2Manager.coffee new file mode 100644 index 0000000000..d7e80d5153 --- /dev/null +++ b/services/web/public/coffee/ide/history/HistoryV2Manager.coffee @@ -0,0 +1,298 @@ +define [ + "moment" + "ide/colors/ColorManager" + "ide/history/controllers/HistoryListController" + "ide/history/controllers/HistoryDiffController" + "ide/history/directives/infiniteScroll" +], (moment, ColorManager) -> + class HistoryManager + constructor: (@ide, @$scope) -> + @reset() + + @$scope.toggleHistory = () => + if @$scope.ui.view == "history" + @hide() + else + @show() + + @$scope.$watch "history.selection.updates", (updates) => + if updates? and updates.length > 0 + @_selectDocFromUpdates() + @reloadDiff() + + @$scope.$on "entity:selected", (event, entity) => + if (@$scope.ui.view == "history") and (entity.type == "doc") + # TODO: Set selection.pathname to entity path name + # @$scope.history.selection.doc = entity + @reloadDiff() + + show: () -> + @$scope.ui.view = "history" + @reset() + + hide: () -> + @$scope.ui.view = "editor" + # Make sure we run the 'open' logic for whatever is currently selected + @$scope.$emit "entity:selected", @ide.fileTreeManager.findSelectedEntity() + + reset: () -> + @$scope.history = { + updates: [] + nextBeforeTimestamp: null + atEnd: false + selection: { + updates: [] + doc: null + range: { + fromV: null + toV: null + } + } + diff: null + } + + autoSelectRecentUpdates: () -> + return if @$scope.history.updates.length == 0 + + @$scope.history.updates[0].selectedTo = true + + indexOfLastUpdateNotByMe = 0 + for update, i in @$scope.history.updates + if @_updateContainsUserId(update, @$scope.user.id) + break + indexOfLastUpdateNotByMe = i + + @$scope.history.updates[indexOfLastUpdateNotByMe].selectedFrom = true + + BATCH_SIZE: 10 + fetchNextBatchOfUpdates: () -> + url = "/project/#{@ide.project_id}/updates?min_count=#{@BATCH_SIZE}" + if @$scope.history.nextBeforeTimestamp? + url += "&before=#{@$scope.history.nextBeforeTimestamp}" + @$scope.history.loading = true + @ide.$http + .get(url) + .then (response) => + { data } = response + console.log "fetchNextBatchOfUpdates", data.updates + @_loadUpdates(data.updates) + @$scope.history.nextBeforeTimestamp = data.nextBeforeTimestamp + if !data.nextBeforeTimestamp? + @$scope.history.atEnd = true + @$scope.history.loading = false + + reloadDiff: () -> + diff = @$scope.history.diff + {updates} = @$scope.history.selection + {fromV, toV, pathname} = @_calculateDiffDataFromSelection() + console.log "[reloadDiff] current diff", diff + console.log "[reloadDiff] new diff data", {fromV, toV, pathname} + + return if !pathname? + + return if diff? and + diff.pathname == pathname and + diff.fromV == fromV and + diff.toV == toV + + @$scope.history.diff = diff = { + fromV: fromV + toV: toV + pathname: pathname + error: false + } + + # TODO: How do we track deleted files now? We can probably show the diffs easily + # with the new system! + if true # !doc.deleted + diff.loading = true + url = "/project/#{@$scope.project_id}/diff" + query = ["pathname=#{encodeURIComponent(pathname)}"] + if diff.fromV? and diff.toV? + query.push "from=#{diff.fromV}", "to=#{diff.toV}" + url += "?" + query.join("&") + + @ide.$http + .get(url) + .then (response) => + { data } = response + diff.loading = false + {text, highlights} = @_parseDiff(data) + diff.text = text + diff.highlights = highlights + .catch () -> + diff.loading = false + diff.error = true + else + diff.deleted = true + diff.restoreInProgress = false + diff.restoreDeletedSuccess = false + diff.restoredDocNewId = null + + restoreDeletedDoc: (doc) -> + url = "/project/#{@$scope.project_id}/doc/#{doc.id}/restore" + @ide.$http.post(url, name: doc.name, _csrf: window.csrfToken) + + restoreDiff: (diff) -> + url = "/project/#{@$scope.project_id}/doc/#{diff.doc.id}/version/#{diff.fromV}/restore" + @ide.$http.post(url, _csrf: window.csrfToken) + + _parseDiff: (diff) -> + row = 0 + column = 0 + highlights = [] + text = "" + for entry, i in diff.diff or [] + content = entry.u or entry.i or entry.d + content ||= "" + text += content + lines = content.split("\n") + startRow = row + startColumn = column + if lines.length > 1 + endRow = startRow + lines.length - 1 + endColumn = lines[lines.length - 1].length + else + endRow = startRow + endColumn = startColumn + lines[0].length + row = endRow + column = endColumn + + range = { + start: + row: startRow + column: startColumn + end: + row: endRow + column: endColumn + } + + if entry.i? or entry.d? + if entry.meta.user? + name = "#{entry.meta.user.first_name} #{entry.meta.user.last_name}" + else + name = "Anonymous" + if entry.meta.user?.id == @$scope.user.id + name = "you" + date = moment(entry.meta.end_ts).format("Do MMM YYYY, h:mm a") + if entry.i? + highlights.push { + label: "Added by #{name} on #{date}" + highlight: range + hue: ColorManager.getHueForUserId(entry.meta.user?.id) + } + else if entry.d? + highlights.push { + label: "Deleted by #{name} on #{date}" + strikeThrough: range + hue: ColorManager.getHueForUserId(entry.meta.user?.id) + } + + return {text, highlights} + + _loadUpdates: (updates = []) -> + previousUpdate = @$scope.history.updates[@$scope.history.updates.length - 1] + + for update in updates or [] + for user in update.meta.users or [] + if user? + user.hue = ColorManager.getHueForUserId(user.id) + + if !previousUpdate? or !moment(previousUpdate.meta.end_ts).isSame(update.meta.end_ts, "day") + update.meta.first_in_day = true + + update.selectedFrom = false + update.selectedTo = false + update.inSelection = false + + previousUpdate = update + + firstLoad = @$scope.history.updates.length == 0 + + @$scope.history.updates = + @$scope.history.updates.concat(updates) + + @autoSelectRecentUpdates() if firstLoad + + _perDocSummaryOfUpdates: (updates) -> + # Track current_pathname -> original_pathname + original_pathnames = {} + + # Map of original pathname -> doc summary + docs_summary = {} + + updatePathnameWithUpdateVersions = (pathname, update) -> + # docs_summary is indexed by the original pathname the doc + # had at the start, so we have to look this up from the current + # pathname via original_pathname first + if !original_pathnames[pathname]? + original_pathnames[pathname] = pathname + original_pathname = original_pathnames[pathname] + doc_summary = docs_summary[original_pathname] ?= { + fromV: update.fromV, toV: update.toV, + } + doc_summary.fromV = Math.min( + doc_summary.fromV, + update.fromV + ) + doc_summary.toV = Math.max( + doc_summary.toV, + update.toV + ) + + # Put updates in ascending chronological order + updates = updates.slice().reverse() + for update in updates + for pathname in update.pathnames or [] + updatePathnameWithUpdateVersions(pathname, update) + for project_op in update.project_ops or [] + if project_op.rename? + rename = project_op.rename + updatePathnameWithUpdateVersions(rename.pathname, update) + original_pathnames[rename.newPathname] = original_pathnames[rename.pathname] + delete original_pathnames[rename.pathname] + if project_op.add? + add = project_op.add + updatePathnameWithUpdateVersions(add.pathname, update) + + return docs_summary + + _calculateDiffDataFromSelection: () -> + fromV = toV = pathname = null + + selected_pathname = @$scope.history.selection.pathname + + console.log "[_calculateDiffDataFromSelection]", @$scope.history.selection.updates + for pathname, doc of @_perDocSummaryOfUpdates(@$scope.history.selection.updates) + console.log "[_calculateDiffDataFromSelection] doc:", pathname, doc + if pathname == selected_pathname + {fromV, toV} = doc + break + + return {fromV, toV, pathname} + + # Set the track changes selected doc to one of the docs in the range + # of currently selected updates. If we already have a selected doc + # then prefer this one if present. + _selectDocFromUpdates: () -> + affected_docs = @_perDocSummaryOfUpdates(@$scope.history.selection.updates) + + selected_pathname = @$scope.history.selection.pathname + if selected_pathname? and affected_docs[selected_pathname] + # Selected doc is already open + else + # Set to first possible candidate + for pathname, doc of affected_docs + selected_pathname = pathname + break + + @$scope.history.selection.pathname = selected_pathname + if selected_pathname? + entity = @ide.fileTreeManager.findEntityByPath(selected_pathname) + if entity? + @ide.fileTreeManager.selectEntity(entity) + + _updateContainsUserId: (update, user_id) -> + for user in update.meta.users + return true if user?.id == user_id + return false diff --git a/services/web/test/unit_frontend/coffee/HistoryManagerTests.coffee b/services/web/test/unit_frontend/coffee/HistoryManagerV2Tests.coffee similarity index 55% rename from services/web/test/unit_frontend/coffee/HistoryManagerTests.coffee rename to services/web/test/unit_frontend/coffee/HistoryManagerV2Tests.coffee index 8806cad66c..4517fc16dd 100644 --- a/services/web/test/unit_frontend/coffee/HistoryManagerTests.coffee +++ b/services/web/test/unit_frontend/coffee/HistoryManagerV2Tests.coffee @@ -1,23 +1,23 @@ Path = require 'path' SandboxedModule = require "sandboxed-module" -modulePath = Path.join __dirname, '../../../public/js/ide/history/HistoryManager' +modulePath = Path.join __dirname, '../../../public/js/ide/history/HistoryV2Manager' sinon = require("sinon") expect = require("chai").expect -describe "HistoryManager", -> +describe "HistoryV2Manager", -> beforeEach -> @moment = {} @ColorManager = {} SandboxedModule.require modulePath, globals: "define": (dependencies, builder) => - @HistoryManager = builder(@moment, @ColorManager) + @HistoryV2Manager = builder(@moment, @ColorManager) @scope = $watch: sinon.stub() $on: sinon.stub() @ide = {} - @historyManager = new @HistoryManager(@ide, @scope) + @historyManager = new @HistoryV2Manager(@ide, @scope) it "should setup the history scope on intialization", -> expect(@scope.history).to.deep.equal({ @@ -38,16 +38,16 @@ describe "HistoryManager", -> describe "_perDocSummaryOfUpdates", -> it "should return the range of updates for the docs", -> result = @historyManager._perDocSummaryOfUpdates([{ - docs: ["main.tex"] + pathnames: ["main.tex"] fromV: 7, toV: 9 },{ - docs: ["main.tex", "foo.tex"] + pathnames: ["main.tex", "foo.tex"] fromV: 4, toV: 6 },{ - docs: ["main.tex"] + pathnames: ["main.tex"] fromV: 3, toV: 3 },{ - docs: ["foo.tex"] + pathnames: ["foo.tex"] fromV: 0, toV: 2 }]) @@ -58,7 +58,7 @@ describe "HistoryManager", -> it "should track renames", -> result = @historyManager._perDocSummaryOfUpdates([{ - docs: ["main2.tex"] + pathnames: ["main2.tex"] fromV: 5, toV: 9 },{ project_ops: [{ @@ -69,7 +69,7 @@ describe "HistoryManager", -> }], fromV: 4, toV: 4 },{ - docs: ["main1.tex"] + pathnames: ["main1.tex"] fromV: 3, toV: 3 },{ project_ops: [{ @@ -80,10 +80,54 @@ describe "HistoryManager", -> }], fromV: 2, toV: 2 },{ - docs: ["main0.tex"] + pathnames: ["main0.tex"] fromV: 0, toV: 1 }]) expect(result).to.deep.equal({ "main0.tex": { fromV: 0, toV: 9 } }) + + it "should track single renames", -> + result = @historyManager._perDocSummaryOfUpdates([{ + project_ops: [{ + rename: { + pathname: "main1.tex", + newPathname: "main2.tex" + } + }], + fromV: 4, toV: 5 + }]) + + expect(result).to.deep.equal({ + "main1.tex": { fromV: 4, toV: 5 } + }) + + it "should track additions", -> + result = @historyManager._perDocSummaryOfUpdates([{ + project_ops: [{ + add: + pathname: "main.tex" + }] + fromV: 0, toV: 1 + }, { + pathnames: ["main.tex"] + fromV: 1, toV: 4 + }]) + + expect(result).to.deep.equal({ + "main.tex": { fromV: 0, toV: 4 } + }) + + it "should track single additions", -> + result = @historyManager._perDocSummaryOfUpdates([{ + project_ops: [{ + add: + pathname: "main.tex" + }] + fromV: 0, toV: 1 + }]) + + expect(result).to.deep.equal({ + "main.tex": { fromV: 0, toV: 1 } + }) From 8a3fadbfc1356cd6d2eca67c3fb2b59b5d3403ca Mon Sep 17 00:00:00 2001 From: James Allen Date: Wed, 13 Dec 2017 16:48:48 +0000 Subject: [PATCH 44/56] Style the edit/add/rename options --- services/web/app/views/project/editor/history.pug | 13 +++++++++---- .../coffee/ide/history/HistoryManager.coffee | 1 + .../coffee/ide/history/HistoryV2Manager.coffee | 9 +-------- .../web/public/stylesheets/app/editor/history.less | 14 ++++++++++++-- .../coffee/HistoryManagerV2Tests.coffee | 1 + 5 files changed, 24 insertions(+), 14 deletions(-) diff --git a/services/web/app/views/project/editor/history.pug b/services/web/app/views/project/editor/history.pug index bdc834352f..ba7740c199 100644 --- a/services/web/app/views/project/editor/history.pug +++ b/services/web/app/views/project/editor/history.pug @@ -135,12 +135,17 @@ div#history(ng-show="ui.view == 'history'") div.description(ng-click="select()") div.time {{ update.meta.end_ts | formatDate:'h:mm a' }} div.docs(ng-repeat="pathname in update.pathnames") + i.fa.fa-pencil(ng-if="history.isV2") span.doc {{ pathname }} div.docs(ng-repeat="project_op in update.project_ops") span(ng-if="project_op.rename") - | Renamed {{ project_op.rename.pathname }} to {{ project_op.rename.newPathname }} + span Renamed + span.doc {{ project_op.rename.pathname }} + span to + span.doc {{ project_op.rename.newPathname }} span(ng-if="project_op.add") - | Added {{ project_op.add.pathname }} + span Added + span.doc {{ project_op.add.pathname }} div.users div.user(ng-repeat="update_user in update.meta.users") .color-square(ng-if="update_user != null", ng-style="{'background-color': 'hsl({{ update_user.hue }}, 70%, 50%)'}") @@ -170,8 +175,8 @@ div#history(ng-show="ui.view == 'history'") 'other': 'changes'\ }" ) - | in {{history.diff.doc.name}} - .toolbar-right + | in {{history.diff.pathname}} + .toolbar-right(ng-if="!history.isV2") a.btn.btn-danger.btn-sm( href, ng-click="openRestoreDiffModal()" diff --git a/services/web/public/coffee/ide/history/HistoryManager.coffee b/services/web/public/coffee/ide/history/HistoryManager.coffee index b2ecc8c8ef..f896fbb8b4 100644 --- a/services/web/public/coffee/ide/history/HistoryManager.coffee +++ b/services/web/public/coffee/ide/history/HistoryManager.coffee @@ -100,6 +100,7 @@ define [ end_ts: end_ts doc: doc error: false + pathname: doc.name } if !doc.deleted diff --git a/services/web/public/coffee/ide/history/HistoryV2Manager.coffee b/services/web/public/coffee/ide/history/HistoryV2Manager.coffee index d7e80d5153..01c9a6ad2d 100644 --- a/services/web/public/coffee/ide/history/HistoryV2Manager.coffee +++ b/services/web/public/coffee/ide/history/HistoryV2Manager.coffee @@ -37,6 +37,7 @@ define [ reset: () -> @$scope.history = { + isV2: true updates: [] nextBeforeTimestamp: null atEnd: false @@ -129,14 +130,6 @@ define [ diff.restoreDeletedSuccess = false diff.restoredDocNewId = null - restoreDeletedDoc: (doc) -> - url = "/project/#{@$scope.project_id}/doc/#{doc.id}/restore" - @ide.$http.post(url, name: doc.name, _csrf: window.csrfToken) - - restoreDiff: (diff) -> - url = "/project/#{@$scope.project_id}/doc/#{diff.doc.id}/version/#{diff.fromV}/restore" - @ide.$http.post(url, _csrf: window.csrfToken) - _parseDiff: (diff) -> row = 0 column = 0 diff --git a/services/web/public/stylesheets/app/editor/history.less b/services/web/public/stylesheets/app/editor/history.less index b6dca4b7cc..558f460e3c 100644 --- a/services/web/public/stylesheets/app/editor/history.less +++ b/services/web/public/stylesheets/app/editor/history.less @@ -169,10 +169,20 @@ font-size: 0.8rem; line-height: @line-height-computed; } - .docs { - font-weight: bold; + .action { font-size: 0.9rem; } + .docs { + font-size: 0.9rem; + .doc { + font-weight: bold; + } + i { + font-size: 0.8rem; + color: @gray; + margin-right: 4px; + } + } } li.loading-changes, li.empty-message { padding: 6px; diff --git a/services/web/test/unit_frontend/coffee/HistoryManagerV2Tests.coffee b/services/web/test/unit_frontend/coffee/HistoryManagerV2Tests.coffee index 4517fc16dd..19b5156b1b 100644 --- a/services/web/test/unit_frontend/coffee/HistoryManagerV2Tests.coffee +++ b/services/web/test/unit_frontend/coffee/HistoryManagerV2Tests.coffee @@ -21,6 +21,7 @@ describe "HistoryV2Manager", -> it "should setup the history scope on intialization", -> expect(@scope.history).to.deep.equal({ + isV2: true updates: [] nextBeforeTimestamp: null atEnd: false From d84580f12dd47ebf69ca207e55205057695596bf Mon Sep 17 00:00:00 2001 From: James Allen Date: Wed, 13 Dec 2017 17:09:55 +0000 Subject: [PATCH 45/56] Label actions with text rather than icons --- .../web/app/views/project/editor/history.pug | 19 +++++++++--------- .../stylesheets/app/editor/history.less | 20 +++++++++---------- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/services/web/app/views/project/editor/history.pug b/services/web/app/views/project/editor/history.pug index ba7740c199..6621cdb2d2 100644 --- a/services/web/app/views/project/editor/history.pug +++ b/services/web/app/views/project/editor/history.pug @@ -134,18 +134,17 @@ div#history(ng-show="ui.view == 'history'") div.description(ng-click="select()") div.time {{ update.meta.end_ts | formatDate:'h:mm a' }} + div.action.action-edited(ng-if="history.isV2 && update.pathnames.length > 0") + | Edited div.docs(ng-repeat="pathname in update.pathnames") - i.fa.fa-pencil(ng-if="history.isV2") - span.doc {{ pathname }} + .doc {{ pathname }} div.docs(ng-repeat="project_op in update.project_ops") - span(ng-if="project_op.rename") - span Renamed - span.doc {{ project_op.rename.pathname }} - span to - span.doc {{ project_op.rename.newPathname }} - span(ng-if="project_op.add") - span Added - span.doc {{ project_op.add.pathname }} + div(ng-if="project_op.rename") + .action Renamed + .doc {{ project_op.rename.pathname }} → {{ project_op.rename.newPathname }} + div(ng-if="project_op.add") + .action Created + .doc {{ project_op.add.pathname }} div.users div.user(ng-repeat="update_user in update.meta.users") .color-square(ng-if="update_user != null", ng-style="{'background-color': 'hsl({{ update_user.hue }}, 70%, 50%)'}") diff --git a/services/web/public/stylesheets/app/editor/history.less b/services/web/public/stylesheets/app/editor/history.less index 558f460e3c..2824fd2e32 100644 --- a/services/web/public/stylesheets/app/editor/history.less +++ b/services/web/public/stylesheets/app/editor/history.less @@ -169,18 +169,18 @@ font-size: 0.8rem; line-height: @line-height-computed; } - .action { + .doc { font-size: 0.9rem; + font-weight: bold; } - .docs { - font-size: 0.9rem; - .doc { - font-weight: bold; - } - i { - font-size: 0.8rem; - color: @gray; - margin-right: 4px; + .action { + color: @gray; + text-transform: uppercase; + font-size: 0.7em; + margin-bottom: -2px; + margin-top: 2px; + &-edited { + margin-top: 0; } } } From d7a26e27e5c7cee5c75127011597fba5e827cfe7 Mon Sep 17 00:00:00 2001 From: James Allen Date: Thu, 14 Dec 2017 09:32:38 +0000 Subject: [PATCH 46/56] Run front end tests in CI --- services/web/Jenkinsfile | 15 ++++++--------- services/web/Makefile | 19 ++++++++++++------- ...tend_unit_tests => compile_frontend_tests} | 0 .../bin/{frontend_unit_test => frontend_test} | 0 services/web/docker-shared.template.yml | 1 + services/web/package.json | 4 ++-- 6 files changed, 21 insertions(+), 18 deletions(-) rename services/web/bin/{compile_frontend_unit_tests => compile_frontend_tests} (100%) rename services/web/bin/{frontend_unit_test => frontend_test} (100%) diff --git a/services/web/Jenkinsfile b/services/web/Jenkinsfile index 6421296330..57df9c7091 100644 --- a/services/web/Jenkinsfile +++ b/services/web/Jenkinsfile @@ -71,16 +71,9 @@ pipeline { } } - stage('Unit Tests') { + stage('Test') { steps { - sh 'make clean install' // Removes js files, so do before compile - sh 'make test_unit MOCHA_ARGS="--reporter=tap"' - } - } - - stage('Acceptance Tests') { - steps { - sh 'make test_acceptance MOCHA_ARGS="--reporter=tap"' + sh 'make ci' } } @@ -155,6 +148,10 @@ pipeline { } post { + always { + sh 'make ci_clean' + } + failure { mail(from: "${EMAIL_ALERT_FROM}", to: "${EMAIL_ALERT_TO}", diff --git a/services/web/Makefile b/services/web/Makefile index 98695a8c1f..558b59e5e6 100644 --- a/services/web/Makefile +++ b/services/web/Makefile @@ -16,9 +16,10 @@ add_dev: docker-shared.yml $(NPM) install --save-dev ${P} install: docker-shared.yml + bin/generate_volumes_file $(NPM) install -clean: +clean: ci_clean rm -f app.js rm -rf app/js rm -rf test/unit/js @@ -30,9 +31,8 @@ clean: rm -rf $$dir/test/unit/js; \ rm -rf $$dir/test/acceptance/js; \ done - # Regenerate docker-shared.yml - not stictly a 'clean', - # but lets `make clean install` work nicely - bin/generate_volumes_file + +ci_clean: # Deletes node_modules volume docker-compose down --volumes @@ -40,11 +40,14 @@ clean: docker-shared.yml: bin/generate_volumes_file -test: test_unit test_acceptance +test: test_unit test_frontend test_acceptance test_unit: docker-shared.yml docker-compose ${DOCKER_COMPOSE_FLAGS} run --rm test_unit npm -q run test:unit -- ${MOCHA_ARGS} +test_frontend: docker-shared.yml + docker-compose ${DOCKER_COMPOSE_FLAGS} run --rm test_unit npm -q run test:frontend -- ${MOCHA_ARGS} + test_acceptance: test_acceptance_app test_acceptance_modules test_acceptance_app: test_acceptance_app_start_service test_acceptance_app_run test_acceptance_app_stop_service @@ -71,7 +74,9 @@ test_acceptance_modules: docker-shared.yml test_acceptance_module: docker-shared.yml cd $(MODULE) && make test_acceptance +ci: install test + .PHONY: - all add install update test test_unit test_acceptance \ + all add install update test test_unit test_unit_frontend test_acceptance \ test_acceptance_start_service test_acceptance_stop_service \ - test_acceptance_run + test_acceptance_run ci ci_clean diff --git a/services/web/bin/compile_frontend_unit_tests b/services/web/bin/compile_frontend_tests similarity index 100% rename from services/web/bin/compile_frontend_unit_tests rename to services/web/bin/compile_frontend_tests diff --git a/services/web/bin/frontend_unit_test b/services/web/bin/frontend_test similarity index 100% rename from services/web/bin/frontend_unit_test rename to services/web/bin/frontend_test diff --git a/services/web/docker-shared.template.yml b/services/web/docker-shared.template.yml index d697e59e96..5f45c48c82 100644 --- a/services/web/docker-shared.template.yml +++ b/services/web/docker-shared.template.yml @@ -24,6 +24,7 @@ services: - ./app/views:/app/app/views:ro - ./config:/app/config - ./test/unit/coffee:/app/test/unit/coffee:ro + - ./test/unit_frontend/coffee:/app/test/unit_frontend/coffee:ro - ./test/acceptance/coffee:/app/test/acceptance/coffee:ro - ./test/acceptance/files:/app/test/acceptance/files:ro - ./test/smoke/coffee:/app/test/smoke/coffee:ro diff --git a/services/web/package.json b/services/web/package.json index b5e843a371..398ce3dc78 100644 --- a/services/web/package.json +++ b/services/web/package.json @@ -15,9 +15,9 @@ "test:acceptance:dir": "npm -q run compile:acceptance_tests && npm -q run test:acceptance:wait_for_app && npm -q run test:acceptance:run -- $@", "test:acceptance": "npm -q run test:acceptance:dir -- $@ test/acceptance/js", "test:unit": "npm -q run compile:backend && npm -q run compile:unit_tests && bin/unit_test $@", - "test:frontend_unit": "npm -q run compile:frontend && npm -q run compile:frontend_unit_tests && bin/frontend_unit_test $@", + "test:frontend": "npm -q run compile:frontend && npm -q run compile:frontend_tests && bin/frontend_test $@", "compile:unit_tests": "bin/compile_unit_tests", - "compile:frontend_unit_tests": "bin/compile_frontend_unit_tests", + "compile:frontend_tests": "bin/compile_frontend_tests", "compile:acceptance_tests": "bin/compile_acceptance_tests", "compile:frontend": "bin/compile_frontend", "compile:backend": "bin/compile_backend", From b0812864ac5214f96dc18bef0e14fcea08a68c01 Mon Sep 17 00:00:00 2001 From: James Allen Date: Thu, 14 Dec 2017 09:43:59 +0000 Subject: [PATCH 47/56] Clean up CI output --- services/web/Jenkinsfile | 2 +- services/web/Makefile | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/services/web/Jenkinsfile b/services/web/Jenkinsfile index 57df9c7091..152d980787 100644 --- a/services/web/Jenkinsfile +++ b/services/web/Jenkinsfile @@ -60,7 +60,7 @@ pipeline { sh 'git config --global core.logallrefupdates false' sh 'mv app/views/external/robots.txt public/robots.txt' sh 'mv app/views/external/googlebdb0f8f7f4a17241.html public/googlebdb0f8f7f4a17241.html' - sh 'npm install' + sh 'npm --quiet install' sh 'npm rebuild' // It's too easy to end up shrinkwrapping to an outdated version of translations. // Ensure translations are always latest, regardless of shrinkwrap diff --git a/services/web/Makefile b/services/web/Makefile index 558b59e5e6..046a0c1a81 100644 --- a/services/web/Makefile +++ b/services/web/Makefile @@ -74,7 +74,9 @@ test_acceptance_modules: docker-shared.yml test_acceptance_module: docker-shared.yml cd $(MODULE) && make test_acceptance -ci: install test +ci: + MOCHA_ARGS="--reporter tap" \ + $(MAKE) install test .PHONY: all add install update test test_unit test_unit_frontend test_acceptance \ From b4a5e5a041c4d111e43e359c2da4368c839ab01f Mon Sep 17 00:00:00 2001 From: James Allen Date: Thu, 14 Dec 2017 10:07:17 +0000 Subject: [PATCH 48/56] Tidy up HistoryV2Manager --- .../ide/history/HistoryV2Manager.coffee | 63 ++++++++----------- 1 file changed, 26 insertions(+), 37 deletions(-) diff --git a/services/web/public/coffee/ide/history/HistoryV2Manager.coffee b/services/web/public/coffee/ide/history/HistoryV2Manager.coffee index 01c9a6ad2d..8198738e16 100644 --- a/services/web/public/coffee/ide/history/HistoryV2Manager.coffee +++ b/services/web/public/coffee/ide/history/HistoryV2Manager.coffee @@ -22,8 +22,7 @@ define [ @$scope.$on "entity:selected", (event, entity) => if (@$scope.ui.view == "history") and (entity.type == "doc") - # TODO: Set selection.pathname to entity path name - # @$scope.history.selection.doc = entity + @$scope.history.selection.pathname = _ide.fileTreeManager.getEntityPath(entity) @reloadDiff() show: () -> @@ -43,7 +42,7 @@ define [ atEnd: false selection: { updates: [] - doc: null + pathname: null range: { fromV: null toV: null @@ -52,6 +51,7 @@ define [ diff: null } + MAX_RECENT_UPDATES_TO_SELECT: 2 autoSelectRecentUpdates: () -> return if @$scope.history.updates.length == 0 @@ -59,7 +59,7 @@ define [ indexOfLastUpdateNotByMe = 0 for update, i in @$scope.history.updates - if @_updateContainsUserId(update, @$scope.user.id) + if @_updateContainsUserId(update, @$scope.user.id) or i > @MAX_RECENT_UPDATES_TO_SELECT break indexOfLastUpdateNotByMe = i @@ -75,7 +75,6 @@ define [ .get(url) .then (response) => { data } = response - console.log "fetchNextBatchOfUpdates", data.updates @_loadUpdates(data.updates) @$scope.history.nextBeforeTimestamp = data.nextBeforeTimestamp if !data.nextBeforeTimestamp? @@ -86,10 +85,10 @@ define [ diff = @$scope.history.diff {updates} = @$scope.history.selection {fromV, toV, pathname} = @_calculateDiffDataFromSelection() - console.log "[reloadDiff] current diff", diff - console.log "[reloadDiff] new diff data", {fromV, toV, pathname} - return if !pathname? + if !pathname? + @$scope.history.diff = null + return return if diff? and diff.pathname == pathname and @@ -103,32 +102,24 @@ define [ error: false } - # TODO: How do we track deleted files now? We can probably show the diffs easily - # with the new system! - if true # !doc.deleted - diff.loading = true - url = "/project/#{@$scope.project_id}/diff" - query = ["pathname=#{encodeURIComponent(pathname)}"] - if diff.fromV? and diff.toV? - query.push "from=#{diff.fromV}", "to=#{diff.toV}" - url += "?" + query.join("&") + diff.loading = true + url = "/project/#{@$scope.project_id}/diff" + query = ["pathname=#{encodeURIComponent(pathname)}"] + if diff.fromV? and diff.toV? + query.push "from=#{diff.fromV}", "to=#{diff.toV}" + url += "?" + query.join("&") - @ide.$http - .get(url) - .then (response) => - { data } = response - diff.loading = false - {text, highlights} = @_parseDiff(data) - diff.text = text - diff.highlights = highlights - .catch () -> - diff.loading = false - diff.error = true - else - diff.deleted = true - diff.restoreInProgress = false - diff.restoreDeletedSuccess = false - diff.restoredDocNewId = null + @ide.$http + .get(url) + .then (response) => + { data } = response + diff.loading = false + {text, highlights} = @_parseDiff(data) + diff.text = text + diff.highlights = highlights + .catch () -> + diff.loading = false + diff.error = true _parseDiff: (diff) -> row = 0 @@ -255,14 +246,12 @@ define [ selected_pathname = @$scope.history.selection.pathname - console.log "[_calculateDiffDataFromSelection]", @$scope.history.selection.updates for pathname, doc of @_perDocSummaryOfUpdates(@$scope.history.selection.updates) - console.log "[_calculateDiffDataFromSelection] doc:", pathname, doc if pathname == selected_pathname {fromV, toV} = doc - break + return {fromV, toV, pathname} - return {fromV, toV, pathname} + return {} # Set the track changes selected doc to one of the docs in the range # of currently selected updates. If we already have a selected doc From 810b5e0e9a9629ba347e1ea88b0ec6a41d06edea Mon Sep 17 00:00:00 2001 From: James Allen Date: Thu, 14 Dec 2017 10:18:17 +0000 Subject: [PATCH 49/56] Fix front end tests --- services/web/docker-shared.template.yml | 7 ++----- .../test/unit_frontend/coffee/HistoryManagerV2Tests.coffee | 2 +- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/services/web/docker-shared.template.yml b/services/web/docker-shared.template.yml index 5f45c48c82..8acc63ef81 100644 --- a/services/web/docker-shared.template.yml +++ b/services/web/docker-shared.template.yml @@ -13,11 +13,8 @@ services: - ./npm-shrinkwrap.json:/app/npm-shrinkwrap.json - node_modules:/app/node_modules - ./bin:/app/bin - # Copying the whole public dir is fine for now, and needed for - # some unit tests to pass, but we will want to isolate the coffee - # and vendor js files, so that the compiled js files are not written - # back to the local filesystem. - - ./public:/app/public + - ./public/coffee:/app/public/coffee:ro + - ./public/js/ace-1.2.5:/app/public/js/ace-1.2.5 - ./app.coffee:/app/app.coffee:ro - ./app/coffee:/app/app/coffee:ro - ./app/templates:/app/app/templates:ro diff --git a/services/web/test/unit_frontend/coffee/HistoryManagerV2Tests.coffee b/services/web/test/unit_frontend/coffee/HistoryManagerV2Tests.coffee index 19b5156b1b..9ad7ba9a2e 100644 --- a/services/web/test/unit_frontend/coffee/HistoryManagerV2Tests.coffee +++ b/services/web/test/unit_frontend/coffee/HistoryManagerV2Tests.coffee @@ -27,7 +27,7 @@ describe "HistoryV2Manager", -> atEnd: false selection: { updates: [] - doc: null + pathname: null range: { fromV: null toV: null From dfe6e26946ddd7f0e4ba9be0a178a032d8e37715 Mon Sep 17 00:00:00 2001 From: James Allen Date: Thu, 14 Dec 2017 14:57:34 +0000 Subject: [PATCH 50/56] test_unit_frontend -> test_frontend in Makefile --- services/web/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/Makefile b/services/web/Makefile index 046a0c1a81..5db188e2b7 100644 --- a/services/web/Makefile +++ b/services/web/Makefile @@ -79,6 +79,6 @@ ci: $(MAKE) install test .PHONY: - all add install update test test_unit test_unit_frontend test_acceptance \ + all add install update test test_unit test_frontend test_acceptance \ test_acceptance_start_service test_acceptance_stop_service \ test_acceptance_run ci ci_clean From 8311101ec0e2a86fcaf0bd88ecb57e15f085aba0 Mon Sep 17 00:00:00 2001 From: James Allen Date: Thu, 14 Dec 2017 15:38:20 +0000 Subject: [PATCH 51/56] Split project_history.enable in initializeHistoryForNewProjects and sendProjectStructureOps --- .../Features/DocumentUpdater/DocumentUpdaterHandler.coffee | 2 +- .../app/coffee/Features/History/HistoryController.coffee | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee index f7dd9f3e17..8dbb52d10b 100644 --- a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee +++ b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee @@ -205,7 +205,7 @@ module.exports = DocumentUpdaterHandler = callback new Error("doc updater returned a non-success status code: #{res.statusCode}") updateProjectStructure : (project_id, userId, changes, callback = (error) ->)-> - return callback() if !settings.apis.project_history?.enabled + return callback() if !settings.apis.project_history?.sendProjectStructureOps docUpdates = DocumentUpdaterHandler._getRenameUpdates('doc', changes.oldDocs, changes.newDocs) fileUpdates = DocumentUpdaterHandler._getRenameUpdates('file', changes.oldFiles, changes.newFiles) diff --git a/services/web/app/coffee/Features/History/HistoryController.coffee b/services/web/app/coffee/Features/History/HistoryController.coffee index 70e0828160..0935118a7c 100644 --- a/services/web/app/coffee/Features/History/HistoryController.coffee +++ b/services/web/app/coffee/Features/History/HistoryController.coffee @@ -6,7 +6,7 @@ ProjectDetailsHandler = require "../Project/ProjectDetailsHandler" module.exports = HistoryController = initializeProject: (callback = (error, history_id) ->) -> - return callback() if !settings.apis.project_history?.enabled + return callback() if !settings.apis.project_history?.initializeHistoryForNewProjects request.post { url: "#{settings.apis.project_history.url}/project" }, (error, res, body)-> @@ -33,7 +33,8 @@ module.exports = HistoryController = # find out which type of history service this project uses ProjectDetailsHandler.getDetails project_id, (err, project) -> return next(err) if err? - if project?.overleaf?.history?.display + history = project.overleaf?.history + if history?.id? and history?.display req.useProjectHistory = true else req.useProjectHistory = false @@ -58,7 +59,7 @@ module.exports = HistoryController = buildHistoryServiceUrl: (useProjectHistory) -> # choose a history service, either document-level (trackchanges) # or project-level (project_history) - if settings.apis.project_history?.enabled && useProjectHistory + if useProjectHistory return settings.apis.project_history.url else return settings.apis.trackchanges.url From ffa2e231fdec7e104bf58ca8eaf4a4e720eb7d2b Mon Sep 17 00:00:00 2001 From: James Allen Date: Fri, 15 Dec 2017 16:11:16 +0000 Subject: [PATCH 52/56] Fix up tests --- services/web/config/settings.defaults.coffee | 3 +- .../DocumentUpdaterHandlerTests.coffee | 4 +- .../History/HistoryControllerTests.coffee | 124 ++++++------------ 3 files changed, 47 insertions(+), 84 deletions(-) diff --git a/services/web/config/settings.defaults.coffee b/services/web/config/settings.defaults.coffee index 65a8bf1e91..844bffae3c 100644 --- a/services/web/config/settings.defaults.coffee +++ b/services/web/config/settings.defaults.coffee @@ -111,7 +111,8 @@ module.exports = settings = trackchanges: url : "http://localhost:3015" project_history: - enabled: process.env.PROJECT_HISTORY_ENABLED == 'true' or false + sendProjectStructureOps: process.env.PROJECT_HISTORY_ENABLED == 'true' or false + initializeHistoryForNewProjects: process.env.PROJECT_HISTORY_ENABLED == 'true' or false url : "http://localhost:3054" docstore: url : "http://#{process.env['DOCSTORE_HOST'] or 'localhost'}:3016" diff --git a/services/web/test/unit/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee b/services/web/test/unit/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee index 14ccaa3a33..5eb3c057ab 100644 --- a/services/web/test/unit/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee +++ b/services/web/test/unit/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee @@ -393,7 +393,7 @@ describe 'DocumentUpdaterHandler', -> describe "with project history disabled", -> beforeEach -> - @settings.apis.project_history.enabled = false + @settings.apis.project_history.sendProjectStructureOps = false @request.post = sinon.stub() @handler.updateProjectStructure @project_id, @user_id, {}, @callback @@ -406,7 +406,7 @@ describe 'DocumentUpdaterHandler', -> describe "with project history enabled", -> beforeEach -> - @settings.apis.project_history.enabled = true + @settings.apis.project_history.sendProjectStructureOps = true @url = "#{@settings.apis.documentupdater.url}/project/#{@project_id}" @request.post = sinon.stub().callsArgWith(1, null, {statusCode: 204}, "") diff --git a/services/web/test/unit/coffee/History/HistoryControllerTests.coffee b/services/web/test/unit/coffee/History/HistoryControllerTests.coffee index e03189526a..3e0464254e 100644 --- a/services/web/test/unit/coffee/History/HistoryControllerTests.coffee +++ b/services/web/test/unit/coffee/History/HistoryControllerTests.coffee @@ -31,7 +31,7 @@ describe "HistoryController", -> describe "for a project with project history", -> beforeEach -> - @ProjectDetailsHandler.getDetails = sinon.stub().callsArgWith(1, null, {overleaf:{history:{display:true}}}) + @ProjectDetailsHandler.getDetails = sinon.stub().callsArgWith(1, null, {overleaf:{history:{id: 42, display:true}}}) @HistoryController.selectHistoryApi @req, @res, @next it "should set the flag for project history to true", -> @@ -57,93 +57,55 @@ describe "HistoryController", -> on: (event, handler) -> @events[event] = handler @request.returns @proxy - describe "with project history enabled", -> + describe "for a project with the project history flag", -> beforeEach -> - @settings.apis.project_history.enabled = true + @req.useProjectHistory = true + @HistoryController.proxyToHistoryApi @req, @res, @next - describe "for a project with the project history flag", -> - beforeEach -> - @req.useProjectHistory = true - @HistoryController.proxyToHistoryApi @req, @res, @next + it "should get the user id", -> + @AuthenticationController.getLoggedInUserId + .calledWith(@req) + .should.equal true - it "should get the user id", -> - @AuthenticationController.getLoggedInUserId - .calledWith(@req) - .should.equal true + it "should call the project history api", -> + @request + .calledWith({ + url: "#{@settings.apis.project_history.url}#{@req.url}" + method: @req.method + headers: + "X-User-Id": @user_id + }) + .should.equal true - it "should call the project history api", -> - @request - .calledWith({ - url: "#{@settings.apis.project_history.url}#{@req.url}" - method: @req.method - headers: - "X-User-Id": @user_id - }) - .should.equal true + it "should pipe the response to the client", -> + @proxy.pipe + .calledWith(@res) + .should.equal true - it "should pipe the response to the client", -> - @proxy.pipe - .calledWith(@res) - .should.equal true - - describe "for a project without the project history flag", -> - beforeEach -> - @req.useProjectHistory = false - @HistoryController.proxyToHistoryApi @req, @res, @next - - it "should get the user id", -> - @AuthenticationController.getLoggedInUserId - .calledWith(@req) - .should.equal true - - it "should call the track changes api", -> - @request - .calledWith({ - url: "#{@settings.apis.trackchanges.url}#{@req.url}" - method: @req.method - headers: - "X-User-Id": @user_id - }) - .should.equal true - - it "should pipe the response to the client", -> - @proxy.pipe - .calledWith(@res) - .should.equal true - - describe "with project history disabled", -> + describe "for a project without the project history flag", -> beforeEach -> - @settings.apis.project_history.enabled = false + @req.useProjectHistory = false + @HistoryController.proxyToHistoryApi @req, @res, @next - describe "for a project with the project history flag", -> - beforeEach -> - @req.useProjectHistory = true - @HistoryController.proxyToHistoryApi @req, @res, @next + it "should get the user id", -> + @AuthenticationController.getLoggedInUserId + .calledWith(@req) + .should.equal true - it "should call the track changes api", -> - @request - .calledWith({ - url: "#{@settings.apis.trackchanges.url}#{@req.url}" - method: @req.method - headers: - "X-User-Id": @user_id - }) - .should.equal true + it "should call the track changes api", -> + @request + .calledWith({ + url: "#{@settings.apis.trackchanges.url}#{@req.url}" + method: @req.method + headers: + "X-User-Id": @user_id + }) + .should.equal true - describe "for a project without the project history flag", -> - beforeEach -> - @req.useProjectHistory = false - @HistoryController.proxyToHistoryApi @req, @res, @next - - it "should call the track changes api", -> - @request - .calledWith({ - url: "#{@settings.apis.trackchanges.url}#{@req.url}" - method: @req.method - headers: - "X-User-Id": @user_id - }) - .should.equal true + it "should pipe the response to the client", -> + @proxy.pipe + .calledWith(@res) + .should.equal true describe "with an error", -> beforeEach -> @@ -156,7 +118,7 @@ describe "HistoryController", -> describe "initializeProject", -> describe "with project history enabled", -> beforeEach -> - @settings.apis.project_history.enabled = true + @settings.apis.project_history.initializeHistoryForNewProjects = true describe "project history returns a successful response", -> beforeEach -> @@ -212,7 +174,7 @@ describe "HistoryController", -> describe "with project history disabled", -> beforeEach -> - @settings.apis.project_history.enabled = false + @settings.apis.project_history.initializeHistoryForNewProjects = false @HistoryController.initializeProject @callback it "should return the callback", -> From 5463b608adbac2af8b6cbf5e320dc04003533b17 Mon Sep 17 00:00:00 2001 From: James Allen Date: Mon, 18 Dec 2017 16:39:27 +0000 Subject: [PATCH 53/56] Add add{File|Doc}WithoutUpdatingHistory methods to allow importing OL projects with existing history --- .../Features/History/HistoryController.coffee | 23 ----- .../Features/History/HistoryManager.coffee | 26 ++++++ .../Project/ProjectCreationHandler.coffee | 4 +- .../Features/Project/ProjectDuplicator.coffee | 2 +- .../Project/ProjectEntityHandler.coffee | 88 +++++++++++-------- services/web/app/coffee/models/Project.coffee | 1 + .../coffee/ProjectStructureTests.coffee | 2 +- .../History/HistoryControllerTests.coffee | 65 -------------- .../coffee/History/HistoryManagerTests.coffee | 86 ++++++++++++++++++ .../ProjectCreationHandlerTests.coffee | 8 +- .../Project/ProjectDuplicatorTests.coffee | 12 +-- .../Project/ProjectEntityHandlerTests.coffee | 51 +++++++++++ 12 files changed, 229 insertions(+), 139 deletions(-) create mode 100644 services/web/app/coffee/Features/History/HistoryManager.coffee create mode 100644 services/web/test/unit/coffee/History/HistoryManagerTests.coffee diff --git a/services/web/app/coffee/Features/History/HistoryController.coffee b/services/web/app/coffee/Features/History/HistoryController.coffee index 0935118a7c..76476c8248 100644 --- a/services/web/app/coffee/Features/History/HistoryController.coffee +++ b/services/web/app/coffee/Features/History/HistoryController.coffee @@ -5,29 +5,6 @@ AuthenticationController = require "../Authentication/AuthenticationController" ProjectDetailsHandler = require "../Project/ProjectDetailsHandler" module.exports = HistoryController = - initializeProject: (callback = (error, history_id) ->) -> - return callback() if !settings.apis.project_history?.initializeHistoryForNewProjects - request.post { - url: "#{settings.apis.project_history.url}/project" - }, (error, res, body)-> - return callback(error) if error? - - if res.statusCode >= 200 and res.statusCode < 300 - try - project = JSON.parse(body) - catch error - return callback(error) - - overleaf_id = project?.project?.id - if !overleaf_id - error = new Error("project-history did not provide an id", project) - return callback(error) - - callback null, { overleaf_id } - else - error = new Error("project-history returned a non-success status code: #{res.statusCode}") - callback error - selectHistoryApi: (req, res, next = (error) ->) -> project_id = req.params?.Project_id # find out which type of history service this project uses diff --git a/services/web/app/coffee/Features/History/HistoryManager.coffee b/services/web/app/coffee/Features/History/HistoryManager.coffee new file mode 100644 index 0000000000..75f552c907 --- /dev/null +++ b/services/web/app/coffee/Features/History/HistoryManager.coffee @@ -0,0 +1,26 @@ +request = require "request" +settings = require "settings-sharelatex" + +module.exports = HistoryManager = + initializeProject: (callback = (error, history_id) ->) -> + return callback() if !settings.apis.project_history?.initializeHistoryForNewProjects + request.post { + url: "#{settings.apis.project_history.url}/project" + }, (error, res, body)-> + return callback(error) if error? + + if res.statusCode >= 200 and res.statusCode < 300 + try + project = JSON.parse(body) + catch error + return callback(error) + + overleaf_id = project?.project?.id + if !overleaf_id + error = new Error("project-history did not provide an id", project) + return callback(error) + + callback null, { overleaf_id } + else + error = new Error("project-history returned a non-success status code: #{res.statusCode}") + callback error \ No newline at end of file diff --git a/services/web/app/coffee/Features/Project/ProjectCreationHandler.coffee b/services/web/app/coffee/Features/Project/ProjectCreationHandler.coffee index cd78cddc09..ceddc2ad61 100644 --- a/services/web/app/coffee/Features/Project/ProjectCreationHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectCreationHandler.coffee @@ -7,7 +7,7 @@ Project = require('../../models/Project').Project Folder = require('../../models/Folder').Folder ProjectEntityHandler = require('./ProjectEntityHandler') ProjectDetailsHandler = require('./ProjectDetailsHandler') -HistoryController = require('../History/HistoryController') +HistoryManager = require('../History/HistoryManager') User = require('../../models/User').User fs = require('fs') Path = require "path" @@ -27,7 +27,7 @@ module.exports = ProjectCreationHandler = if projectHistoryId? ProjectCreationHandler._createBlankProject owner_id, projectName, projectHistoryId, callback else - HistoryController.initializeProject (error, history) -> + HistoryManager.initializeProject (error, history) -> return callback(error) if error? ProjectCreationHandler._createBlankProject owner_id, projectName, history?.overleaf_id, callback diff --git a/services/web/app/coffee/Features/Project/ProjectDuplicator.coffee b/services/web/app/coffee/Features/Project/ProjectDuplicator.coffee index 815e31eb27..d4640db966 100644 --- a/services/web/app/coffee/Features/Project/ProjectDuplicator.coffee +++ b/services/web/app/coffee/Features/Project/ProjectDuplicator.coffee @@ -21,7 +21,7 @@ module.exports = ProjectDuplicator = if !doc?._id? return callback() content = docContents[doc._id.toString()] - projectEntityHandler.addDocWithProject newProject, desFolder._id, doc.name, content.lines, owner_id, (err, newDoc)-> + projectEntityHandler.addDoc newProject, desFolder._id, doc.name, content.lines, owner_id, (err, newDoc)-> if err? logger.err err:err, "error copying doc" return callback(err) diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index 446786fa57..180c080b2c 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -149,14 +149,35 @@ module.exports = ProjectEntityHandler = else DocstoreManager.getDoc project_id, doc_id, options, callback - addDoc: (project_id, folder_id, docName, docLines, userId, callback = (error, doc, folder_id) ->)=> - ProjectGetter.getProjectWithOnlyFolders project_id, (err, project) -> + addDoc: (project_or_id, folder_id, docName, docLines, userId, callback = (error, doc, folder_id) ->)=> + ProjectEntityHandler.addDocWithoutUpdatingHistory project_or_id, folder_id, docName, docLines, userId, (error, doc, folder_id, result) -> + return callback(error) if error? + newDocs = [ + doc: doc + path: result?.path?.fileSystem + docLines: docLines.join('\n') + ] + project_id = project_or_id._id or project_or_id + DocumentUpdaterHandler.updateProjectStructure project_id, userId, {newDocs}, (error) -> + return callback(error) if error? + callback null, doc, folder_id + + addDocWithoutUpdatingHistory: (project_or_id, folder_id, docName, docLines, userId, callback = (error, doc, folder_id) ->)=> + # This method should never be called directly, except when importing a project + # from Overleaf. It skips sending updates to the project history, which will break + # the history unless you are making sure it is updated in some other way. + getProject = (cb) -> + if project_or_id._id? # project + return cb(null, project_or_id) + else # id + return ProjectGetter.getProjectWithOnlyFolders project_or_id, cb + getProject (error, project) -> if err? logger.err project_id:project_id, err:err, "error getting project for add doc" return callback(err) - ProjectEntityHandler.addDocWithProject project, folder_id, docName, docLines, userId, callback + ProjectEntityHandler._addDocWithProject project, folder_id, docName, docLines, userId, callback - addDocWithProject: (project, folder_id, docName, docLines, userId, callback = (error, doc, folder_id) ->)=> + _addDocWithProject: (project, folder_id, docName, docLines, userId, callback = (error, doc, folder_id, result) ->)=> project_id = project._id logger.log project_id: project_id, folder_id: folder_id, doc_name: docName, "adding doc to project with project" confirmFolder project, folder_id, (folder_id)=> @@ -176,14 +197,7 @@ module.exports = ProjectEntityHandler = rev: 0 }, (err) -> return callback(err) if err? - newDocs = [ - doc: doc - path: result?.path?.fileSystem - docLines: docLines.join('\n') - ] - DocumentUpdaterHandler.updateProjectStructure project_id, userId, {newDocs}, (error) -> - return callback(error) if error? - callback null, doc, folder_id + callback(null, doc, folder_id, result) restoreDoc: (project_id, doc_id, name, callback = (error, doc, folder_id) ->) -> # getDoc will return the deleted doc's lines, but we don't actually remove @@ -192,37 +206,37 @@ module.exports = ProjectEntityHandler = return callback(error) if error? ProjectEntityHandler.addDoc project_id, null, name, lines, callback - addFile: (project_id, folder_id, fileName, path, userId, callback = (error, fileRef, folder_id) ->)-> + addFileWithoutUpdatingHistory: (project_id, folder_id, fileName, path, userId, callback = (error, fileRef, folder_id, result, fileStoreUrl) ->)-> ProjectGetter.getProjectWithOnlyFolders project_id, (err, project) -> if err? logger.err project_id:project_id, err:err, "error getting project for add file" return callback(err) - ProjectEntityHandler.addFileWithProject project, folder_id, fileName, path, userId, callback - - addFileWithProject: (project, folder_id, fileName, path, userId, callback = (error, fileRef, folder_id) ->)-> - project_id = project._id - logger.log project_id: project._id, folder_id: folder_id, file_name: fileName, path:path, "adding file" - return callback(err) if err? - confirmFolder project, folder_id, (folder_id)-> - fileRef = new File name : fileName - FileStoreHandler.uploadFileFromDisk project._id, fileRef._id, path, (err, fileStoreUrl)-> - if err? - logger.err err:err, project_id: project._id, folder_id: folder_id, file_name: fileName, fileRef:fileRef, "error uploading image to s3" - return callback(err) - ProjectEntityHandler._putElement project, folder_id, fileRef, "file", (err, result)=> + logger.log project_id: project._id, folder_id: folder_id, file_name: fileName, path:path, "adding file" + return callback(err) if err? + confirmFolder project, folder_id, (folder_id)-> + fileRef = new File name : fileName + FileStoreHandler.uploadFileFromDisk project._id, fileRef._id, path, (err, fileStoreUrl)-> if err? - logger.err err:err, project_id: project._id, folder_id: folder_id, file_name: fileName, fileRef:fileRef, "error adding file with project" + logger.err err:err, project_id: project._id, folder_id: folder_id, file_name: fileName, fileRef:fileRef, "error uploading image to s3" return callback(err) - tpdsUpdateSender.addFile {project_id:project._id, file_id:fileRef._id, path:result?.path?.fileSystem, project_name:project.name, rev:fileRef.rev}, (err) -> - return callback(err) if err? - newFiles = [ - file: fileRef - path: result?.path?.fileSystem - url: fileStoreUrl - ] - DocumentUpdaterHandler.updateProjectStructure project_id, userId, {newFiles}, (error) -> - return callback(error) if error? - callback null, fileRef, folder_id + ProjectEntityHandler._putElement project, folder_id, fileRef, "file", (err, result)=> + if err? + logger.err err:err, project_id: project._id, folder_id: folder_id, file_name: fileName, fileRef:fileRef, "error adding file with project" + return callback(err) + tpdsUpdateSender.addFile {project_id:project._id, file_id:fileRef._id, path:result?.path?.fileSystem, project_name:project.name, rev:fileRef.rev}, (err) -> + return callback(err) if err? + callback(null, fileRef, folder_id, result, fileStoreUrl) + + addFile: (project_id, folder_id, fileName, path, userId, callback = (error, fileRef, folder_id) ->)-> + ProjectEntityHandler.addFileWithoutUpdatingHistory project_id, folder_id, fileName, path, userId, (error, fileRef, folder_id, result, fileStoreUrl) -> + newFiles = [ + file: fileRef + path: result?.path?.fileSystem + url: fileStoreUrl + ] + DocumentUpdaterHandler.updateProjectStructure project_id, userId, {newFiles}, (error) -> + return callback(error) if error? + callback null, fileRef, folder_id replaceFile: (project_id, file_id, fsPath, userId, callback)-> self = ProjectEntityHandler diff --git a/services/web/app/coffee/models/Project.coffee b/services/web/app/coffee/models/Project.coffee index e0013f8c5f..b434d35ab9 100644 --- a/services/web/app/coffee/models/Project.coffee +++ b/services/web/app/coffee/models/Project.coffee @@ -56,6 +56,7 @@ ProjectSchema = new Schema read_token : { type: String } history : id : { type: Number } + display : { type: Boolean } ProjectSchema.statics.getProject = (project_or_id, fields, callback)-> if project_or_id._id? diff --git a/services/web/test/acceptance/coffee/ProjectStructureTests.coffee b/services/web/test/acceptance/coffee/ProjectStructureTests.coffee index e54d4fac9d..5dd4fed251 100644 --- a/services/web/test/acceptance/coffee/ProjectStructureTests.coffee +++ b/services/web/test/acceptance/coffee/ProjectStructureTests.coffee @@ -59,7 +59,7 @@ describe "ProjectStructureChanges", -> @dup_project_id = body.project_id done() - it "should version the dosc created", -> + it "should version the docs created", -> updates = MockDocUpdaterApi.getProjectStructureUpdates(@dup_project_id).docUpdates expect(updates.length).to.equal(2) _.each updates, (update) => diff --git a/services/web/test/unit/coffee/History/HistoryControllerTests.coffee b/services/web/test/unit/coffee/History/HistoryControllerTests.coffee index 3e0464254e..f0669a5902 100644 --- a/services/web/test/unit/coffee/History/HistoryControllerTests.coffee +++ b/services/web/test/unit/coffee/History/HistoryControllerTests.coffee @@ -114,68 +114,3 @@ describe "HistoryController", -> it "should pass the error up the call chain", -> @next.calledWith(@error).should.equal true - - describe "initializeProject", -> - describe "with project history enabled", -> - beforeEach -> - @settings.apis.project_history.initializeHistoryForNewProjects = true - - describe "project history returns a successful response", -> - beforeEach -> - @overleaf_id = 1234 - @res = statusCode: 200 - @body = JSON.stringify(project: id: @overleaf_id) - @request.post = sinon.stub().callsArgWith(1, null, @res, @body) - - @HistoryController.initializeProject @callback - - it "should call the project history api", -> - @request.post.calledWith( - url: "#{@settings.apis.project_history.url}/project" - ).should.equal true - - it "should return the callback with the overleaf id", -> - @callback.calledWithExactly(null, { @overleaf_id }).should.equal true - - describe "project history returns a response without the project id", -> - beforeEach -> - @res = statusCode: 200 - @body = JSON.stringify(project: {}) - @request.post = sinon.stub().callsArgWith(1, null, @res, @body) - - @HistoryController.initializeProject @callback - - it "should return the callback with an error", -> - @callback - .calledWith(sinon.match.has("message", "project-history did not provide an id")) - .should.equal true - - describe "project history returns a unsuccessful response", -> - beforeEach -> - @res = statusCode: 404 - @request.post = sinon.stub().callsArgWith(1, null, @res) - - @HistoryController.initializeProject @callback - - it "should return the callback with an error", -> - @callback - .calledWith(sinon.match.has("message", "project-history returned a non-success status code: 404")) - .should.equal true - - describe "project history errors", -> - beforeEach -> - @error = sinon.stub() - @request.post = sinon.stub().callsArgWith(1, @error) - - @HistoryController.initializeProject @callback - - it "should return the callback with the error", -> - @callback.calledWithExactly(@error).should.equal true - - describe "with project history disabled", -> - beforeEach -> - @settings.apis.project_history.initializeHistoryForNewProjects = false - @HistoryController.initializeProject @callback - - it "should return the callback", -> - @callback.calledWithExactly().should.equal true diff --git a/services/web/test/unit/coffee/History/HistoryManagerTests.coffee b/services/web/test/unit/coffee/History/HistoryManagerTests.coffee new file mode 100644 index 0000000000..9b5f80df04 --- /dev/null +++ b/services/web/test/unit/coffee/History/HistoryManagerTests.coffee @@ -0,0 +1,86 @@ +chai = require('chai') +chai.should() +sinon = require("sinon") +modulePath = "../../../../app/js/Features/History/HistoryManager" +SandboxedModule = require('sandboxed-module') + +describe "HistoryManager", -> + beforeEach -> + @callback = sinon.stub() + @user_id = "user-id-123" + @AuthenticationController = + getLoggedInUserId: sinon.stub().returns(@user_id) + @HistoryManager = SandboxedModule.require modulePath, requires: + "request" : @request = sinon.stub() + "settings-sharelatex": @settings = {} + @settings.apis = + trackchanges: + enabled: false + url: "http://trackchanges.example.com" + project_history: + url: "http://project_history.example.com" + + describe "initializeProject", -> + describe "with project history enabled", -> + beforeEach -> + @settings.apis.project_history.initializeHistoryForNewProjects = true + + describe "project history returns a successful response", -> + beforeEach -> + @overleaf_id = 1234 + @res = statusCode: 200 + @body = JSON.stringify(project: id: @overleaf_id) + @request.post = sinon.stub().callsArgWith(1, null, @res, @body) + + @HistoryManager.initializeProject @callback + + it "should call the project history api", -> + @request.post.calledWith( + url: "#{@settings.apis.project_history.url}/project" + ).should.equal true + + it "should return the callback with the overleaf id", -> + @callback.calledWithExactly(null, { @overleaf_id }).should.equal true + + describe "project history returns a response without the project id", -> + beforeEach -> + @res = statusCode: 200 + @body = JSON.stringify(project: {}) + @request.post = sinon.stub().callsArgWith(1, null, @res, @body) + + @HistoryManager.initializeProject @callback + + it "should return the callback with an error", -> + @callback + .calledWith(sinon.match.has("message", "project-history did not provide an id")) + .should.equal true + + describe "project history returns a unsuccessful response", -> + beforeEach -> + @res = statusCode: 404 + @request.post = sinon.stub().callsArgWith(1, null, @res) + + @HistoryManager.initializeProject @callback + + it "should return the callback with an error", -> + @callback + .calledWith(sinon.match.has("message", "project-history returned a non-success status code: 404")) + .should.equal true + + describe "project history errors", -> + beforeEach -> + @error = sinon.stub() + @request.post = sinon.stub().callsArgWith(1, @error) + + @HistoryManager.initializeProject @callback + + it "should return the callback with the error", -> + @callback.calledWithExactly(@error).should.equal true + + describe "with project history disabled", -> + beforeEach -> + @settings.apis.project_history.initializeHistoryForNewProjects = false + @HistoryManager.initializeProject @callback + + it "should return the callback", -> + @callback.calledWithExactly().should.equal true diff --git a/services/web/test/unit/coffee/Project/ProjectCreationHandlerTests.coffee b/services/web/test/unit/coffee/Project/ProjectCreationHandlerTests.coffee index 470b538bd9..378e909984 100644 --- a/services/web/test/unit/coffee/Project/ProjectCreationHandlerTests.coffee +++ b/services/web/test/unit/coffee/Project/ProjectCreationHandlerTests.coffee @@ -38,7 +38,7 @@ describe 'ProjectCreationHandler', -> setRootDoc: sinon.stub().callsArg(2) @ProjectDetailsHandler = validateProjectName: sinon.stub().yields() - @HistoryController = + @HistoryManager = initializeProject: sinon.stub().callsArg(0) @user = @@ -53,7 +53,7 @@ describe 'ProjectCreationHandler', -> '../../models/User': User:@User '../../models/Project':{Project:@ProjectModel} '../../models/Folder':{Folder:@FolderModel} - '../History/HistoryController': @HistoryController + '../History/HistoryManager': @HistoryManager './ProjectEntityHandler':@ProjectEntityHandler "./ProjectDetailsHandler":@ProjectDetailsHandler "settings-sharelatex": @Settings = {} @@ -66,7 +66,7 @@ describe 'ProjectCreationHandler', -> describe 'Creating a Blank project', -> beforeEach -> @overleaf_id = 1234 - @HistoryController.initializeProject = sinon.stub().callsArgWith(0, null, { @overleaf_id }) + @HistoryManager.initializeProject = sinon.stub().callsArgWith(0, null, { @overleaf_id }) @ProjectModel::save = sinon.stub().callsArg(0) describe "successfully", -> @@ -83,7 +83,7 @@ describe 'ProjectCreationHandler', -> it "should initialize the project overleaf if history id not provided", (done)-> @handler.createBlankProject ownerId, projectName, done - @HistoryController.initializeProject.calledWith().should.equal true + @HistoryManager.initializeProject.calledWith().should.equal true it "should set the overleaf id if overleaf id not provided", (done)-> @handler.createBlankProject ownerId, projectName, (err, project)=> diff --git a/services/web/test/unit/coffee/Project/ProjectDuplicatorTests.coffee b/services/web/test/unit/coffee/Project/ProjectDuplicatorTests.coffee index b489014e7e..b7afb79f83 100644 --- a/services/web/test/unit/coffee/Project/ProjectDuplicatorTests.coffee +++ b/services/web/test/unit/coffee/Project/ProjectDuplicatorTests.coffee @@ -64,7 +64,7 @@ describe 'ProjectDuplicator', -> @projectOptionsHandler = setCompiler : sinon.stub() @entityHandler = - addDocWithProject: sinon.stub().callsArgWith(5, null, {name:"somDoc"}) + addDoc: sinon.stub().callsArgWith(5, null, {name:"somDoc"}) copyFileFromExistingProjectWithProject: sinon.stub().callsArgWith(5) setRootDoc: sinon.stub() addFolderWithProject: sinon.stub().callsArgWith(3, null, @newFolder) @@ -112,13 +112,13 @@ describe 'ProjectDuplicator', -> done() it 'should use the same compiler', (done)-> - @entityHandler.addDocWithProject.callsArgWith(5, null, @rootFolder.docs[0], @owner._id) + @entityHandler.addDoc.callsArgWith(5, null, @rootFolder.docs[0], @owner._id) @duplicator.duplicate @owner, @old_project_id, "", (err, newProject)=> @projectOptionsHandler.setCompiler.calledWith(@stubbedNewProject._id, @project.compiler).should.equal true done() it 'should use the same root doc', (done)-> - @entityHandler.addDocWithProject.callsArgWith(5, null, @rootFolder.docs[0], @owner._id) + @entityHandler.addDoc.callsArgWith(5, null, @rootFolder.docs[0], @owner._id) @duplicator.duplicate @owner, @old_project_id, "", (err, newProject)=> @entityHandler.setRootDoc.calledWith(@stubbedNewProject._id, @rootFolder.docs[0]._id).should.equal true done() @@ -139,13 +139,13 @@ describe 'ProjectDuplicator', -> it 'should copy all the docs', (done)-> @duplicator.duplicate @owner, @old_project_id, "", (err, newProject)=> @DocstoreManager.getAllDocs.calledWith(@old_project_id).should.equal true - @entityHandler.addDocWithProject + @entityHandler.addDoc .calledWith(@stubbedNewProject, @stubbedNewProject.rootFolder[0]._id, @doc0.name, @doc0_lines, @owner._id) .should.equal true - @entityHandler.addDocWithProject + @entityHandler.addDoc .calledWith(@stubbedNewProject, @newFolder._id, @doc1.name, @doc1_lines, @owner._id) .should.equal true - @entityHandler.addDocWithProject + @entityHandler.addDoc .calledWith(@stubbedNewProject, @newFolder._id, @doc2.name, @doc2_lines, @owner._id) .should.equal true done() diff --git a/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee b/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee index f62690e226..75d19f459c 100644 --- a/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee +++ b/services/web/test/unit/coffee/Project/ProjectEntityHandlerTests.coffee @@ -496,6 +496,51 @@ describe 'ProjectEntityHandler', -> .calledWith(project_id, userId, {newDocs}) .should.equal true + describe 'addDocWithoutUpdatingHistory', -> + beforeEach -> + @name = "some new doc" + @lines = ['1234','abc'] + @path = "/path/to/doc" + + @ProjectEntityHandler._putElement = sinon.stub().callsArgWith(4, null, {path:{fileSystem:@path}}) + @callback = sinon.stub() + @tpdsUpdateSender.addDoc = sinon.stub().callsArg(1) + @DocstoreManager.updateDoc = sinon.stub().yields(null, true, 0) + + @ProjectEntityHandler.addDocWithoutUpdatingHistory project_id, folder_id, @name, @lines, userId, @callback + + # Created doc + @doc = @ProjectEntityHandler._putElement.args[0][2] + @doc.name.should.equal @name + expect(@doc.lines).to.be.undefined + + it 'should call put element', -> + @ProjectEntityHandler._putElement.calledWith(@project, folder_id, @doc).should.equal true + + it 'should return doc and parent folder', -> + @callback.calledWith(null, @doc, folder_id).should.equal true + + it 'should call third party data store', -> + @tpdsUpdateSender.addDoc + .calledWith({ + project_id: project_id + doc_id: doc_id + path: @path + project_name: @project.name + rev: 0 + }) + .should.equal true + + it "should send the doc lines to the doc store", -> + @DocstoreManager.updateDoc + .calledWith(project_id, @doc._id.toString(), @lines) + .should.equal true + + it "should not should send the change in project structure to the doc updater", () -> + @documentUpdaterHandler.updateProjectStructure + .called + .should.equal false + describe "restoreDoc", -> beforeEach -> @name = "doc-name" @@ -584,6 +629,12 @@ describe 'ProjectEntityHandler', -> @ProjectEntityHandler.addFile project_id, folder_id, fileName, {}, userId, () -> + it "should not send the change in project structure to the doc updater when called as addFileWithoutUpdatingHistory", (done) -> + @documentUpdaterHandler.updateProjectStructure = sinon.stub().yields() + @ProjectEntityHandler.addFileWithoutUpdatingHistory project_id, folder_id, fileName, {}, userId, () => + @documentUpdaterHandler.updateProjectStructure.called.should.equal false + done() + describe 'replaceFile', -> beforeEach -> @projectLocator From 731cd57250b30911f775bd548a1dfd2a2ddefc6f Mon Sep 17 00:00:00 2001 From: James Allen Date: Tue, 19 Dec 2017 11:52:15 +0000 Subject: [PATCH 54/56] Make intermediate argument signature clearer --- .../Project/ProjectEntityHandler.coffee | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index 180c080b2c..941bc5e988 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -150,11 +150,11 @@ module.exports = ProjectEntityHandler = DocstoreManager.getDoc project_id, doc_id, options, callback addDoc: (project_or_id, folder_id, docName, docLines, userId, callback = (error, doc, folder_id) ->)=> - ProjectEntityHandler.addDocWithoutUpdatingHistory project_or_id, folder_id, docName, docLines, userId, (error, doc, folder_id, result) -> + ProjectEntityHandler.addDocWithoutUpdatingHistory project_or_id, folder_id, docName, docLines, userId, (error, doc, folder_id, path) -> return callback(error) if error? newDocs = [ doc: doc - path: result?.path?.fileSystem + path: path docLines: docLines.join('\n') ] project_id = project_or_id._id or project_or_id @@ -177,7 +177,7 @@ module.exports = ProjectEntityHandler = return callback(err) ProjectEntityHandler._addDocWithProject project, folder_id, docName, docLines, userId, callback - _addDocWithProject: (project, folder_id, docName, docLines, userId, callback = (error, doc, folder_id, result) ->)=> + _addDocWithProject: (project, folder_id, docName, docLines, userId, callback = (error, doc, folder_id, path) ->)=> project_id = project._id logger.log project_id: project_id, folder_id: folder_id, doc_name: docName, "adding doc to project with project" confirmFolder project, folder_id, (folder_id)=> @@ -197,7 +197,7 @@ module.exports = ProjectEntityHandler = rev: 0 }, (err) -> return callback(err) if err? - callback(null, doc, folder_id, result) + callback(null, doc, folder_id, result?.path?.fileSystem) restoreDoc: (project_id, doc_id, name, callback = (error, doc, folder_id) ->) -> # getDoc will return the deleted doc's lines, but we don't actually remove @@ -206,7 +206,7 @@ module.exports = ProjectEntityHandler = return callback(error) if error? ProjectEntityHandler.addDoc project_id, null, name, lines, callback - addFileWithoutUpdatingHistory: (project_id, folder_id, fileName, path, userId, callback = (error, fileRef, folder_id, result, fileStoreUrl) ->)-> + addFileWithoutUpdatingHistory: (project_id, folder_id, fileName, path, userId, callback = (error, fileRef, folder_id, path, fileStoreUrl) ->)-> ProjectGetter.getProjectWithOnlyFolders project_id, (err, project) -> if err? logger.err project_id:project_id, err:err, "error getting project for add file" @@ -225,13 +225,13 @@ module.exports = ProjectEntityHandler = return callback(err) tpdsUpdateSender.addFile {project_id:project._id, file_id:fileRef._id, path:result?.path?.fileSystem, project_name:project.name, rev:fileRef.rev}, (err) -> return callback(err) if err? - callback(null, fileRef, folder_id, result, fileStoreUrl) + callback(null, fileRef, folder_id, result?.path?.fileSystem, fileStoreUrl) - addFile: (project_id, folder_id, fileName, path, userId, callback = (error, fileRef, folder_id) ->)-> - ProjectEntityHandler.addFileWithoutUpdatingHistory project_id, folder_id, fileName, path, userId, (error, fileRef, folder_id, result, fileStoreUrl) -> + addFile: (project_id, folder_id, fileName, fsPath, userId, callback = (error, fileRef, folder_id) ->)-> + ProjectEntityHandler.addFileWithoutUpdatingHistory project_id, folder_id, fileName, fsPath, userId, (error, fileRef, folder_id, path, fileStoreUrl) -> newFiles = [ file: fileRef - path: result?.path?.fileSystem + path: path url: fileStoreUrl ] DocumentUpdaterHandler.updateProjectStructure project_id, userId, {newFiles}, (error) -> From 2d8b47295f7a245575bceca62daf822a218f5a8f Mon Sep 17 00:00:00 2001 From: James Allen Date: Tue, 19 Dec 2017 11:53:17 +0000 Subject: [PATCH 55/56] Build from module branch --- services/web/Jenkinsfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/Jenkinsfile b/services/web/Jenkinsfile index 152d980787..84a143c5d7 100644 --- a/services/web/Jenkinsfile +++ b/services/web/Jenkinsfile @@ -43,7 +43,7 @@ pipeline { checkout([$class: 'GitSCM', branches: [[name: '*/master']], extensions: [[$class: 'RelativeTargetDirectory', relativeTargetDir: 'modules/learn-wiki'], [$class: 'CloneOption', shallow: true]], userRemoteConfigs: [[credentialsId: 'GIT_DEPLOY_KEY', url: 'git@bitbucket.org:sharelatex/learn-wiki-web-module.git']]]) checkout([$class: 'GitSCM', branches: [[name: '*/master']], extensions: [[$class: 'RelativeTargetDirectory', relativeTargetDir: 'modules/templates'], [$class: 'CloneOption', shallow: true]], userRemoteConfigs: [[credentialsId: 'GIT_DEPLOY_KEY', url: 'git@github.com:sharelatex/templates-webmodule.git']]]) checkout([$class: 'GitSCM', branches: [[name: '*/master']], extensions: [[$class: 'RelativeTargetDirectory', relativeTargetDir: 'modules/track-changes'], [$class: 'CloneOption', shallow: true]], userRemoteConfigs: [[credentialsId: 'GIT_DEPLOY_KEY', url: 'git@github.com:sharelatex/track-changes-web-module.git']]]) - checkout([$class: 'GitSCM', branches: [[name: '*/master']], extensions: [[$class: 'RelativeTargetDirectory', relativeTargetDir: 'modules/overleaf-integration'], [$class: 'CloneOption', shallow: true]], userRemoteConfigs: [[credentialsId: 'GIT_DEPLOY_KEY', url: 'git@github.com:sharelatex/overleaf-integration-web-module.git']]]) + checkout([$class: 'GitSCM', branches: [[name: '*/ja-dont-send-history-updates-on-import']], extensions: [[$class: 'RelativeTargetDirectory', relativeTargetDir: 'modules/overleaf-integration'], [$class: 'CloneOption', shallow: true]], userRemoteConfigs: [[credentialsId: 'GIT_DEPLOY_KEY', url: 'git@github.com:sharelatex/overleaf-integration-web-module.git']]]) checkout([$class: 'GitSCM', branches: [[name: '*/master']], extensions: [[$class: 'RelativeTargetDirectory', relativeTargetDir: 'modules/overleaf-account-merge'], [$class: 'CloneOption', shallow: true]], userRemoteConfigs: [[credentialsId: 'GIT_DEPLOY_KEY', url: 'git@github.com:sharelatex/overleaf-account-merge.git']]]) } } From 1653de9301b25c1e8b6577b62eee3530df4813b3 Mon Sep 17 00:00:00 2001 From: James Allen Date: Tue, 19 Dec 2017 15:01:17 +0000 Subject: [PATCH 56/56] Revert "Build from module branch" This reverts commit 34d4b72ddd8b16bd34013e7bea9459940fbf0032. --- services/web/Jenkinsfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/Jenkinsfile b/services/web/Jenkinsfile index 84a143c5d7..152d980787 100644 --- a/services/web/Jenkinsfile +++ b/services/web/Jenkinsfile @@ -43,7 +43,7 @@ pipeline { checkout([$class: 'GitSCM', branches: [[name: '*/master']], extensions: [[$class: 'RelativeTargetDirectory', relativeTargetDir: 'modules/learn-wiki'], [$class: 'CloneOption', shallow: true]], userRemoteConfigs: [[credentialsId: 'GIT_DEPLOY_KEY', url: 'git@bitbucket.org:sharelatex/learn-wiki-web-module.git']]]) checkout([$class: 'GitSCM', branches: [[name: '*/master']], extensions: [[$class: 'RelativeTargetDirectory', relativeTargetDir: 'modules/templates'], [$class: 'CloneOption', shallow: true]], userRemoteConfigs: [[credentialsId: 'GIT_DEPLOY_KEY', url: 'git@github.com:sharelatex/templates-webmodule.git']]]) checkout([$class: 'GitSCM', branches: [[name: '*/master']], extensions: [[$class: 'RelativeTargetDirectory', relativeTargetDir: 'modules/track-changes'], [$class: 'CloneOption', shallow: true]], userRemoteConfigs: [[credentialsId: 'GIT_DEPLOY_KEY', url: 'git@github.com:sharelatex/track-changes-web-module.git']]]) - checkout([$class: 'GitSCM', branches: [[name: '*/ja-dont-send-history-updates-on-import']], extensions: [[$class: 'RelativeTargetDirectory', relativeTargetDir: 'modules/overleaf-integration'], [$class: 'CloneOption', shallow: true]], userRemoteConfigs: [[credentialsId: 'GIT_DEPLOY_KEY', url: 'git@github.com:sharelatex/overleaf-integration-web-module.git']]]) + checkout([$class: 'GitSCM', branches: [[name: '*/master']], extensions: [[$class: 'RelativeTargetDirectory', relativeTargetDir: 'modules/overleaf-integration'], [$class: 'CloneOption', shallow: true]], userRemoteConfigs: [[credentialsId: 'GIT_DEPLOY_KEY', url: 'git@github.com:sharelatex/overleaf-integration-web-module.git']]]) checkout([$class: 'GitSCM', branches: [[name: '*/master']], extensions: [[$class: 'RelativeTargetDirectory', relativeTargetDir: 'modules/overleaf-account-merge'], [$class: 'CloneOption', shallow: true]], userRemoteConfigs: [[credentialsId: 'GIT_DEPLOY_KEY', url: 'git@github.com:sharelatex/overleaf-account-merge.git']]]) } }