diff --git a/services/web/.nvmrc b/services/web/.nvmrc new file mode 100644 index 0000000000..994fe99096 --- /dev/null +++ b/services/web/.nvmrc @@ -0,0 +1 @@ +0.10.22 \ No newline at end of file diff --git a/services/web/app/coffee/Features/Analytics/AnalyticsController.coffee b/services/web/app/coffee/Features/Analytics/AnalyticsController.coffee index f9431a0f5c..18a7de5e24 100644 --- a/services/web/app/coffee/Features/Analytics/AnalyticsController.coffee +++ b/services/web/app/coffee/Features/Analytics/AnalyticsController.coffee @@ -1,7 +1,15 @@ AnalyticsManager = require "./AnalyticsManager" +Errors = require "../Errors/Errors" +AuthenticationController = require("../Authentication/AuthenticationController") module.exports = AnalyticsController = recordEvent: (req, res, next) -> - AnalyticsManager.recordEvent req.session?.user?._id, req.params.event, req.body, (error) -> - return next(error) if error? - res.send 204 + user_id = AuthenticationController.getLoggedInUserId(req) or req.sessionID + AnalyticsManager.recordEvent user_id, req.params.event, req.body, (error) -> + if error instanceof Errors.ServiceNotConfiguredError + # ignore, no-op + return res.send(204) + else if error? + return next(error) + else + return res.send 204 diff --git a/services/web/app/coffee/Features/Analytics/AnalyticsManager.coffee b/services/web/app/coffee/Features/Analytics/AnalyticsManager.coffee index 9f34d6d9d1..12e5b68c84 100644 --- a/services/web/app/coffee/Features/Analytics/AnalyticsManager.coffee +++ b/services/web/app/coffee/Features/Analytics/AnalyticsManager.coffee @@ -2,6 +2,7 @@ settings = require "settings-sharelatex" logger = require "logger-sharelatex" _ = require "underscore" request = require "request" +Errors = require '../Errors/Errors' makeRequest = (opts, callback)-> @@ -10,12 +11,20 @@ makeRequest = (opts, callback)-> opts.url = "#{settings.apis.analytics.url}#{urlPath}" request opts, callback else - callback() - + callback(new Errors.ServiceNotConfiguredError('Analytics service not configured')) module.exports = + identifyUser: (user_id, old_user_id, callback = (error)->)-> + opts = + body: + old_user_id:old_user_id + json:true + method:"POST" + timeout:1000 + url: "/user/#{user_id}/identify" + makeRequest opts, callback recordEvent: (user_id, event, segmentation = {}, callback = (error) ->) -> if user_id+"" == settings.smokeTest?.userId+"" diff --git a/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee b/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee index 485b046a85..3c43323887 100644 --- a/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee +++ b/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee @@ -87,6 +87,7 @@ module.exports = AuthenticationController = LoginRateLimiter.recordSuccessfulLogin(email) AuthenticationController._recordSuccessfulLogin(user._id) Analytics.recordEvent(user._id, "user-logged-in", {ip:req.ip}) + Analytics.identifyUser(user._id, req.sessionID) logger.log email: email, user_id: user._id.toString(), "successful log in" req.session.justLoggedIn = true # capture the request ip for use when creating the session diff --git a/services/web/app/coffee/Features/Authorization/AuthorizationManager.coffee b/services/web/app/coffee/Features/Authorization/AuthorizationManager.coffee index ded0b6f979..90c8cdb485 100644 --- a/services/web/app/coffee/Features/Authorization/AuthorizationManager.coffee +++ b/services/web/app/coffee/Features/Authorization/AuthorizationManager.coffee @@ -4,6 +4,8 @@ User = require("../../models/User").User PrivilegeLevels = require("./PrivilegeLevels") PublicAccessLevels = require("./PublicAccessLevels") Errors = require("../Errors/Errors") +ObjectId = require("mongojs").ObjectId + module.exports = AuthorizationManager = # Get the privilege level that the user has for the project @@ -13,6 +15,8 @@ module.exports = AuthorizationManager = # * becausePublic: true if the access level is only because the project is public. getPrivilegeLevelForProject: (user_id, project_id, callback = (error, privilegeLevel, becausePublic) ->) -> getPublicAccessLevel = () -> + if !ObjectId.isValid(project_id) + return callback(new Error("invalid project id")) Project.findOne { _id: project_id }, { publicAccesLevel: 1 }, (error, project) -> return callback(error) if error? if !project? diff --git a/services/web/app/coffee/Features/Blog/BlogController.coffee b/services/web/app/coffee/Features/Blog/BlogController.coffee index 0f0d9383c7..eb2b3fad94 100644 --- a/services/web/app/coffee/Features/Blog/BlogController.coffee +++ b/services/web/app/coffee/Features/Blog/BlogController.coffee @@ -10,7 +10,7 @@ module.exports = BlogController = url = req.url?.toLowerCase() blogUrl = "#{settings.apis.blog.url}#{url}" - extensionsToProxy = [".png", ".xml", ".jpeg", ".json", ".zip", ".eps", ".gif"] + extensionsToProxy = [".png", ".xml", ".jpeg", ".jpg", ".json", ".zip", ".eps", ".gif"] shouldProxy = _.find extensionsToProxy, (extension)-> url.indexOf(extension) != -1 @@ -42,4 +42,4 @@ module.exports = BlogController = upstream = request.get(originUrl) upstream.on "error", (error) -> logger.error err: error, "blog proxy error" - upstream.pipe res \ No newline at end of file + upstream.pipe res diff --git a/services/web/app/coffee/Features/Email/EmailSender.coffee b/services/web/app/coffee/Features/Email/EmailSender.coffee index 69574c8276..04b4d39132 100644 --- a/services/web/app/coffee/Features/Email/EmailSender.coffee +++ b/services/web/app/coffee/Features/Email/EmailSender.coffee @@ -72,6 +72,7 @@ module.exports = client.sendMail options, (err, res)-> if err? logger.err err:err, "error sending message" + err = new Error('Cannot send email') else logger.log "Message sent to #{options.to}" callback(err) diff --git a/services/web/app/coffee/Features/Errors/Errors.coffee b/services/web/app/coffee/Features/Errors/Errors.coffee index 0bbff1f19b..92228f7b0b 100644 --- a/services/web/app/coffee/Features/Errors/Errors.coffee +++ b/services/web/app/coffee/Features/Errors/Errors.coffee @@ -5,5 +5,14 @@ NotFoundError = (message) -> return error NotFoundError.prototype.__proto__ = Error.prototype + +ServiceNotConfiguredError = (message) -> + error = new Error(message) + error.name = "ServiceNotConfiguredError" + error.__proto__ = ServiceNotConfiguredError.prototype + return error + + module.exports = Errors = - NotFoundError: NotFoundError \ No newline at end of file + NotFoundError: NotFoundError + ServiceNotConfiguredError: ServiceNotConfiguredError diff --git a/services/web/app/coffee/Features/FileStore/FileStoreHandler.coffee b/services/web/app/coffee/Features/FileStore/FileStoreHandler.coffee index eb5c2cd03f..10545cca76 100644 --- a/services/web/app/coffee/Features/FileStore/FileStoreHandler.coffee +++ b/services/web/app/coffee/Features/FileStore/FileStoreHandler.coffee @@ -13,6 +13,9 @@ module.exports = FileStoreHandler = if err? logger.err err:err, project_id:project_id, file_id:file_id, fsPath:fsPath, "error stating file" callback(err) + if !stat? + logger.err project_id:project_id, file_id:file_id, fsPath:fsPath, "stat is not available, can not check file from disk" + return callback(new Error("error getting stat, not available")) if !stat.isFile() logger.log project_id:project_id, file_id:file_id, fsPath:fsPath, "tried to upload symlink, not contining" return callback(new Error("can not upload symlink")) diff --git a/services/web/app/coffee/Features/Project/ProjectApiController.coffee b/services/web/app/coffee/Features/Project/ProjectApiController.coffee index b16991ac62..f832a75b54 100644 --- a/services/web/app/coffee/Features/Project/ProjectApiController.coffee +++ b/services/web/app/coffee/Features/Project/ProjectApiController.coffee @@ -4,11 +4,9 @@ logger = require("logger-sharelatex") module.exports = - getProjectDetails : (req, res)-> + getProjectDetails : (req, res, next)-> {project_id} = req.params ProjectDetailsHandler.getDetails project_id, (err, projDetails)-> - if err? - logger.log err:err, project_id:project_id, "something went wrong getting project details" - return res.sendStatus 500 + return next(err) if err? res.json(projDetails) diff --git a/services/web/app/coffee/Features/Project/ProjectDetailsHandler.coffee b/services/web/app/coffee/Features/Project/ProjectDetailsHandler.coffee index 7bd1d33561..9c823c9f17 100644 --- a/services/web/app/coffee/Features/Project/ProjectDetailsHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectDetailsHandler.coffee @@ -5,6 +5,7 @@ logger = require("logger-sharelatex") tpdsUpdateSender = require '../ThirdPartyDataStore/TpdsUpdateSender' _ = require("underscore") PublicAccessLevels = require("../Authorization/PublicAccessLevels") +Errors = require("../Errors/Errors") module.exports = @@ -13,6 +14,7 @@ module.exports = if err? logger.err err:err, project_id:project_id, "error getting project" return callback(err) + return callback(new Errors.NotFoundError("project not found")) if !project? UserGetter.getUser project.owner_ref, (err, user) -> return callback(err) if err? details = diff --git a/services/web/app/coffee/Features/Project/ProjectDuplicator.coffee b/services/web/app/coffee/Features/Project/ProjectDuplicator.coffee index 6cae962b04..d945326395 100644 --- a/services/web/app/coffee/Features/Project/ProjectDuplicator.coffee +++ b/services/web/app/coffee/Features/Project/ProjectDuplicator.coffee @@ -15,9 +15,11 @@ module.exports = ProjectDuplicator = _copyDocs: (newProject, originalRootDoc, originalFolder, desFolder, docContents, callback)-> setRootDoc = _.once (doc_id)-> projectEntityHandler.setRootDoc newProject._id, doc_id - - jobs = originalFolder.docs.map (doc)-> + docs = originalFolder.docs or [] + jobs = docs.map (doc)-> return (cb)-> + if !doc?._id? + return callback() content = docContents[doc._id.toString()] projectEntityHandler.addDocWithProject newProject, desFolder._id, doc.name, content.lines, (err, newDoc)-> if err? @@ -30,7 +32,8 @@ module.exports = ProjectDuplicator = async.series jobs, callback _copyFiles: (newProject, originalProject_id, originalFolder, desFolder, callback)-> - jobs = originalFolder.fileRefs.map (file)-> + fileRefs = originalFolder.fileRefs or [] + jobs = fileRefs.map (file)-> return (cb)-> projectEntityHandler.copyFileFromExistingProjectWithProject newProject, desFolder._id, originalProject_id, file, cb async.parallelLimit jobs, 5, callback @@ -40,10 +43,14 @@ module.exports = ProjectDuplicator = ProjectGetter.getProject newProject_id, {rootFolder:true, name:true}, (err, newProject)-> if err? logger.err project_id:newProject_id, "could not get project" - return cb(err) + return callback(err) - jobs = originalFolder.folders.map (childFolder)-> + folders = originalFolder.folders or [] + + jobs = folders.map (childFolder)-> return (cb)-> + if !childFolder?._id? + return cb() projectEntityHandler.addFolderWithProject newProject, desFolder?._id, childFolder.name, (err, newFolder)-> return cb(err) if err? ProjectDuplicator._copyFolderRecursivly newProject_id, originalProject_id, originalRootDoc, childFolder, newFolder, docContents, cb diff --git a/services/web/app/coffee/Features/Project/ProjectLocator.coffee b/services/web/app/coffee/Features/Project/ProjectLocator.coffee index 44f68123d6..62b495e5d2 100644 --- a/services/web/app/coffee/Features/Project/ProjectLocator.coffee +++ b/services/web/app/coffee/Features/Project/ProjectLocator.coffee @@ -26,6 +26,8 @@ module.exports = ProjectLocator = element = _.find searchFolder[elementType], (el)-> el?._id+'' == element_id+'' #need to ToString both id's for robustness if !element? && searchFolder.folders? && searchFolder.folders.length != 0 _.each searchFolder.folders, (folder, index)-> + if !folder? + return newPath = {} newPath[key] = value for own key,value of path #make a value copy of the string newPath.fileSystem += "/#{folder.name}" diff --git a/services/web/app/coffee/Features/StaticPages/StaticPagesRouter.coffee b/services/web/app/coffee/Features/StaticPages/StaticPagesRouter.coffee index f543d20c2e..07923d48d0 100644 --- a/services/web/app/coffee/Features/StaticPages/StaticPagesRouter.coffee +++ b/services/web/app/coffee/Features/StaticPages/StaticPagesRouter.coffee @@ -9,6 +9,7 @@ module.exports = webRouter.get '/tos', HomeController.externalPage("tos", "Terms of Service") webRouter.get '/about', HomeController.externalPage("about", "About Us") + webRouter.get '/security', HomeController.externalPage("security", "Security") webRouter.get '/privacy_policy', HomeController.externalPage("privacy", "Privacy Policy") webRouter.get '/planned_maintenance', HomeController.externalPage("planned_maintenance", "Planned Maintenance") @@ -20,4 +21,4 @@ module.exports = webRouter.get '/dropbox', HomeController.externalPage("dropbox", "Dropbox and ShareLaTeX") webRouter.get '/university', UniversityController.getIndexPage - webRouter.get '/university/*', UniversityController.getPage \ No newline at end of file + webRouter.get '/university/*', UniversityController.getPage diff --git a/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee b/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee index 60387c0dc0..fddd4f3567 100644 --- a/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee +++ b/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee @@ -469,33 +469,39 @@ module.exports = RecurlyWrapper = logger.err err:error, subscriptionId:subscriptionId, daysUntilExpire:daysUntilExpire, "error exending trial" callback(error) ) + + listAccountActiveSubscriptions: (account_id, callback = (error, subscriptions) ->) -> + RecurlyWrapper.apiRequest { + url: "accounts/#{account_id}/subscriptions" + qs: + state: "active" + expect404: true + }, (error, response, body) -> + return callback(error) if error? + if response.statusCode == 404 + return callback null, [] + else + RecurlyWrapper._parseSubscriptionsXml body, callback + + _parseSubscriptionsXml: (xml, callback) -> + RecurlyWrapper._parseXmlAndGetAttribute xml, "subscriptions", callback _parseSubscriptionXml: (xml, callback) -> - RecurlyWrapper._parseXml xml, (error, data) -> - return callback(error) if error? - if data? and data.subscription? - recurlySubscription = data.subscription - else - return callback "I don't understand the response from Recurly" - callback null, recurlySubscription + RecurlyWrapper._parseXmlAndGetAttribute xml, "subscription", callback _parseAccountXml: (xml, callback) -> - RecurlyWrapper._parseXml xml, (error, data) -> - return callback(error) if error? - if data? and data.account? - account = data.account - else - return callback "I don't understand the response from Recurly" - callback null, account + RecurlyWrapper._parseXmlAndGetAttribute xml, "account", callback _parseBillingInfoXml: (xml, callback) -> + RecurlyWrapper._parseXmlAndGetAttribute xml, "billing_info", callback + + _parseXmlAndGetAttribute: (xml, attribute, callback) -> RecurlyWrapper._parseXml xml, (error, data) -> return callback(error) if error? - if data? and data.billing_info? - billingInfo = data.billing_info + if data? and data[attribute]? + return callback null, data[attribute] else - return callback "I don't understand the response from Recurly" - callback null, billingInfo + return callback(new Error("I don't understand the response from Recurly")) _parseXml: (xml, callback) -> convertDataTypes = (data) -> diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionController.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionController.coffee index 84de417bb6..cc3013a42d 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionController.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionController.coffee @@ -46,31 +46,39 @@ module.exports = SubscriptionController = if hasSubscription or !plan? res.redirect "/user/subscription" else - currency = req.query.currency?.toUpperCase() - GeoIpLookup.getCurrencyCode req.query?.ip || req.ip, (err, recomendedCurrency, countryCode)-> - return next(err) if err? - if recomendedCurrency? and !currency? - currency = recomendedCurrency - RecurlyWrapper.sign { - subscription: - plan_code : req.query.planCode - currency: currency - account_code: user._id - }, (error, signature) -> - return next(error) if error? - res.render "subscriptions/new", - title : "subscribe" - plan_code: req.query.planCode - currency: currency - countryCode:countryCode - plan:plan - showStudentPlan: req.query.ssp - recurlyConfig: JSON.stringify - currency: currency - subdomain: Settings.apis.recurly.subdomain - showCouponField: req.query.scf - showVatField: req.query.svf - couponCode: req.query.cc or "" + # LimitationsManager.userHasSubscription only checks Mongo. Double check with + # Recurly as well at this point (we don't do this most places for speed). + SubscriptionHandler.validateNoSubscriptionInRecurly user._id, (error, valid) -> + return next(error) if error? + if !valid + res.redirect "/user/subscription" + return + else + currency = req.query.currency?.toUpperCase() + GeoIpLookup.getCurrencyCode req.query?.ip || req.ip, (err, recomendedCurrency, countryCode)-> + return next(err) if err? + if recomendedCurrency? and !currency? + currency = recomendedCurrency + RecurlyWrapper.sign { + subscription: + plan_code : req.query.planCode + currency: currency + account_code: user._id + }, (error, signature) -> + return next(error) if error? + res.render "subscriptions/new", + title : "subscribe" + plan_code: req.query.planCode + currency: currency + countryCode:countryCode + plan:plan + showStudentPlan: req.query.ssp + recurlyConfig: JSON.stringify + currency: currency + subdomain: Settings.apis.recurly.subdomain + showCouponField: req.query.scf + showVatField: req.query.svf + couponCode: req.query.cc or "" diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionHandler.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionHandler.coffee index ba95895dcd..99da3270a8 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionHandler.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionHandler.coffee @@ -11,15 +11,28 @@ Analytics = require("../Analytics/AnalyticsManager") module.exports = + validateNoSubscriptionInRecurly: (user_id, callback = (error, valid) ->) -> + RecurlyWrapper.listAccountActiveSubscriptions user_id, (error, subscriptions = []) -> + return callback(error) if error? + if subscriptions.length > 0 + SubscriptionUpdater.syncSubscription subscriptions[0], user_id, (error) -> + return callback(error) if error? + return callback(null, false) + else + return callback(null, true) createSubscription: (user, subscriptionDetails, recurly_token_id, callback)-> self = @ clientTokenId = "" - RecurlyWrapper.createSubscription user, subscriptionDetails, recurly_token_id, (error, recurlySubscription)-> + @validateNoSubscriptionInRecurly user._id, (error, valid) -> return callback(error) if error? - SubscriptionUpdater.syncSubscription recurlySubscription, user._id, (error) -> + if !valid + return callback(new Error("user already has subscription in recurly")) + RecurlyWrapper.createSubscription user, subscriptionDetails, recurly_token_id, (error, recurlySubscription)-> return callback(error) if error? - callback() + SubscriptionUpdater.syncSubscription recurlySubscription, user._id, (error) -> + return callback(error) if error? + callback() updateSubscription: (user, plan_code, coupon_code, callback)-> logger.log user:user, plan_code:plan_code, coupon_code:coupon_code, "updating subscription" diff --git a/services/web/app/views/project/editor/editor.pug b/services/web/app/views/project/editor/editor.pug index 9924fe1221..ce24108f77 100644 --- a/services/web/app/views/project/editor/editor.pug +++ b/services/web/app/views/project/editor/editor.pug @@ -19,7 +19,8 @@ div.full-size( 'rp-size-mini': (!ui.reviewPanelOpen && reviewPanel.hasEntries),\ 'rp-size-expanded': ui.reviewPanelOpen,\ 'rp-layout-left': reviewPanel.layoutToLeft,\ - 'rp-loading-threads': reviewPanel.loadingThreads\ + 'rp-loading-threads': reviewPanel.loadingThreads,\ + 'rp-new-comment-ui': reviewPanel.newAddCommentUI\ }" ) .loading-panel(ng-show="!editor.sharejs_doc || editor.opening") diff --git a/services/web/app/views/project/editor/review-panel.pug b/services/web/app/views/project/editor/review-panel.pug index b9dea40207..da14b69a99 100644 --- a/services/web/app/views/project/editor/review-panel.pug +++ b/services/web/app/views/project/editor/review-panel.pug @@ -1,10 +1,18 @@ #review-panel - a.rp-track-changes-indicator( - href - ng-if="editor.wantTrackChanges" - ng-click="toggleReviewPanel();" - ng-class="{ 'rp-track-changes-indicator-on-dark' : darkTheme }" - ) !{translate("track_changes_is_on")} + .rp-in-editor-widgets + a.rp-track-changes-indicator( + href + ng-if="editor.wantTrackChanges" + ng-click="toggleReviewPanel();" + ng-class="{ 'rp-track-changes-indicator-on-dark' : darkTheme }" + ) !{translate("track_changes_is_on")} + a.rp-add-comment-btn( + href + ng-if="reviewPanel.newAddCommentUI && reviewPanel.entries[editor.open_doc_id]['add-comment'] != null" + ng-click="addNewComment();" + ) + i.fa.fa-comment + |  #{translate("add_comment")} .review-panel-toolbar resolved-comments-dropdown( class="rp-flex-block" @@ -314,7 +322,7 @@ script(type='text/ng-template', id='resolvedCommentEntryTemplate') script(type='text/ng-template', id='addCommentEntryTemplate') div .rp-entry-callout.rp-entry-callout-add-comment - .rp-entry-indicator( + .rp-entry-indicator.rp-entry-indicator-add-comment( ng-if="!commentState.adding" ng-click="startNewComment(); onIndicatorClick();" tooltip=translate("add_comment") diff --git a/services/web/public/coffee/ide/editor/EditorManager.coffee b/services/web/public/coffee/ide/editor/EditorManager.coffee index e01a12cb8b..a1d00d3180 100644 --- a/services/web/public/coffee/ide/editor/EditorManager.coffee +++ b/services/web/public/coffee/ide/editor/EditorManager.coffee @@ -121,11 +121,22 @@ define [ _bindToDocumentEvents: (doc, sharejs_doc) -> sharejs_doc.on "error", (error, meta) => - if error?.message?.match "maxDocLength" + if error?.message? + message = error.message + else if typeof error == "string" + message = error + else + message = "" + if message.match "maxDocLength" @ide.showGenericMessageModal( "Document Too Long" "Sorry, this file is too long to be edited manually. Please upload it directly." ) + else if message.match "too many comments or tracked changes" + @ide.showGenericMessageModal( + "Too many comments or tracked changes" + "Sorry, this file has too many comments or tracked changes. Please try accepting or rejecting some existing changes, or resolving and deleting some comments." + ) else @ide.socket.disconnect() @ide.reportError(error, meta) diff --git a/services/web/public/coffee/ide/editor/directives/aceEditor/track-changes/TrackChangesManager.coffee b/services/web/public/coffee/ide/editor/directives/aceEditor/track-changes/TrackChangesManager.coffee index f51d1ed41e..0dd6a9f06b 100644 --- a/services/web/public/coffee/ide/editor/directives/aceEditor/track-changes/TrackChangesManager.coffee +++ b/services/web/public/coffee/ide/editor/directives/aceEditor/track-changes/TrackChangesManager.coffee @@ -73,16 +73,24 @@ define [ _scrollTimeout = null , 200 + @_resetCutState() + onCut = () => @onCut() + onPaste = () => @onPaste() + bindToAce = () => @editor.on "changeSelection", onChangeSelection @editor.on "change", onChangeSelection # Selection also moves with updates elsewhere in the document @editor.on "changeSession", onChangeSession + @editor.on "cut", onCut + @editor.on "paste", onPaste @editor.renderer.on "resize", onResize unbindFromAce = () => @editor.off "changeSelection", onChangeSelection @editor.off "change", onChangeSelection @editor.off "changeSession", onChangeSession + @editor.off "cut", onCut + @editor.off "paste", onPaste @editor.renderer.off "resize", onResize @$scope.$watch "trackChangesEnabled", (enabled) => @@ -244,6 +252,49 @@ define [ @_onCommentAdded(comment) @broadcastChange() + _resetCutState: () -> + @_cutState = { + text: null + comments: [] + docId: null + } + + onCut: () -> + @_resetCutState() + selection = @editor.getSelectionRange() + selection_start = @_aceRangeToShareJs(selection.start) + selection_end = @_aceRangeToShareJs(selection.end) + @_cutState.text = @editor.getSelectedText() + @_cutState.docId = @$scope.docId + for comment in @rangesTracker.comments + comment_start = comment.op.p + comment_end = comment_start + comment.op.c.length + if selection_start <= comment_start and comment_end <= selection_end + @_cutState.comments.push { + offset: comment.op.p - selection_start + text: comment.op.c + comment: comment + } + + onPaste: () => + @editor.once "change", (change) => + return if change.action != "insert" + pasted_text = change.lines.join("\n") + paste_offset = @_aceRangeToShareJs(change.start) + # We have to wait until the change has been processed by the range tracker, + # since if we move the ops into place beforehand, they will be moved again + # when the changes are processed by the range tracker. This ranges:dirty + # event is fired after the doc has applied the changes to the range tracker. + @$scope.sharejsDoc.on "ranges:dirty.paste", () => + @$scope.sharejsDoc.off "ranges:dirty.paste" # Doc event emitter uses namespaced events + if pasted_text == @_cutState.text and @$scope.docId == @_cutState.docId + for {comment, offset, text} in @_cutState.comments + op = { c: text, p: paste_offset + offset, t: comment.id } + @$scope.sharejsDoc.submitOp op # Resubmitting an existing comment op (by thread id) will move it + @_resetCutState() + # Check that comments still match text. Will throw error if not. + @rangesTracker.validate(@editor.getValue()) + checkMapping: () -> # TODO: reintroduce this check session = @editor.getSession() diff --git a/services/web/public/coffee/ide/file-tree/FileTreeManager.coffee b/services/web/public/coffee/ide/file-tree/FileTreeManager.coffee index c4ad4b30a4..dd6f813430 100644 --- a/services/web/public/coffee/ide/file-tree/FileTreeManager.coffee +++ b/services/web/public/coffee/ide/file-tree/FileTreeManager.coffee @@ -173,6 +173,8 @@ define [ @_findEntityByPathInFolder @$scope.rootFolder, path _findEntityByPathInFolder: (folder, path) -> + if !path? or !folder? + return null parts = path.split("/") name = parts.shift() rest = parts.join("/") diff --git a/services/web/public/coffee/ide/review-panel/RangesTracker.coffee b/services/web/public/coffee/ide/review-panel/RangesTracker.coffee index a9c43e9816..14193f628d 100644 --- a/services/web/public/coffee/ide/review-panel/RangesTracker.coffee +++ b/services/web/public/coffee/ide/review-panel/RangesTracker.coffee @@ -1,3 +1,6 @@ +# This file is shared between document-updater and web, so that the server and client share +# an identical track changes implementation. Do not edit it directly in web or document-updater, +# instead edit it at https://github.com/sharelatex/ranges-tracker, where it has a suite of tests load = () -> class RangesTracker # The purpose of this class is to track a set of inserts and deletes to a document, like @@ -78,6 +81,13 @@ load = () -> @comments = @comments.filter (c) -> c.id != comment_id @_markAsDirty comment, "comment", "removed" + moveCommentId: (comment_id, position, text) -> + for comment in @comments + if comment.id == comment_id + comment.op.p = position + comment.op.c = text + @_markAsDirty comment, "comment", "moved" + getChange: (change_id) -> change = null for c in @changes @@ -90,6 +100,18 @@ load = () -> change = @getChange(change_id) return if !change? @_removeChange(change) + + validate: (text) -> + for change in @changes + if change.op.i? + content = text.slice(change.op.p, change.op.p + change.op.i.length) + if content != change.op.i + throw new Error("Change (#{JSON.stringify(change)}) doesn't match text (#{JSON.stringify(content)})") + for comment in @comments + content = text.slice(comment.op.p, comment.op.p + comment.op.c.length) + if content != comment.op.c + throw new Error("Comment (#{JSON.stringify(comment)}) doesn't match text (#{JSON.stringify(content)})") + return true applyOp: (op, metadata = {}) -> metadata.ts ?= new Date() @@ -110,17 +132,21 @@ load = () -> @applyOp(op, metadata) addComment: (op, metadata) -> - # TODO: Don't allow overlapping comments? - @comments.push comment = { - id: op.t or @newId() - op: # Copy because we'll modify in place - c: op.c - p: op.p - t: op.t - metadata - } - @_markAsDirty comment, "comment", "added" - return comment + existing = @getComment(op.t) + if existing? + @moveCommentId(op.t, op.p, op.c) + return existing + else + @comments.push comment = { + id: op.t or @newId() + op: # Copy because we'll modify in place + c: op.c + p: op.p + t: op.t + metadata + } + @_markAsDirty comment, "comment", "added" + return comment applyInsertToComments: (op) -> for comment in @comments 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 5067799571..d4c40084bd 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, $http, $modal, event_tracking, localStorage) -> + App.controller "ReviewPanelController", ($scope, $element, ide, $timeout, $http, $modal, event_tracking, sixpack, localStorage) -> $reviewPanelEl = $element.find "#review-panel" $scope.SubViews = @@ -27,6 +27,14 @@ define [ layoutToLeft: false rendererData: {} loadingThreads: false + newAddCommentUI: false # Test new UI for adding comments; remove afterwards. + + $scope.shouldABAddCommentBtn = false + if $scope.user.signUpDate >= '2017-03-27' + sixpack.participate "add-comment-btn", [ "default", "editor-corner" ], (variation) -> + $scope.shouldABAddCommentBtn = true + $scope.variationABAddCommentBtn = variation + $scope.reviewPanel.newAddCommentUI = (variation == "editor-corner") window.addEventListener "beforeunload", () -> collapsedStates = {} @@ -163,7 +171,11 @@ define [ $scope.$watch (() -> entries = $scope.reviewPanel.entries[$scope.editor.open_doc_id] or {} - Object.keys(entries).length + permEntries = {} + for entry, entryData of entries + if entry != "add-comment" or !$scope.reviewPanel.newAddCommentUI + permEntries[entry] = entryData + Object.keys(permEntries).length ), (nEntries) -> $scope.reviewPanel.hasEntries = nEntries > 0 and $scope.project.features.trackChangesVisible @@ -323,11 +335,17 @@ define [ $scope.$broadcast "change:reject", entry_id event_tracking.sendMB "rp-change-rejected", { view: if $scope.ui.reviewPanelOpen then $scope.reviewPanel.subView else 'mini' } + $scope.addNewComment = () -> + $scope.$broadcast "comment:start_adding" + $scope.toggleReviewPanel() + $scope.startNewComment = () -> $scope.$broadcast "comment:select_line" $timeout () -> $scope.$broadcast "review-panel:layout" - + if $scope.shouldABAddCommentBtn and !$scope.ui.reviewPanelOpen + sixpack.convert "add-comment-btn" + $scope.submitNewComment = (content) -> return if !content? or content == "" doc_id = $scope.editor.open_doc_id diff --git a/services/web/public/coffee/ide/review-panel/directives/addCommentEntry.coffee b/services/web/public/coffee/ide/review-panel/directives/addCommentEntry.coffee index 2ec79efcd5..9d33bcb35a 100644 --- a/services/web/public/coffee/ide/review-panel/directives/addCommentEntry.coffee +++ b/services/web/public/coffee/ide/review-panel/directives/addCommentEntry.coffee @@ -8,13 +8,16 @@ define [ onStartNew: "&" onSubmit: "&" onCancel: "&" - onIndicatorClick: "&" + onIndicatorClick: "&" layoutToLeft: "=" link: (scope, element, attrs) -> scope.state = isAdding: false content: "" + scope.$on "comment:start_adding", () -> + scope.startNewComment() + scope.startNewComment = () -> scope.state.isAdding = true scope.onStartNew() diff --git a/services/web/public/js/ace-1.2.5/worker-latex.js b/services/web/public/js/ace-1.2.5/worker-latex.js index 3fa9bceb07..ccd7e2da9b 100644 --- a/services/web/public/js/ace-1.2.5/worker-latex.js +++ b/services/web/public/js/ace-1.2.5/worker-latex.js @@ -1931,7 +1931,7 @@ var InterpretTokens = function (TokeniseResult, ErrorReporter) { if (newPos === null) { continue; } else {i = newPos;}; } else if (seq === "hbox" || seq === "text" || seq === "mbox" || seq === "footnote" || seq === "intertext" || seq === "shortintertext" || seq === "textnormal" || seq === "tag" || seq === "reflectbox" || seq === "textrm") { nextGroupMathMode = false; - } else if (seq === "rotatebox" || seq === "scalebox") { + } else if (seq === "rotatebox" || seq === "scalebox" || seq == "feynmandiagram") { newPos = readOptionalGeneric(TokeniseResult, i); if (newPos === null) { /* do nothing */ } else {i = newPos;}; newPos = readDefinition(TokeniseResult, i); @@ -2179,7 +2179,7 @@ var EnvHandler = function (ErrorReporter) { this._beginMathMode = function (thisEnv) { var currentMathMode = this.getMathMode(); // undefined, null, $, $$, name of mathmode env if (currentMathMode) { - ErrorFrom(thisEnv, thisEnv.name + " used inside existing math mode " + getName(currentMathMode), + ErrorFrom(thisEnv, getName(thisEnv) + " used inside existing math mode " + getName(currentMathMode), {suppressIfEditing:true, errorAtStart: true, mathMode:true}); }; thisEnv.mathMode = thisEnv; diff --git a/services/web/public/stylesheets/app/editor/review-panel.less b/services/web/public/stylesheets/app/editor/review-panel.less index 7d2dbb1c9f..c069c536e2 100644 --- a/services/web/public/stylesheets/app/editor/review-panel.less +++ b/services/web/public/stylesheets/app/editor/review-panel.less @@ -9,13 +9,10 @@ @rp-border-grey : #d9d9d9; @rp-green : #2c8e30; -@rp-dim-green : #cae3cb; @rp-green-on-dark : rgba(37, 107, 41, 0.5); @rp-red : #c5060b; -@rp-dim-red : #f3cdce; @rp-yellow : #f3b111; @rp-yellow-on-dark : rgba(194, 93, 11, 0.5); -@rp-dim-yellow : #ffe9b2; @rp-grey : #aaaaaa; @rp-type-blue : #6b7797; @@ -93,7 +90,7 @@ } #review-panel { - display: none; + display: block; .rp-size-expanded & { display: flex; flex-direction: column; @@ -101,7 +98,6 @@ overflow: visible; } .rp-size-mini & { - display: block; width: @review-off-width; z-index: 6; } @@ -155,7 +151,13 @@ } .rp-entry-list { + display: none; width: 100%; + + .rp-size-expanded &, + .rp-size-mini & { + display: block; + } .rp-state-current-file & { position: absolute; @@ -197,6 +199,10 @@ right: 4px; z-index: 1; } + + .rp-new-comment-ui &-add-comment { + display: none; + } } .rp-entry-wrapper { @@ -448,6 +454,12 @@ display: block; padding: 5px 10px; border-radius: 3px; + + .rp-in-editor-widgets & { + white-space: nowrap; + border-radius: 0; + border-bottom-left-radius: 3px; + } } .rp-new-comment { @@ -563,6 +575,10 @@ border-color: @rp-yellow; } } + + .rp-size-mini.rp-new-comment-ui &-add-comment { + display: none; + } } .rp-overview-file-header { @@ -610,11 +626,12 @@ } .rp-nav { - display: flex; + display: none; flex-shrink: 0; - .rp-size-mini & { - display: none; + .rp-size-expanded & { + display: flex; } + .rp-state-current-file & { position: absolute; bottom: 0; @@ -748,13 +765,16 @@ .rp-loading-threads & { display: none; } + z-index: 6 // Appear above text selection } .track-changes-comment-marker { - background-color: @rp-dim-yellow; + background-color: @rp-yellow; + opacity: 0.3; } .track-changes-added-marker { - background-color: @rp-dim-green; + background-color: @rp-green; + opacity: 0.3; } .track-changes-deleted-marker { border-left: 2px dotted @rp-red; @@ -871,43 +891,49 @@ } } -.rp-track-changes-indicator { - display: none; +.rp-in-editor-widgets { position: absolute; top: 0; - right: @review-off-width; - padding: 5px 10px; - background-color: rgba(240, 240, 240, 0.9); - color: @rp-type-blue; - text-align: center; - border-bottom-left-radius: 3px; - font-size: 10px; + right: 0; + font-size: 11px; z-index: 2; - white-space: nowrap; - - &.rp-track-changes-indicator-on-dark { - background-color: rgba(88, 88, 88, .8); - color: #FFF; - - &:hover, - &:focus { - background-color: rgba(88, 88, 88, 1); - color: #FFF; - } - } - - &:hover, - &:focus { - outline: 0; - text-decoration: none; - background-color: rgba(240, 240, 240, 1); - color: @rp-type-blue; - } - + .rp-size-mini & { - display: block; + right: @review-off-width; } } + .rp-track-changes-indicator { + display: block; + padding: 5px 10px; + background-color: rgba(240, 240, 240, 0.9); + color: @rp-type-blue; + text-align: center; + border-bottom-left-radius: 3px; + white-space: nowrap; + + &.rp-track-changes-indicator-on-dark { + background-color: rgba(88, 88, 88, .8); + color: #FFF; + + &:hover, + &:focus { + background-color: rgba(88, 88, 88, 1); + color: #FFF; + } + } + + &:hover, + &:focus { + outline: 0; + text-decoration: none; + background-color: rgba(240, 240, 240, 1); + color: @rp-type-blue; + } + + .rp-size-expanded & { + display: none; + } + } // Helper class for elements which aren't treated as flex-items by IE10, e.g: // * inline items; diff --git a/services/web/test/UnitTests/coffee/Analytics/AnalyticsControllerTests.coffee b/services/web/test/UnitTests/coffee/Analytics/AnalyticsControllerTests.coffee new file mode 100644 index 0000000000..2c52b8b5f0 --- /dev/null +++ b/services/web/test/UnitTests/coffee/Analytics/AnalyticsControllerTests.coffee @@ -0,0 +1,44 @@ +should = require('chai').should() +SandboxedModule = require('sandboxed-module') +assert = require('assert') +path = require('path') +modulePath = path.join __dirname, '../../../../app/js/Features/Analytics/AnalyticsController' +sinon = require("sinon") +expect = require("chai").expect + + +describe 'AnalyticsController', -> + + beforeEach -> + @AuthenticationController = + getLoggedInUserId: sinon.stub() + + @AnalyticsManager = + recordEvent: sinon.stub().callsArgWith(3) + + @req = + params: + event:"i_did_something" + body:"stuff" + sessionID: "sessionIDHere" + + @res = + send:-> + @controller = SandboxedModule.require modulePath, requires: + "./AnalyticsManager":@AnalyticsManager + "../Authentication/AuthenticationController":@AuthenticationController + "logger-sharelatex": + log:-> + + describe "recordEvent", -> + + it "should use the user_id", (done)-> + @AuthenticationController.getLoggedInUserId.returns("1234") + @controller.recordEvent @req, @res + @AnalyticsManager.recordEvent.calledWith("1234", @req.params["event"], @req.body).should.equal true + done() + + it "should use the session id", (done)-> + @controller.recordEvent @req, @res + @AnalyticsManager.recordEvent.calledWith(@req.sessionID, @req.params["event"], @req.body).should.equal true + done() diff --git a/services/web/test/UnitTests/coffee/Authentication/AuthenticationControllerTests.coffee b/services/web/test/UnitTests/coffee/Authentication/AuthenticationControllerTests.coffee index 94e930c7b1..f44968a0e5 100644 --- a/services/web/test/UnitTests/coffee/Authentication/AuthenticationControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Authentication/AuthenticationControllerTests.coffee @@ -254,6 +254,8 @@ describe "AuthenticationController", -> @cb = sinon.stub() @LoginRateLimiter.processLoginRequest.callsArgWith(1, null, true) @AuthenticationManager.authenticate = sinon.stub().callsArgWith(2, null, @user) + @req.sessionID = Math.random() + @AnalyticsManager.identifyUser = sinon.stub() @AuthenticationController.doPassportLogin(@req, @req.body.email, @req.body.password, @cb) it "should attempt to authorise the user", -> @@ -261,6 +263,9 @@ describe "AuthenticationController", -> .calledWith(email: @email.toLowerCase(), @password) .should.equal true + it "should call identifyUser", -> + @AnalyticsManager.identifyUser.calledWith(@user._id, @req.sessionID).should.equal true + it "should setup the user data in the background", -> @UserHandler.setupLoginData.calledWith(@user).should.equal true diff --git a/services/web/test/UnitTests/coffee/Authorization/AuthorizationManagerTests.coffee b/services/web/test/UnitTests/coffee/Authorization/AuthorizationManagerTests.coffee index fcacce5164..d779934055 100644 --- a/services/web/test/UnitTests/coffee/Authorization/AuthorizationManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/Authorization/AuthorizationManagerTests.coffee @@ -136,7 +136,20 @@ describe "AuthorizationManager", -> it "should return a NotFoundError", -> @AuthorizationManager.getPrivilegeLevelForProject @user_id, @project_id, (error) -> error.should.be.instanceof Errors.NotFoundError - + + describe "when the project id is not valid", -> + beforeEach -> + @AuthorizationManager.isUserSiteAdmin.withArgs(@user_id).yields(null, false) + @CollaboratorsHandler.getMemberIdPrivilegeLevel + .withArgs(@user_id, @project_id) + .yields(null, "readOnly") + + it "should return a error", (done)-> + @AuthorizationManager.getPrivilegeLevelForProject undefined, "not project id", (err) => + @Project.findOne.called.should.equal false + expect(err).to.exist + done() + describe "canUserReadProject", -> beforeEach -> @AuthorizationManager.getPrivilegeLevelForProject = sinon.stub() diff --git a/services/web/test/UnitTests/coffee/Email/EmailSenderTests.coffee b/services/web/test/UnitTests/coffee/Email/EmailSenderTests.coffee index beba91fcb3..dc504907bf 100644 --- a/services/web/test/UnitTests/coffee/Email/EmailSenderTests.coffee +++ b/services/web/test/UnitTests/coffee/Email/EmailSenderTests.coffee @@ -51,17 +51,19 @@ describe "EmailSender", -> it "should set the properties on the email to send", (done)-> @sesClient.sendMail.callsArgWith(1) - @sender.sendEmail @opts, => + @sender.sendEmail @opts, (err) => + expect(err).to.not.exist args = @sesClient.sendMail.args[0][0] args.html.should.equal @opts.html args.to.should.equal @opts.to args.subject.should.equal @opts.subject done() - it "should return the error", (done)-> + it "should return a non-specific error", (done)-> @sesClient.sendMail.callsArgWith(1, "error") @sender.sendEmail {}, (err)=> - err.should.equal "error" + err.should.exist + err.toString().should.equal 'Error: Cannot send email' done() diff --git a/services/web/test/UnitTests/coffee/FileStore/FileStoreHandlerTests.coffee b/services/web/test/UnitTests/coffee/FileStore/FileStoreHandlerTests.coffee index 7452c6fb79..01990787e1 100644 --- a/services/web/test/UnitTests/coffee/FileStore/FileStoreHandlerTests.coffee +++ b/services/web/test/UnitTests/coffee/FileStore/FileStoreHandlerTests.coffee @@ -96,6 +96,13 @@ describe "FileStoreHandler", -> @fs.createReadStream.called.should.equal false done() + describe "symlink", -> + it "should not read file stat returns nothing", (done)-> + @fs.lstat = sinon.stub().callsArgWith(1, null, null) + @handler.uploadFileFromDisk @project_id, @file_id, @fsPath, => + @fs.createReadStream.called.should.equal false + done() + describe "when upload fails", -> beforeEach -> @writeStream.on = (type, cb) -> diff --git a/services/web/test/UnitTests/coffee/Project/ProjectApiControllerTests.coffee b/services/web/test/UnitTests/coffee/Project/ProjectApiControllerTests.coffee index 9476a80019..ddf23c0bac 100644 --- a/services/web/test/UnitTests/coffee/Project/ProjectApiControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Project/ProjectApiControllerTests.coffee @@ -20,6 +20,7 @@ describe 'Project api controller', -> session: destroy:sinon.stub() @res = {} + @next = sinon.stub() @projDetails = {name:"something"} @@ -34,9 +35,7 @@ describe 'Project api controller', -> @controller.getProjectDetails @req, @res - it "should send a 500 if there is an error", (done)-> + it "should send a 500 if there is an error", ()-> @ProjectDetailsHandler.getDetails.callsArgWith(1, "error") - @res.sendStatus = (resCode)=> - resCode.should.equal 500 - done() - @controller.getProjectDetails @req, @res + @controller.getProjectDetails @req, @res, @next + @next.calledWith("error").should.equal true diff --git a/services/web/test/UnitTests/coffee/Project/ProjectDetailsHandlerTests.coffee b/services/web/test/UnitTests/coffee/Project/ProjectDetailsHandlerTests.coffee index b9e8ca27c6..501e48f1a5 100644 --- a/services/web/test/UnitTests/coffee/Project/ProjectDetailsHandlerTests.coffee +++ b/services/web/test/UnitTests/coffee/Project/ProjectDetailsHandlerTests.coffee @@ -1,5 +1,6 @@ should = require('chai').should() modulePath = "../../../../app/js/Features/Project/ProjectDetailsHandler" +Errors = require "../../../../app/js/Features/Errors/Errors" SandboxedModule = require('sandboxed-module') sinon = require('sinon') assert = require("chai").assert @@ -48,6 +49,13 @@ describe 'ProjectDetailsHandler', -> assert.equal(details.something, undefined) done() + it "should return an error for a non-existent project", (done)-> + @ProjectGetter.getProject.callsArg(2, null, null) + err = new Errors.NotFoundError("project not found") + @handler.getDetails "0123456789012345678901234", (error, details) => + err.should.eql error + done() + it "should return the error", (done)-> error = "some error" @ProjectGetter.getProject.callsArgWith(2, error) diff --git a/services/web/test/UnitTests/coffee/Project/ProjectDuplicatorTests.coffee b/services/web/test/UnitTests/coffee/Project/ProjectDuplicatorTests.coffee index 7897ae47c8..a1101cfbe0 100644 --- a/services/web/test/UnitTests/coffee/Project/ProjectDuplicatorTests.coffee +++ b/services/web/test/UnitTests/coffee/Project/ProjectDuplicatorTests.coffee @@ -9,7 +9,7 @@ describe 'ProjectDuplicator', -> @level2folder = name: "level2folderName" _id:"level2folderId" - docs:[@doc2 = {_id: "doc2_id", name:"level2folderDocName"}] + docs:[@doc2 = {_id: "doc2_id", name:"level2folderDocName"}, undefined] folders:[] fileRefs:[{name:"file2", _id:"file2"}] @level1folder = @@ -17,12 +17,12 @@ describe 'ProjectDuplicator', -> _id:"level1folderId" docs:[@doc1 = {_id: "doc1_id", name:"level1folderDocName"}] folders:[@level2folder] - fileRefs:[{name:"file1", _id:"file1"}] + fileRefs:[{name:"file1", _id:"file1"}, null] @rootFolder = name:"rootFolder" _id:"rootFolderId" docs:[@doc0 = {_id: "doc0_id", name:"rootDocHere"}] - folders:[@level1folder] + folders:[@level1folder, {}] fileRefs:[{name:"file0", _id:"file0"}] @project = _id: @old_project_id = "this_is_the_old_project_id" @@ -117,7 +117,7 @@ describe 'ProjectDuplicator', -> @projectOptionsHandler.setCompiler.calledWith(@stubbedNewProject._id, @project.compiler).should.equal true done() - it 'should use the same root docccccccc', (done)-> + it 'should use the same root doc', (done)-> @entityHandler.addDocWithProject.callsArgWith(4, null, @rootFolder.docs[0]) @duplicator.duplicate @owner, @old_project_id, "", (err, newProject)=> @entityHandler.setRootDoc.calledWith(@stubbedNewProject._id, @rootFolder.docs[0]._id).should.equal true diff --git a/services/web/test/UnitTests/coffee/Project/ProjectLocatorTests.coffee b/services/web/test/UnitTests/coffee/Project/ProjectLocatorTests.coffee index 1d982d90af..9257c2e83e 100644 --- a/services/web/test/UnitTests/coffee/Project/ProjectLocatorTests.coffee +++ b/services/web/test/UnitTests/coffee/Project/ProjectLocatorTests.coffee @@ -17,7 +17,7 @@ file1 = name:"file1", _id:"dsa9lkdsad" subSubFile = name:"subSubFile", _id:"d1d2dk" subSubDoc = name:"subdoc.txt", _id:"321dmdwi" secondSubFolder = name:"secondSubFolder", _id:"dsa3e23", docs:[subSubDoc], fileRefs:[subSubFile], folders:[] -subFolder = name:"subFolder", _id:"dsadsa93", folders:[secondSubFolder], docs:[], fileRefs:[] +subFolder = name:"subFolder", _id:"dsadsa93", folders:[secondSubFolder, null], docs:[], fileRefs:[] subFolder1 = name:"subFolder1", _id:"123asdjoij" rootFolder = diff --git a/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee b/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee index c55efdb3c5..d951653310 100644 --- a/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee +++ b/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee @@ -1030,3 +1030,35 @@ describe "RecurlyWrapper", -> @call (err, result) => expect(err).to.be.instanceof Error done() + + describe "listAccountActiveSubscriptions", -> + beforeEach -> + @user_id = "mock-user-id" + @callback = sinon.stub() + @RecurlyWrapper.apiRequest = sinon.stub().yields(null, @response = {"mock": "response"}, @body = "") + @RecurlyWrapper._parseSubscriptionsXml = sinon.stub().yields(null, @subscriptions = ["mock", "subscriptions"]) + + describe "with an account", -> + beforeEach -> + @RecurlyWrapper.listAccountActiveSubscriptions @user_id, @callback + + it "should send a request to Recurly", -> + @RecurlyWrapper.apiRequest + .calledWith({ + url: "accounts/#{@user_id}/subscriptions" + qs: + state: "active" + expect404: true + }) + .should.equal true + + it "should return the subscriptions", -> + @callback.calledWith(null, @subscriptions).should.equal true + + describe "without an account", -> + beforeEach -> + @response.statusCode = 404 + @RecurlyWrapper.listAccountActiveSubscriptions @user_id, @callback + + it "should return an empty array of subscriptions", -> + @callback.calledWith(null, []).should.equal true \ No newline at end of file diff --git a/services/web/test/UnitTests/coffee/Subscription/SubscriptionControllerTests.coffee b/services/web/test/UnitTests/coffee/Subscription/SubscriptionControllerTests.coffee index 3a43c8988e..b95fba1cdd 100644 --- a/services/web/test/UnitTests/coffee/Subscription/SubscriptionControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Subscription/SubscriptionControllerTests.coffee @@ -17,8 +17,7 @@ mockSubscriptions = account: account_code: "user-123" -describe "SubscriptionController sanboxed", -> - +describe "SubscriptionController", -> beforeEach -> @user = {email:"tom@yahoo.com", _id: 'one', signUpDate: new Date('2000-10-01')} @activeRecurlySubscription = mockSubscriptions["subscription-123-active"] @@ -150,6 +149,7 @@ describe "SubscriptionController sanboxed", -> describe "paymentPage", -> beforeEach -> @req.headers = {} + @SubscriptionHandler.validateNoSubscriptionInRecurly = sinon.stub().yields(null, true) @GeoIpLookup.getCurrencyCode.callsArgWith(1, null, @stubbedCurrencyCode) describe "with a user without a subscription", -> @@ -209,6 +209,16 @@ describe "SubscriptionController sanboxed", -> opts.currency.should.equal @stubbedCurrencyCode done() @SubscriptionController.paymentPage @req, @res + + describe "with a recurly subscription already", -> + it "should redirect to the subscription dashboard", (done)-> + @LimitationsManager.userHasSubscription.callsArgWith(1, null, false) + @SubscriptionHandler.validateNoSubscriptionInRecurly = sinon.stub().yields(null, false) + @res.redirect = (url)=> + url.should.equal "/user/subscription" + done() + @SubscriptionController.paymentPage(@req, @res) + describe "successful_subscription", -> beforeEach (done) -> diff --git a/services/web/test/UnitTests/coffee/Subscription/SubscriptionHandlerTests.coffee b/services/web/test/UnitTests/coffee/Subscription/SubscriptionHandlerTests.coffee index 935ed6d025..1e2cec4bfa 100644 --- a/services/web/test/UnitTests/coffee/Subscription/SubscriptionHandlerTests.coffee +++ b/services/web/test/UnitTests/coffee/Subscription/SubscriptionHandlerTests.coffee @@ -16,7 +16,7 @@ mockRecurlySubscriptions = account: account_code: "user-123" -describe "Subscription Handler sanboxed", -> +describe "SubscriptionHandler", -> beforeEach -> @Settings = @@ -33,7 +33,7 @@ describe "Subscription Handler sanboxed", -> @activeRecurlySubscription = mockRecurlySubscriptions["subscription-123-active"] @User = {} @user = - _id: "user_id_here_" + _id: @user_id = "user_id_here_" @subscription = recurlySubscription_id: @activeRecurlySubscription.uuid @RecurlyWrapper = @@ -77,23 +77,33 @@ describe "Subscription Handler sanboxed", -> describe "createSubscription", -> - beforeEach (done) -> + beforeEach -> + @callback = sinon.stub() @subscriptionDetails = cvv:"123" number:"12345" @recurly_token_id = "45555666" - @SubscriptionHandler.createSubscription(@user, @subscriptionDetails, @recurly_token_id, done) + @SubscriptionHandler.validateNoSubscriptionInRecurly = sinon.stub().yields(null, true) - it "should create the subscription with the wrapper", (done)-> - @RecurlyWrapper.createSubscription.calledWith(@user, @subscriptionDetails, @recurly_token_id).should.equal true - done() + describe "successfully", -> + beforeEach -> + @SubscriptionHandler.createSubscription(@user, @subscriptionDetails, @recurly_token_id, @callback) - it "should sync the subscription to the user", (done)-> - @SubscriptionUpdater.syncSubscription.calledOnce.should.equal true - @SubscriptionUpdater.syncSubscription.args[0][0].should.deep.equal @activeRecurlySubscription - @SubscriptionUpdater.syncSubscription.args[0][1].should.deep.equal @user._id - done() + it "should create the subscription with the wrapper", -> + @RecurlyWrapper.createSubscription.calledWith(@user, @subscriptionDetails, @recurly_token_id).should.equal true + it "should sync the subscription to the user", -> + @SubscriptionUpdater.syncSubscription.calledOnce.should.equal true + @SubscriptionUpdater.syncSubscription.args[0][0].should.deep.equal @activeRecurlySubscription + @SubscriptionUpdater.syncSubscription.args[0][1].should.deep.equal @user._id + + describe "when there is already a subscription in Recurly", -> + beforeEach -> + @SubscriptionHandler.validateNoSubscriptionInRecurly = sinon.stub().yields(null, false) + @SubscriptionHandler.createSubscription(@user, @subscriptionDetails, @recurly_token_id, @callback) + + it "should return an error", -> + @callback.calledWith(new Error("user already has subscription in recurly")) describe "updateSubscription", -> describe "with a user with a subscription", -> @@ -145,8 +155,6 @@ describe "Subscription Handler sanboxed", -> updateOptions = @RecurlyWrapper.updateSubscription.args[0][1] updateOptions.plan_code.should.equal @plan_code - - describe "cancelSubscription", -> describe "with a user without a subscription", -> beforeEach (done) -> @@ -210,5 +218,39 @@ describe "Subscription Handler sanboxed", -> @SubscriptionUpdater.syncSubscription.args[0][0].should.deep.equal @activeRecurlySubscription @SubscriptionUpdater.syncSubscription.args[0][1].should.deep.equal @user._id + describe "validateNoSubscriptionInRecurly", -> + beforeEach -> + @subscriptions = [] + @RecurlyWrapper.listAccountActiveSubscriptions = sinon.stub().yields(null, @subscriptions) + @SubscriptionUpdater.syncSubscription = sinon.stub().yields() + @callback = sinon.stub() + describe "with no subscription in recurly", -> + beforeEach -> + @subscriptions.push @subscription = { "mock": "subscription" } + @SubscriptionHandler.validateNoSubscriptionInRecurly @user_id, @callback + it "should call RecurlyWrapper.listAccountActiveSubscriptions with the user id", -> + @RecurlyWrapper.listAccountActiveSubscriptions + .calledWith(@user_id) + .should.equal true + + it "should sync the subscription", -> + @SubscriptionUpdater.syncSubscription + .calledWith(@subscription, @user_id) + .should.equal true + + it "should call the callback with valid == false", -> + @callback.calledWith(null, false).should.equal true + + describe "with a subscription in recurly", -> + beforeEach -> + @SubscriptionHandler.validateNoSubscriptionInRecurly @user_id, @callback + + it "should not sync the subscription", -> + @SubscriptionUpdater.syncSubscription + .called + .should.equal false + + it "should call the callback with valid == true", -> + @callback.calledWith(null, true).should.equal true