Skip to content

Transaction refactoring

Jano requested to merge transaction_refactoring into master

What does this MR do?

This branch refactors the classes in the Service namespace and moves their functionality to Transaction classes or other places that fit better.

Background

@NerdyProjects, me and some others discussed our project structure in an online conference during hackweek. We agreed on being unhappy with the current practice of putting business logic into the Gateway classes and found that in some places, this separation of concern is solved using classes called "Service" in the "Service" namespace. However, the "Service" namespace also contained utitlities, and as, depending on your definition, nearly every class in our project could be called "Service" (and we don't have "GatewayService" or "ControllerService"), we found the name "Service" to be too generic.

@Caluera skimmed the book on that our code structure is based upon and found out that they solved this problem with using Transaction classes. Also, in the book, all Gateway methods are named very technically (like insert..., update..., select...), so you see that they just do database queries and don't confuse them with business transactions.

This MR tries to establish the code structure portrayed by the book Modernizing Legacy Applications In PHP, or more precisely, the way I interpreted it in https://gitlab.com/foodsharing-dev/foodsharing/-/blob/9f0f337e11a69c061b9fadc9209e08034faa7cb3/docs/php-structure.md.

How confident are you it won't break things if deployed?

I used the refactoring tools of PHPStorm and double-checked the code by reading, and I clicked through the pages that have modules that got refactored. So I think this won't break anything.

Still, I don't want to merge this before !887 (merged) to save @NerdyProjects from merge conflicts regarding StoreService (now StoreTransactions). That's why I haven't set the label State 4 - Wants a review yet.

Checklist

  • added a test, or explain why one is not needed/possible...
  • no unrelated changes
  • asked someone for a code review
  • joined #foodsharing-beta channel at https://slackin.yunity.org
  • added an entry to CHANGELOG.md (description, merge request link, username(s))
  • Once your MR has been merged, you are responsible to update the #foodsharing-beta Slack channel about what has been changed here. They will test your work in different browsers, roles or other settings
Edited by Chris Oelmueller

Merge request reports