Fix Git History (#1028) (#1032)

* No Git history for selected file
Fixes #1024

* No context menu item if not in a repository

Author: fcollonval

jest.config.js

@@ -22,11 +22,17 @@ module.exports = {
   preset: 'ts-jest/presets/js-with-babel',
   moduleFileExtensions: ['ts', 'tsx', 'js', 'jsx', 'json', 'node'],
   modulePathIgnorePatterns: [
+    '<rootDir>/build',
     '<rootDir>/jupyterlab_git',
     '<rootDir>/jupyter-config'
   ],
   setupFiles: ['<rootDir>/testutils/jest-setup-files.js'],
-  testPathIgnorePatterns: ['/lib/', '/node_modules/', '/jupyterlab_git/'],
+  testPathIgnorePatterns: [
+    '/lib/',
+    '/node_modules/',
+    '/jupyterlab_git/',
+    '/ui-tests/'
+  ],
   testRegex: '/tests/.*.spec.ts[x]?$',
   transformIgnorePatterns: [`/node_modules/(?!${esModules}).+`],
   globals: {

src/commandsAndMenu.tsx

@@ -1057,7 +1057,11 @@ export function addFileBrowserContextMenu(
     const items = toArray(filebrowser.selectedItems());
     const statuses = new Set<Git.Status>(
       items
-        .map(item => model.getFile(item.path)?.status)
+        .map(item =>
+          model.pathRepository === null
+            ? undefined
+            : model.getFile(item.path)?.status
+        )
         .filter(status => typeof status !== 'undefined')
     );
 

src/model.ts

@@ -294,24 +294,41 @@ export class GitExtension implements IGitExtension {
   /**
    * Match files status information based on a provided file path.
    *
-   * If the file is tracked and has no changes, a StatusFile of unmodified will be returned
+   * If the file is tracked and has no changes, a StatusFile of unmodified will be returned.
    *
    * @param path the file path relative to the server root
+   * @returns The file status or null if path repository is null or path not in repository
    */
-  getFile(path: string): Git.IStatusFile {
-    return (
-      this._status.files.find(status => {
-        return this.getRelativeFilePath(status.to) === path;
-      }) ?? {
-        x: '',
-        y: '',
-        to: path,
-        from: '',
-        is_binary: null,
-        status: 'unmodified',
-        type: this._resolveFileType(path)
+  getFile(path: string): Git.IStatusFile | null {
+    if (this.pathRepository === null) {
+      return null;
+    }
+    const fileStatus = this._status.files.find(status => {
+      return this.getRelativeFilePath(status.to) === path;
+    });
+
+    if (!fileStatus) {
+      const relativePath = PathExt.relative(
+        '/' + this.pathRepository,
+        '/' + path
+      );
+
+      if (relativePath.startsWith('../')) {
+        return null;
+      } else {
+        return {
+          x: '',
+          y: '',
+          to: relativePath,
+          from: '',
+          is_binary: null,
+          status: 'unmodified',
+          type: this._resolveFileType(path)
+        };
       }
-    );
+    } else {
+      return fileStatus;
+    }
   }
 
   /**

tests/model.spec.tsx

@@ -26,7 +26,7 @@ describe('IGitExtension', () => {
     };
 
     mockResponses = {
-      responses: {...defaultMockedResponses}
+      responses: { ...defaultMockedResponses }
     };
 
     mockGit.requestAPI.mockImplementation(mockedRequestAPI(mockResponses));
@@ -85,7 +85,7 @@ describe('IGitExtension', () => {
         expect(model.isReady).toBe(false);
       });
 
-      mockResponses.path = DEFAULT_REPOSITORY_PATH
+      mockResponses.path = DEFAULT_REPOSITORY_PATH;
       model.pathRepository = mockResponses.path;
       expect(model.isReady).toBe(false);
       await model.ready;
@@ -269,32 +269,57 @@ describe('IGitExtension', () => {
     });
   });
 
+  describe('#getFile', () => {
+    it.each([
+      ['dir1/dir2/repo/somefolder/file', 'somefolder/file', 'dir1/dir2/repo'],
+      ['repo/file', 'file', 'repo'],
+      ['repo/somefolder/file', 'somefolder/file', 'repo'],
+      ['somefolder/file', 'somefolder/file', ''],
+      ['file', 'file', ''],
+      ['file', null, null],
+      ['other_repo/file', null, 'repo'],
+      ['root/other_repo/file', null, 'root/repo']
+    ])(
+      '%s should return unmodified status with path relative to the repository',
+      async (path, expected, repo) => {
+        // Given
+        mockResponses.path = repo;
+        model.pathRepository = repo;
+        await model.ready;
+        // When
+        const status = model.getFile(path);
+        // Then
+        if (expected === null) {
+          expect(status).toBeNull();
+        } else {
+          expect(status.status).toEqual('unmodified');
+          expect(status.to).toEqual(expected);
+        }
+      }
+    );
+  });
+
   describe('#getRelativeFilePath', () => {
-    it('should return relative path correctly ', async () => {
-      const testData = [
-        [
-          'somefolder/file',
-          'dir1/dir2/repo',
-          'dir1/dir2/repo/somefolder/file'
-        ],
-        ['file', 'repo', 'repo/file'],
-        ['somefolder/file', 'repo', 'repo/somefolder/file'],
-        ['somefolder/file', '', 'somefolder/file'],
-        ['file', '', 'file'],
-        ['file', null, null]
-      ];
-
-      for (const testDatum of testData) {
+    it.each([
+      ['somefolder/file', 'dir1/dir2/repo', 'dir1/dir2/repo/somefolder/file'],
+      ['file', 'repo', 'repo/file'],
+      ['somefolder/file', 'repo', 'repo/somefolder/file'],
+      ['somefolder/file', '', 'somefolder/file'],
+      ['file', '', 'file'],
+      ['file', null, null]
+    ])(
+      '%s should return relative path correctly ',
+      async (path, repo, expected) => {
         // Given
-        mockResponses.path = testDatum[1];
-        model.pathRepository = testDatum[1];
+        mockResponses.path = repo;
+        model.pathRepository = repo;
         await model.ready;
         // When
-        const relativePath = model.getRelativeFilePath(testDatum[0]);
+        const relativePath = model.getRelativeFilePath(path);
         // Then
-        expect(relativePath).toEqual(testDatum[2]);
+        expect(relativePath).toEqual(expected);
       }
-    });
+    );
   });
 
   describe('#checkout', () => {