Contrib.social

The service should not initialise countries and currencies

Open on Drupal.org →
Created on 5 February 2024, over 1 year ago
Updated 29 February 2024, over 1 year ago

Problem/Motivation

The constructor of the InvoiceNinjaSynchronizer service is currently calling this:

    $this->initCountries();
    $this->initCurrencies();

That's problematic because that service gets initialised pretty often and early in the bootstrap. It gets initialised even if the current PHP process doesn't even need that service, e.g. when opening an ECA model which loads all the action plugins, and that will do because the sync service in an injected dependency to that action plugin.

This is causing a few issues:

  • It is initialised even when not needed.
  • This throws an exception, if the IN is not configured yet, e.g. in line 320 where new InvoiceNinja($token) requires a string, but NULL could be provided.

Proposed resolution

The initialisation should only be done when really needed.

I would probably define the variable $InvoiceNinjaClient in that class. And the function getInvoiceNinjaClient() initialised the client, if it isn't defined yet.

The country and currency initialisation should probably only be done, if the values are actually need, which currently only seems to be the case in getClientData(). Sounds like the service might be used sometimes even without a need for those statics, but calling them could be slow.

📌 Task
Status

Fixed

Version

1.0

Component

Code

Created by

🇩🇪Germany jurgenhaas Gottmadingen

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024