Show tags in history sidebar (#1272)

* show tags in history sidebar * switch to ITag interface * fix tests * fix PastCommitNode.spec test * remove hard coded length of tagsList * change tet_tag.py to ITag interface * fix test_git_tag_success test * fix test_tag * fix tests
jupyterlab · Oct 10, 2023 · 6fd13f4 · 6fd13f4
1 parent 2b268a7
commit 6fd13f4
Show file tree

Hide file tree

Showing 14 changed files with 148 additions and 30 deletions.
diff --git a/jupyterlab_git/git.py b/jupyterlab_git/git.py
@@ -1736,16 +1736,25 @@ async def version(self):
         return None
 
     async def tags(self, path):
-        """List all tags of the git repository.
+        """List all tags of the git repository, including the commit each tag points to.
 
         path: str
             Git path repository
         """
-        command = ["git", "tag", "--list"]
+        formats = ["refname:short", "objectname"]
+        command = [
+            "git",
+            "for-each-ref",
+            "--format=" + "%09".join("%({})".format(f) for f in formats),
+            "refs/tags",
+        ]
         code, output, error = await self.__execute(command, cwd=path)
         if code != 0:
             return {"code": code, "command": " ".join(command), "message": error}
-        tags = [tag for tag in output.split("\n") if len(tag) > 0]
+        tags = []
+        for tag_name, commit_id in (line.split("\t") for line in output.splitlines()):
+            tag = {"name": tag_name, "baseCommitId": commit_id}
+            tags.append(tag)
         return {"code": code, "tags": tags}
 
     async def tag_checkout(self, path, tag):

diff --git a/jupyterlab_git/handlers.py b/jupyterlab_git/handlers.py
@@ -861,7 +861,7 @@ async def get(self):
 
 class GitTagHandler(GitHandler):
     """
-    Handler for 'git tag '. Fetches list of all tags in current repository
+    Handler for 'git for-each-ref refs/tags'. Fetches list of all tags in current repository
     """
 
     @tornado.web.authenticated

diff --git a/jupyterlab_git/tests/test_tag.py b/jupyterlab_git/tests/test_tag.py
@@ -10,16 +10,22 @@
 @pytest.mark.asyncio
 async def test_git_tag_success():
     with patch("jupyterlab_git.git.execute") as mock_execute:
-        tag = "1.0.0"
+        output_tags = "v1.0.0\t6db57bf4987d387d439acd16ddfe8d54d46e8f4\nv2.0.1\t2aeae86b6010dd1f05b820d8753cff8349c181a6"
+
         # Given
-        mock_execute.return_value = maybe_future((0, tag, ""))
+        mock_execute.return_value = maybe_future((0, output_tags, ""))
 
         # When
         actual_response = await Git().tags("test_curr_path")
 
         # Then
         mock_execute.assert_called_once_with(
-            ["git", "tag", "--list"],
+            [
+                "git",
+                "for-each-ref",
+                "--format=%(refname:short)%09%(objectname)",
+                "refs/tags",
+            ],
             cwd="test_curr_path",
             timeout=20,
             env=None,
@@ -28,7 +34,21 @@ async def test_git_tag_success():
             is_binary=False,
         )
 
-        assert {"code": 0, "tags": [tag]} == actual_response
+        expected_response = {
+            "code": 0,
+            "tags": [
+                {
+                    "name": "v1.0.0",
+                    "baseCommitId": "6db57bf4987d387d439acd16ddfe8d54d46e8f4",
+                },
+                {
+                    "name": "v2.0.1",
+                    "baseCommitId": "2aeae86b6010dd1f05b820d8753cff8349c181a6",
+                },
+            ],
+        }
+
+        assert expected_response == actual_response
 
 
 @pytest.mark.asyncio

diff --git a/src/__tests__/test-components/HistorySideBar.spec.tsx b/src/__tests__/test-components/HistorySideBar.spec.tsx
@@ -27,6 +27,7 @@ describe('HistorySideBar', () => {
       }
     ],
     branches: [],
+    tagsList: [],
     model: {
       selectedHistoryFile: null
     } as GitExtension,

diff --git a/src/__tests__/test-components/PastCommitNode.spec.tsx b/src/__tests__/test-components/PastCommitNode.spec.tsx
@@ -48,6 +48,27 @@ describe('PastCommitNode', () => {
     }
   ];
   const branches: Git.IBranch[] = notMatchingBranches.concat(matchingBranches);
+  const matchingTags: Git.ITag[] = [
+    {
+      name: '1.0.0',
+      baseCommitId: '2414721b194453f058079d897d13c4e377f92dc6'
+    },
+    {
+      name: 'feature-1',
+      baseCommitId: '2414721b194453f058079d897d13c4e377f92dc6'
+    }
+  ];
+  const notMatchingTags: Git.ITag[] = [
+    {
+      name: 'feature-2',
+      baseCommitId: '798438398'
+    },
+    {
+      name: 'patch-007',
+      baseCommitId: '238848848'
+    }
+  ];
+  const tags: Git.ITag[] = notMatchingTags.concat(matchingTags);
   const toggleCommitExpansion = jest.fn();
   const props: IPastCommitNodeProps = {
     model: null,
@@ -59,6 +80,7 @@ describe('PastCommitNode', () => {
       pre_commits: ['pre_commit']
     },
     branches: branches,
+    tagsList: tags,
     commands: null,
     trans,
     onCompareWithSelected: null,
@@ -84,6 +106,14 @@ describe('PastCommitNode', () => {
     expect(node.text()).not.toMatch('name2');
   });
 
+  test('Includes only relevant tag info', () => {
+    const node = shallow(<PastCommitNode {...props} />);
+    expect(node.text()).toMatch('1.0.0');
+    expect(node.text()).toMatch('feature-1');
+    expect(node.text()).not.toMatch('feature-2');
+    expect(node.text()).not.toMatch('patch-007');
+  });
+
   test('Toggle show details', () => {
     // simulates SinglePastCommitInfo child
     const node = shallow(

diff --git a/src/__tests__/test-components/TagMenu.spec.tsx b/src/__tests__/test-components/TagMenu.spec.tsx
@@ -20,16 +20,20 @@ jest.mock('@jupyterlab/apputils');
 
 const TAGS = [
   {
-    name: '1.0.0'
+    name: '1.0.0',
+    baseCommitId: '4738782743'
   },
   {
-    name: 'feature-1'
+    name: 'feature-1',
+    baseCommitId: '7432743264'
   },
   {
-    name: 'feature-2'
+    name: 'feature-2',
+    baseCommitId: '798438398'
   },
   {
-    name: 'patch-007'
+    name: 'patch-007',
+    baseCommitId: '238848848'
   }
 ];
 
@@ -78,7 +82,7 @@ describe('TagMenu', () => {
       pastCommits: [],
       logger: new Logger(),
       model: model as IGitExtension,
-      tagsList: TAGS.map(tag => tag.name),
+      tagsList: TAGS,
       trans: trans,
       ...props
     };

diff --git a/src/components/GitPanel.tsx b/src/components/GitPanel.tsx
@@ -93,7 +93,7 @@ export interface IGitPanelState {
   /**
    * List of tags.
    */
-  tagsList: string[];
+  tagsList: Git.ITag[];
 
   /**
    * List of changed files.
@@ -598,6 +598,7 @@ export class GitPanel extends React.Component<IGitPanelProps, IGitPanelState> {
       <React.Fragment>
         <HistorySideBar
           branches={this.state.branches}
+          tagsList={this.state.tagsList}
           commits={this.state.pastCommits}
           model={this.props.model}
           commands={this.props.commands}

diff --git a/src/components/HistorySideBar.tsx b/src/components/HistorySideBar.tsx
@@ -32,6 +32,11 @@ export interface IHistorySideBarProps {
    */
   branches: Git.IBranch[];
 
+  /**
+   * List of tags.
+   */
+  tagsList: Git.ITag[];
+
   /**
    * Git extension data model.
    */
@@ -175,6 +180,7 @@ export const HistorySideBar: React.FunctionComponent<IHistorySideBarProps> = (
             const commonProps = {
               commit,
               branches: props.branches,
+              tagsList: props.tagsList,
               model: props.model,
               commands: props.commands,
               trans: props.trans

diff --git a/src/components/NewTagDialog.tsx b/src/components/NewTagDialog.tsx
@@ -369,14 +369,14 @@ export const NewTagDialogBox: React.FunctionComponent<INewTagDialogProps> = (
    */
   const createTag = async (): Promise<void> => {
     const tagName = nameState;
-    const commitId = baseCommitIdState;
+    const baseCommitId = baseCommitIdState;
 
     props.logger.log({
       level: Level.RUNNING,
       message: props.trans.__('Creating tag…')
     });
     try {
-      await props.model.setTag(tagName, commitId);
+      await props.model.setTag(tagName, baseCommitId);
     } catch (err) {
       setErrorState(err.message.replace(/^fatal:/, ''));
       props.logger.log({

diff --git a/src/components/PastCommitNode.tsx b/src/components/PastCommitNode.tsx
@@ -42,6 +42,11 @@ export interface IPastCommitNodeProps {
    */
   branches: Git.IBranch[];
 
+  /**
+   * List of tags.
+   */
+  tagsList: Git.ITag[];
+
   /**
    * Extension data model.
    */
@@ -199,6 +204,7 @@ export class PastCommitNode extends React.Component<
           )}
         </div>
         <div className={branchWrapperClass}>{this._renderBranches()}</div>
+        <div className={branchWrapperClass}>{this._renderTags()}</div>
         <div className={commitBodyClass}>
           {this.props.commit.commit_msg}
           {this.props.expanded && this.props.children}
@@ -250,6 +256,39 @@ export class PastCommitNode extends React.Component<
     );
   }
 
+  /**
+   * Renders tags information.
+   *
+   * @returns array of React elements
+   */
+  private _renderTags(): React.ReactElement[] {
+    const curr = this.props.commit.commit;
+    const tags: Git.ITag[] = [];
+    for (let i = 0; i < this.props.tagsList.length; i++) {
+      const tag = this.props.tagsList[i];
+      if (tag.baseCommitId && tag.baseCommitId === curr) {
+        tags.push(tag);
+      }
+    }
+    return tags.map(this._renderTag, this);
+  }
+
+  /**
+   * Renders individual tag data.
+   *
+   * @param tag - tag data
+   * @returns React element
+   */
+  private _renderTag(tag: Git.ITag): React.ReactElement {
+    return (
+      <React.Fragment key={tag.name}>
+        <span className={classes(branchClass, localBranchClass)}>
+          {tag.name}
+        </span>
+      </React.Fragment>
+    );
+  }
+
   /**
    * Callback invoked upon clicking on an individual commit.
    *

diff --git a/src/components/TagMenu.tsx b/src/components/TagMenu.tsx
@@ -91,7 +91,7 @@ export interface ITagMenuProps {
   /**
    * Current list of tags.
    */
-  tagsList: string[];
+  tagsList: Git.ITag[];
 
   /**
    * Boolean indicating whether branching is disabled.
@@ -215,7 +215,7 @@ export class TagMenu extends React.Component<ITagMenuProps, ITagMenuState> {
     // Perform a "simple" filter... (TODO: consider implementing fuzzy filtering)
     const filter = this.state.filter;
     const tags = this.props.tagsList.filter(
-      tag => !filter || tag.includes(filter)
+      tag => !filter || tag.name.includes(filter)
     );
     return (
       <FixedSizeList
@@ -225,7 +225,7 @@ export class TagMenu extends React.Component<ITagMenuProps, ITagMenuState> {
         )}
         itemCount={tags.length}
         itemData={tags}
-        itemKey={(index, data) => data[index]}
+        itemKey={(index, data) => data[index].name}
         itemSize={ITEM_HEIGHT}
         style={{ overflowX: 'hidden', paddingTop: 0, paddingBottom: 0 }}
         width={'auto'}
@@ -243,18 +243,18 @@ export class TagMenu extends React.Component<ITagMenuProps, ITagMenuState> {
    */
   private _renderItem = (props: ListChildComponentProps): JSX.Element => {
     const { data, index, style } = props;
-    const tag = data[index] as string;
+    const tag = data[index] as Git.ITag;
 
     return (
       <ListItem
         button
-        title={this.props.trans.__('Checkout to tag: %1', tag)}
+        title={this.props.trans.__('Checkout to tag: %1', tag.name)}
         className={listItemClass}
-        onClick={this._onTagClickFactory(tag)}
+        onClick={this._onTagClickFactory(tag.name)}
         style={style}
       >
         <tagIcon.react className={listItemIconClass} tag="span" />
-        <span className={nameClass}>{tag}</span>
+        <span className={nameClass}>{tag.name}</span>
       </ListItem>
     );
   };

diff --git a/src/components/Toolbar.tsx b/src/components/Toolbar.tsx
@@ -48,7 +48,7 @@ export interface IToolbarProps {
   /**
    * Current list of tags.
    */
-  tagsList: string[];
+  tagsList: Git.ITag[];
 
   /**
    * Boolean indicating whether branching is disabled.

diff --git a/src/model.ts b/src/model.ts
@@ -90,7 +90,7 @@ export class GitExtension implements IGitExtension {
   /**
    * Tags list for the current repository.
    */
-  get tagsList(): string[] {
+  get tagsList(): Git.ITag[] {
     return this._tagsList;
   }
 
@@ -1784,7 +1784,7 @@ export class GitExtension implements IGitExtension {
   }
 
   /**
-   * Retrieve the list of tags in the repository.
+   * Retrieve the list of tags in the repository, with the respective commits they point to.
    *
    * @returns promise which resolves upon retrieving the tag list
    *
@@ -1845,7 +1845,7 @@ export class GitExtension implements IGitExtension {
   async setTag(tag: string, commitId: string): Promise<void> {
     const path = await this._getPathRepository();
     await this._taskHandler.execute<void>('git:tag:create', async () => {
-      return await requestAPI<void>(URLExt.join(path, 'new_tag'), 'POST', {
+      return await requestAPI<void>(URLExt.join(path, 'tag'), 'POST', {
         tag_id: tag,
         commit_id: commitId
       });
@@ -2195,7 +2195,7 @@ export class GitExtension implements IGitExtension {
   private _stash: Git.IStash;
   private _pathRepository: string | null = null;
   private _branches: Git.IBranch[] = [];
-  private _tagsList: string[] = [];
+  private _tagsList: Git.ITag[] = [];
   private _currentBranch: Git.IBranch | null = null;
   private _docmanager: IDocumentManager | null;
   private _docRegistry: DocumentRegistry | null;