Merge pull request #1413 from sharelatex/pr-fix-history-default-file-selection

Fix history default file selection and bug when comparing with version zero

GitOrigin-RevId: 7a123eaec908abd0178eba435f6820ad1e858da1

services/web/app/views/project/editor/history/fileTreeV2.pug

@@ -4,7 +4,7 @@ aside.file-tree.full-size(
 )
 	.history-file-tree-inner
 		history-file-tree(
-			file-tree="currentFileTree"
+			files="history.selection.files"
 			selected-pathname="history.selection.pathname"
 			on-selected-file-change="handleFileSelection(file)"
 			is-loading="history.loadingFileTree"
@@ -13,7 +13,7 @@ aside.file-tree.full-size(
 script(type="text/ng-template", id="historyFileTreeTpl")
 	.history-file-tree
 		history-file-entity(
-			ng-repeat="fileEntity in $ctrl.fileTree | orderBy : [ '-type', 'operation', 'name' ]"
+			ng-repeat="fileEntity in $ctrl._fileTree | orderBy : [ '-type', 'operation', 'name' ]"
 			file-entity="fileEntity"
 			ng-show="!$ctrl.isLoading"
 		)

services/web/public/src/ide/history/HistoryV2Manager.js

@@ -56,7 +56,7 @@ define([
         this._localStorageShowOnlyLabelsProjKey = `history.userPrefs.showOnlyLabels.${
           $scope.project_id
         }`
-
+        this._previouslySelectedPathname = null
         this.hardReset()
 
         this.$scope.toggleHistory = () => {
@@ -78,8 +78,8 @@ define([
               let [newTo, newFrom] = newRange
               let [prevTo, prevFrom] = prevRange
               if (
-                newTo &&
-                newFrom &&
+                newTo != null &&
+                newFrom != null &&
                 newTo !== prevTo &&
                 newFrom !== prevFrom
               ) {
@@ -149,10 +149,10 @@ define([
               fromV: null,
               toV: null
             },
-            diff: null, // When history.viewMode == HistoryViewModes.COMPARE
-            files: [], // When history.viewMode == HistoryViewModes.COMPARE
-            update: null, // When history.viewMode == HistoryViewModes.POINT_IN_TIME
-            label: null, // When history.viewMode == HistoryViewModes.POINT_IN_TIME
+            diff: null,
+            files: [],
+            update: null,
+            label: null,
             file: null
           },
           error: null,
@@ -298,6 +298,17 @@ define([
           .get(url)
           .then(response => {
             this.$scope.history.selection.files = response.data.diff
+            for (let file of this.$scope.history.selection.files) {
+              if (file.newPathname != null) {
+                file.oldPathname = file.pathname
+                file.pathname = file.newPathname
+                delete file.newPathname
+              }
+            }
+            this.autoSelectFile()
+          })
+          .catch(err => {
+            console.error(err)
           })
           .finally(() => {
             this.$scope.history.loadingFileTree = false
@@ -309,7 +320,8 @@ define([
 
       selectFile(file) {
         if (file != null && file.pathname != null) {
-          this.$scope.history.selection.pathname = file.pathname
+          this.$scope.history.selection.pathname = this._previouslySelectedPathname =
+            file.pathname
           this.$scope.history.selection.file = file
           if (this.$scope.history.viewMode === HistoryViewModes.POINT_IN_TIME) {
             this.loadFileAtPointInTime()
@@ -319,6 +331,50 @@ define([
         }
       }
 
+      autoSelectFile() {
+        let selectedPathname = null
+        let files = this.$scope.history.selection.files
+        let fileToSelect = null
+        let previouslySelectedFile = null
+        const orderedOpTypes = ['edited', 'added', 'renamed', 'removed']
+
+        if (this._previouslySelectedPathname != null) {
+          previouslySelectedFile = _.find(files, {
+            pathname: this._previouslySelectedPathname
+          })
+        }
+
+        if (previouslySelectedFile != null) {
+          fileToSelect = previouslySelectedFile
+        } else {
+          for (let opType of orderedOpTypes) {
+            let fileWithMatchingOpType = _.find(files, { operation: opType })
+            if (fileWithMatchingOpType != null) {
+              fileToSelect = fileWithMatchingOpType
+              break
+            }
+          }
+          if (fileToSelect == null) {
+            let mainFile = _.find(files, function(file) {
+              return /main\.tex$/.test(file.pathname)
+            })
+            if (mainFile != null) {
+              fileToSelect = mainFile
+            } else {
+              let anyTeXFile = _.find(files, function(file) {
+                return /\.tex$/.test(file.pathname)
+              })
+              if (anyTeXFile != null) {
+                fileToSelect = anyTeXFile
+              } else {
+                fileToSelect = files[0]
+              }
+            }
+          }
+        }
+        this.selectFile(fileToSelect)
+      }
+
       autoSelectRecentUpdates() {
         if (this.$scope.history.updates.length === 0) {
           return

services/web/public/src/ide/history/components/historyFileEntity.js

@@ -91,8 +91,8 @@ define([
         ctrl.handleClick = _handleFileClick
         $scope.$watch(
           () => ctrl.historyFileTreeController.selectedPathname,
-          newPathname =>
-            (ctrl.isSelected = ctrl.fileEntity.pathname === newPathname)
+          selectedPathname =>
+            (ctrl.isSelected = ctrl.fileEntity.pathname === selectedPathname)
         )
       }
     }

services/web/public/src/ide/history/components/historyFileTree.js

@@ -9,14 +9,52 @@
  * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
  */
 define(['base'], function(App) {
-  const historyFileTreeController = function($scope, $element, $attrs) {
+  const historyFileTreeController = function($scope, $element, $attrs, _) {
     const ctrl = this
     ctrl.handleEntityClick = file => ctrl.onSelectedFileChange({ file })
+    ctrl._fileTree = []
+
+    $scope.$watch('$ctrl.files', function(files) {
+      if (files != null && files.length > 0) {
+        ctrl._fileTree = _.reduce(files, _reducePathsToTree, [])
+      }
+    })
+
+    function _reducePathsToTree(currentFileTree, fileObject) {
+      const filePathParts = fileObject.pathname.split('/')
+      let currentFileTreeLocation = currentFileTree
+      for (let index = 0; index < filePathParts.length; index++) {
+        var fileTreeEntity
+        var pathPart = filePathParts[index]
+        const isFile = index === filePathParts.length - 1
+        if (isFile) {
+          fileTreeEntity = _.clone(fileObject)
+          fileTreeEntity.name = pathPart
+          fileTreeEntity.type = 'file'
+          currentFileTreeLocation.push(fileTreeEntity)
+        } else {
+          fileTreeEntity = _.find(
+            currentFileTreeLocation,
+            entity => entity.name === pathPart
+          )
+          if (fileTreeEntity == null) {
+            fileTreeEntity = {
+              name: pathPart,
+              type: 'folder',
+              children: []
+            }
+            currentFileTreeLocation.push(fileTreeEntity)
+          }
+          currentFileTreeLocation = fileTreeEntity.children
+        }
+      }
+      return currentFileTree
+    }
   }
 
   return App.component('historyFileTree', {
     bindings: {
-      fileTree: '<',
+      files: '<',
       selectedPathname: '<',
       onSelectedFileChange: '&',
       isLoading: '<'

services/web/public/src/ide/history/controllers/HistoryV2FileTreeController.js

@@ -15,88 +15,9 @@ define(['base'], App =>
   App.controller('HistoryV2FileTreeController', [
     '$scope',
     'ide',
-    '_',
-    function($scope, ide, _) {
-      let _reducePathsToTree
-      let _previouslySelectedPathname = null
-      $scope.currentFileTree = []
-
-      const _pathnameExistsInFiles = (pathname, files) =>
-        _.any(files, file => file.pathname === pathname)
-
-      const _getSelectedDefaultFile = function(files) {
-        let selectedPathname = null
-        if (
-          _previouslySelectedPathname != null &&
-          _pathnameExistsInFiles(_previouslySelectedPathname, files)
-        ) {
-          selectedPathname = _previouslySelectedPathname
-        } else {
-          const mainFile = _.find(files, file =>
-            /main\.tex$/.test(file.pathname)
-          )
-          if (mainFile != null) {
-            selectedPathname = _previouslySelectedPathname = mainFile.pathname
-          } else {
-            selectedPathname = _previouslySelectedPathname = files[0].pathname
-          }
-        }
-        return _.find(files, { pathname: selectedPathname })
-      }
-
+    function($scope, ide) {
       $scope.handleFileSelection = file => {
-        _previouslySelectedPathname = file.pathname
         ide.historyManager.selectFile(file)
       }
-
-      $scope.$watch('history.selection.files', function(files) {
-        if (files != null && files.length > 0) {
-          $scope.currentFileTree = _.reduce(files, _reducePathsToTree, [])
-          ide.historyManager.selectFile(_getSelectedDefaultFile(files))
-        }
-      })
-
-      return (_reducePathsToTree = function(currentFileTree, fileObject) {
-        const filePathParts = fileObject.newPathname
-          ? fileObject.newPathname.split('/')
-          : fileObject.pathname.split('/')
-        let currentFileTreeLocation = currentFileTree
-        for (let index = 0; index < filePathParts.length; index++) {
-          var fileTreeEntity
-          var pathPart = filePathParts[index]
-          const isFile = index === filePathParts.length - 1
-          if (isFile) {
-            fileTreeEntity = {
-              name: pathPart,
-              pathname: fileObject.pathname,
-              type: 'file',
-              operation: fileObject.operation
-            }
-            if (fileObject.operation === 'renamed') {
-              fileTreeEntity.pathname = fileObject.newPathname
-              fileTreeEntity.oldPathname = fileObject.pathname
-            }
-            if (fileObject.operation === 'removed' && fileObject.deletedAtV) {
-              fileTreeEntity.deletedAtV = fileObject.deletedAtV
-            }
-            currentFileTreeLocation.push(fileTreeEntity)
-          } else {
-            fileTreeEntity = _.find(
-              currentFileTreeLocation,
-              entity => entity.name === pathPart
-            )
-            if (fileTreeEntity == null) {
-              fileTreeEntity = {
-                name: pathPart,
-                type: 'folder',
-                children: []
-              }
-              currentFileTreeLocation.push(fileTreeEntity)
-            }
-            currentFileTreeLocation = fileTreeEntity.children
-          }
-        }
-        return currentFileTree
-      })
     }
   ]))

services/web/test/unit_frontend/src/ide/history/HistoryV2ManagerTests.js

@@ -200,4 +200,178 @@ define(['ide/history/HistoryV2Manager'], HistoryV2Manager =>
       this.$scope.$digest()
       expect(this.$scope.history.userHasFullFeature).to.equal(false)
     })
+
+    describe('autoSelectFile', function() {
+      beforeEach(function() {
+        this.mockedFilesList = [
+          {
+            pathname: 'main.tex'
+          },
+          {
+            pathname: 'references.bib'
+          },
+          {
+            pathname: 'universe.jpg'
+          },
+          {
+            pathname: 'chapters/chapter2.tex'
+          },
+          {
+            pathname: 'chapters/draft.tex',
+            operation: 'removed',
+            deletedAtV: 47
+          },
+          {
+            pathname: 'chapters/chapter3.tex',
+            operation: 'added'
+          },
+          {
+            pathname: 'chapters/chapter1.tex',
+            operation: 'edited'
+          },
+          {
+            pathname: 'chapters/foo.tex',
+            oldPathname: 'chapters/bar.tex',
+            operation: 'renamed'
+          }
+        ]
+        this.mockedMainTex = this.mockedFilesList[0]
+        this.mockedReferencesFile = this.mockedFilesList[1]
+        this.mockedRemovedFile = this.mockedFilesList[4]
+        this.mockedAddedFile = this.mockedFilesList[5]
+        this.mockedEditedFile = this.mockedFilesList[6]
+        this.mockedRenamedFile = this.mockedFilesList[7]
+
+        this.$scope.history.selection.files = this.mockedFilesList
+      })
+
+      describe('with a previously selected file', function() {
+        it('should prefer the previously selected file if it is available', function() {
+          this.historyManager._previouslySelectedPathname = this.mockedReferencesFile.pathname
+          this.historyManager.autoSelectFile()
+          expect(this.$scope.history.selection.file).to.deep.equal(
+            this.mockedReferencesFile
+          )
+        })
+
+        it('should ignore the previously selected file if it is not available', function() {
+          this.historyManager._previouslySelectedPathname = 'non/existent.file'
+          this.historyManager.autoSelectFile()
+          expect(this.$scope.history.selection.file.pathname).to.not.equal(
+            'non/existent.file'
+          )
+        })
+      })
+
+      describe('without a previously selected file, with a list of files containing operations', function() {
+        it('should prefer edited files', function() {
+          this.historyManager.autoSelectFile()
+          expect(this.$scope.history.selection.file).to.deep.equal(
+            this.mockedEditedFile
+          )
+        })
+
+        it('should prefer added files if no edited files are present', function() {
+          const indexOfEditedFile = this.$scope.history.selection.files.indexOf(
+            this.mockedEditedFile
+          )
+          this.$scope.history.selection.files.splice(indexOfEditedFile, 1)
+          this.historyManager.autoSelectFile()
+          expect(this.$scope.history.selection.file).to.deep.equal(
+            this.mockedAddedFile
+          )
+        })
+
+        it('should prefer renamed files if no edited or added files are present', function() {
+          const indexOfEditedFile = this.$scope.history.selection.files.indexOf(
+            this.mockedEditedFile
+          )
+          this.$scope.history.selection.files.splice(indexOfEditedFile, 1)
+          const indexOfAddedFile = this.$scope.history.selection.files.indexOf(
+            this.mockedAddedFile
+          )
+          this.$scope.history.selection.files.splice(indexOfAddedFile, 1)
+          this.historyManager.autoSelectFile()
+          expect(this.$scope.history.selection.file).to.deep.equal(
+            this.mockedRenamedFile
+          )
+        })
+
+        it('should prefer removed files if no edited, added or renamed files are present', function() {
+          const indexOfEditedFile = this.$scope.history.selection.files.indexOf(
+            this.mockedEditedFile
+          )
+          this.$scope.history.selection.files.splice(indexOfEditedFile, 1)
+          const indexOfAddedFile = this.$scope.history.selection.files.indexOf(
+            this.mockedAddedFile
+          )
+          this.$scope.history.selection.files.splice(indexOfAddedFile, 1)
+          const indexOfRenamedFile = this.$scope.history.selection.files.indexOf(
+            this.mockedRenamedFile
+          )
+          this.$scope.history.selection.files.splice(indexOfRenamedFile, 1)
+          this.historyManager.autoSelectFile()
+          expect(this.$scope.history.selection.file).to.deep.equal(
+            this.mockedRemovedFile
+          )
+        })
+      })
+
+      describe('without a previously selected file, with a list of files without operations', function() {
+        beforeEach(function() {
+          this.mockedFilesListWithNoOps = [
+            {
+              pathname: 'main.tex'
+            },
+            {
+              pathname: 'references.bib'
+            },
+            {
+              pathname: 'other.tex'
+            },
+            {
+              pathname: 'universe.jpg'
+            }
+          ]
+          this.mockedMainTex = this.mockedFilesListWithNoOps[0]
+          this.mockedReferencesFile = this.mockedFilesListWithNoOps[1]
+          this.mockedOtherTexFile = this.mockedFilesListWithNoOps[2]
+
+          this.$scope.history.selection.files = this.mockedFilesListWithNoOps
+        })
+
+        it('should prefer main.tex if it exists', function() {
+          this.historyManager.autoSelectFile()
+          expect(this.$scope.history.selection.file).to.deep.equal(
+            this.mockedMainTex
+          )
+        })
+
+        it('should prefer another tex file if main.tex does not exist', function() {
+          const indexOfMainTex = this.$scope.history.selection.files.indexOf(
+            this.mockedMainTex
+          )
+          this.$scope.history.selection.files.splice(indexOfMainTex, 1)
+          this.historyManager.autoSelectFile()
+          expect(this.$scope.history.selection.file).to.deep.equal(
+            this.mockedOtherTexFile
+          )
+        })
+
+        it('should pick the first available file if no tex files are available', function() {
+          const indexOfMainTex = this.$scope.history.selection.files.indexOf(
+            this.mockedMainTex
+          )
+          this.$scope.history.selection.files.splice(indexOfMainTex, 1)
+          const indexOfOtherTexFile = this.$scope.history.selection.files.indexOf(
+            this.mockedOtherTexFile
+          )
+          this.$scope.history.selection.files.splice(indexOfOtherTexFile, 1)
+          this.historyManager.autoSelectFile()
+          expect(this.$scope.history.selection.file).to.deep.equal(
+            this.mockedReferencesFile
+          )
+        })
+      })
+    })
   }))