Add ECA condition plugin "is TFA setup"

Created on 30 November 2023, 7 months ago
Updated 4 December 2023, 7 months ago

Problem/Motivation

I'm not sure if the maintainers of TFA have explored ECA, however my experience is this module is already at the Views v2 stage in terms of awesomeness.

So while D7 had a module tfa_rules , modern plugin architecture is pretty neat, and I'm wondering whether TFA could provide its own plugins for ECA either directly in the main module, or a submodule.

This would allow ECA like this:

Steps to reproduce

n/a

Proposed resolution

Here's an anonymised version of a plugin we're using - would you accept a pull request or would you see this going into another contrib project.
https://gist.github.com/simesy/7bfbca281cf1e1594f20e2d233b2186e

Remaining tasks

User interface changes

API changes

Data model changes

Feature request
Status

Needs review

Version

2.0

Component

Code

Created by

🇦🇺Australia sime Canberra

Live updates comments and jobs are added and updated live.
  • Needs tests

    The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @sime
  • 🇦🇺Australia sime Canberra
  • 🇦🇺Australia sime Canberra
  • 🇺🇸United States cmlara

    I'm aware of ECA although to date I have not personally given it more than a cursory glance. Even though ECA's install count is small (around 2k reported at the moment) it does seem it might possibly become the replacement for Rules in the future (though I would be unable to opine on the strengths of either) especially because no D8+ stable release of Rules yet exists.

    Cursory discussions indicate there there may indeed be at least one use for such a plugin (redirect on login) and I could see that some ohter uses might exist (such as only allow TFA enabled accounts access to certain nodes). As such I can see value in such a plugin existing.

    The primary call isReady() is in a class marked @internal in 2.x. I would prefer to keep it so in order to allow TFA the flexibility to respond to changing needs in the future.

    Given the above I would be supportive of an MR to add this into the main repo.

  • Assigned to sime
  • Status changed to Needs work 7 months ago
  • 🇦🇺Australia sime Canberra

    That's good news.

  • 🇮🇳India Bhanu951

    I would suggest to go with submodule in TFA instead of standalone plugin in the TFA module, as this plugin will have external dependency of ECA Module which won't be present in all sites which are planning to use this module.

  • 🇦🇺Australia sime Canberra

    Yes i assume tfa_eca?

  • 🇺🇸United States cmlara

    as this plugin will have external dependency of ECA Module which won't be present in all sites which are planning to use this module.

    It is my understanding that plugins are scoped to the ecosystem they work with eg tfa\Plugins\ECA, and should only be loaded when the plugin manager for ECA is installed.

    There is 🐛 AttributeClassDiscovery fails while trying to include non valid plugin class Fixed in core, however that applies only to Attribute based plugins and to my understanding should only pose an issue if the ECA namespace is listed for discovery and the plugin has a dependency on yet another module that is not installed.

    No other code should attempt discovery for tfa\Plugins\ECA except for ECA, or some other module that provides all of ECA's API. So long as our plugin doesn't use source beyond that of TFA or ECA it shouldn't pose an issue.

    ECA will likely becomes a require-dev for testing however that is acceptable dependency as it doesn't impact end users.

  • Merge request !64Add ECA condition to test if TFA is setup. → (Open) created by sime
  • Status changed to Needs review 7 months ago
  • 🇦🇺Australia sime Canberra
Production build 0.69.0 2024