Skip to content

TCOSMM-32: Fix delay based date calculation - #3

Open
shahrukh-compuco wants to merge 1 commit into
3.32-patchesfrom
tcosmm-32-fix-delay-based-date-calc
Open

TCOSMM-32: Fix delay based date calculation#3
shahrukh-compuco wants to merge 1 commit into
3.32-patchesfrom
tcosmm-32-fix-delay-based-date-calc

Conversation

@shahrukh-compuco

Copy link
Copy Markdown
Contributor

Overview

When a CiviRule action that creates a case activity (e.g. "Add activity to case") has its Activity Date configured as "Delay based on date field" using a Case custom date field, the calculated date was being ignored and the activity was always created with today's date. This PR fixes the delay calculation so the configured date field is respected for Case (and all other supported entities), not just Contact and Activity.

Before

  • Activity Date delay based on a Contact or Activity custom date field: works.
  • Activity Date delay based on a Case (or Contribution, Membership, Participant, Event, Relationship, etc.) custom date field: silently fails and the created activity defaults to today's date.
  • The failure was invisible — no log entry, no error.

After

  • The delay is calculated correctly from the custom date field of whichever entity was selected in the delay configuration (Case, Contact, Activity, Contribution, Membership, Participant, Event, Relationship, …).
  • If the custom field value genuinely can't be fetched, the fallback behaviour is unchanged (activity date = now), but a warning is now written to the CiviCRM log instead of failing silently.

Technical Details

Root cause was in CRM_Civirules_Delay_DelayBasedOnDateField::addInCustomField():

  1. It had a hardcoded switch that only handled Activity, with every other entity falling through to a Civi\Api4\Contact::get() call. For a Case custom field this queried Contact with the case ID and a custom field that doesn't exist on Contact, threw, and the empty catch swallowed the exception — so delayTo() fell back to now.
  2. delayTo() passed $triggerData->getEntity() (the trigger's primary entity) into the lookup instead of $this->entity (the entity the delay was configured for), even though both $data and the field prefix are derived from $this->entity.

Changes:

  • Replaced the hardcoded switch with a generic lookup:
  $customData = civicrm_api4(ucfirst($entity), 'get', [
    'select' => [$customFieldName],
    'where' => [['id', '=', $data['id']]],
    'checkPermissions' => FALSE,
    'limit' => 1,
  ])->first();
  • delayTo() now passes $this->entity into addInCustomField().
  • ucfirst() guards against triggers that register lowercase entity names (e.g. the Birthday cron trigger registers 'contact') — APIv4 entity names are case-sensitive, so without this the previously-working Birthday path would regress.
  • checkPermissions => FALSE (and CustomField::get(FALSE)): delayed actions run via the queue/cron, often with no logged-in user, where the old permission-checked calls could silently fail the same way.
  • catch (API_Exception) widened to catch (Exception)API_Exception is a deprecated alias (removed in newer cores) — and the catch now logs a warning via Civi::log('civirules').
  • Added guard clauses for a missing $data['id'] and empty custom field values (previous behaviour preserved: fall back to now).

All callers of delayTo() were audited (Engine::getActionDelay(), Activity/Add incl. the AddToCase subclass, Activity/Edit, CreateActivityFromEvent, Case/SetDateFieldOnCase) — the change is fully internal to the delay class and the previously-working Contact/Activity paths issue identical queries to before.

Core overrides

None

Comments

  • To reproduce the original bug: create a rule on a Case trigger with the "Add activity to case" action, set Activity Date to Delay based on date field → Case → any custom date field, trigger the rule, and observe the activity dated today. With this patch the activity is dated from the custom field ± the configured offset.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors CRM_Civirules_Delay_DelayBasedOnDateField to simplify custom field retrieval, replacing a switch statement with a dynamic APIv4 call and adding robust error logging. Feedback suggests adding explicit null checks for $customField and $customData to prevent PHP 8.x warnings when no records are found.

Comment thread CRM/Civirules/Delay/DelayBasedOnDateField.php
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants