Add button to show less commits (#3562)

* Add a button to show less commits

* Fix lint issue

* Fix test

* Add tests

Author: sroy3

extension/src/experiments/index.ts

@@ -180,7 +180,7 @@ export class Experiments extends BaseRepository<TableData> {
     return this.cliData.managedUpdate()
   }
 
-  public getMoreCommits(numberOfCommitsToShow: number) {
+  public changeNbOfCommits(numberOfCommitsToShow: number) {
     return this.cliData.update(
       ExperimentFlag.NUM_COMMIT,
       numberOfCommitsToShow.toString()
@@ -581,7 +581,7 @@ export class Experiments extends BaseRepository<TableData> {
           AvailableCommands.GIT_GET_NUM_COMMITS,
           this.dvcRoot
         ),
-      (nbOfCommits: number) => this.getMoreCommits(nbOfCommits)
+      (nbOfCommits: number) => this.changeNbOfCommits(nbOfCommits)
     )
 
     this.dispose.track(

extension/src/experiments/webview/contract.ts

@@ -102,6 +102,7 @@ export type TableData = {
   hasMoreCommits: boolean
   hasRunningExperiment: boolean
   hasValidDvcYaml: boolean
+  isShowingMoreCommits: boolean
   rows: Row[]
   sorts: SortDefinition[]
   filteredCounts: FilteredCounts

extension/src/experiments/webview/messages.ts

@@ -47,11 +47,13 @@ export class WebviewMessages {
   private hasConfig = false
   private hasValidDvcYaml = true
   private hasMoreCommits = false
+  private isShowingMoreCommits = true
+
   private numberOfCommitsToShow = Number.parseInt(NUM_OF_COMMITS_TO_SHOW, 10)
 
   private readonly addStage: () => Promise<boolean>
   private readonly getNumCommits: () => Promise<number>
-  private readonly getMoreCommits: (nbOfCommits: number) => Promise<void>
+  private readonly changeNbOfCommits: (nbOfCommits: number) => Promise<void>
 
   constructor(
     dvcRoot: string,
@@ -68,7 +70,7 @@ export class WebviewMessages {
     hasStages: () => Promise<string>,
     addStage: () => Promise<boolean>,
     getNumCommits: () => Promise<number>,
-    getMoreCommits: (nbOfCommits: number) => Promise<void>
+    changeNbOfCommits: (nbOfCommits: number) => Promise<void>
   ) {
     this.dvcRoot = dvcRoot
     this.experiments = experiments
@@ -81,10 +83,10 @@ export class WebviewMessages {
     this.hasStages = hasStages
     this.addStage = addStage
     this.getNumCommits = getNumCommits
-    this.getMoreCommits = getMoreCommits
+    this.changeNbOfCommits = changeNbOfCommits
 
     void this.changeHasConfig()
-    void this.changeHasMoreCommits()
+    void this.changeHasMoreOrLessCommits()
   }
 
   public async changeHasConfig(update?: boolean) {
@@ -216,22 +218,27 @@ export class WebviewMessages {
         )
 
       case MessageFromWebviewType.SHOW_MORE_COMMITS:
-        return this.changeNumberOfCommits()
+        return this.changeCommitsToShow(1)
+
+      case MessageFromWebviewType.SHOW_LESS_COMMITS:
+        return this.changeCommitsToShow(-1)
 
       default:
         Logger.error(`Unexpected message: ${JSON.stringify(message)}`)
     }
   }
 
-  private async changeHasMoreCommits() {
-    this.hasMoreCommits =
-      (await this.getNumCommits()) > this.numberOfCommitsToShow
+  private async changeHasMoreOrLessCommits() {
+    const availableNbCommits = await this.getNumCommits()
+    this.hasMoreCommits = availableNbCommits > this.numberOfCommitsToShow
+    this.isShowingMoreCommits =
+      Math.min(this.numberOfCommitsToShow, availableNbCommits) > 1
   }
 
-  private async changeNumberOfCommits() {
-    this.numberOfCommitsToShow = this.numberOfCommitsToShow + 2
-    await this.getMoreCommits(this.numberOfCommitsToShow)
-    await this.changeHasMoreCommits()
+  private async changeCommitsToShow(change: number) {
+    this.numberOfCommitsToShow = this.numberOfCommitsToShow + 2 * change
+    await this.changeNbOfCommits(this.numberOfCommitsToShow)
+    await this.changeHasMoreOrLessCommits()
   }
 
   private getWebviewData() {
@@ -250,6 +257,7 @@ export class WebviewMessages {
       hasMoreCommits: this.hasMoreCommits,
       hasRunningExperiment: this.experiments.hasRunningExperiment(),
       hasValidDvcYaml: this.hasValidDvcYaml,
+      isShowingMoreCommits: this.isShowingMoreCommits,
       rows: this.experiments.getRowData(),
       sorts: this.experiments.getSorts()
     }

extension/src/test/fixtures/expShow/base/tableData.ts

@@ -13,6 +13,7 @@ const tableDataFixture: TableData = {
   hasRunningExperiment: true,
   hasValidDvcYaml: true,
   hasColumns: true,
+  isShowingMoreCommits: true,
   sorts: [],
   changes: [],
   columnOrder: [],

extension/src/test/fixtures/expShow/dataTypes/tableData.ts

@@ -13,6 +13,7 @@ export const data: TableData = {
   hasMoreCommits: true,
   hasRunningExperiment: false,
   hasValidDvcYaml: true,
+  isShowingMoreCommits: true,
   sorts: [],
   columns,
   hasColumns: true,

extension/src/test/fixtures/expShow/deeplyNested/tableData.ts

@@ -16,6 +16,7 @@ const data: TableData = {
   hasMoreCommits: true,
   hasRunningExperiment: false,
   hasValidDvcYaml: true,
+  isShowingMoreCommits: true,
   sorts: [
     {
       path: 'params:params.yaml:nested1.doubled',

extension/src/test/fixtures/expShow/survival/tableData.ts

@@ -13,6 +13,7 @@ const data: TableData = {
   hasColumns: true,
   hasMoreCommits: true,
   hasValidDvcYaml: true,
+  isShowingMoreCommits: true,
   sorts: [],
   changes: [],
   columnOrder: [],

extension/src/test/suite/experiments/index.test.ts

@@ -136,14 +136,14 @@ suite('Experiments Test Suite', () => {
       const { experiments, mockUpdateExperimentsData } =
         buildExperiments(disposable)
 
-      await experiments.getMoreCommits(5)
+      await experiments.changeNbOfCommits(5)
 
       expect(mockUpdateExperimentsData).to.be.calledWithExactly(
         ExperimentFlag.NUM_COMMIT,
         '5'
       )
 
-      await experiments.getMoreCommits(7)
+      await experiments.changeNbOfCommits(7)
 
       expect(mockUpdateExperimentsData).to.be.calledWithExactly(
         ExperimentFlag.NUM_COMMIT,
@@ -316,6 +316,33 @@ suite('Experiments Test Suite', () => {
         hasMoreCommits: false
       })
     }).timeout(WEBVIEW_TEST_TIMEOUT)
+
+    it('should set isShowingMoreCommits to true if it is showing more than the current commit', async () => {
+      const { experiments, messageSpy } = buildExperiments(
+        disposable,
+        expShowFixture
+      )
+
+      await experiments.showWebview()
+
+      expect(messageSpy).to.be.calledWithMatch({
+        isShowingMoreCommits: true
+      })
+    }).timeout(WEBVIEW_TEST_TIMEOUT)
+
+    it('should set isShowingMoreCommits to false it is showing only the current commit', async () => {
+      stub(GitReader.prototype, 'getNumCommits').resolves(1)
+      const { experiments, messageSpy } = buildExperiments(
+        disposable,
+        expShowFixture
+      )
+
+      await experiments.showWebview()
+
+      expect(messageSpy).to.be.calledWithMatch({
+        isShowingMoreCommits: false
+      })
+    }).timeout(WEBVIEW_TEST_TIMEOUT)
   })
 
   describe('handleMessageFromWebview', () => {
@@ -1352,7 +1379,7 @@ suite('Experiments Test Suite', () => {
     it('should handle a message to show more commits', async () => {
       const { experiments, messageSpy } = setupExperimentsAndMockCommands()
 
-      const getMoreCommitsSpy = spy(experiments, 'getMoreCommits')
+      const getMoreCommitsSpy = spy(experiments, 'changeNbOfCommits')
 
       const webview = await experiments.showWebview()
       messageSpy.resetHistory()
@@ -1363,6 +1390,24 @@ suite('Experiments Test Suite', () => {
       })
 
       expect(getMoreCommitsSpy).to.be.calledOnce
+      expect(getMoreCommitsSpy).to.be.calledWith(5)
+    }).timeout(WEBVIEW_TEST_TIMEOUT)
+
+    it('should handle a message to show less commits', async () => {
+      const { experiments, messageSpy } = setupExperimentsAndMockCommands()
+
+      const getLessCommitsSpy = spy(experiments, 'changeNbOfCommits')
+
+      const webview = await experiments.showWebview()
+      messageSpy.resetHistory()
+      const mockMessageReceived = getMessageReceivedEmitter(webview)
+
+      mockMessageReceived.fire({
+        type: MessageFromWebviewType.SHOW_LESS_COMMITS
+      })
+
+      expect(getLessCommitsSpy).to.be.calledOnce
+      expect(getLessCommitsSpy).to.be.calledWith(1)
     }).timeout(WEBVIEW_TEST_TIMEOUT)
   })
 

extension/src/webview/contract.ts

@@ -67,7 +67,8 @@ export enum MessageFromWebviewType {
   INSTALL_DVC = 'install-dvc',
   SETUP_WORKSPACE = 'setup-workspace',
   ZOOM_PLOT = 'zoom-plot',
-  SHOW_MORE_COMMITS = 'show-more-commits'
+  SHOW_MORE_COMMITS = 'show-more-commits',
+  SHOW_LESS_COMMITS = 'show-less-commits'
 }
 
 export type ColumnResizePayload = {
@@ -232,6 +233,7 @@ export type MessageFromWebview =
   | { type: MessageFromWebviewType.ZOOM_PLOT; payload?: string }
   | { type: MessageFromWebviewType.OPEN_EXPERIMENTS_WEBVIEW }
   | { type: MessageFromWebviewType.SHOW_MORE_COMMITS }
+  | { type: MessageFromWebviewType.SHOW_LESS_COMMITS }
 
 export type MessageToWebview<T extends WebviewData> = {
   type: MessageToWebviewType.SET_DATA

webview/src/experiments/components/App.test.tsx

@@ -1541,5 +1541,27 @@ describe('App', () => {
         type: MessageFromWebviewType.SHOW_MORE_COMMITS
       })
     })
+
+    it('should display a show less commits button if the table data isShowingMoreCommits is set to true', () => {
+      renderTable({ ...tableDataFixture, isShowingMoreCommits: true })
+
+      expect(screen.getByTestId('show-less-commits')).toBeInTheDocument()
+    })
+
+    it('should not display a show less commits button if the table data isShowingMoreCommits is set to false', () => {
+      renderTable({ ...tableDataFixture, isShowingMoreCommits: false })
+
+      expect(screen.queryByTestId('show-less-commits')).not.toBeInTheDocument()
+    })
+
+    it('should send a message to show less commits when the show less commits button is clicked', () => {
+      renderTable({ ...tableDataFixture, isShowingMoreCommits: true })
+
+      fireEvent.click(screen.getByTestId('show-less-commits'))
+
+      expect(mockPostMessage).toHaveBeenCalledWith({
+        type: MessageFromWebviewType.SHOW_LESS_COMMITS
+      })
+    })
   })
 })