Skip to content

Commit

Permalink
fixed unexpected behaviors around saving an engagement and polling fo… (
Browse files Browse the repository at this point in the history
#285)

* fixed unexpected behaviors around saving an engagement and polling for updates

* removed environment class

* removed useless comments
  • Loading branch information
sdstolworthy authored Jul 31, 2020
1 parent f3ba194 commit cfb5790
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ describe('Engagement Context', () => {
cleanup();
});

test('by default, engagements are undefined', () => {
test('by default, engagements are an empty array', () => {
const { result } = getHook();
expect(result.current.engagements).toEqual(undefined);
expect(result.current.engagements).toEqual([]);
});

test('Fetch Engagements', async () => {
Expand Down Expand Up @@ -246,7 +246,11 @@ describe('Engagement Context', () => {
result.current.getEngagements();
await waitForNextUpdate();
});
expect(result.current.engagements[0]).toEqual(initialEngagement);
await act(async () => {
result.current.setCurrentEngagement(result.current.engagements[0]);
await waitForNextUpdate();
});
expect(result.current.currentEngagement).toEqual(initialEngagement);
let modifiedEngagement = {
...initialEngagement,
customer_contact_email: '[email protected]',
Expand All @@ -255,7 +259,7 @@ describe('Engagement Context', () => {
result.current.saveEngagement(modifiedEngagement);
await waitForNextUpdate();
});
expect(result.current.engagements[0]).toEqual(modifiedEngagement);
expect(result.current.currentEngagement).toEqual(modifiedEngagement);
});
test('saveEngagement reverts to the initial engagement when the save was unsuccessful', async () => {
const initialEngagement = Engagement.fromFake(true);
Expand Down
23 changes: 13 additions & 10 deletions src/context/engagement_context/engagement_context.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ export const EngagementProvider = ({

const [error] = useState<any>();
const [isLoading] = useState<boolean>(false);
const [engagements, setEngagements] = useState<Engagement[]>(undefined);
const [engagements, setEngagements] = useState<Engagement[]>([]);
const [currentEngagement, setCurrentEngagement] = useState<
Engagement | undefined
>();
Expand Down Expand Up @@ -179,16 +179,23 @@ export const EngagementProvider = ({
[_updateEngagementInPlace, engagementService, _validateAuthStatus]
);

const _checkHasUpdateRef = useRef(async () => false);

useEffect(() => {
const checkUpdate = async () => {
return await engagementService.checkHasUpdates(currentEngagement);
};
_checkHasUpdateRef.current = checkUpdate;
}, [currentEngagement, engagementService]);

const createEngagementPoll = useCallback(
async (engagement: Engagement): Promise<EngagementPoll> => {
await _validateAuthStatus();
return new EngagementPoll(
new EngagementPollIntervalStrategy(
setInterval(async () => {
await _validateAuthStatusRef.current();
const hasUpdates = await engagementService.checkHasUpdates(
engagement
);
const hasUpdates = await _checkHasUpdateRef.current();
if (hasUpdates) {
feedbackContext.showAlert(
'Another user edited this engagement. In order to continue, you must refresh the page. By refreshing, your unsaved changes will be overwritten."',
Expand All @@ -206,12 +213,7 @@ export const EngagementProvider = ({
)
);
},
[
_validateAuthStatus,
_refreshEngagementData,
engagementService,
feedbackContext,
]
[_validateAuthStatus, _refreshEngagementData, feedbackContext]
);

const getEngagement = useCallback(
Expand Down Expand Up @@ -335,6 +337,7 @@ export const EngagementProvider = ({
);
feedbackContext.hideLoader();
_updateEngagementInPlace(returnedEngagement);
setCurrentEngagement(returnedEngagement);
} catch (e) {
_updateEngagementInPlace(oldEngagement);
feedbackContext.hideLoader();
Expand Down
32 changes: 24 additions & 8 deletions src/routes/dashboard/__snapshots__/dashboard.spec.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,9 @@ Object {
class=""
data-cy="numbers_of_l"
data-pf-content="true"
/>
>
0
</h1>
<button
class="pf-c-button pf-m-link pf-m-inline"
data-cy="button_l"
Expand Down Expand Up @@ -125,7 +127,9 @@ Object {
class=""
data-cy="numbers_of_p"
data-pf-content="true"
/>
>
0
</h1>
<button
class="pf-c-button pf-m-link pf-m-inline"
data-cy="button_p"
Expand Down Expand Up @@ -185,7 +189,9 @@ Object {
class=""
data-cy="numbers_of_c"
data-pf-content="true"
/>
>
0
</h1>
<button
class="pf-c-button pf-m-link pf-m-inline"
data-cy="button_c"
Expand Down Expand Up @@ -245,7 +251,9 @@ Object {
class=""
data-cy="numbers_of_a"
data-pf-content="true"
/>
>
0
</h1>
<button
class="pf-c-button pf-m-link pf-m-inline"
data-cy="button_a"
Expand Down Expand Up @@ -335,7 +343,9 @@ Object {
class=""
data-cy="numbers_of_l"
data-pf-content="true"
/>
>
0
</h1>
<button
class="pf-c-button pf-m-link pf-m-inline"
data-cy="button_l"
Expand Down Expand Up @@ -395,7 +405,9 @@ Object {
class=""
data-cy="numbers_of_p"
data-pf-content="true"
/>
>
0
</h1>
<button
class="pf-c-button pf-m-link pf-m-inline"
data-cy="button_p"
Expand Down Expand Up @@ -455,7 +467,9 @@ Object {
class=""
data-cy="numbers_of_c"
data-pf-content="true"
/>
>
0
</h1>
<button
class="pf-c-button pf-m-link pf-m-inline"
data-cy="button_c"
Expand Down Expand Up @@ -515,7 +529,9 @@ Object {
class=""
data-cy="numbers_of_a"
data-pf-content="true"
/>
>
0
</h1>
<button
class="pf-c-button pf-m-link pf-m-inline"
data-cy="button_a"
Expand Down

0 comments on commit cfb5790

Please sign in to comment.