CIVIPLMMSR-568: Ensure scheduled_date is always set when submitting a mailing

[erawat]

Summary

Patch for CIVIPLMMSR-568: Adds ensureScheduledDate() private method in CRM_Mailing_BAO_Mailing to prevent mailings from silently disappearing when scheduled_date is lost during the submit pipeline. Called from both add() and doSubmitActions().

Core PR: civicrm#35126

Patch commit

CIVIPLMMSR-568: Ensure scheduled_date is always set when submitting a mailing
PR: https://github.com/civicrm/civicrm-core/pull/35126

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces critical defensive programming to the CiviCRM mailing submission process. It addresses a potential issue where scheduled mailings could silently fail to be sent because their scheduled_date was inadvertently lost during the submission pipeline, even when a scheduled_id indicated they were meant to be scheduled. By ensuring scheduled_date is always populated when a mailing is scheduled, this change significantly improves the reliability and robustness of the mailing system, preventing mailings from disappearing from the scheduled list.

Highlights

• Mailing Submission Robustness: Implemented defensive checks in the mailing submission pipeline to ensure that a scheduled_date is always present when a mailing is marked as scheduled (scheduled_id is set). This prevents mailings from becoming unsendable due to a lost scheduled_date.
• CRM/Mailing/BAO/Mailing.php Update: Added a new conditional block within doSubmitActions to detect cases where scheduled_id is present but scheduled_date is missing or 'null'. In such scenarios, scheduled_date is automatically set to the current timestamp, and a warning is logged.
• api/v3/Mailing.php Update: Introduced a similar defensive check in civicrm_api3_mailing_submit to ensure scheduled_date is populated with the current timestamp if scheduled_id is set but scheduled_date is empty, addressing edge cases in the API submission process.

Changelog

• CRM/Mailing/BAO/Mailing.php
  • Added a check within doSubmitActions to ensure scheduled_date is set if scheduled_id is present but scheduled_date is missing or 'null'.
  • Implemented logic to default scheduled_date to the current timestamp (YmdHis) and log a warning if the condition is met.
• api/v3/Mailing.php
  • Introduced a conditional block in civicrm_api3_mailing_submit to set scheduled_date to the current timestamp if scheduled_id is present but scheduled_date is empty.
  • Added comments explaining the purpose of this defensive check to prevent mailings from being unschedulable.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request adds defensive guards to ensure scheduled_date is set when a mailing is submitted with a scheduled_id, preventing mailings from being lost in the pipeline. The changes are implemented in both the Mailing BAO and the v3 Mailing API, which is a good approach to cover different submission pathways. My review includes suggestions to improve consistency between these two new checks, particularly regarding the validation condition, logging, and the method for obtaining the current timestamp.

[gemini-code-assist]

This defensive check is a good addition. However, for consistency and robustness, it should be aligned with the similar check added in CRM_Mailing_BAO_Mailing::doSubmitActions.

1. The condition should also check for the string 'null', which is a common pattern in CiviCRM for representing null values from forms/APIs.
2. A warning should be logged when this condition is met, to help with debugging potential issues in the submission pipeline.
3. CRM_Utils_Date::currentDBDate() should be used for getting the current timestamp to maintain consistency across the codebase.

  if (!empty($updateParams['scheduled_id']) && (empty($updateParams['scheduled_date']) || $updateParams['scheduled_date'] === 'null')) {
    Civi::log()->warning('civicrm_api3_mailing_submit: scheduled_id is set but scheduled_date is missing for mailing {id}. Defaulting to now.', [
      'id' => $params['id'],
    ]);
    $updateParams['scheduled_date'] = CRM_Utils_Date::currentDBDate();
  }

[gemini-code-assist]

For consistency with CiviCRM's coding standards, it's better to use the framework's utility functions for date operations. CRM_Utils_Date::currentDBDate() should be used instead of the native PHP date() function. This ensures that any future changes to date handling in the framework are automatically applied here.

      $params['scheduled_date'] = CRM_Utils_Date::currentDBDate();

[shahrukh-compuco]

@erawat I think it would be better to do this on add function as well just before
$result = static::writeRecord($params);