Chi-2357 add link contact case #1954

acedeywin · 2023-12-19T10:27:05Z

Primary reviewer:

Description

This PR adds the blue banner to link from contact to case as well as an Undo button on the orange banner after a case has been removed from the contact.

Checklist

Related Issues

Fixes CHI-2357

Verification steps

Search for contact
Click on a contact that has been connected to a case
Verify that you can see the blue banner when you preview the contact
Click the "Remove from Case" button to remove the case from the contact
Verify that there is an "Undo" button on the orange banner
Click on the "Undo" button to undo the previous action

stephenhand

A few things suggested that would streamline the implementation, but the logic looks correct

stephenhand · 2023-12-19T17:26:18Z

plugin-hrm-form/src/components/caseMergingBanners/ContactAddedToCaseBanner.tsx

-  const { savedContact } = selectContactByTaskSid(state, taskId);
-  const connectedCase = selectCaseByCaseId(state, savedContact.caseId ?? '')?.connectedCase;
-  const caseId = savedContact?.caseId;
+const mapStateToProps = (state: RootState, { taskId, contactId }: OwnProps) => {


We should change the ContactAddedToCaseBanner to always have a contactId passed in, it's available on TabbedForms.tsx which is the other place where this is used, so we may as well pass it in there too.

We can then simplify this logic to always expect the contact ID and always use that to look up the contact

stephenhand · 2023-12-19T17:33:15Z

plugin-hrm-form/src/states/contacts/reducer.ts

@@ -222,6 +223,14 @@ export function reduce(
        },
      };
    }
