Problem/Motivation

The AddonStorage class uses static functions to make database calls against the "conreg_member_addons" table. It makes extensive use of \Drupal calls, which is a pattern that is discouraged.

Steps to reproduce

N/A

Proposed resolution

The class should be converted to a service using dependency injection to access dependencies, and allowing it to be injected as a dependency to other classes.

The class should be moved into the src/Service directory.

Remaining tasks

  • Move the class to src/Service.
  • Add constructor for dependency injection.
  • Remove "static" from function definitions.
  • Make all \Drupal calls use injected dependencies.
  • Add service to services.yml file.
  • Also add alias for service using full class name.
  • Update all static calls to AddonStorage to use new service. Where possible use dependency injection, but in some cases \Drupal::service(AddonStorage::class)-> will be needed.

User interface changes

None.

API changes

None.

Data model changes

None.

Issue fork conreg-3572718

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

lostcarpark created an issue. See original summary.

lostcarpark’s picture

lostcarpark commented
Issue summary: View changes
ojchris’s picture

ojchris commented

I'll work this.

ojchris’s picture

ojchris commented

Made the conversion and aslo converted the Addon class but maintained addons in src folder cause several classes using the namespace. Left the functions getAddon, markPaid, getMemberAddons, saveAddons static cause of callers from other classes.

ojchris opened merge request !31

ojchris’s picture

ojchris commented
Status: Active » Needs review

Raised MR. I've got doubts about making Addons class a service but did any way. @lostcarpark let if you want to keep it a utility as it was.

lostcarpark’s picture

lostcarpark commented

Hi @ojchris,

Nice work overall.

There are some coding standards issues that need looking at.

I think converting Addons class to a service is a good idea, but if you want to do that, you should fully convert it, and not have static calls to functions in the service. This would require finding all calls to the class and converting to service calls - there shouldn't be too many of those.

If you prefer to keep Addons as a static class, and have a follow up issue to convert to a service, I'm okay with that.

lostcarpark’s picture

lostcarpark commented
Status: Needs review » Needs work
ojchris’s picture

ojchris commented
Status: Needs work » Needs review

- Revert Addons class to utility
- Apply drupal standards to addons class and AddonStorage service
- Update AddonStorage constructor to use property promotion
- Set services _defaults, remove repeated autowire: true setting