From afb8bb6a42609a199926da868893fa9aef5d5eec Mon Sep 17 00:00:00 2001 From: James Allen Date: Tue, 11 Mar 2014 12:13:46 +0000 Subject: [PATCH 1/7] Add in restore button to track changes --- .../TrackChangesController.coffee | 2 +- services/web/app/coffee/router.coffee | 1 + services/web/app/views/templates.jade | 6 ++- .../coffee/file-tree/FileTreeManager.coffee | 5 ++ services/web/public/coffee/models/User.coffee | 3 ++ .../track-changes/ChangeListView.coffee | 12 ++++- .../track-changes/TrackChangesManager.coffee | 48 +++++++++++++++++-- .../coffee/track-changes/models/Change.coffee | 10 ++-- .../coffee/track-changes/models/Diff.coffee | 7 ++- services/web/public/coffee/utils/Modal.coffee | 6 +-- .../public/stylesheets/less/trackchanges.less | 27 +++++++++-- .../TrackChangesControllerTests.coffee | 13 +++-- 12 files changed, 113 insertions(+), 27 deletions(-) diff --git a/services/web/app/coffee/Features/TrackChanges/TrackChangesController.coffee b/services/web/app/coffee/Features/TrackChanges/TrackChangesController.coffee index bb7193b5a7..1d7674e9da 100644 --- a/services/web/app/coffee/Features/TrackChanges/TrackChangesController.coffee +++ b/services/web/app/coffee/Features/TrackChanges/TrackChangesController.coffee @@ -6,7 +6,7 @@ module.exports = TrackChangesController = proxyToTrackChangesApi: (req, res, next = (error) ->) -> url = settings.apis.trackchanges.url + req.url logger.log url: url, "proxying to track-changes api" - getReq = request.get(url) + getReq = request(url: url, method: req.method) getReq.pipe(res) getReq.on "error", (error) -> logger.error err: error, "track-changes API error" diff --git a/services/web/app/coffee/router.coffee b/services/web/app/coffee/router.coffee index 4c9f88b31a..46d702f13f 100644 --- a/services/web/app/coffee/router.coffee +++ b/services/web/app/coffee/router.coffee @@ -125,6 +125,7 @@ module.exports = class Router app.get "/project/:Project_id/doc/:doc_id/updates", SecutiryManager.requestCanAccessProject, TrackChangesController.proxyToTrackChangesApi app.get "/project/:Project_id/doc/:doc_id/diff", SecutiryManager.requestCanAccessProject, TrackChangesController.proxyToTrackChangesApi + app.post "/project/:Project_id/doc/:doc_id/version/:version_id/restore", SecutiryManager.requestCanAccessProject, TrackChangesController.proxyToTrackChangesApi app.post '/project/:project_id/leave', AuthenticationController.requireLogin(), CollaboratorsController.removeSelfFromProject app.get '/project/:Project_id/collaborators', SecutiryManager.requestCanAccessProject(allow_auth_token: true), CollaboratorsController.getCollaborators diff --git a/services/web/app/views/templates.jade b/services/web/app/views/templates.jade index ca7c07fa78..d63f2efeb0 100644 --- a/services/web/app/views/templates.jade +++ b/services/web/app/views/templates.jade @@ -140,8 +140,7 @@ button.btn.btn-primary ok script(type="text/template")#genericModalButtonTemplate - a(href="#",class="btn {{ class }}") {{ text }} - + button(class="btn {{ class }}") {{ text }} script(type="text/template")#editorPanelTemplate #editorArea(style='display: none;') @@ -445,6 +444,9 @@ div.color-square(style="background-color: hsl({{hue}}, 100%, 70%);") span {{name}} + div(class='restore') + a(href="#") Restore to here + script(type='text/template')#changeListTemplate ul.change-list.nav.nav-pills.nav-stacked li.loading-changes Loading... diff --git a/services/web/public/coffee/file-tree/FileTreeManager.coffee b/services/web/public/coffee/file-tree/FileTreeManager.coffee index 6d7364dd02..6526d91dad 100644 --- a/services/web/public/coffee/file-tree/FileTreeManager.coffee +++ b/services/web/public/coffee/file-tree/FileTreeManager.coffee @@ -144,6 +144,11 @@ define [ # it's not the root folder so keep going path = entity.get("name") + "/" + path return path + + getNameOfEntityId: (entity_id) -> + entity = @getEntity(entity_id) + return if !entity? + return entity.get("name") # RENAMING renameSelected: () -> diff --git a/services/web/public/coffee/models/User.coffee b/services/web/public/coffee/models/User.coffee index 1e101bee34..f83df16139 100644 --- a/services/web/public/coffee/models/User.coffee +++ b/services/web/public/coffee/models/User.coffee @@ -49,4 +49,7 @@ define [ find: (id) -> @loadedModel ||= {} return @loadedModel[id] + + getAnonymousUser: () -> + return User.findOrBuild("anonymous", { first_name: "Anonymous", email: "anon@sharelatex.com" }) } diff --git a/services/web/public/coffee/track-changes/ChangeListView.coffee b/services/web/public/coffee/track-changes/ChangeListView.coffee index 6456e912b4..937542d43f 100644 --- a/services/web/public/coffee/track-changes/ChangeListView.coffee +++ b/services/web/public/coffee/track-changes/ChangeListView.coffee @@ -70,6 +70,9 @@ define [ delete @hoverFromIndex @resetHoverStates() + view.on "click:restore", (e) => + @trigger "restore", view.model + view.resetSelector(index, @selectedFromIndex, @selectedToIndex) resetAllSelectors: () -> @@ -77,11 +80,11 @@ define [ view.resetSelector(i, @selectedFromIndex, @selectedToIndex) resetHoverStates: () -> - if @hoverToIndex? + if @hoverToIndex? and @hoverToIndex != @selectedToIndex @$("ul").addClass("hover-state") for view, i in @itemViews view.resetHoverState(i, @selectedFromIndex, @hoverToIndex) - else if @hoverFromIndex? + else if @hoverFromIndex? and @hoverFromIndex != @selectedFromIndex @$("ul").addClass("hover-state") for view, i in @itemViews view.resetHoverState(i, @hoverFromIndex, @selectedToIndex) @@ -152,6 +155,7 @@ define [ @trigger "mouseenter:from", args... "mouseleave .change-selector-from": (args...) -> @trigger "mouseleave:from", args... + "click .restore a": "onRestoreClick" template : $("#changeListItemTemplate").html() @@ -183,6 +187,10 @@ define [ onFromSelectorClick: (e) -> @trigger "selected:from", e, @ + onRestoreClick: (e) -> + e.preventDefault() + @trigger "click:restore", e, @ + isSelectedFrom: () -> @$(".change-selector-from").is(":checked") diff --git a/services/web/public/coffee/track-changes/TrackChangesManager.coffee b/services/web/public/coffee/track-changes/TrackChangesManager.coffee index 58d470b51b..cd56626e56 100644 --- a/services/web/public/coffee/track-changes/TrackChangesManager.coffee +++ b/services/web/public/coffee/track-changes/TrackChangesManager.coffee @@ -3,21 +3,23 @@ define [ "track-changes/models/Diff" "track-changes/ChangeListView" "track-changes/DiffView" -], (ChangeList, Diff, ChangeListView, DiffView) -> + "utils/Modal" + "moment" +], (ChangeList, Diff, ChangeListView, DiffView, Modal, moment) -> class TrackChangesManager template: $("#trackChangesPanelTemplate").html() constructor: (@ide) -> @$el = $(@template) $("#editorWrapper").append(@$el) - @hideEl() + @hide() @ide.editor.on "change:doc", () => - @hideEl() + @hide() @$el.find(".track-changes-close").on "click", (e) => e.preventDefault - @hideEl() + @hide() show: () -> @project_id = window.userSettings.project_id @@ -44,14 +46,50 @@ define [ ) @diff.fetch() + @changeListView.on "restore", (change) => + @restore(change) + @showEl() showEl: -> @ide.editor.hide() @$el.show() - hideEl: () -> + hide: () -> @ide.editor.show() @$el.hide() + restore: (change) -> + name = @ide.fileTreeManager.getNameOfEntityId(@doc_id) + date = moment(change.get("start_ts")).format("Do MMM YYYY, h:mm:ss a") + modal = new Modal({ + title: "Restore document" + message: "Are you sure you want to restore #{name} to before the changes on #{date}" + buttons: [{ + text: "Cancel" + }, { + text: "Restore" + class: "btn-success" + close: false + callback: ($button) => + $button.text("Restoring...") + $button.prop("disabled", true) + @doRestore change.get("version"), (error) => + modal.remove() + @hide() + }] + }) + + doRestore: (version, callback = (error) ->) -> + $.ajax { + url: "/project/#{@project_id}/doc/#{@doc_id}/version/#{version}/restore" + type: "POST" + headers: + "X-CSRF-Token": window.csrfToken + success: () -> + callback() + error: (error) -> + callback(error) + } + return TrackChangesManager diff --git a/services/web/public/coffee/track-changes/models/Change.coffee b/services/web/public/coffee/track-changes/models/Change.coffee index fcefb43661..845c44d5ec 100644 --- a/services/web/public/coffee/track-changes/models/Change.coffee +++ b/services/web/public/coffee/track-changes/models/Change.coffee @@ -4,9 +4,13 @@ define [ ], (User)-> Change = Backbone.Model.extend parse: (change) -> - return { + model = { start_ts: change.meta.start_ts end_ts: change.meta.end_ts - user: User.findOrBuild(change.meta.user.id, change.meta.user) version: change.v - } \ No newline at end of file + } + if change.meta.user? + model.user = User.findOrBuild(change.meta.user.id, change.meta.user) + else + model.user = User.getAnonymousUser() + return model \ No newline at end of file diff --git a/services/web/public/coffee/track-changes/models/Diff.coffee b/services/web/public/coffee/track-changes/models/Diff.coffee index 7d301273e2..52ad05c439 100644 --- a/services/web/public/coffee/track-changes/models/Diff.coffee +++ b/services/web/public/coffee/track-changes/models/Diff.coffee @@ -8,6 +8,9 @@ define [ parse: (diff) -> for entry in diff.diff - if entry.meta? and entry.meta.user? - entry.meta.user = User.findOrBuild(entry.meta.user.id, entry.meta.user) + if entry.meta? + if entry.meta.user? + entry.meta.user = User.findOrBuild(entry.meta.user.id, entry.meta.user) + else + entry.meta.user = User.getAnonymousUser() return diff diff --git a/services/web/public/coffee/utils/Modal.coffee b/services/web/public/coffee/utils/Modal.coffee index 709445b6fd..d2adc70df8 100644 --- a/services/web/public/coffee/utils/Modal.coffee +++ b/services/web/public/coffee/utils/Modal.coffee @@ -19,8 +19,9 @@ define [ button.on "click", (e) -> e.preventDefault() if buttonOptions.callback? - buttonOptions.callback() - self.remove() + buttonOptions.callback(button) + if !buttonOptions.close? or buttonOptions.close + self.remove() @$el.modal # make sure we control when the modal is hidden @@ -39,7 +40,6 @@ define [ @$el.find('input').focus() - remove: () -> @$el.modal("hide") Backbone.View.prototype.remove.call(this) diff --git a/services/web/public/stylesheets/less/trackchanges.less b/services/web/public/stylesheets/less/trackchanges.less index 3dc4a94a42..031356225f 100644 --- a/services/web/public/stylesheets/less/trackchanges.less +++ b/services/web/public/stylesheets/less/trackchanges.less @@ -71,11 +71,9 @@ ul.change-list { li { - padding: 6px 4px; position: relative; border-bottom: 1px solid #ccc; cursor: pointer; - min-height: 38px; .change-selectors { .change-selector-from { position: absolute; @@ -98,7 +96,8 @@ } } .change-description { - padding-left: 26px; + padding: 6px 4px 6px 30px; + min-height: 38px; } .change-name { font-size: 0.9em; @@ -112,6 +111,15 @@ margin-right: 4px; margin-bottom: -1px; } + .restore { + a { + display: block; + padding: 4px; + text-align: center; + border-top: 1px solid #ccc; + } + display: none; + } &:hover { background-color: #eaeaea; } @@ -137,12 +145,16 @@ li.selected-from { .change-selectors { .range { - bottom: 10px; + bottom: 37px; } .change-selector-from { opacity: 1; + bottom: 32px; } } + .restore { + display: block; + } } } ul.change-list.hover-state { @@ -182,5 +194,12 @@ } } } + li.selected-from.hover-selected-from { + .change-selectors { + .range { + bottom: 37px; + } + } + } } } \ No newline at end of file diff --git a/services/web/test/UnitTests/coffee/TrackChanges/TrackChangesControllerTests.coffee b/services/web/test/UnitTests/coffee/TrackChanges/TrackChangesControllerTests.coffee index e98305b939..9aa558a77e 100644 --- a/services/web/test/UnitTests/coffee/TrackChanges/TrackChangesControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/TrackChanges/TrackChangesControllerTests.coffee @@ -7,13 +7,13 @@ SandboxedModule = require('sandboxed-module') describe "TrackChangesController", -> beforeEach -> @TrackChangesController = SandboxedModule.require modulePath, requires: - "request" : @request = {} + "request" : @request = sinon.stub() "settings-sharelatex": @settings = {} "logger-sharelatex": @logger = {log: sinon.stub(), error: sinon.stub()} describe "proxyToTrackChangesApi", -> beforeEach -> - @req = { url: "/mock/url" } + @req = { url: "/mock/url", method: "POST" } @res = "mock-res" @next = sinon.stub() @settings.apis = @@ -23,13 +23,16 @@ describe "TrackChangesController", -> events: {} pipe: sinon.stub() on: (event, handler) -> @events[event] = handler - @request.get = sinon.stub().returns @proxy + @request.returns @proxy @TrackChangesController.proxyToTrackChangesApi @req, @res, @next describe "successfully", -> it "should call the track changes api", -> - @request.get - .calledWith("#{@settings.apis.trackchanges.url}#{@req.url}") + @request + .calledWith({ + url: "#{@settings.apis.trackchanges.url}#{@req.url}" + method: @req.method + }) .should.equal true it "should pipe the response to the client", -> From 1a9c86417cb243f1a5bbc6abe90fa1073ae77a0d Mon Sep 17 00:00:00 2001 From: James Allen Date: Tue, 11 Mar 2014 12:14:36 +0000 Subject: [PATCH 2/7] Add question mark to restore modal --- .../web/public/coffee/track-changes/TrackChangesManager.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/public/coffee/track-changes/TrackChangesManager.coffee b/services/web/public/coffee/track-changes/TrackChangesManager.coffee index cd56626e56..b6b2da45a2 100644 --- a/services/web/public/coffee/track-changes/TrackChangesManager.coffee +++ b/services/web/public/coffee/track-changes/TrackChangesManager.coffee @@ -64,7 +64,7 @@ define [ date = moment(change.get("start_ts")).format("Do MMM YYYY, h:mm:ss a") modal = new Modal({ title: "Restore document" - message: "Are you sure you want to restore #{name} to before the changes on #{date}" + message: "Are you sure you want to restore #{name} to before the changes on #{date}?" buttons: [{ text: "Cancel" }, { From da9fa286120a995c0261933562c78c9a4d801fca Mon Sep 17 00:00:00 2001 From: James Allen Date: Tue, 11 Mar 2014 12:16:51 +0000 Subject: [PATCH 3/7] Remove mention of Dropbox in external update dialog --- services/web/public/coffee/editor/Editor.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/public/coffee/editor/Editor.coffee b/services/web/public/coffee/editor/Editor.coffee index 7c86598ad6..d4313fe6eb 100644 --- a/services/web/public/coffee/editor/Editor.coffee +++ b/services/web/public/coffee/editor/Editor.coffee @@ -224,7 +224,7 @@ define [ document.on "externalUpdate", () => Modal.createModal title: "Document Updated Externally" - message: "This document was just updated externally (probably via Dropbox). Any recent changes you have made may have been overwritten. To see previous versions please look in the history." + message: "This document was just updated externally. Any recent changes you have made may have been overwritten. To see previous versions please look in the history." buttons:[ text: "Ok" ] From be64b510e8309bb1a3e78c3941a76b4352a165f9 Mon Sep 17 00:00:00 2001 From: James Allen Date: Tue, 11 Mar 2014 18:01:14 +0000 Subject: [PATCH 4/7] Support the new update format from the track changes api --- .../TrackChangesController.coffee | 22 +++++++++++------ services/web/app/views/templates.jade | 11 +++++---- .../web/public/coffee/pdf/PdfManager.coffee | 1 - .../track-changes/ChangeListView.coffee | 24 ++++++++++--------- .../track-changes/TrackChangesManager.coffee | 4 ++-- .../coffee/track-changes/models/Change.coffee | 12 ++++++---- .../track-changes/models/ChangeList.coffee | 2 +- .../public/stylesheets/less/trackchanges.less | 2 +- .../TrackChangesControllerTests.coffee | 10 ++++++++ 9 files changed, 56 insertions(+), 32 deletions(-) diff --git a/services/web/app/coffee/Features/TrackChanges/TrackChangesController.coffee b/services/web/app/coffee/Features/TrackChanges/TrackChangesController.coffee index 1d7674e9da..f548cadde6 100644 --- a/services/web/app/coffee/Features/TrackChanges/TrackChangesController.coffee +++ b/services/web/app/coffee/Features/TrackChanges/TrackChangesController.coffee @@ -1,13 +1,21 @@ logger = require "logger-sharelatex" request = require "request" settings = require "settings-sharelatex" +AuthenticationController = require "../Authentication/AuthenticationController" module.exports = TrackChangesController = proxyToTrackChangesApi: (req, res, next = (error) ->) -> - url = settings.apis.trackchanges.url + req.url - logger.log url: url, "proxying to track-changes api" - getReq = request(url: url, method: req.method) - getReq.pipe(res) - getReq.on "error", (error) -> - logger.error err: error, "track-changes API error" - next(error) \ No newline at end of file + AuthenticationController.getLoggedInUserId req, (error, user_id) -> + return next(error) if error? + url = settings.apis.trackchanges.url + req.url + logger.log url: url, "proxying to track-changes api" + getReq = request( + url: url + method: req.method + headers: + "X-User-Id": user_id + ) + getReq.pipe(res) + getReq.on "error", (error) -> + logger.error err: error, "track-changes API error" + next(error) \ No newline at end of file diff --git a/services/web/app/views/templates.jade b/services/web/app/views/templates.jade index d63f2efeb0..04b152dbf5 100644 --- a/services/web/app/views/templates.jade +++ b/services/web/app/views/templates.jade @@ -439,14 +439,17 @@ input(type="radio",name="toVersion").change-selector-to div(class='change-description') - div(class='change-date') {{date}} - div(class='change-name') - div.color-square(style="background-color: hsl({{hue}}, 100%, 70%);") - span {{name}} + div(class='change-date') {{date}} + div {{{users}}} div(class='restore') a(href="#") Restore to here + script(type='text/template')#changeListItemUserTemplate + div(class='change-name') + div.color-square(style="background-color: hsl({{hue}}, 100%, 70%);") + span {{name}} + script(type='text/template')#changeListTemplate ul.change-list.nav.nav-pills.nav-stacked li.loading-changes Loading... diff --git a/services/web/public/coffee/pdf/PdfManager.coffee b/services/web/public/coffee/pdf/PdfManager.coffee index ee98a1b179..8315a6a311 100644 --- a/services/web/public/coffee/pdf/PdfManager.coffee +++ b/services/web/public/coffee/pdf/PdfManager.coffee @@ -143,7 +143,6 @@ define [ @view.showLog() if outputFiles? - console.log "outputFiles", outputFiles @view.showOutputFileDownloadLinks(outputFiles) fetchLogAndUpdateView: (pdfExists) -> diff --git a/services/web/public/coffee/track-changes/ChangeListView.coffee b/services/web/public/coffee/track-changes/ChangeListView.coffee index 937542d43f..36b190ee5f 100644 --- a/services/web/public/coffee/track-changes/ChangeListView.coffee +++ b/services/web/public/coffee/track-changes/ChangeListView.coffee @@ -158,24 +158,26 @@ define [ "click .restore a": "onRestoreClick" - template : $("#changeListItemTemplate").html() + templates: + item: $("#changeListItemTemplate").html() + user: $("#changeListItemUserTemplate").html() initialize: -> @render() render: -> - @$el.html Mustache.to_html(@template, @modelView()) - return this - - modelView: -> - modelView = { - hue: @model.get("user").hue() + userHtml = for user in @model.get("users") + Mustache.to_html @templates.user, { + hue: user.hue() + name: user.name() + } + data = { date: moment(parseInt(@model.get("end_ts"), 10)).calendar() - name: @model.get("user").name() + users: userHtml.join("") } - # modelView.start_ts = util.formatDate(modelView.start_ts) - # modelView.end_ts = util.formatDate(modelView.end_ts) - return modelView + + @$el.html Mustache.to_html(@templates.item, data) + return this onClick: (e) -> e.preventDefault() diff --git a/services/web/public/coffee/track-changes/TrackChangesManager.coffee b/services/web/public/coffee/track-changes/TrackChangesManager.coffee index b6b2da45a2..99265838ee 100644 --- a/services/web/public/coffee/track-changes/TrackChangesManager.coffee +++ b/services/web/public/coffee/track-changes/TrackChangesManager.coffee @@ -37,8 +37,8 @@ define [ @diff = new Diff({ project_id: @project_id doc_id: @doc_id - from: fromModel.get("version") - to: toModel.get("version") + from: fromModel.get("fromVersion") + to: toModel.get("toVersion") }) @diffView = new DiffView( model: @diff diff --git a/services/web/public/coffee/track-changes/models/Change.coffee b/services/web/public/coffee/track-changes/models/Change.coffee index 845c44d5ec..bd1913e67f 100644 --- a/services/web/public/coffee/track-changes/models/Change.coffee +++ b/services/web/public/coffee/track-changes/models/Change.coffee @@ -7,10 +7,12 @@ define [ model = { start_ts: change.meta.start_ts end_ts: change.meta.end_ts - version: change.v + fromVersion: change.fromV + toVersion: change.toV } - if change.meta.user? - model.user = User.findOrBuild(change.meta.user.id, change.meta.user) - else - model.user = User.getAnonymousUser() + model.users = [] + for user in change.meta.users or [] + model.users.push User.findOrBuild(user.id, user) + if model.users.length == 0 + model.users.push User.getAnonymousUser() return model \ No newline at end of file diff --git a/services/web/public/coffee/track-changes/models/ChangeList.coffee b/services/web/public/coffee/track-changes/models/ChangeList.coffee index cfc7673a6a..17902223d2 100644 --- a/services/web/public/coffee/track-changes/models/ChangeList.coffee +++ b/services/web/public/coffee/track-changes/models/ChangeList.coffee @@ -12,7 +12,7 @@ define [ url = "/project/#{@options.project_id}/doc/#{@options.doc_id}/updates?limit=#{@batchSize}" if @models.length > 0 last = @models[@models.length - 1] - url += "&to=#{last.get("version") - 1}" + url += "&to=#{last.get("fromVersion") - 1}" return url parse: (json) -> diff --git a/services/web/public/stylesheets/less/trackchanges.less b/services/web/public/stylesheets/less/trackchanges.less index 031356225f..584a2cbfd8 100644 --- a/services/web/public/stylesheets/less/trackchanges.less +++ b/services/web/public/stylesheets/less/trackchanges.less @@ -28,7 +28,7 @@ background-color: white; .track-changes-header { - background-color: black; + background-color: #282828; h3 { color: white; padding-left: 8px; diff --git a/services/web/test/UnitTests/coffee/TrackChanges/TrackChangesControllerTests.coffee b/services/web/test/UnitTests/coffee/TrackChanges/TrackChangesControllerTests.coffee index 9aa558a77e..ad9f1ed04a 100644 --- a/services/web/test/UnitTests/coffee/TrackChanges/TrackChangesControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/TrackChanges/TrackChangesControllerTests.coffee @@ -10,12 +10,14 @@ describe "TrackChangesController", -> "request" : @request = sinon.stub() "settings-sharelatex": @settings = {} "logger-sharelatex": @logger = {log: sinon.stub(), error: sinon.stub()} + "../Authentication/AuthenticationController": @AuthenticationController = {} describe "proxyToTrackChangesApi", -> beforeEach -> @req = { url: "/mock/url", method: "POST" } @res = "mock-res" @next = sinon.stub() + @user_id = "user-id-123" @settings.apis = trackchanges: url: "http://trackchanges.example.com" @@ -24,14 +26,22 @@ describe "TrackChangesController", -> pipe: sinon.stub() on: (event, handler) -> @events[event] = handler @request.returns @proxy + @AuthenticationController.getLoggedInUserId = sinon.stub().callsArgWith(1, null, @user_id) @TrackChangesController.proxyToTrackChangesApi @req, @res, @next describe "successfully", -> + 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 From e0da6da4c6a1427ee6d08779e4eed2077eaf2831 Mon Sep 17 00:00:00 2001 From: James Allen Date: Wed, 12 Mar 2014 16:46:20 +0000 Subject: [PATCH 5/7] Improve hover info in diff --- .../coffee/track-changes/DiffView.coffee | 113 +++++++++++------- .../public/stylesheets/less/trackchanges.less | 20 ++-- 2 files changed, 78 insertions(+), 55 deletions(-) diff --git a/services/web/public/coffee/track-changes/DiffView.coffee b/services/web/public/coffee/track-changes/DiffView.coffee index 9c433b24d2..396de3de67 100644 --- a/services/web/public/coffee/track-changes/DiffView.coffee +++ b/services/web/public/coffee/track-changes/DiffView.coffee @@ -2,8 +2,9 @@ define [ "ace/ace" "ace/mode/latex" "ace/range" + "moment" "libs/backbone" -], (Ace, LatexMode, Range)-> +], (Ace, LatexMode, Range, moment)-> DiffView = Backbone.View.extend initialize: () -> @model.on "change:diff", () => @render() @@ -19,6 +20,7 @@ define [ session.setMode(new LatexMode.Mode()) session.setUseWrapMode(true) @insertMarkers() + @insertNameTag() return @ createAceEditor: () -> @@ -44,6 +46,7 @@ define [ insertMarkers: () -> row = 0 column = 0 + @entries = [] for entry, i in @model.get("diff") or [] content = entry.u or entry.i or entry.d content ||= "" @@ -62,9 +65,12 @@ define [ range = new Range.Range( startRow, startColumn, endRow, endColumn ) - @addMarker(range, "change-marker-#{i}", entry) + entry.range = range + @addMarker(range, entry) + if entry.i? or entry.d? + @entries.push entry - addMarker: (range, id, entry) -> + addMarker: (range, entry) -> session = @aceEditor.getSession() markerBackLayer = @aceEditor.renderer.$markerBack markerFrontLayer = @aceEditor.renderer.$markerFront @@ -72,10 +78,9 @@ define [ if entry.i? or entry.d? hue = entry.meta.user.hue() if entry.i? - @_addMarkerWithCustomStyle session, markerBackLayer, range, "deleted-change-background", false, """ + @_addMarkerWithCustomStyle session, markerBackLayer, range, "inserted-change-background", false, """ background-color : hsl(#{hue}, 70%, 85%); """ - tag = "Added by #{entry.meta.user.name()}" if entry.d? @_addMarkerWithCustomStyle session, markerBackLayer, range, "deleted-change-background", false, """ background-color : hsl(#{hue}, 70%, 95%); @@ -84,13 +89,6 @@ define [ height: #{Math.round(lineHeight/2) - 1}px; border-bottom: 2px solid hsl(#{hue}, 70%, 40%); """ - tag = "Deleted by #{entry.meta.user.name()}" - - date = moment(parseInt(entry.meta.end_ts, 10)).format("Do MMM YYYY, h:mm:ss a") - tag += " on #{date}" - @_addNameTag session, id, range, tag, """ - background-color : hsl(#{hue}, 70%, 95%); - """ _addMarkerWithCustomStyle: (session, markerLayer, range, klass, foreground, style) -> session.addMarker range, klass, (html, range, left, top, config) -> @@ -100,42 +98,65 @@ define [ markerLayer.drawSingleLineMarker(html, range, "#{klass} ace_start", config, 0, style) , foreground - _addNameTag: (session, id, range, content, style) -> - @nameMarkers ||= [] - @nameMarkers.push - range: range - id: id - startRange = new Range.Range( - range.start.row, range.start.column - range.start.row, range.start.column + 1 - ) - session.addMarker startRange, "change-name-marker", (html, range, left, top, config) -> - html.push """ -
-
#{content}
-
- """ - , true + insertNameTag: () -> + @$ace = $(@aceEditor.renderer.container).find(".ace_scroller") + @$nameTagEl = $("
") + @$nameTagEl.css({ + position: "absolute" + }) + @$nameTagEl.hide() + @$ace.append(@$nameTagEl) + + _drawNameTag: (entry, position) -> + @$nameTagEl.show() + + if entry.i? + text = "Added by #{entry.meta.user.name()}" + else if entry.d? + text = "Deleted by #{entry.meta.user.name()}" + date = moment(parseInt(entry.meta.end_ts, 10)).format("Do MMM YYYY, h:mm a") + text += " on #{date}" + @$nameTagEl.text(text) + + position = @aceEditor.renderer.textToScreenCoordinates(position.row, position.column) + offset = @$ace.offset() + position.pageX = position.pageX - offset.left + position.pageY = position.pageY - offset.top + height = @$ace.height() + + hue = entry.meta.user.hue() + css = { + "background-color" : "hsl(#{hue}, 70%, 90%)"; + } + + if position.pageX + @$nameTagEl.width() < @$ace.width() + css["left"] = position.pageX + css["right"] = "auto" + else + css["right"] = 0 + css["left"] = "auto" + + if position.pageY > 2 * @$nameTagEl.height() + css["bottom"] = height - position.pageY + css["top"] = "auto" + else + css["top"] = position.pageY + @aceEditor.renderer.lineHeight + css["bottom"] = "auto" + + @$nameTagEl.css css + + _hideNameTag: () -> + @$nameTagEl.hide() updateVisibleNames: (e) -> - for marker in @nameMarkers or [] - if marker.range.contains(e.position.row, e.position.column) - $("##{marker.id}").find(".name").show() - else - $("##{marker.id}").find(".name").hide() + visibleName = false + for entry in @entries or [] + if entry.range.contains(e.position.row, e.position.column) + @_drawNameTag(entry, e.position) + visibleName = true + break + if !visibleName + @_hideNameTag() return DiffView diff --git a/services/web/public/stylesheets/less/trackchanges.less b/services/web/public/stylesheets/less/trackchanges.less index 584a2cbfd8..06959e6b30 100644 --- a/services/web/public/stylesheets/less/trackchanges.less +++ b/services/web/public/stylesheets/less/trackchanges.less @@ -16,6 +16,9 @@ left: 0; right: 0; bottom: 0; + .ace_active-line, .ace_cursor-layer { + display: none; + } } } @@ -53,20 +56,19 @@ } } - .deleted-change-background, .deleted-change-foreground, .inserted-change, .change-name-marker { + .deleted-change-background, .deleted-change-foreground, .inserted-change-background, .change-name-marker { position: absolute; z-index: 2; } .change-name-marker { - .name { - font-size: 0.8em; - padding: 2px 6px; - .border-radius(3px 3px 3px 3px); - position: absolute; - border: 1px solid #999; - left: 0; - } + font-size: 0.8em; + padding: 2px 6px; + .border-radius(3px 3px 3px 3px); + position: absolute; + border: 1px solid #999; + left: 0; + white-space: pre; } ul.change-list { From 05443b85daec1dd55dbd11368bf41a12a5340263 Mon Sep 17 00:00:00 2001 From: James Allen Date: Wed, 12 Mar 2014 17:09:11 +0000 Subject: [PATCH 6/7] Automatically show a sensible diff to start --- .../track-changes/ChangeListView.coffee | 31 ++++++------ .../track-changes/TrackChangesManager.coffee | 48 ++++++++++++++----- 2 files changed, 51 insertions(+), 28 deletions(-) diff --git a/services/web/public/coffee/track-changes/ChangeListView.coffee b/services/web/public/coffee/track-changes/ChangeListView.coffee index 36b190ee5f..ac8e918622 100644 --- a/services/web/public/coffee/track-changes/ChangeListView.coffee +++ b/services/web/public/coffee/track-changes/ChangeListView.coffee @@ -7,7 +7,7 @@ define [ template: $("#changeListTemplate").html() events: - "scroll" : "loadUntilFull" + "scroll" : () -> @loadUntilFull() initialize: () -> @itemViews = [] @@ -39,20 +39,13 @@ define [ view.$el.insertBefore(elementAtIndex) view.on "click", (e, v) => - @selectedToIndex = index - @selectedFromIndex = index - @resetAllSelectors() - @triggerChangeDiff() + @setSelectionRange(index, index) view.on "selected:to", (e, v) => - @selectedToIndex = index - @resetAllSelectors() - @triggerChangeDiff() + @setSelectionRange(@selectedFromIndex, index) view.on "selected:from", (e, v) => - @selectedFromIndex = index - @resetAllSelectors() - @triggerChangeDiff() + @setSelectionRange(index, @selectedToIndex) view.on "mouseenter:to", (e) => @hoverToIndex = index @@ -75,6 +68,12 @@ define [ view.resetSelector(index, @selectedFromIndex, @selectedToIndex) + setSelectionRange: (fromIndex, toIndex) -> + @selectedFromIndex = fromIndex + @selectedToIndex = toIndex + @resetAllSelectors() + @triggerChangeDiff() + resetAllSelectors: () -> for view, i in @itemViews view.resetSelector(i, @selectedFromIndex, @selectedToIndex) @@ -102,23 +101,23 @@ define [ atEndOfListView: -> @$el.scrollTop() + @$el.height() >= @$(".change-list").height() - 30 - loadUntilFull: (e, callback) -> + loadUntilFull: (callback = (error) ->) -> if (@listShorterThanContainer() or @atEndOfListView()) and not @atEndOfCollection and not @loading @showLoading() @hideEmptyMessage() @collection.fetchNextBatch - error: => + error: (error) => @hideLoading() @showEmptyMessageIfCollectionEmpty() - callback() if callback? + callback(error) success: (collection, response) => @hideLoading() if response.updates.length == @collection.batchSize - @loadUntilFull(e, callback) + @loadUntilFull(callback) else @atEndOfCollection = true @showEmptyMessageIfCollectionEmpty() - callback() if callback? + callback() else callback() if callback? diff --git a/services/web/public/coffee/track-changes/TrackChangesManager.coffee b/services/web/public/coffee/track-changes/TrackChangesManager.coffee index 99265838ee..62a7aaf524 100644 --- a/services/web/public/coffee/track-changes/TrackChangesManager.coffee +++ b/services/web/public/coffee/track-changes/TrackChangesManager.coffee @@ -31,26 +31,50 @@ define [ el : @$el.find(".change-list-area") ) @changeListView.render() - @changeListView.loadUntilFull() + @changeListView.loadUntilFull (error) => + @autoSelectDiff() @changeListView.on "change_diff", (fromModel, toModel) => - @diff = new Diff({ - project_id: @project_id - doc_id: @doc_id - from: fromModel.get("fromVersion") - to: toModel.get("toVersion") - }) - @diffView = new DiffView( - model: @diff - el: @$el.find(".track-changes-diff") - ) - @diff.fetch() + @showDiff(fromModel, toModel) @changeListView.on "restore", (change) => @restore(change) @showEl() + autoSelectDiff: () -> + if @changes.models.length == 0 + return + + # Find all change until the last one we made + fromIndex = null + for change, i in @changes.models + if ide.user in change.get("users") + if i > 0 + fromIndex = i - 1 + else + fromIndex = 0 + break + fromIndex = 0 if !fromIndex + + toChange = @changes.models[0] + fromChange = @changes.models[fromIndex] + @showDiff(fromChange, toChange) + @changeListView.setSelectionRange(fromIndex, 0) + + showDiff: (fromModel, toModel) -> + @diff = new Diff({ + project_id: @project_id + doc_id: @doc_id + from: fromModel.get("fromVersion") + to: toModel.get("toVersion") + }) + @diffView = new DiffView( + model: @diff + el: @$el.find(".track-changes-diff") + ) + @diff.fetch() + showEl: -> @ide.editor.hide() @$el.show() From 451ff14e654361026f923ae954b33eb58f5f2e0e Mon Sep 17 00:00:00 2001 From: James Allen Date: Wed, 12 Mar 2014 17:13:38 +0000 Subject: [PATCH 7/7] Improve track changes css --- services/web/public/stylesheets/less/trackchanges.less | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/services/web/public/stylesheets/less/trackchanges.less b/services/web/public/stylesheets/less/trackchanges.less index 06959e6b30..d4adeb4495 100644 --- a/services/web/public/stylesheets/less/trackchanges.less +++ b/services/web/public/stylesheets/less/trackchanges.less @@ -16,7 +16,7 @@ left: 0; right: 0; bottom: 0; - .ace_active-line, .ace_cursor-layer { + .ace_active-line, .ace_cursor-layer, .ace_gutter-active-line { display: none; } } @@ -126,6 +126,9 @@ background-color: #eaeaea; } } + li.loading-changes, li.empty-message { + padding: 6px; + } li.selected { background-color: #eaeaea; .change-selectors {