+    case t.SET_REMOVED_CASE_ID: {


We should enhance the CaseMergingBannersState that @murilovmachado created in state/case/caseBanners.ts rather than creating another state structure. We can then also reuse those actions

@stephenhand - The SET_REMOVED_CASE_ID is a new action type that would hold the caseId of the connected contact after the contact has been removed from the case. This caseId will subsequently be used when the user wants to Undo the initial action

I understand that - I'm saying we already have a redux state & associated actions for managing whether a 'removed from case' banner should be displayed on a contact. We don't want 2 different states for managing it on 2 different pages, a contact should either be showing a 'removed from case' banner or it shouldn't.

The only thing missing from the existing state is the case ID used in the undo functionality

stephenhand · 2023-12-19T17:36:06Z

plugin-hrm-form/src/components/caseMergingBanners/ContactRemovedFromCaseBanner.tsx


 type OwnProps = {
  taskId: string;
+  savedContact?: Contact;
+  showRemovedFromCaseBanner?: boolean;


This name is confusing, would showUndoButton be more clear?

@stephenhand - showRemovedFromCaseBanner state is for the blue banner state and was previously implemented by @murilovmachado. I was only reusing it. I think this was what you meant in your previous comment.

In this code, the part of the banner toggled by this variable isn't the whole banner, but the just the undo button. The banner itself, the warning triangle and main message are displayed whether this variable is set or not, which is why this is a misleading name in this context

stephenhand · 2023-12-19T17:40:18Z

plugin-hrm-form/src/components/caseMergingBanners/ContactRemovedFromCaseBanner.tsx


 type OwnProps = {
  taskId: string;
+  savedContact?: Contact;


Again, I think we should simplify this logic by making the banner always expect a contactId, which it then uses to look up the contact. TabbedForms.tsx which is the other place this banner appears already has the ID loaded from state, so it's trivial to pass it in their too and simplify this logic

We should expect a contactId rather than a full contact object, since we only use the ID anyway, anmd this is conistent with the other banner

@stephenhand - I wouldn't just need the contactId, else I would pass just it. I also needed to get the contact state since I will be needing it for the Undo action.

You only look to use the ID in the code? Even if you do use the rest of the contact data it should be mapped from the state via it's ID in this component rather than prop drilled whole. This is consistent with the other banner interface and more aligned with our general move away from prop drilling and more direct use of redux state

stephenhand · 2023-12-20T14:15:41Z

plugin-hrm-form/src/components/caseMergingBanners/ContactRemovedFromCaseBanner.tsx


 type OwnProps = {
  taskId: string;
+  savedContact?: Contact;
+  showRemovedFromCaseBanner?: boolean;


In this code, the part of the banner toggled by this variable isn't the whole banner, but the just the undo button. The banner itself, the warning triangle and main message are displayed whether this variable is set or not, which is why this is a misleading name in this context

stephenhand · 2023-12-20T14:19:49Z

plugin-hrm-form/src/components/caseMergingBanners/ContactRemovedFromCaseBanner.tsx


 type OwnProps = {
  taskId: string;
+  savedContact?: Contact;


You only look to use the ID in the code? Even if you do use the rest of the contact data it should be mapped from the state via it's ID in this component rather than prop drilled whole. This is consistent with the other banner interface and more aligned with our general move away from prop drilling and more direct use of redux state

stephenhand · 2023-12-20T14:26:08Z

plugin-hrm-form/src/states/contacts/reducer.ts

@@ -222,6 +223,14 @@ export function reduce(
        },
      };
    }
+    case t.SET_REMOVED_CASE_ID: {


I understand that - I'm saying we already have a redux state & associated actions for managing whether a 'removed from case' banner should be displayed on a contact. We don't want 2 different states for managing it on 2 different pages, a contact should either be showing a 'removed from case' banner or it shouldn't.

The only thing missing from the existing state is the case ID used in the undo functionality

stephenhand · 2023-12-20T14:29:59Z

plugin-hrm-form/src/components/caseMergingBanners/ContactAddedToCaseBanner.tsx

-  const caseId = savedContact?.caseId;
+const mapStateToProps = (state: RootState, { taskId, contactId }: OwnProps) => {
+  const offlineSavedContact = selectContactByTaskSid(state, taskId)?.savedContact;
+  const existingSavedContact = state[namespace][contactFormsBase].existingContacts[contactId]?.savedContact;


[Sorry, missed this in the first pass] We have a selectContactStateByContactId selector function we should use for this.

We should be using selector functions to map state to components in all new code now- in this case we already have one, but where they don't exist yet we should add them

@stephenhand - All feedback has been implemented.

…than contact IDs in the case connectedContacts.

…into CHI-2357-add_link_contact_case

stephenhand

Updates look good - a small issue came up in testing that if you open a contact from the case timeline, remove it, then go back to the case, the contact is still there. I made a small update to the logic used to select the connected contacts for a case from redux that fixed this

So LGTM now.

stephenhand · 2023-12-22T13:45:48Z

Updates look good - a small issue came up in testing that if you open a contact from the case timeline, remove it, then go back to the case, the contact is still there. I made a small update to the logic used to select the connected contacts for a case from redux that fixed this

So LGTM now.

Except I broke it 🤦 - fixing it now

…nised transfer code slightly, updated tests

…into CHI-2357-add_link_contact_case

stephenhand · 2023-12-22T14:30:16Z

All good now!

acedeywin added 4 commits December 19, 2023 11:10

ft: first implementation for chi-2357

d2b419f

ch: refator caseId

8685b34

implement the Undo button

b1c860d

ft: clean up code for review

193e9dd

acedeywin requested review from murilovmachado and stephenhand December 19, 2023 10:27

ch: fix failing test

e54e847

stephenhand reviewed Dec 19, 2023

View reviewed changes

acedeywin requested a review from stephenhand December 20, 2023 11:12

stephenhand reviewed Dec 20, 2023

View reviewed changes

ch: implement review feebdback

5a82780

acedeywin requested a review from stephenhand December 22, 2023 06:33

stephenhand added 2 commits December 22, 2023 12:17

Update selector for case contacts to use case ids on contacts rather …

8ed4470

…than contact IDs in the case connectedContacts.

Merge remote-tracking branch 'origin/CHI-2357-add_link_contact_case' …

a2ee648

…into CHI-2357-add_link_contact_case

stephenhand approved these changes Dec 22, 2023

View reviewed changes

annaliseirby090 and others added 7 commits December 22, 2023 14:45

Enable case merging for TH (#1960)

c1ce3b1

Enable case merging for CA Stg (#1961)

5e144a5

Removed twilio sync based transfers & associated legacy types. Reorga…

1f80e73

…nised transfer code slightly, updated tests

Linter

ce4af4c

Update selector for case contacts to handle no case

be54e9b

Merge remote-tracking branch 'origin/CHI-2357-add_link_contact_case' …

a0e629c

…into CHI-2357-add_link_contact_case

Merge branch 'master' into CHI-2357-add_link_contact_case

d139c9d

acedeywin merged commit d6a63f5 into master Dec 22, 2023
8 checks passed

acedeywin deleted the CHI-2357-add_link_contact_case branch December 22, 2023 15:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Chi-2357 add link contact case #1954

Chi-2357 add link contact case #1954

acedeywin commented Dec 19, 2023 •

edited by jira bot

Loading

stephenhand left a comment

stephenhand Dec 19, 2023

stephenhand Dec 19, 2023

acedeywin Dec 20, 2023 •

edited

Loading

stephenhand Dec 20, 2023

stephenhand Dec 19, 2023

acedeywin Dec 20, 2023 •

edited

Loading

stephenhand Dec 20, 2023

stephenhand Dec 19, 2023

acedeywin Dec 20, 2023

stephenhand Dec 20, 2023

stephenhand Dec 20, 2023

stephenhand Dec 20, 2023

stephenhand Dec 20, 2023

stephenhand Dec 20, 2023

acedeywin Dec 22, 2023

stephenhand left a comment

stephenhand commented Dec 22, 2023

stephenhand commented Dec 22, 2023

Chi-2357 add link contact case #1954

Chi-2357 add link contact case #1954

Conversation

acedeywin commented Dec 19, 2023 • edited by jira bot Loading

Description

Checklist

Related Issues

Verification steps

stephenhand left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

acedeywin Dec 20, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

acedeywin Dec 20, 2023 • edited Loading

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

stephenhand left a comment

Choose a reason for hiding this comment

stephenhand commented Dec 22, 2023

stephenhand commented Dec 22, 2023

acedeywin commented Dec 19, 2023 •

edited by jira bot

Loading

acedeywin Dec 20, 2023 •

edited

Loading

acedeywin Dec 20, 2023 •

edited

Loading