Problem/Motivation

ECA is getting popular although as of today it still has far less installations than Rules module. However, ECA seems to have an upward trending install count, but Rules is trending downward. As of today, Rules has a compatible Drupal 11 version. So Rules integration will remain supported.

Proposed resolution

Implement the ECA integration in a new submodule.

Remaining tasks

  • Code a new role_expire_eca submodule
  • Write tests for the new submodule
  • Add documentation on role_expire main README file

Original report by kopeboy

Rules is not D10 ready yet, while a new substitute module ECA has emerged, which is already stable, has even more features and better admin UI.

I would suggest you have a look into that and put the Rules packed with this module in a submodule for better organization.

Issue fork role_expire-3362875

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

kopeboy created an issue. See original summary.

rcodina’s picture

rcodina
Primary language Catalan
Location Barcelona
commented

I agree ECA integration would be awesome. Patches are welcome. However, I would like #3036511: Add a new configurable email notification system to be committed first.

rcodina’s picture

rcodina
Primary language Catalan
Location Barcelona
commented

Rules integration is moving to a submodule with the new Drupal 11 compatible version (4.x branch).

rcodina’s picture

rcodina
Primary language Catalan
Location Barcelona
commented
Title: Make Rules integration a sub-module, provide ECA integration » Provide ECA integration
Priority: Normal » Major
rcodina’s picture

rcodina
Primary language Catalan
Location Barcelona
commented
Version: 3.x-dev » 4.x-dev
drupgirl’s picture

drupgirl commented

+1 for eca integration

rcodina’s picture

rcodina
Primary language Catalan
Location Barcelona
commented
Issue summary: View changes
rcodina’s picture

rcodina
Primary language Catalan
Location Barcelona
commented
Issue summary: View changes
rcodina’s picture

rcodina
Primary language Catalan
Location Barcelona
commented
Component: Miscellaneous » Code

kuzyawkk made their first commit to this issue’s fork.

kuzyawkk’s picture

kuzyawkk commented
Assigned: Unassigned » kuzyawkk
plasticpippo’s picture

plasticpippo commented

+1 for ECA integration
if you integrate it with ECA there is no need for #3036511: Add a new configurable email notification system as it can be done within ECA

kuzyawkk opened merge request !24

kuzyawkk’s picture

kuzyawkk commented
Assigned: kuzyawkk » Unassigned
Status: Active » Needs review

Yes, I added implementation for the ECA module similar to the Rules module.
I did not add any additional features on my own.

_tarik_’s picture

_tarik_ commented
Assigned: Unassigned » _tarik_
Status: Needs review » Needs work

I see pipelines found problems with phpcs and phpstan, and the tests are missing, while they are mentioned in the issue description.
I have some free time to complete this task, so I've assigned it to myself.

_tarik_’s picture

_tarik_ commented
Assigned: _tarik_ » Unassigned
Status: Needs work » Needs review

I’ve finished my changes here, so the issue is ready for review.

I did these steps in the scope of my work:
1. Manually tested changes that did @kuzyawkk (testing environment: eca version 2.1.13, Drupal core - 10.5)
2. Fixed errors from phpcs and phpstan
3. Added tests that cover both plugins: role_expire_eca_remove_expire_time and role_expire_eca_set_expire_time

rcodina’s picture

rcodina
Primary language Catalan
Location Barcelona
commented

Added missing features to match Rules module integration.