Skip to content
New issue

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

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

Already on GitHub? # to your account

Improve DonationNotifier implementation #244

Merged
merged 2 commits into from
Apr 4, 2024
Merged

Conversation

gbirke
Copy link
Member

@gbirke gbirke commented Mar 19, 2024

Split interface for donor and template mailer
Use value objects instead of arrays

This is a backwards-breaking change that needs new implementations in
the Fundraising Application

@gbirke gbirke mentioned this pull request Mar 19, 2024
@gbirke gbirke force-pushed the upgrade-phpstan-level branch from fbe7f25 to 916cb0b Compare March 20, 2024 15:58
@gbirke gbirke force-pushed the donation-mailer-interface-oop branch from 670c1c9 to af4651d Compare March 20, 2024 16:00
@gbirke gbirke force-pushed the donation-mailer-interface-oop branch from af4651d to ea3d64d Compare March 20, 2024 22:35
Base automatically changed from upgrade-phpstan-level to main March 21, 2024 15:21
gbirke added a commit to wmde/fundraising-application that referenced this pull request Mar 21, 2024
Create adapter classes that transform the templateArguments objects into
arrays, passing them on to the TemplateBasedMailer which only accepts
arrays.

This is an adaptation for wmde/fundraising-donations#244
gbirke added a commit to wmde/fundraising-application that referenced this pull request Mar 22, 2024
Create adapter classes that transform the templateArguments objects into
arrays, passing them on to the TemplateBasedMailer which only accepts
arrays.

This is an adaptation for wmde/fundraising-donations#244
@Abban Abban requested review from Abban, moiikana and Sperling-0 March 26, 2024 08:32
Since user can no longer cancel donations, remove all code that
distinguishes between users and administrators.

Remove reference to mailer interface, since that is no longer needed.

This'll help improving the TemplateMailerInterface to accept only
objects.

Add `allow-plugins` section to composer.json to allow for new code
style.

The changes in the CancelDonationUseCase and CancelDonationRequest
constructor are a BC break.
@gbirke gbirke force-pushed the donation-mailer-interface-oop branch 2 times, most recently from f9300c3 to 6e6b3c7 Compare March 26, 2024 09:52
Copy link
Member

@Abban Abban left a comment

Choose a reason for hiding this comment

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

Even just seeing those arrays as objects in a PR makes things feel safer

@@ -59,7 +59,8 @@
"config": {
"discard-changes": true,
"allow-plugins": {
"composer/package-versions-deprecated": true
"composer/package-versions-deprecated": true,
"dealerdirect/phpcodesniffer-composer-installer": true
Copy link
Member

Choose a reason for hiding this comment

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

Every time I see this I get a fright

@@ -0,0 +1,11 @@
<?php
declare( strict_types=1 );
Copy link
Member

Choose a reason for hiding this comment

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

There should be a line break before this, I think?

Split interface for donor and template mailer
Use value objects instead of arrays

This is a backwards-breaking change that needs new implementations in
the Fundraising Application
@gbirke gbirke force-pushed the donation-mailer-interface-oop branch from 6e6b3c7 to 95178dc Compare April 2, 2024 12:36
@gbirke gbirke merged commit 25473d5 into main Apr 4, 2024
5 checks passed
@gbirke gbirke deleted the donation-mailer-interface-oop branch April 4, 2024 10:34
gbirke added a commit to wmde/fundraising-application that referenced this pull request Apr 5, 2024
Create adapter classes that transform the templateArguments objects into
arrays, passing them on to the TemplateBasedMailer which only accepts
arrays.

This is an adaptation for wmde/fundraising-donations#244
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants