Add processing comment (#21)

joachimdalen · May 16, 2022 · 52ccc1a · 52ccc1a
1 parent 08deb15
commit 52ccc1a
Show file tree

Hide file tree

Showing 12 changed files with 357 additions and 56 deletions.
diff --git a/.azext/changelog-cache.json b/.azext/changelog-cache.json
@@ -1,5 +1,12 @@
 {
   "issues": [
+    {
+      "id": 1168793478,
+      "number": 4,
+      "submitter": "joachimdalen",
+      "title": "Approval / Rejection comments",
+      "url": "https://github.com/joachimdalen/azdevops-acceptance-criterias/issues/4"
+    },
     {
       "id": 1168792610,
       "number": 3,
@@ -16,6 +23,13 @@
     }
   ],
   "pullRequests": [
+    {
+      "id": 936583972,
+      "number": 21,
+      "submitter": "joachimdalen",
+      "title": "Add processing comment",
+      "url": "https://github.com/joachimdalen/azdevops-acceptance-criterias/pull/21"
+    },
     {
       "id": 930221978,
       "number": 19,

diff --git a/.azext/changelog.json b/.azext/changelog.json
@@ -1,6 +1,6 @@
 [
   {
-    "publishDate": "2022-05-XX",
+    "publishDate": "2022-05-16",
     "version": "1.1.0",
     "changes": [
       {
@@ -38,10 +38,16 @@
         "changes": [
           {
             "type": "feature",
-            "description": "Added processing history",
+            "description": "Added processing history. See [history](https://devops-extensions.dev/docs/extensions/acceptance-criterias/processing/history)",
             "pullRequest": 19,
             "issue": 3
           },
+          {
+            "type": "feature",
+            "description": "Added processing comments. See [approvals and rejections](https://devops-extensions.dev/docs/extensions/acceptance-criterias/processing#approvals-and-rejections)",
+            "pullRequest": 21,
+            "issue": 4
+          },
           {
             "type": "fix",
             "description": "Fixed an issue where you could uncheck chacklist items while waiting for approval",

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,6 +1,6 @@
 # Changelog
 
-## 1.1.0 (2022-05-XX)
+## 1.1.0 (2022-05-16)
 
 ### 🐛 Fixes (1)
 
@@ -32,14 +32,19 @@
 - Remove unused code
   - Changed in [PR#19 - Refactor and introduce base history](https://github.com/joachimdalen/azdevops-acceptance-criterias/pull/19)
 
-### 🚀 Features (1)
+### 🚀 Features (2)
 
 #### `[email protected]`
 
-- Added processing history
+- Added processing history. See [history](https://devops-extensions.dev/docs/extensions/acceptance-criterias/processing/history)
+
   - Suggested in [GH#3 - Processing history](https://github.com/joachimdalen/azdevops-acceptance-criterias/issues/3)
   - Added in [PR#19 - Refactor and introduce base history](https://github.com/joachimdalen/azdevops-acceptance-criterias/pull/19)
 
+- Added processing comments. See [approvals and rejections](https://devops-extensions.dev/docs/extensions/acceptance-criterias/processing#approvals-and-rejections)
+  - Suggested in [GH#4 - Approval / Rejection comments](https://github.com/joachimdalen/azdevops-acceptance-criterias/issues/4)
+  - Added in [PR#21 - Add processing comment](https://github.com/joachimdalen/azdevops-acceptance-criterias/pull/21)
+
 ### 📝 Documentation (1)
 
 #### `[email protected]`

diff --git a/src/__tests__/common/services/CriteriaHistoryService.test.ts b/src/__tests__/common/services/CriteriaHistoryService.test.ts
@@ -0,0 +1,172 @@
+import { IInternalIdentity } from '@joachimdalen/azdevops-ext-core/CommonTypes';
+
+import CriteriaHistoryService from '../../../common/services/CriteriaHistoryService';
+import { StorageService } from '../../../common/services/StorageService';
+import { HistoryDocument, HistoryEvent, ProcessEvent } from '../../../common/types';
+
+const identity: IInternalIdentity = {
+  displayName: 'Test User',
+  entityId: '1234',
+  entityType: 'User',
+  id: '54321',
+  descriptor: 'user1234',
+  image: '/image.png'
+};
+
+const historyWithContent: HistoryDocument = {
+  __etag: 1,
+  id: 'AC-1-2',
+  items: [
+    {
+      date: new Date(),
+      event: HistoryEvent.Completed,
+      actor: identity
+    }
+  ]
+};
+
+describe('CriteriaHistoryService', () => {
+  const getHistorySpy = jest.spyOn(StorageService.prototype, 'getHistory');
+  const setHistorySpy = jest.spyOn(StorageService.prototype, 'setHistory');
+
+  beforeEach(() => {
+    jest.clearAllMocks();
+    getHistorySpy.mockReset();
+  });
+
+  describe('getProcessEvent', () => {
+    it('should return approved event without actor', () => {
+      const service = new CriteriaHistoryService();
+
+      const evnt = service.getProcessEvent(ProcessEvent.Approve);
+      expect(evnt.event).toEqual(HistoryEvent.Approved);
+      expect(evnt.actor).toBeUndefined();
+      expect(evnt.properties).toBeUndefined();
+      expect(evnt.date).not.toBeUndefined();
+    });
+
+    it('should return rejected event without actor', () => {
+      const service = new CriteriaHistoryService();
+
+      const evnt = service.getProcessEvent(ProcessEvent.Reject);
+      expect(evnt.event).toEqual(HistoryEvent.Rejected);
+      expect(evnt.actor).toBeUndefined();
+      expect(evnt.properties).toBeUndefined();
+      expect(evnt.date).not.toBeUndefined();
+    });
+
+    it('should return complete event without actor', () => {
+      const service = new CriteriaHistoryService();
+
+      const evnt = service.getProcessEvent(ProcessEvent.Complete);
+      expect(evnt.event).toEqual(HistoryEvent.Completed);
+      expect(evnt.actor).toBeUndefined();
+      expect(evnt.properties).toBeUndefined();
+      expect(evnt.date).not.toBeUndefined();
+    });
+
+    it('should return reset-to-new event without actor', () => {
+      const service = new CriteriaHistoryService();
+
+      const evnt = service.getProcessEvent(ProcessEvent.ResetToNew);
+      expect(evnt.event).toEqual(HistoryEvent.ReOpened);
+      expect(evnt.actor).toBeUndefined();
+      expect(evnt.properties).toBeUndefined();
+      expect(evnt.date).not.toBeUndefined();
+    });
+
+    it('should return resubmit-for-approval event without actor', () => {
+      const service = new CriteriaHistoryService();
+
+      const evnt = service.getProcessEvent(ProcessEvent.ResubmitForApproval);
+      expect(evnt.event).toEqual(HistoryEvent.ReApprove);
+      expect(evnt.actor).toBeUndefined();
+      expect(evnt.properties).toBeUndefined();
+      expect(evnt.date).not.toBeUndefined();
+    });
+
+    it('should return event with actor', () => {
+      const service = new CriteriaHistoryService();
+
+      const evnt = service.getProcessEvent(ProcessEvent.Approve, identity);
+      expect(evnt.event).toEqual(HistoryEvent.Approved);
+      expect(evnt.actor).toEqual(identity);
+      expect(evnt.properties).toBeUndefined();
+      expect(evnt.date).not.toBeUndefined();
+    });
+    it('should return event with actor and comment', () => {
+      const service = new CriteriaHistoryService();
+
+      const evnt = service.getProcessEvent(ProcessEvent.Approve, identity, 'Some comment');
+      expect(evnt.event).toEqual(HistoryEvent.Approved);
+      expect(evnt.actor).toEqual(identity);
+      expect(evnt.properties).not.toBeUndefined();
+      expect(evnt.properties?.comment).toEqual('Some comment');
+      expect(evnt.date).not.toBeUndefined();
+    });
+  });
+
+  describe('getHistory', () => {
+    it('should return default when 404 is thrown', async () => {
+      getHistorySpy.mockRejectedValue({ status: 404 });
+      const service = new CriteriaHistoryService();
+
+      const result = await service.getHistory('AC-1-1');
+
+      expect(result).not.toBeUndefined();
+      expect(result.__etag).toEqual(-1);
+      expect(result.items.length).toEqual(0);
+    });
+
+    it('should return default when fetched is undefined', async () => {
+      getHistorySpy.mockResolvedValue(undefined);
+      const service = new CriteriaHistoryService();
+
+      const result = await service.getHistory('AC-1-1');
+
+      expect(result).not.toBeUndefined();
+      expect(result.__etag).toEqual(-1);
+      expect(result.items.length).toEqual(0);
+    });
+
+    it('should return history', async () => {
+      getHistorySpy.mockResolvedValue(historyWithContent);
+      const service = new CriteriaHistoryService();
+
+      const result = await service.getHistory('AC-1-1');
+
+      expect(result).not.toBeUndefined();
+      expect(result.__etag).toEqual(1);
+      expect(result.items.length).toEqual(1);
+    });
+
+    it('should throw when error is not 404', async () => {
+      getHistorySpy.mockRejectedValue({ status: 500 });
+      const service = new CriteriaHistoryService();
+
+      expect(async () => await service.getHistory('AC-1-1')).rejects.toThrowError();
+    });
+  });
+
+  describe('createOrUpdate', () => {
+    it('should update item', async () => {
+      getHistorySpy.mockResolvedValue(historyWithContent);
+      setHistorySpy.mockImplementation(d => new Promise(resolve => resolve(d)));
+
+      const service = new CriteriaHistoryService();
+
+      const result = await service.createOrUpdate('AC-1-1', {
+        date: new Date(),
+        event: HistoryEvent.Completed,
+        actor: identity,
+        properties: {
+          comment: 'Hello'
+        }
+      });
+
+      expect(result).not.toBeUndefined();
+      expect(result.items.length).toEqual(2);
+      expect(result.items[0].event).toEqual(HistoryEvent.Completed);
+    });
+  });
+});
diff --git a/src/__tests__/criteria-panel/components/ProcessingContainer.test.tsx b/src/__tests__/criteria-panel/components/ProcessingContainer.test.tsx
@@ -15,8 +15,28 @@ describe('ProcessingContainer', () => {
     fireEvent.click(approve);
     fireEvent.click(save);
 
-    expect(process).toHaveBeenCalledWith('1234', ProcessEvent.Approve);
+    expect(process).toHaveBeenCalledWith('1234', ProcessEvent.Approve, undefined);
   });
+
+  it('should approve with comment when entered', async () => {
+    const process = jest.fn();
+    render(<ProcessingContainer criteriaId="1234" processCriteria={process} />);
+
+    const approve = screen.getByRole('radio', { name: 'Approve' });
+
+    const comment = screen.getByPlaceholderText(
+      'A short reason for rejecting or approving the criteria'
+    );
+    fireEvent.change(comment, { target: { value: 'I approve this' } });
+
+    const save = screen.getByRole('button');
+
+    fireEvent.click(approve);
+    fireEvent.click(save);
+
+    expect(process).toHaveBeenCalledWith('1234', ProcessEvent.Approve, 'I approve this');
+  });
+
   it('should reject when selected', async () => {
     const process = jest.fn();
     render(<ProcessingContainer criteriaId="1234" processCriteria={process} />);
@@ -28,6 +48,25 @@ describe('ProcessingContainer', () => {
     fireEvent.click(reject);
     fireEvent.click(save);
 
-    expect(process).toHaveBeenCalledWith('1234', ProcessEvent.Reject);
+    expect(process).toHaveBeenCalledWith('1234', ProcessEvent.Reject, undefined);
+  });
+
+  it('should reject with comment when entered', async () => {
+    const process = jest.fn();
+    render(<ProcessingContainer criteriaId="1234" processCriteria={process} />);
+
+    const reject = screen.getByRole('radio', { name: 'Reject' });
+
+    const comment = screen.getByPlaceholderText(
+      'A short reason for rejecting or approving the criteria'
+    );
+    fireEvent.change(comment, { target: { value: 'I reject this' } });
+
+    const save = screen.getByRole('button');
+
+    fireEvent.click(reject);
+    fireEvent.click(save);
+
+    expect(process).toHaveBeenCalledWith('1234', ProcessEvent.Reject, 'I reject this');
   });
 });
diff --git a/src/admin-hub/tabs/orphaned/tabs/OrphanedCriteriaDetailsTab.tsx b/src/admin-hub/tabs/orphaned/tabs/OrphanedCriteriaDetailsTab.tsx
@@ -100,7 +100,6 @@ const OrphanedCriteriaDetailsTab = (): React.ReactElement => {
         tableItem: OrphanedCriteriaDetail,
         ariaRowIndex?: number
       ) => {
-        console.log('rerender');
         return (
           <SimpleTableCell columnIndex={columnIndex} tableColumn={tableColumn}>
             {tableItem.type === 'Unknown' ? (