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

MAE-501: Fix unable to send direct debit notifications. #231

Conversation

erawat
Copy link
Member

@erawat erawat commented Mar 15, 2021

Overview

When using Find contributions / Find memberships to search for contribution or memberships, Manual Direct Debit extensions provide a feature to send direct debit notifications on the actions using Manual Direct Templates.

The implementation was extended CiviCRM core functions which were moved from the class to traits since CiviCRM 5.34 here.

This PR refactors the code in the Manual Direct Debit to align with the changes in the CiviCRM core.

Before

Error was present when sending email from "Send Direct Debit Notifications" actions when using Direct Debit templates or CiviCRM templates.

After

Email can be sent when using Send Direct Debit Notifications when using either

  • Direct Debit templates.
  • CiviCRM templates.

Technical Details

This PR also takes an opportunity to refactor codes to fix or address as the following:

  • Decouple the functions by abstracting function onto abstract class
  • Fix warning issue on overriding method that have different signature.

PHP does not allow different method signature when override method from the parent class. As the sendEmail method in the child class requires different arguments, the method name has been changed to address the warning.
This abstract common class to decouple functions that duplicate in Contribution and Membership Email classes
The methods in parents class have been moved from the parent class to the traits. In order to use the functions in the traits which is not static, we need to change the context of method that calls the function in traits to have the same context.
Since the common code have been moved from common class to the traits in CiviCRM core, we need to refactor these classes to align with CiviCRM core.
@erawat erawat marked this pull request as ready for review March 15, 2021 13:35
@erawat erawat changed the title MAE-501: Fix unable to send direct debit notification. MAE-501: Fix unable to send direct debit notifications. Mar 15, 2021
@erawat erawat merged commit 61d5b24 into MAE-483-workstream-civi5.35 Mar 16, 2021
@erawat erawat deleted the MAE-501-fix-unable-to-send-directdebit-notification branch March 16, 2021 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants