Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dev/core#1599/Deceased Contact's Membership Marked to Deceased via Inline #16724

Merged
merged 1 commit into from
Mar 15, 2020

Conversation

kartik1000
Copy link
Contributor

Signed-off-by: Kartik Kathuria [email protected]

Overview
The Deceased Contact's Membership will also be Deceased, if you mark a contact as deceased even via Inline. The BAO Contact Create function will call the updateMemberhship function and change the Contact's Membership Status. Added Test as well for the same.

@civibot
Copy link

civibot bot commented Mar 9, 2020

(Standard links)

@civibot civibot bot added the master label Mar 9, 2020
@kartik1000
Copy link
Contributor Author

@eileenmcnaughton I have fixed any checkstyle warnings, Please review.

@eileenmcnaughton
Copy link
Contributor

Can we now remove these lines ...

if (isset($params['contact_id'])) {
// process membership status for deceased contact
$deceasedParams = [
'contact_id' => CRM_Utils_Array::value('contact_id', $params),
'is_deceased' => CRM_Utils_Array::value('is_deceased', $params, FALSE),
'deceased_date' => CRM_Utils_Array::value('deceased_date', $params, NULL),
];
$updateMembershipMsg = CRM_Member_BAO_Membership::updateMembershipStatus($deceasedParams, $this->_contactType);
}

@eileenmcnaughton
Copy link
Contributor

One of the 2 fails - api.v4.Entity.ConformanceTest.testConformance - is a known bug in our test suite :-(

The other one is being caused (I think) because your test is not deleting the membership it created. This might help - #16756

As an aside we will want you to squash your git commits down to a single commit - the general trick is

  1. make sure you have a coffee first
  2. git rebase -i origin/master
  3. when you have your commits right git push -f

* Test that after checking the person as 'Deceased', the Membership is also 'Deceased' both through inline and normal edit.
*/
public function testDeceasedMembershipInline() {
$membershipStatusID = $this->membershipStatusCreate();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually the class you are using already created a membershipStatus in the setup so you can use that - from the setup

    $this->_contactID = $this->individualCreate();
    $this->_membershipTypeID = $this->membershipTypeCreate(['member_of_contact_id' => $this->_contactID]);
    $this->_membershipStatusID = $this->membershipStatusCreate('test status');

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I use, the membershipStatus in the setup, i think the teardown function will automatically delete that membership so then I won't have to call the delete function and it will pass the 2nd test as well. Am I right? or will I have to call the delete function even then?

@kartik1000
Copy link
Contributor Author

Ok @eileenmcnaughton, Thanks! I will make the desired changes and let you know :)

@kartik1000
Copy link
Contributor Author

Can we now remove these lines ...

if (isset($params['contact_id'])) {
// process membership status for deceased contact
$deceasedParams = [
'contact_id' => CRM_Utils_Array::value('contact_id', $params),
'is_deceased' => CRM_Utils_Array::value('is_deceased', $params, FALSE),
'deceased_date' => CRM_Utils_Array::value('deceased_date', $params, NULL),
];
$updateMembershipMsg = CRM_Member_BAO_Membership::updateMembershipStatus($deceasedParams, $this->_contactType);
}

@eileenmcnaughton, I am not sure of removing this, because the normal edit still works through Form/Contact.php So, removing this might break that. I will also try and confirm it.

Signed-off-by: Kartik Kathuria <[email protected]>

Signed-off-by: Kartik Kathuria <[email protected]>

Signed-off-by: Kartik Kathuria <[email protected]>

Signed-off-by: Kartik Kathuria <[email protected]>
@kartik1000
Copy link
Contributor Author

@eileenmcnaughton, all tests have passed and I have even squashed the commits. Please check.

@eileenmcnaughton
Copy link
Contributor

Great - I tested this & it works - yay - for merging your commit to core!

@eileenmcnaughton eileenmcnaughton merged commit 12d5ca9 into civicrm:master Mar 15, 2020
@colemanw
Copy link
Member

FYI @kartik1000 I've been trying to get rid of unnecessary uses of CRM_Utils_Array::value in favor of the ?? coalesce operator which is simpler.

I've done #16874 to clean it up (and some other code cleanup in that file while I was at it).

@kartik1000
Copy link
Contributor Author

Great @colemanw! Yes if it makes it simpler. Why not.

@colemanw
Copy link
Member

No problem. Thanks again for your contribution @kartik1000

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants