From a1615e6d846c7471dfc527bac79ebf1559b95f3f Mon Sep 17 00:00:00 2001 From: James Allen Date: Tue, 28 Nov 2017 18:18:15 +0000 Subject: [PATCH 01/13] 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 02/13] 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 03/13] 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 04/13] 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 05/13] 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 06/13] 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 07/13] 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 08/13] 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 09/13] 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 10/13] 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 11/13] 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 12/13] 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 13/13] 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", ->