Uploaded image for project: 'CiviCRM'
  1. CiviCRM
  2. CRM-19786

ActionSchedule assumes presence of tokens which are not registered

    Details

    • Type: Bug
    • Status: Done/Fixed
    • Priority: Trivial
    • Resolution: Fixed/Completed
    • Affects Version/s: 4.7
    • Fix Version/s: None
    • Component/s: None
    • Labels:
    • Versioning Impact:
      Minor (add functionality in backwards-compatible manner)
    • Documentation Required?:
      Developer Doc
    • Funding Source:
      Needs Funding
    • Verified?:
      No

      Description

      A couple of tests appear to depend on the assumption that a default set of tokens are available. This is what happens when TokenCompatSubscriber is not aware of the tokens loaded.

      As part of CRM-19757, I submitted some changes to TokenCompatSubscriber which allow it to pass $returnProperties to CRM_Contact_BAO_Query::apiQuery(), in order to ensure tokens used in messages are registered. This permits tokens like {contact.email_greeting} to be replaced in contexts such as scheduled reminders.

      The PR for CRM-19757 broke a couple of other tests, eg see the results at https://test.civicrm.org/job/CiviCRM-Core-PR/13027

      It turns out that's because those tests (via CRM_Core_BAO_ActionSchedule) assume the presence of tokens currently loaded by default (when no returnProperties are passed to the apiQuery function).

      We want the currently implemented tokens restored, but we don't want to break ActionSchedule, which currently directly references the token value to send its email.

      Some options then are -

      • Ensure that all contact tokens are always available. This is probably safest for backwards compatibility, but the token interface seems to go to some effort to avoid loading tokens if not required, so it doesn't feel "in the spirit" of existing code.
      • Ensure an arbitrary set of tokens are always loaded, and that other tokens are loaded dynamically. This is an intermediate approach.
      • Only load the required tokens in each context, based on the currently registered messages. Register an unused message in order to ensure the token value is populated (this feels oblique) and then directly reference the token value (as currently).
      • Only load the required tokens in each context, based on the currently registered messages. Use another method to obtain the contact's display name.

      There's probably a better approach, but I wanted to write this much up so I can submit a PR for discussion.

      Setting to Minor because this may trigger some further breakage (as a result of CRM-19757 attempt to try and resolve existing breakage).

      Tim Otten keen to discuss this with you, since you're the boss of the Tokens

      See eg this ugly oblique method (IMO) of registering a dummy message, or this method which looks up display_name separately. I'm not certain of either of these approaches TBH, but I wanted to show the first one to demonstrate that it works.

        Attachments

          Activity

            People

            • Assignee:
              Unassigned
              Reporter:
              xurizaemon Chris Burgess
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: