OCLOMRS-472: Implement the Retire/Un-Retire button #367

brucemakallan · 2019-03-18T14:17:41Z

JIRA TICKET NAME:

Implement the Retire/Un-Retire button

Summary:

Ensure that one can Retire custom Concepts as well as Unretire them

brucemakallan · 2019-03-18T14:18:18Z

This PR is currently a Work In Progress

coveralls · 2019-03-19T10:02:31Z

Pull Request Test Coverage Report for Build 2571

26 of 26 (100.0%) changed or added relevant lines in 3 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.06%) to 96.985%

Totals
Change from base Build 2545:	0.06%
Covered Lines:	1897
Relevant Lines:	1932

💛 - Coveralls

AGMETEOR

LGTM

dkayiwa · 2019-03-25T11:09:47Z

package.json

@@ -30,7 +30,7 @@
    "uuid": "^3.3.2"
  },
  "scripts": {
-    "start": "chmod +x ./init.sh && ./init.sh && cp env-config.js ./public/ && npm run build:css && react-scripts start",


How is this related to the ticket?

I have created a separate ticket for it. I'll remove the change from this one

dkayiwa · 2019-03-27T16:09:43Z

src/tests/Dashboard/action/dictionaryAction.test.js

+      });
+    });
+    const store = mockStore({ payload: {} });
+    return store.dispatch(retireConcept(sampleConcept.url, { retired: false })).then(() => {


How is this testing for should unretire a concept?

I have updated the retireConcept() action to return the retired concept and I am now testing directly for the retired flag. expect(result.retired).toEqual(true); when retiring, and expect(result.retired).toEqual(false); when un-retiring.

dkayiwa · 2019-03-28T07:38:51Z

src/tests/Dashboard/action/dictionaryAction.test.js

@@ -381,6 +382,49 @@ describe('Test suite for dictionary actions', () => {
      expect(store.getActions()).toEqual(expectedActions);
    });
  });
+
+  it('should retire a concept', () => {


When should retire a concept happen?

This action is triggered when the retire button is clicked

Will you keep around to keep answering the maintainers of this code whenever they ask this?

I have updated the message accordingly

dkayiwa · 2019-03-28T07:39:04Z

src/tests/Dashboard/action/dictionaryAction.test.js

+    });
+  });
+
+  it('should unretire a concept', () => {


When should unretire a concept happen?

This action is triggered when the unretire button is clicked

I have updated the message accordingly.

dkayiwa · 2019-03-28T09:24:36Z

src/tests/Dashboard/action/dictionaryAction.test.js

+        response: sampleRetiredConcept,
+      });
+    });
+    const store = mockStore({ payload: {} });


Where are is the simulation of clicking the retire button?

DictionaryConcepts.test.jsx lines 528 - 621

I mean in this very test?

This is the Action that is triggered when the button is clicked. The test is only for the Action and the button click test is in the lines I mentioned. Therefore the simulation for the button click is not (and should not be) in this test.

Then i find this confusing "should retire a concept when the retire button is clicked"

I understand the concern. Should I change it to "should retire a concept when the retireConcept method is triggered"?

I have changed the title to should retire a concept when the retireConcept action is triggered with the true argument

dkayiwa · 2019-03-28T09:24:46Z

src/tests/Dashboard/action/dictionaryAction.test.js

+        response: sampleConcept,
+      });
+    });
+    const store = mockStore({ payload: {} });


Same as above

DictionaryConcepts.test.jsx lines 528 - 621

Same as above

I have changed the title to should unretire a concept when the retireConcept action is triggered with the false argument

dkayiwa · 2019-03-28T10:09:22Z

src/redux/actions/dictionaries/dictionaryActionCreators.js

@@ -18,6 +18,7 @@ import {
 import { filterPayload } from '../../reducers/util';
 import { addDictionaryReference } from '../bulkConcepts';
 import api from '../../api';
+import instance from '../../../config/axiosConfig';


What is the meaning of instance?

I have used a more appropriate name (axiosInstance)

brucemakallan force-pushed the OCLOMRS-472 branch 2 times, most recently from d90326d to 1406abd Compare March 19, 2019 09:58

brucemakallan force-pushed the OCLOMRS-472 branch 9 times, most recently from a41b222 to a07295b Compare March 21, 2019 08:55

AGMETEOR approved these changes Mar 21, 2019

View reviewed changes

alexkayabula approved these changes Mar 21, 2019

View reviewed changes

dkayiwa reviewed Mar 25, 2019

View reviewed changes

brucemakallan force-pushed the OCLOMRS-472 branch from a07295b to b8f6285 Compare March 25, 2019 11:14

judeinno approved these changes Mar 27, 2019

View reviewed changes

brucemakallan force-pushed the OCLOMRS-472 branch 3 times, most recently from b6cc573 to 308021e Compare March 27, 2019 12:10

dkayiwa reviewed Mar 27, 2019

View reviewed changes

brucemakallan force-pushed the OCLOMRS-472 branch 2 times, most recently from 11f195e to a2702cb Compare March 28, 2019 03:46

dkayiwa reviewed Mar 28, 2019

View reviewed changes

brucemakallan force-pushed the OCLOMRS-472 branch from a2702cb to e9f4b38 Compare March 28, 2019 09:02

dkayiwa reviewed Mar 28, 2019

View reviewed changes

brucemakallan force-pushed the OCLOMRS-472 branch from e9f4b38 to 35e3ac2 Compare March 28, 2019 10:50

OCLOMRS-472: Implement the Retire/Un-Retire button

d7211b8

brucemakallan force-pushed the OCLOMRS-472 branch from 35e3ac2 to d7211b8 Compare March 28, 2019 12:07

dkayiwa merged commit 8281ee5 into openmrs:master Mar 28, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OCLOMRS-472: Implement the Retire/Un-Retire button #367

OCLOMRS-472: Implement the Retire/Un-Retire button #367

brucemakallan commented Mar 18, 2019

brucemakallan commented Mar 18, 2019

coveralls commented Mar 19, 2019 •

edited

AGMETEOR left a comment

dkayiwa Mar 25, 2019

brucemakallan Mar 25, 2019

dkayiwa Mar 27, 2019

brucemakallan Mar 28, 2019

dkayiwa Mar 28, 2019

brucemakallan Mar 28, 2019

dkayiwa Mar 28, 2019

brucemakallan Mar 28, 2019

dkayiwa Mar 28, 2019

brucemakallan Mar 28, 2019

brucemakallan Mar 28, 2019

dkayiwa Mar 28, 2019

brucemakallan Mar 28, 2019

dkayiwa Mar 28, 2019

brucemakallan Mar 28, 2019

dkayiwa Mar 28, 2019

brucemakallan Mar 28, 2019

brucemakallan Mar 28, 2019

dkayiwa Mar 28, 2019

brucemakallan Mar 28, 2019

dkayiwa Mar 28, 2019

brucemakallan Mar 28, 2019

dkayiwa Mar 28, 2019

brucemakallan Mar 28, 2019

OCLOMRS-472: Implement the Retire/Un-Retire button #367

OCLOMRS-472: Implement the Retire/Un-Retire button #367

Conversation

brucemakallan commented Mar 18, 2019

JIRA TICKET NAME:

Summary:

brucemakallan commented Mar 18, 2019

coveralls commented Mar 19, 2019 • edited

Pull Request Test Coverage Report for Build 2571

💛 - Coveralls

AGMETEOR left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Mar 19, 2019 •

edited