Page MenuHomePhabricator

Abstract "donation access"
Closed, ResolvedPublic

Description

Event Timeline

What is currently done looks structurally sane enough to me. Please outline your concerns if you have a different view. (Though better read the below first.)

The naming is not so good ATM though. We have the ShowDonationConfirmation UC, which is used by the confirmation route and by the membership application one. If the interaction with the BC is just getting Donation info, then the UC can be named as such; it does not need to care about what kind of page you present it on (or if you are even using something like pages). So I suggest to rename the UC to GetDonation.

The UC does have a problem which was already there before it was used in multiple places. It returns the Donation entity in the Response Model. This violates the architecture rules. I'm thinking we made an exception here since the violation did not seem to have too bad consequences while creating a DTO with all the Donation info is annoying (and not approved by the ministry of laziness). Now the membership application stuff uses this route, the problem consequence is worse not just in that more places now get access to the Donation entity, but that something outside of the Donation BC is getting this access. Hence I think it is time to create this DTO.

There also is that the code in apply-for-membership might be better off in its own UC, though this seems much less important than the other points here, so much so that even if the UC approach is clearly better, it likely is not worth the effort at present.

Integration into FundraisingFrontend in https://github.com/wmde/FundraisingFrontend/pull/1169 - BC changes were in master, blocking update of BC (other stories) in FundraisingFrontend without this.

kai.nissen claimed this task.