Contrib.social

getParentEntity in SmartDateRule Preventing New Instance Creation

Open on Drupal.org →
Created on 30 April 2024, 11 months ago

Problem/Motivation

I have a site that has several SmartDate Recur rules that are supposed to create new instances when needed on cron runs. I noticed recently that while the list of instances was correct in a rule's instances field and in the UI, the new instances were not being created as rows in my content type's node__field_xxx. (In my case this is node__field_date.) Because of this, a view that looks for a current date instance was not working.

After some digging, I tracked this down to an issue with a few of the rules that had field_name set to a field that no longer existed. When the getParentEntity() method ran during cron, and $field_name was used in the entity query with a deleted field, cron stopped the process and never got to the process of making new instances. The code was checking the entity_type was defined but was not checking bundle or field name.

Steps to reproduce

Create a SmartDate Recur rule on a field. Delete the field and run cron

Proposed resolution

I added a method "validateRule" to SmartDateRule. This returns TRUE if entity type, bundle and field name are all defined and field name is defined in the type/bundle and FALSE if any of the checks fail.

I then added a call to this new method before getParentEntity is called.

In addition, I created a drush command, "smart_date_recur:prune-invalid-rules" that checks the rules and removes invalid ones from the database.

Remaining tasks

User interface changes

API changes

Data model changes

🐛 Bug report
Status

Active

Version

4.1

Component

Smart Date Recur

Created by

esomething

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

Merge Requests

Comments & Activities

  • Issue created by @esomething
  • Assigned to esomething
  • Comment 11 months ago →
    esomething
  • Merge request !118Issue 3444258 : Validate rule before calling getParentEntity → (Merged) created by esomething
  • Pipeline finished with Failed
    11 months ago
    Total: 5467s
    #160307
  • Issue was unassigned.
  • Comment 11 months ago →
    esomething
  • First commit to issue fork.
  • Status changed to Needs review 11 months ago
  • Comment 11 months ago →
    🇨🇦Canada mandclu

    Thanks for identifying this, and for providing a fix. I updated the code to resolve the issues identified by the CI, and I refactored the new validateRule() method to make the logic easier to follow.

    Please validate that the result still works for your use case.

  • Comment 11 months ago →
    esomething

    Thank you for doing the code cleanup. I've created patches but this was my first MR.

    It all looks good to me. I just made a couple of minor changes to the comments in the code to reflect the refactoring.

  • Pipeline finished with Failed
    11 months ago
    Total: 366s
    #161052
  • Pipeline finished with Skipped
    11 months ago
    #163516
  • Pipeline finished with Skipped
    11 months ago
    #163517
  • Comment 11 months ago →
    System Message
  • Status changed to Fixed 11 months ago
  • Comment 11 months ago →
    🇨🇦Canada mandclu

    Great. Merged in, and should have a new release out shortly.

  • Merge request !120Fixed dependency injection issue → (Open) created by esomething
  • Pipeline finished with Failed
    11 months ago
    Total: 183s
    #169352
  • Pipeline finished with Success
    11 months ago
    Total: 223s
    #169355
  • Comment 11 months ago →
    🇨🇦Canada mandclu

    @esomething please don't open a new MR on an issue that's already merged. Your changes seem worthwhile so please open a new issue for them.

  • Comment 11 months ago →
    esomething

    Sorry for that. Would you prefer a new MR or a new issue ticket?

  • Comment 11 months ago →
    🇨🇦Canada mandclu

    If you could file a new issue that would be great. Thanks!

  • Comment 10 months ago →
    System Message

    Automatically closed - issue fixed for 2 weeks with no activity.

  • Comment about 1 month ago →
    🇺🇸United States foodslover

    Based on a client request, I developed a patch to facilitate the bulk deletion of events older than 365 days. This patch includes configurable parameters to streamline the cleanup process, making it a valuable tool for anyone needing to manage outdated events efficiently.

    Key Features:

    1. - Bulk deletion of outdated events while preserving recent ones.
    2. - Automatic removal of expired recurrence rules that are no longer in use.
    3. - Updates to recurring events to ensure accurate start and end dates.
    4. - Email summary reports generated upon completion for tracking and verification.

    Usage:

    To execute the cleanup, run the following Drush command:
    ```sh
    drush smart_date_recur:prune-old-events 365 field_name event email_Address
    ```

    Note: Please remember to run `drush cr` after executing this command to ensure that the changes are reflected on the node editing page.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024