From 293ba1fc4c4cc2244a126e864a823bad77de33b3 Mon Sep 17 00:00:00 2001 From: James Allen Date: Fri, 9 Dec 2016 15:43:08 +0000 Subject: [PATCH] Fetch all ranges from docstore when viewing overview panel --- .../Features/Docstore/DocstoreManager.coffee | 15 +++++++++ .../Features/Editor/EditorController.coffee | 1 - .../HistoryController.coffee} | 4 +-- .../HistoryManager.coffee} | 2 +- .../InactiveProjectManager.coffee | 3 -- .../Features/Ranges/RangesController.coffee | 11 +++++++ .../Features/Ranges/RangesManager.coffee | 8 +++++ services/web/app/coffee/router.coffee | 11 ++++--- .../views/project/editor/review-panel.jade | 3 ++ .../controllers/ReviewPanelController.coffee | 24 +++++++++++++- .../Docstore/DocstoreManagerTests.coffee | 32 +++++++++++++++++++ .../HistoryControllerTests.coffee} | 10 +++--- .../HistoryManagerTests.coffee} | 14 ++++---- 13 files changed, 114 insertions(+), 24 deletions(-) rename services/web/app/coffee/Features/{TrackChanges/TrackChangesController.coffee => History/HistoryController.coffee} (84%) rename services/web/app/coffee/Features/{TrackChanges/TrackChangesManager.coffee => History/HistoryManager.coffee} (96%) create mode 100644 services/web/app/coffee/Features/Ranges/RangesController.coffee create mode 100644 services/web/app/coffee/Features/Ranges/RangesManager.coffee rename services/web/test/UnitTests/coffee/{TrackChanges/TrackChangesControllerTests.coffee => History/HistoryControllerTests.coffee} (83%) rename services/web/test/UnitTests/coffee/{TrackChanges/TrackChangesManagerTests.coffee => History/HistoryManagerTests.coffee} (81%) diff --git a/services/web/app/coffee/Features/Docstore/DocstoreManager.coffee b/services/web/app/coffee/Features/Docstore/DocstoreManager.coffee index cf48dfe07b..06dd14c17b 100644 --- a/services/web/app/coffee/Features/Docstore/DocstoreManager.coffee +++ b/services/web/app/coffee/Features/Docstore/DocstoreManager.coffee @@ -29,6 +29,21 @@ module.exports = DocstoreManager = error = new Error("docstore api responded with non-success code: #{res.statusCode}") logger.error err: error, project_id: project_id, "error getting all docs from docstore" callback(error) + + getAllRanges: (project_id, callback = (error) ->) -> + logger.log { project_id }, "getting all doc ranges for project in docstore api" + url = "#{settings.apis.docstore.url}/project/#{project_id}/ranges" + request.get { + url: url + json: true + }, (error, res, docs) -> + return callback(error) if error? + if 200 <= res.statusCode < 300 + callback(null, docs) + else + error = new Error("docstore api responded with non-success code: #{res.statusCode}") + logger.error err: error, project_id: project_id, "error getting all doc ranges from docstore" + callback(error) getDoc: (project_id, doc_id, options = {}, callback = (error, lines, rev, version) ->) -> if typeof(options) == "function" diff --git a/services/web/app/coffee/Features/Editor/EditorController.coffee b/services/web/app/coffee/Features/Editor/EditorController.coffee index 476ba96174..b5abab3bf9 100644 --- a/services/web/app/coffee/Features/Editor/EditorController.coffee +++ b/services/web/app/coffee/Features/Editor/EditorController.coffee @@ -7,7 +7,6 @@ ProjectDetailsHandler = require('../Project/ProjectDetailsHandler') ProjectDeleter = require("../Project/ProjectDeleter") DocumentUpdaterHandler = require('../DocumentUpdater/DocumentUpdaterHandler') EditorRealTimeController = require("./EditorRealTimeController") -TrackChangesManager = require("../TrackChanges/TrackChangesManager") async = require('async') LockManager = require("../../infrastructure/LockManager") _ = require('underscore') diff --git a/services/web/app/coffee/Features/TrackChanges/TrackChangesController.coffee b/services/web/app/coffee/Features/History/HistoryController.coffee similarity index 84% rename from services/web/app/coffee/Features/TrackChanges/TrackChangesController.coffee rename to services/web/app/coffee/Features/History/HistoryController.coffee index bc6e00a29a..d4f42b38b1 100644 --- a/services/web/app/coffee/Features/TrackChanges/TrackChangesController.coffee +++ b/services/web/app/coffee/Features/History/HistoryController.coffee @@ -3,8 +3,8 @@ request = require "request" settings = require "settings-sharelatex" AuthenticationController = require "../Authentication/AuthenticationController" -module.exports = TrackChangesController = - proxyToTrackChangesApi: (req, res, next = (error) ->) -> +module.exports = HistoryController = + proxyToHistoryApi: (req, res, next = (error) ->) -> user_id = AuthenticationController.getLoggedInUserId req url = settings.apis.trackchanges.url + req.url logger.log url: url, "proxying to track-changes api" diff --git a/services/web/app/coffee/Features/TrackChanges/TrackChangesManager.coffee b/services/web/app/coffee/Features/History/HistoryManager.coffee similarity index 96% rename from services/web/app/coffee/Features/TrackChanges/TrackChangesManager.coffee rename to services/web/app/coffee/Features/History/HistoryManager.coffee index ddcfe3e44a..ea3f492613 100644 --- a/services/web/app/coffee/Features/TrackChanges/TrackChangesManager.coffee +++ b/services/web/app/coffee/Features/History/HistoryManager.coffee @@ -2,7 +2,7 @@ settings = require "settings-sharelatex" request = require "request" logger = require "logger-sharelatex" -module.exports = TrackChangesManager = +module.exports = HistoryManager = flushProject: (project_id, callback = (error) ->) -> logger.log project_id: project_id, "flushing project in track-changes api" url = "#{settings.apis.trackchanges.url}/project/#{project_id}/flush" diff --git a/services/web/app/coffee/Features/InactiveData/InactiveProjectManager.coffee b/services/web/app/coffee/Features/InactiveData/InactiveProjectManager.coffee index a2afee0573..5c984dcb5d 100644 --- a/services/web/app/coffee/Features/InactiveData/InactiveProjectManager.coffee +++ b/services/web/app/coffee/Features/InactiveData/InactiveProjectManager.coffee @@ -5,8 +5,6 @@ DocstoreManager = require("../Docstore/DocstoreManager") ProjectGetter = require("../Project/ProjectGetter") ProjectUpdateHandler = require("../Project/ProjectUpdateHandler") Project = require("../../models/Project").Project -TrackChangesManager = require("../TrackChanges/TrackChangesManager") - MILISECONDS_IN_DAY = 86400000 module.exports = InactiveProjectManager = @@ -52,7 +50,6 @@ module.exports = InactiveProjectManager = logger.log project_id:project_id, "deactivating inactive project" jobs = [ (cb)-> DocstoreManager.archiveProject project_id, cb - # (cb)-> TrackChangesManager.archiveProject project_id, cb (cb)-> ProjectUpdateHandler.markAsInactive project_id, cb ] async.series jobs, (err)-> diff --git a/services/web/app/coffee/Features/Ranges/RangesController.coffee b/services/web/app/coffee/Features/Ranges/RangesController.coffee new file mode 100644 index 0000000000..96a42588ac --- /dev/null +++ b/services/web/app/coffee/Features/Ranges/RangesController.coffee @@ -0,0 +1,11 @@ +RangesManager = require "./RangesManager" +logger = require "logger-sharelatex" + +module.exports = RangesController = + getAllRanges: (req, res, next) -> + project_id = req.params.project_id + logger.log {project_id}, "request for project ranges" + RangesManager.getAllRanges project_id, (error, docs = []) -> + return next(error) if error? + docs = ({id: d._id, ranges: d.ranges} for d in docs) + res.json docs diff --git a/services/web/app/coffee/Features/Ranges/RangesManager.coffee b/services/web/app/coffee/Features/Ranges/RangesManager.coffee new file mode 100644 index 0000000000..1fdf55b4a8 --- /dev/null +++ b/services/web/app/coffee/Features/Ranges/RangesManager.coffee @@ -0,0 +1,8 @@ +DocumentUpdaterHandler = require "../DocumentUpdater/DocumentUpdaterHandler" +DocstoreManager = require "../Docstore/DocstoreManager" + +module.exports = RangesManager = + getAllRanges: (project_id, callback = (error, docs) ->) -> + DocumentUpdaterHandler.flushProjectToMongo project_id, (error) -> + return callback(error) if error? + DocstoreManager.getAllRanges project_id, callback \ No newline at end of file diff --git a/services/web/app/coffee/router.coffee b/services/web/app/coffee/router.coffee index 14ac3b8d22..36e26782ba 100644 --- a/services/web/app/coffee/router.coffee +++ b/services/web/app/coffee/router.coffee @@ -25,7 +25,7 @@ ClsiCookieManager = require("./Features/Compile/ClsiCookieManager") HealthCheckController = require("./Features/HealthCheck/HealthCheckController") ProjectDownloadsController = require "./Features/Downloads/ProjectDownloadsController" FileStoreController = require("./Features/FileStore/FileStoreController") -TrackChangesController = require("./Features/TrackChanges/TrackChangesController") +HistoryController = require("./Features/History/HistoryController") PasswordResetRouter = require("./Features/PasswordReset/PasswordResetRouter") StaticPagesRouter = require("./Features/StaticPages/StaticPagesRouter") ChatController = require("./Features/Chat/ChatController") @@ -40,6 +40,7 @@ AuthorizationMiddlewear = require('./Features/Authorization/AuthorizationMiddlew BetaProgramController = require('./Features/BetaProgram/BetaProgramController') AnalyticsRouter = require('./Features/Analytics/AnalyticsRouter') AnnouncementsController = require("./Features/Announcements/AnnouncementsController") +RangesController = require("./Features/Ranges/RangesController") logger = require("logger-sharelatex") _ = require("underscore") @@ -171,9 +172,11 @@ module.exports = class Router webRouter.post '/project/:Project_id/rename', AuthorizationMiddlewear.ensureUserCanAdminProject, ProjectController.renameProject - webRouter.get "/project/:Project_id/updates", AuthorizationMiddlewear.ensureUserCanReadProject, TrackChangesController.proxyToTrackChangesApi - webRouter.get "/project/:Project_id/doc/:doc_id/diff", AuthorizationMiddlewear.ensureUserCanReadProject, TrackChangesController.proxyToTrackChangesApi - webRouter.post "/project/:Project_id/doc/:doc_id/version/:version_id/restore", AuthorizationMiddlewear.ensureUserCanReadProject, TrackChangesController.proxyToTrackChangesApi + webRouter.get "/project/:Project_id/updates", AuthorizationMiddlewear.ensureUserCanReadProject, HistoryController.proxyToHistoryApi + webRouter.get "/project/:Project_id/doc/:doc_id/diff", AuthorizationMiddlewear.ensureUserCanReadProject, HistoryController.proxyToHistoryApi + webRouter.post "/project/:Project_id/doc/:doc_id/version/:version_id/restore", AuthorizationMiddlewear.ensureUserCanReadProject, HistoryController.proxyToHistoryApi + + webRouter.get "/project/:project_id/ranges", AuthorizationMiddlewear.ensureUserCanReadProject, RangesController.getAllRanges webRouter.get '/Project/:Project_id/download/zip', AuthorizationMiddlewear.ensureUserCanReadProject, ProjectDownloadsController.downloadProject webRouter.get '/project/download/zip', AuthorizationMiddlewear.ensureUserCanReadMultipleProjects, ProjectDownloadsController.downloadMultipleProjects diff --git a/services/web/app/views/project/editor/review-panel.jade b/services/web/app/views/project/editor/review-panel.jade index be41e80e2c..2da1277572 100644 --- a/services/web/app/views/project/editor/review-panel.jade +++ b/services/web/app/views/project/editor/review-panel.jade @@ -49,8 +49,11 @@ .rp-entry-list( ng-if="reviewPanel.subView === SubViews.OVERVIEW" ) + .rp-overview-loading(ng-if="reviewPanel.overview.loading") + i.fa.fa-spinner.fa-spin .rp-overview-file( ng-repeat="(doc_id, entries) in reviewPanel.entries" + ng-if="!reviewPanel.overview.loading" ) .rp-overview-file-header | {{ getFileName(doc_id) }} diff --git a/services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee b/services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee index d581c388a9..8543e86402 100644 --- a/services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee +++ b/services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee @@ -4,7 +4,7 @@ define [ "ide/colors/ColorManager" "ide/review-panel/RangesTracker" ], (App, EventEmitter, ColorManager, RangesTracker) -> - App.controller "ReviewPanelController", ($scope, $element, ide, $timeout) -> + App.controller "ReviewPanelController", ($scope, $element, ide, $timeout, $http) -> $reviewPanelEl = $element.find "#review-panel" $scope.SubViews = @@ -16,6 +16,8 @@ define [ hasEntries: false subView: $scope.SubViews.CUR_FILE openSubView: $scope.SubViews.CUR_FILE + overview: + loading: false $scope.commentState = adding: false @@ -146,6 +148,11 @@ define [ else # Reset back to what we had when previously open $scope.reviewPanel.subView = $scope.reviewPanel.openSubView + + $scope.$watch "reviewPanel.subView", (view) -> + return if !view? + if view == $scope.SubViews.OVERVIEW + refreshOverviewPanel() $scope.$watch "editor.open_doc_id", (open_doc_id) -> return if !open_doc_id? @@ -164,6 +171,21 @@ define [ $scope.$broadcast "review-panel:toggle" $scope.$broadcast "review-panel:layout" + refreshOverviewPanel = () -> + $scope.reviewPanel.overview.loading = true + $http.get "/project/#{$scope.project_id}/ranges" + .success (docs) -> + for doc in docs + if doc.id != $scope.editor.open_doc_id # this is kept up to date in real-time, don't overwrite + rangesTrackers[doc.id] ?= new RangesTracker() + rangesTrackers[doc.id].comments = doc.ranges?.comments or [] + rangesTrackers[doc.id].changes = doc.ranges?.changes or [] + updateEntries(doc.id) + $scope.reviewPanel.overview.loading = false + .error (error) -> + console.log "loading ranges errored", error + $scope.reviewPanel.overview.loading = false + updateEntries = (doc_id) -> rangesTracker = getChangeTracker(doc_id) entries = getDocEntries(doc_id) diff --git a/services/web/test/UnitTests/coffee/Docstore/DocstoreManagerTests.coffee b/services/web/test/UnitTests/coffee/Docstore/DocstoreManagerTests.coffee index 52603c28bb..abcc55a0b9 100644 --- a/services/web/test/UnitTests/coffee/Docstore/DocstoreManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/Docstore/DocstoreManagerTests.coffee @@ -186,6 +186,38 @@ describe "DocstoreManager", -> }, "error getting all docs from docstore") .should.equal true + describe "getAllRanges", -> + describe "with a successful response code", -> + beforeEach -> + @request.get = sinon.stub().callsArgWith(1, null, statusCode: 204, @docs = [{ _id: "mock-doc-id", ranges: "mock-ranges" }]) + @DocstoreManager.getAllRanges @project_id, @callback + + it "should get all the project doc ranges in the docstore api", -> + @request.get + .calledWith({ + url: "#{@settings.apis.docstore.url}/project/#{@project_id}/ranges" + json: true + }) + .should.equal true + + it "should call the callback with the docs", -> + @callback.calledWith(null, @docs).should.equal true + + describe "with a failed response code", -> + beforeEach -> + @request.get = sinon.stub().callsArgWith(1, null, statusCode: 500, "") + @DocstoreManager.getAllRanges @project_id, @callback + + it "should call the callback with an error", -> + @callback.calledWith(new Error("docstore api responded with non-success code: 500")).should.equal true + + it "should log the error", -> + @logger.error + .calledWith({ + err: new Error("docstore api responded with a non-success code: 500") + project_id: @project_id + }, "error getting all doc ranges from docstore") + .should.equal true describe "archiveProject", -> describe "with a successful response code", -> diff --git a/services/web/test/UnitTests/coffee/TrackChanges/TrackChangesControllerTests.coffee b/services/web/test/UnitTests/coffee/History/HistoryControllerTests.coffee similarity index 83% rename from services/web/test/UnitTests/coffee/TrackChanges/TrackChangesControllerTests.coffee rename to services/web/test/UnitTests/coffee/History/HistoryControllerTests.coffee index bcc57b58b8..577aae6a9d 100644 --- a/services/web/test/UnitTests/coffee/TrackChanges/TrackChangesControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/History/HistoryControllerTests.coffee @@ -1,21 +1,21 @@ chai = require('chai') chai.should() sinon = require("sinon") -modulePath = "../../../../app/js/Features/TrackChanges/TrackChangesController" +modulePath = "../../../../app/js/Features/History/HistoryController" SandboxedModule = require('sandboxed-module') -describe "TrackChangesController", -> +describe "HistoryController", -> beforeEach -> @user_id = "user-id-123" @AuthenticationController = getLoggedInUserId: sinon.stub().returns(@user_id) - @TrackChangesController = SandboxedModule.require modulePath, requires: + @HistoryController = SandboxedModule.require modulePath, requires: "request" : @request = sinon.stub() "settings-sharelatex": @settings = {} "logger-sharelatex": @logger = {log: sinon.stub(), error: sinon.stub()} "../Authentication/AuthenticationController": @AuthenticationController - describe "proxyToTrackChangesApi", -> + describe "proxyToHistoryApi", -> beforeEach -> @req = { url: "/mock/url", method: "POST" } @res = "mock-res" @@ -28,7 +28,7 @@ describe "TrackChangesController", -> pipe: sinon.stub() on: (event, handler) -> @events[event] = handler @request.returns @proxy - @TrackChangesController.proxyToTrackChangesApi @req, @res, @next + @HistoryController.proxyToHistoryApi @req, @res, @next describe "successfully", -> it "should get the user id", -> diff --git a/services/web/test/UnitTests/coffee/TrackChanges/TrackChangesManagerTests.coffee b/services/web/test/UnitTests/coffee/History/HistoryManagerTests.coffee similarity index 81% rename from services/web/test/UnitTests/coffee/TrackChanges/TrackChangesManagerTests.coffee rename to services/web/test/UnitTests/coffee/History/HistoryManagerTests.coffee index 90b36f89c5..65b22812ea 100644 --- a/services/web/test/UnitTests/coffee/TrackChanges/TrackChangesManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/History/HistoryManagerTests.coffee @@ -2,12 +2,12 @@ chai = require('chai') expect = chai.expect chai.should() sinon = require("sinon") -modulePath = "../../../../app/js/Features/TrackChanges/TrackChangesManager" +modulePath = "../../../../app/js/Features/History/HistoryManager" SandboxedModule = require('sandboxed-module') -describe "TrackChangesManager", -> +describe "HistoryManager", -> beforeEach -> - @TrackChangesManager = SandboxedModule.require modulePath, requires: + @HistoryManager = SandboxedModule.require modulePath, requires: "request" : @request = sinon.stub() "settings-sharelatex": @settings = apis: @@ -22,7 +22,7 @@ describe "TrackChangesManager", -> describe "with a successful response code", -> beforeEach -> @request.post = sinon.stub().callsArgWith(1, null, statusCode: 204, "") - @TrackChangesManager.flushProject @project_id, @callback + @HistoryManager.flushProject @project_id, @callback it "should flush the project in the track changes api", -> @request.post @@ -35,7 +35,7 @@ describe "TrackChangesManager", -> describe "with a failed response code", -> beforeEach -> @request.post = sinon.stub().callsArgWith(1, null, statusCode: 500, "") - @TrackChangesManager.flushProject @project_id, @callback + @HistoryManager.flushProject @project_id, @callback it "should call the callback with an error", -> @callback.calledWith(new Error("track-changes api responded with a non-success code: 500")).should.equal true @@ -52,12 +52,12 @@ describe "TrackChangesManager", -> it "should call the post endpoint", (done)-> @request.post.callsArgWith(1, null, {}) - @TrackChangesManager.archiveProject @project_id, (err)=> + @HistoryManager.archiveProject @project_id, (err)=> @request.post.calledWith("#{@settings.apis.trackchanges.url}/project/#{@project_id}/archive") done() it "should return an error on a non success", (done)-> @request.post.callsArgWith(1, null, {statusCode:500}) - @TrackChangesManager.archiveProject @project_id, (err)=> + @HistoryManager.archiveProject @project_id, (err)=> expect(err).to.exist done() \ No newline at end of file