-
-
Notifications
You must be signed in to change notification settings - Fork 814
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
dev/mail#20 : Preview screen doesn't open until recipients list is built on mail compose screen #12509
Conversation
(Standard links)
|
ping @lcdservices @colemanw |
also @davejenx |
ang/crmMailing/services.js
Outdated
@@ -286,6 +286,15 @@ | |||
} | |||
}, | |||
|
|||
// @param mailing Object (per APIv3) | |||
// @return preview content | |||
quickPreview: function quickPreview(mailing) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why don't we catch only mailing.id rather whole mailing object or array?
Thanks @pradpnayak for your feedback. I have made that change, please have a look. |
@monishdeb tested and working as expected. The preview modal opens immediately. |
@seamuslee001 @andrew-cormick-dockery This has been tested but I'd really like you guys to sign off on it too as you got stung by a mailing bug recently |
@lcdservices I have changed my patch, which will now fix the issue where earlier, making any changes on HTML/text content doesn't get reflected in the preview screen, as the content is not saved to DB. On another look, I saw that we don't even need to call |
test this please |
349b78f
to
a76869e
Compare
…t on mail compose screen
…t on mail compose screen
@Monish this looks good. The preview does not depend on the recipients list being built before displaying, and when clicked, will display the mailing content even if autosave is not completed. |
@seamuslee001 or @andrew-cormick-dockery I think this is waiting on the green light from one of you. |
I've reviewed the code and gave this a test using a pretty big mailing and worked as expected. I can't see any ACL issues here. Happy to see this merged |
Flexmailer overrides the `Mailing.preview` API. In civicrm/civicrm-core#12509 (5.6), it became possible to call `Mailing.preview` without an `id` for the mailing. This update Flexmailer `Mailing.preview` to also work without an ID.
Flexmailer overrides the `Mailing.preview` API. In civicrm/civicrm-core#12509 (5.6), it became possible to call `Mailing.preview` without an `id` for the mailing. This update Flexmailer `Mailing.preview` to also work without an ID.
Flexmailer overrides the `Mailing.preview` API. In civicrm/civicrm-core#12509 (5.6), it became possible to call `Mailing.preview` without an `id` for the mailing. This update Flexmailer `Mailing.preview` to also work without an ID, and it expands unit-test coverage to ensure correct operation with or without an ID.
…essor/Flexmailer Overview -------- When using `TokenProcessor` to generate a mailing (e.g. as with Flexmailer/Mosaico), the action-tokens (e.g. `{action.optOutUrl}`) are generated via `CRM_Mailing_ActionTokens`. To properly generate them, `CRM_Mailing_ActionTokens` relies on certain information (e.g. mailing/job ID). However, that information is no longer available when performing a "Preview" -- leading to misbehavior in previews. This patch allows Flexmailer to restore parity for previewing those tokens. Note: This is a pre-requisite for the full fix civicrm/org.civicrm.flexmailer#38 Before (Pre-5.6) ---------------- * When a user begins composing a mailing, CiviMail creates a draft mailing with a concrete ID (e.g. `mailing civicrm#123`). * To preview the mailing, the UI calls `Mailing.preview` API with the ID of the mailing. * Flexmailer/Mosaico generates the preview by calling `TokenProcessor` and therefore `CRM_Mailing_ActionTokens`. * `CRM_Mailing_ActionTokens` has strictness checks. These pass because the ID is available. Before (5.6-5.12) ---------------- As a performance enhancement, CiviCRM 5.6 (PR civicrm#12509; [dev/mail#20](https://lab.civicrm.org/dev/mail/issues/20)) revised the signature for `Mailing.preview` API to allow previews *without* having a specific mailing record/job/ID. Consequently: * When a user begins composing a mailing, CiviMail creates a draft mailing with a concrete ID (e.g. `mailing civicrm#123`). * To preview the mailing, the UI calls `Mailing.preview` API ~~with~~ **without** the ID of the mailing. * Flexmailer/Mosaico generates the preview by calling `TokenProcessor` and therefore `CRM_Mailing_ActionTokens`. * `CRM_Mailing_ActionTokens` has strictness checks. These ~~pass~~ **fail** because the ID is ~~available~~ **unavailable**. After ---------------- * When a user begins composing a mailing, CiviMail creates a draft mailing with a concrete ID (e.g. `mailing civicrm#123`). * To preview the mailing, the UI calls `Mailing.preview` API ~~with~~ **without** the ID of the mailing. * Flexmailer/Mosaico generates the preview by calling `TokenProcessor` and therefore `CRM_Mailing_ActionTokens`. * `CRM_Mailing_ActionTokens` has ~~strictness~~ **less strict** checks. These **pass** because the `context[schema]` hints that a mailing ID *will be available* when needed.
…essor/Flexmailer Overview -------- When using `TokenProcessor` to generate a mailing (e.g. as with Flexmailer/Mosaico), the action-tokens (e.g. `{action.optOutUrl}`) are generated via `CRM_Mailing_ActionTokens`. To properly generate them, `CRM_Mailing_ActionTokens` relies on certain information (e.g. mailing/job ID). However, that information is no longer available when performing a "Preview" -- leading to misbehavior in previews. This patch allows Flexmailer to restore parity for previewing those tokens. Note: This is a pre-requisite for the full fix civicrm/org.civicrm.flexmailer#38 Before (Pre-5.6) ---------------- * When a user begins composing a mailing, CiviMail creates a draft mailing with a concrete ID (e.g. `mailing civicrm#123`). * To preview the mailing, the UI calls `Mailing.preview` API with the ID of the mailing. * Flexmailer/Mosaico generates the preview by calling `TokenProcessor` and therefore `CRM_Mailing_ActionTokens`. * `CRM_Mailing_ActionTokens` has strictness checks. These pass because the ID is available. Before (5.6-5.12) ---------------- As a performance enhancement, CiviCRM 5.6 (PR civicrm#12509; [dev/mail#20](https://lab.civicrm.org/dev/mail/issues/20)) revised the signature for `Mailing.preview` API to allow previews *without* having a specific mailing record/job/ID. Consequently: * When a user begins composing a mailing, CiviMail creates a draft mailing with a concrete ID (e.g. `mailing civicrm#123`). * To preview the mailing, the UI calls `Mailing.preview` API ~~with~~ **without** the ID of the mailing. * Flexmailer/Mosaico generates the preview by calling `TokenProcessor` and therefore `CRM_Mailing_ActionTokens`. * `CRM_Mailing_ActionTokens` has strictness checks. These ~~pass~~ **fail** because the ID is ~~available~~ **unavailable**. After ---------------- * When a user begins composing a mailing, CiviMail creates a draft mailing with a concrete ID (e.g. `mailing civicrm#123`). * To preview the mailing, the UI calls `Mailing.preview` API ~~with~~ **without** the ID of the mailing. * Flexmailer/Mosaico generates the preview by calling `TokenProcessor` and therefore `CRM_Mailing_ActionTokens`. * `CRM_Mailing_ActionTokens` has ~~strictness~~ **less strict** checks. These **pass** because the `context[schema]` hints that a mailing ID *will be available* when needed.
…x (dev/mail#20) Flexmailer overrides the `Mailing.preview` API. In civicrm/civicrm-core#12509 (5.6), it became possible to call `Mailing.preview` without an `id` for the mailing. This update Flexmailer `Mailing.preview` to also work without an ID, and it expands unit-test coverage to ensure correct operation with or without an ID.
…essor/Flexmailer Overview -------- When using `TokenProcessor` to generate a mailing (e.g. as with Flexmailer/Mosaico), the action-tokens (e.g. `{action.optOutUrl}`) are generated via `CRM_Mailing_ActionTokens`. To properly generate them, `CRM_Mailing_ActionTokens` relies on certain information (e.g. mailing/job ID). However, that information is no longer available when performing a "Preview" -- leading to misbehavior in previews. This patch allows Flexmailer to restore parity for previewing those tokens. Before (Pre-5.6) ---------------- * When a user begins composing a mailing, CiviMail creates a draft mailing with a concrete ID (e.g. `mailing civicrm#123`). * To preview the mailing, the UI calls `Mailing.preview` API with the ID of the mailing. * Flexmailer/Mosaico generates the preview by calling `TokenProcessor` and therefore `CRM_Mailing_ActionTokens`. * `CRM_Mailing_ActionTokens` has strictness checks. These pass because the ID is available. Before (5.6-5.12) ---------------- As a performance enhancement, CiviCRM 5.6 (PR civicrm#12509; [dev/mail#20](https://lab.civicrm.org/dev/mail/issues/20)) revised the signature for `Mailing.preview` API to allow previews *without* having a specific mailing record/job/ID. Consequently: * When a user begins composing a mailing, CiviMail creates a draft mailing with a concrete ID (e.g. `mailing civicrm#123`). * To preview the mailing, the UI calls `Mailing.preview` API ~~with~~ **without** the ID of the mailing. * Flexmailer/Mosaico generates the preview by calling `TokenProcessor` and therefore `CRM_Mailing_ActionTokens`. * `CRM_Mailing_ActionTokens` has strictness checks. These ~~pass~~ **fail** because the ID is ~~available~~ **unavailable**. After ---------------- * When a user begins composing a mailing, CiviMail creates a draft mailing with a concrete ID (e.g. `mailing civicrm#123`). * To preview the mailing, the UI calls `Mailing.preview` API ~~with~~ **without** the ID of the mailing. * Flexmailer/Mosaico generates the preview by calling `TokenProcessor` and therefore `CRM_Mailing_ActionTokens`. * `CRM_Mailing_ActionTokens` has ~~strictness~~ **less strict** checks. These **pass** because the `context[schema]` hints that a mailing ID *will be available* when needed.
Overview
To replicate the issue:
Before
The preview window will not open until the recipient count query has completed. This can also be replicated by setting up a mailing with a large/complex group, saving it, then continuing it and immediately clicking preview. the window will not open until the query completes.
After
Preview action doesn't wait for recipient built to complete and opens the preview window instantly.
Technical Details
As per the change it directly calls API and doesn't rely on Promise queue to deliver the result in sequence.