Bug #23217 See Comments

Extension Hook Settings

Version: 3.5.11 Reporter: TJ Draper

There’s a problem with EE_Extensions::call() method. The problem is that each extension row for an add-on in the exp_extensions table can have settings. However, the first hook/row that runs from the call method has it’s settings cached by class name, so when another hook for that addon/class that does have settings is run, it is sent the settings for the first hook/row that ran. The settings cache key should include the row ID for uniqueness. Here are images that hopefully illustrate the problem better than the words describing it perhaps.

Extension Rows

Extension Class

  • Are you saving your extension hook entities manually or with SQL by chance? ExpressionEngine saves the settings to all rows for the class, letting the extension itself have settings, but not individual methods.

    $extension_model = ee('Model')->get('Extension')
     ->filter('enabled', 'y')
     ->filter('class', $extension['class'])
     ->all();
    
    $extension_model->settings = $settings;
    $extension_model->save();

    That behavior has not changed since Extensions were introduced in v1, fwiw. Or is there a situation where the native application is not saving settings for all rows for the class?

    Derek Jones
    06th September, 2017 at 11:55am
  • I’m creating an Extension Model, setting the class, hook, and settings, and saving the extension model. I definitely DO NOT want the same settings to be saved to every row. Each extension row needs it’s own settings and that would certainly be the most intuitive since every row in the DB has its own settings column.

    I guess I‘ve never been in a situation where I was saving extension settings on an add-on that had multiple extensions rows.

    TJ Draper
    06th September, 2017 at 11:59am
  • Why are you saving the settings instead of letting ExpressionEngine save the settings? Per-method settings may be intuitive for the developer, but not the user, which is why the settings have been for the Extension itself (and a class property). And when the app saves them, it would overwrite the different settings you saved manually. Multiple hooks are often used coordinate a single functional goal, so for users to have to understand the set settings per-hook would not be very user friendly.

    Future note; we are in fact more likely to combine all add-on settings into a single table, since again, the user doesn’t need to know or care about what types of components an add-on does; to them it’s a single thing: a plugin, a mini-app, a widget, a section of their control panel, etc.

    Derek Jones
    06th September, 2017 at 12:05pm
  • The user never sees the settings in the case of the add-on I’m working on. I’d hate to lose the ability to have each row have its own settings. I’m not sure what problem it would cause to send a row’s settings to the class as expected by the developer, and still have EE’s extensions settings method save the settings to all extension rows. For cases where the developer is not using EE extension settings page (and in my case where the user never sees the settings), it behaves the way it’s expected to. And for cases where the add-on exposes settings, it still behaves as before since EE is updating all the rows.

    And in fact, if you’re moving toward a model where add-on wide settings are saved in some common location, then the extension settings column could become, truly, developer only data.

    TJ Draper
    06th September, 2017 at 12:10pm
  • The user never sees the settings in the case of the add-on I’m working on. I’d hate to lose the ability to have each row have its own settings.

    But you never had that ability, as the app UI and API have never allowed for separate settings. It saves one settings array to all rows in the collection with that class name. Saving the settings would overwrite all of your rows that differ from one another. Again, this behavior has been consistent for almost 10 years, and is documented how extensions are expected to interact with their settings.

    I agree it would be more clear to someone examining the tables if extension settings had their own table and a single row per class, but that’s the nature of software development. You always find a better way to write something than the developer before you (even if it was you). Either way, you’re diving into private api and expecting it to work in a way it wasn’t designed to.

    Settings are for the user, it’s why ExpressionEngine automatically builds a settings form for you based on your settings array, and has a special method to allow you to customize and write a more complex form UI for the end user. I’m guessing you’re having to go through quite a bit of a dance to hide your extension’s settings from the user. If you want developer settings that will be untouched by the native application, you should store them in your own table rather than using a first-party table in a way that wasn’t intended.

    Derek Jones
    06th September, 2017 at 12:41pm
  • Just to be clear (I’m not trying to be a hard head), ee('Model')->make('Extension'), setting properties, and then saving the model is considered a private API?

    TJ Draper
    06th September, 2017 at 1:02pm
  • I’m not taking it as hard-headed. Yes, in this case that would be private API. That is not how the documentation describes working with extensions to install them or save settings. Public, documented methods are provided and using the model service directly is just making an end-run around those to manipulate the database directly. And aren’t you having to do anything else to trick the system into hiding the settings from the user? If so, isn’t that itself a red flag that you’re operating in private API outside of the entry point’s intent? These are all calculated risks you can make as a developer—it’s not like we block add-ons that aren’t written entirely according to documentation—and those risks are sometimes fine. In this case though you are seeing that problems can arise, when our intent as a developer and desired behavior do not match the application’s intent and behavior.

    You have me curious: what settings are you needing to save to the DB for individual methods, that are never exposed to the user? From an architectural standpoint, doesn’t that feel like something you’d want in your own tables anyway? Or in an add-on config file?

    Derek Jones
    06th September, 2017 at 1:12pm
  • User defined custom extensions. I can’t tell you how many times I’ve needed to write some extension or other custom to a given site.

    https://github.com/tjdraper/executive-ee/blob/master/docs/user-extensions.md

    https://github.com/tjdraper/executive-ee/blob/master/executive/ext.executive.php#L150

    So I need to know what row is calling my routing method (because the class and method are always going to be the same). I’m using the settings to store the class and method. I could move to storing that on my own table, but I still need to know which row is calling my method so I know which settings to get. There’s currently, to the best of my knowledge, no way to know that.

    TJ Draper
    06th September, 2017 at 1:40pm
  • https://github.com/tjdraper/executive-ee/blob/master/docs/extension-designer.md

    TJ Draper
    06th September, 2017 at 1:41pm
  • Circling around after our conversation on Slack, and for posterity:

    • these classes are stored in a different location as you wish to physically separate bespoke extensions from other add-ons
    • your Executive extension acts as a gatekeeper, a middle layer to handle the extension hook calls
    • Using a naming mechanism like methodName__someKey will let your handler know both which method to call in these external classes, as well as a primary key to fetch any info or settings from custom tables, without requiring any modifications to the application.
    Derek Jones
    06th September, 2017 at 3:36pm

You must be signed in to comment on a bug report.

ExpressionEngine News

#eecms, #events, #releases