civicrm / org.civicrm.flexmailer

FlexMailer is now part of civicrm-core. Please see the link below for how to upgrade.
https://civicrm.org/extensions/flexmailer
Other
5 stars 22 forks source link

Syntax Error when view mailing in browser #29

Closed sunilpawar closed 5 years ago

sunilpawar commented 5 years ago

DB syntax error occurred when mailing is views by anonymous user.

'view public CiviMail content' permission granted to anonymous role, user no longer need to access to open email in browser. (e.g. View in your browser => domainname/civicrm/mailing/view?reset=1&id=255) we have page_callback replacement using this extension. which required contact id.

since, its anonymous user. No contact id present and get DB error.

We are using civicrm 4.7.31 version

davidjosephhayes commented 5 years ago

View in Browser was working alpha5.

jmcclelland commented 5 years ago

Here is the full callback:

Feb 12 11:17:48  [info] $Fatal Error Details = Array
(
    [callback] => Array
        (
            [0] => CRM_Core_Error
            [1] => exceptionHandler
        )

    [code] => -2
    [message] => DB Error: syntax error
    [mode] => 16
    [debug_info] => SELECT id, display_name FROM civicrm_contact WHERE id in () [nativecode=1064 ** You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near ')' at line 1]
    [type] => DB_Error
    [user_info] => SELECT id, display_name FROM civicrm_contact WHERE id in () [nativecode=1064 ** You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near ')' at line 1]
    [to_string] => [db_error: message="DB Error: syntax error" code=-2 mode=callback callback=CRM_Core_Error::exceptionHandler prefix="" info="SELECT id, display_name FROM civicrm_contact WHERE id in () [nativecode=1064 ** You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near ')' at line 1]"]
)

Feb 12 11:17:48  [info] $backTrace = #0 /var/www/powerbase/sites/all/modules/civicrm/CRM/Core/Error.php(948): CRM_Core_Error::backtrace("backTrace", TRUE)
#1 /var/www/powerbase/sites/all/modules/civicrm/packages/PEAR.php(921): CRM_Core_Error::exceptionHandler(Object(DB_Error))
#2 /var/www/powerbase/sites/all/modules/civicrm/packages/DB.php(985): PEAR_Error->__construct("DB Error: syntax error", -2, 16, (Array:2), "SELECT id, display_name FROM civicrm_contact WHERE id in () [nativecode=1064 ...")
#3 /var/www/powerbase/sites/all/modules/civicrm/packages/PEAR.php(575): DB_Error->__construct(-2, 16, (Array:2), "SELECT id, display_name FROM civicrm_contact WHERE id in () [nativecode=1064 ...")
#4 /var/www/powerbase/sites/all/modules/civicrm/packages/PEAR.php(223): PEAR->_raiseError(Object(DB_mysqli), NULL, -2, 16, (Array:2), "SELECT id, display_name FROM civicrm_contact WHERE id in () [nativecode=1064 ...", "DB_Error", TRUE)
#5 /var/www/powerbase/sites/all/modules/civicrm/packages/DB/common.php(1907): PEAR->__call("raiseError", (Array:7))
#6 /var/www/powerbase/sites/all/modules/civicrm/packages/DB/mysqli.php(933): DB_common->raiseError(-2, NULL, NULL, "SELECT id, display_name FROM civicrm_contact WHERE id in () [nativecode=1064 ...", "1064 ** You have an error in your SQL syntax; check the manual that correspon...")
#7 /var/www/powerbase/sites/all/modules/civicrm/packages/DB/mysqli.php(403): DB_mysqli->mysqliRaiseError()
#8 /var/www/powerbase/sites/all/modules/civicrm/packages/DB/common.php(1216): DB_mysqli->simpleQuery("SELECT id, display_name FROM civicrm_contact WHERE id in ()")
#9 /var/www/powerbase/sites/all/modules/civicrm/packages/DB/DataObject.php(2415): DB_common->query("SELECT id, display_name FROM civicrm_contact WHERE id in ()")
#10 /var/www/powerbase/sites/all/modules/civicrm/packages/DB/DataObject.php(1607): DB_DataObject->_query("SELECT id, display_name FROM civicrm_contact WHERE id in ()")
#11 /var/www/powerbase/sites/all/modules/civicrm/CRM/Core/DAO.php(438): DB_DataObject->query("SELECT id, display_name FROM civicrm_contact WHERE id in ()")
#12 /var/www/powerbase/sites/all/modules/civicrm/CRM/Core/DAO.php(1411): CRM_Core_DAO->query("SELECT id, display_name FROM civicrm_contact WHERE id in ()", TRUE)
#13 /var/www/powerbase/sites/all/extensions/flexmailer/src/Listener/ToHeader.php(80): CRM_Core_DAO::executeQuery("SELECT id, display_name FROM civicrm_contact WHERE id in ()")
#14 /var/www/powerbase/sites/all/extensions/flexmailer/src/Listener/ToHeader.php(44): Civi\FlexMailer\Listener\ToHeader->getContactNames((Array:1))
#15 /var/www/powerbase/sites/all/modules/civicrm/vendor/symfony/event-dispatcher/EventDispatcher.php(184): Civi\FlexMailer\Listener\ToHeader->onCompose(Object(Civi\FlexMailer\Event\ComposeBatchEvent), "civi.flexmailer.compose", Object(Civi\Core\CiviEventDispatcher))
#16 /var/www/powerbase/sites/all/modules/civicrm/vendor/symfony/event-dispatcher/EventDispatcher.php(46): Symfony\Component\EventDispatcher\EventDispatcher->doDispatch((Array:10), "civi.flexmailer.compose", Object(Civi\FlexMailer\Event\ComposeBatchEvent))
#17 /var/www/powerbase/sites/all/modules/civicrm/Civi/Core/CiviEventDispatcher.php(47): Symfony\Component\EventDispatcher\EventDispatcher->dispatch("civi.flexmailer.compose", Object(Civi\FlexMailer\Event\ComposeBatchEvent))
#18 /var/www/powerbase/sites/all/extensions/flexmailer/src/FlexMailer.php(238): Civi\Core\CiviEventDispatcher->dispatch("civi.flexmailer.compose", Object(Civi\FlexMailer\Event\ComposeBatchEvent))
#19 /var/www/powerbase/sites/all/extensions/flexmailer/src/API/MailingPreview.php(57): Civi\FlexMailer\FlexMailer->fireComposeBatch((Array:1))
#20 [internal function](): Civi\FlexMailer\API\MailingPreview::preview((Array:7))
#21 /var/www/powerbase/sites/all/modules/civicrm/Civi/API/Provider/AdhocProvider.php(131): call_user_func("\Civi\FlexMailer\API\MailingPreview::preview", (Array:7))
#22 /var/www/powerbase/sites/all/modules/civicrm/Civi/API/Kernel.php(169): Civi\API\Provider\AdhocProvider->invoke((Array:7))
#23 /var/www/powerbase/sites/all/modules/civicrm/Civi/API/Kernel.php(100): Civi\API\Kernel->runRequest((Array:7))
#24 /var/www/powerbase/sites/all/modules/civicrm/api/api.php(43): Civi\API\Kernel->runSafe("Mailing", "preview", (Array:2))
#25 /var/www/powerbase/sites/all/extensions/flexmailer/src/Page/ViewMailing.php(43): civicrm_api3("Mailing", "preview", (Array:2))
#26 /var/www/powerbase/sites/all/modules/civicrm/CRM/Core/Invoke.php(309): Civi\FlexMailer\Page\ViewMailing->run((Array:3), NULL)
#27 /var/www/powerbase/sites/all/modules/civicrm/CRM/Core/Invoke.php(84): CRM_Core_Invoke::runItem((Array:15))
#28 /var/www/powerbase/sites/all/modules/civicrm/CRM/Core/Invoke.php(52): CRM_Core_Invoke::_invoke((Array:3))
#29 /var/www/powerbase/sites/all/modules/civicrm/drupal/civicrm.module(445): CRM_Core_Invoke::invoke((Array:3))
#30 /var/www/powerbase/includes/menu.inc(527): civicrm_invoke("mailing", "view")
#31 /var/www/powerbase/index.php(21): menu_execute_active_handler()
#32 {main}

Feb 12 11:17:48  [info] 
$Fatal Error Details = array(3) {
  ["message"]=>
  string(22) "DB Error: syntax error"
  ["code"]=>
  NULL
  ["exception"]=>
  object(CiviCRM_API3_Exception)#342 (8) {
    ["extraParams":"CiviCRM_API3_Exception":private]=>
    array(4) {
      ["error_code"]=>
      string(12) "syntax error"
      ["tip"]=>
      string(62) "add debug=1 to your API call to have more info about the error"
      ["is_error"]=>
      int(1)
      ["error_message"]=>
      string(22) "DB Error: syntax error"
    }
    ["message":protected]=>
    string(22) "DB Error: syntax error"
    ["string":"Exception":private]=>
    string(0) ""
    ["code":protected]=>
    int(0)
    ["file":protected]=>
    string(56) "/var/www/powerbase/sites/all/modules/civicrm/api/api.php"
    ["line":protected]=>
    int(45)
    ["trace":"Exception":private]=>
    array(7) {
      [0]=>
      array(4) {
        ["file"]=>
        string(75) "/var/www/powerbase/sites/all/extensions/flexmailer/src/Page/ViewMailing.php"
        ["line"]=>
        int(43)
        ["function"]=>
        string(12) "civicrm_api3"
        ["args"]=>
        array(3) {
          [0]=>
          string(7) "Mailing"
          [1]=>
          string(7) "preview"
          [2]=>
          array(2) {
            ["id"]=>
            string(3) "189"
            ["version"]=>
            int(3)
          }
        }
      }
      [1]=>
      array(6) {
        ["file"]=>
        string(64) "/var/www/powerbase/sites/all/modules/civicrm/CRM/Core/Invoke.php"
        ["line"]=>
        int(309)
        ["function"]=>
        string(3) "run"
        ["class"]=>
        string(32) "Civi\FlexMailer\Page\ViewMailing"
        ["type"]=>
        string(2) "->"
        ["args"]=>
        array(2) {
          [0]=>
          array(3) {
            [0]=>
            string(7) "civicrm"
            [1]=>
            string(7) "mailing"
            [2]=>
            string(4) "view"
          }
          [1]=>
          NULL
        }
      }
      [2]=>
      array(6) {
        ["file"]=>
        string(64) "/var/www/powerbase/sites/all/modules/civicrm/CRM/Core/Invoke.php"
        ["line"]=>
        int(84)
        ["function"]=>
        string(7) "runItem"
        ["class"]=>
        string(15) "CRM_Core_Invoke"
        ["type"]=>
        string(2) "::"
        ["args"]=>
        array(1) {
          [0]=>
          &array(15) {
            ["id"]=>
            string(3) "376"
            ["domain_id"]=>
            string(1) "1"
            ["path"]=>
            string(20) "civicrm/mailing/view"
            ["title"]=>
            string(12) "View Mailing"
            ["access_callback"]=>
            array(2) {
              [0]=>
              string(19) "CRM_Core_Permission"
              [1]=>
              string(9) "checkMenu"
            }
            ["access_arguments"]=>
            array(2) {
              [0]=>
              array(3) {
                [0]=>
                string(28) "view public CiviMail content"
                [1]=>
                string(15) "access CiviMail"
                [2]=>
                string(16) "approve mailings"
              }
              [1]=>
              string(2) "or"
            }
            ["page_callback"]=>
            string(33) "\Civi\FlexMailer\Page\ViewMailing"
            ["breadcrumb"]=>
            array(2) {
              [0]=>
              array(2) {
                ["title"]=>
                string(7) "CiviCRM"
                ["url"]=>
                string(16) "/civicrm?reset=1"
              }
              [1]=>
              array(2) {
                ["title"]=>
                string(8) "CiviMail"
                ["url"]=>
                string(24) "/civicrm/mailing?reset=1"
              }
            }
            ["component_id"]=>
            string(1) "4"
            ["is_public"]=>
            string(1) "1"
            ["is_ssl"]=>
            string(1) "0"
            ["weight"]=>
            string(3) "800"
            ["type"]=>
            string(1) "1"
            ["page_type"]=>
            string(1) "0"
            ["page_arguments"]=>
            bool(false)
          }
        }
      }
      [3]=>
      array(6) {
        ["file"]=>
        string(64) "/var/www/powerbase/sites/all/modules/civicrm/CRM/Core/Invoke.php"
        ["line"]=>
        int(52)
        ["function"]=>
        string(7) "_invoke"
        ["class"]=>
        string(15) "CRM_Core_Invoke"
        ["type"]=>
        string(2) "::"
        ["args"]=>
        array(1) {
          [0]=>
          array(3) {
            [0]=>
            string(7) "civicrm"
            [1]=>
            string(7) "mailing"
            [2]=>
            string(4) "view"
          }
        }
      }
      [4]=>
      array(6) {
        ["file"]=>
        string(66) "/var/www/powerbase/sites/all/modules/civicrm/drupal/civicrm.module"
        ["line"]=>
        int(445)
        ["function"]=>
        string(6) "invoke"
        ["class"]=>
        string(15) "CRM_Core_Invoke"
        ["type"]=>
        string(2) "::"
        ["args"]=>
        array(1) {
          [0]=>
          array(3) {
            [0]=>
            string(7) "civicrm"
            [1]=>
            string(7) "mailing"
            [2]=>
            string(4) "view"
          }
        }
      }
      [5]=>
      array(4) {
        ["file"]=>
        string(36) "/var/www/powerbase/includes/menu.inc"
        ["line"]=>
        int(527)
        ["function"]=>
        string(14) "civicrm_invoke"
        ["args"]=>
        array(2) {
          [0]=>
          string(7) "mailing"
          [1]=>
          string(4) "view"
        }
      }
      [6]=>
      array(4) {
        ["file"]=>
        string(28) "/var/www/powerbase/index.php"
        ["line"]=>
        int(21)
        ["function"]=>
        string(27) "menu_execute_active_handler"
        ["args"]=>
        array(0) {
        }
      }
    }
    ["previous":"Exception":private]=>
    NULL
  }
}

Feb 12 11:17:48  [info] $backTrace = #0 /var/www/powerbase/sites/all/modules/civicrm/CRM/Core/Error.php(459): CRM_Core_Error::backtrace("backTrace", TRUE)
#1 /var/www/powerbase/sites/all/modules/civicrm/CRM/Core/Invoke.php(55): CRM_Core_Error::handleUnhandledException(Object(CiviCRM_API3_Exception))
#2 /var/www/powerbase/sites/all/modules/civicrm/drupal/civicrm.module(445): CRM_Core_Invoke::invoke((Array:3))
#3 /var/www/powerbase/includes/menu.inc(527): civicrm_invoke("mailing", "view")
#4 /var/www/powerbase/index.php(21): menu_execute_active_handler()
#5 {main}
jmcclelland commented 5 years ago
   $ids = array();
    foreach ($tasks as $task) {
      /** @var FlexMailerTask $task */
      $ids[$task->getContactId()] = $task->getContactId();
    }

    if (empty($ids)) {
      return array();
    }

The problem is that when you are the anonymous user, $ids is set to ([] =>) - which is not empty - it's an array with one element that is empty.

jmcclelland commented 5 years ago

This fixes the immediate problem...

--- ./src/Listener/ToHeader.php 2017-09-20 12:28:07.571448838 -0400
+++ //srv/longshore/platforms/ourpowerbase-d7-c5.7/sites/all/extensions/flexmailer/src/Listener/ToHeader.php    2019-02-12 12:07:49.406972651 -0500
@@ -67,7 +67,10 @@
     $ids = array();
     foreach ($tasks as $task) {
       /** @var FlexMailerTask $task */
-      $ids[$task->getContactId()] = $task->getContactId();
+      $id = $task->getContactId();
+      if ($id) {
+        $ids[$id] = $id;
+      }
     }

     if (!$ids) {

But it still doesn't work because ultimately `Civi/Token/TokenCompatSubscriber->onEvaluate() wants there to be a contactID that is valid so the tokens can be replaced.

jmcclelland commented 5 years ago

Reverting commit 68f6b9faab7be0e3fea34c9f938c219bd3b93a78 seems to solve the problem.

sunilpawar commented 5 years ago

@jmcclelland i did try similar patch ,but it need valid contact id. For immediate fix i commented line

This will switch to default civicrm mailing view.

jmcclelland commented 5 years ago

Thanks @sunilpawar - I came to the same conclusion.

lcdservices commented 5 years ago

+1 for commenting out the alterMenu hook mod. reverting to the core civicrm mailing view works fine.

francescbassas commented 5 years ago

Same problem with 1.0-beta1

pbatroff commented 5 years ago

Yes, me too! 'Fix' works though, but a patched Extension is not ideal!

francescbassas commented 5 years ago

@jitendrapurohit Do not know if you can take a look?

Reverting commit 68f6b9f seems to solve the problem.

agilewarealok commented 5 years ago

Any particular reason we have used mailing.preview API for viewing an email? Applying below patch also works, but it is just a wrapper around MailingPageView class,

diff --git a/src/Page/ViewMailing.php b/src/Page/ViewMailing.php
index 60f9e08..7b789b5 100644
--- a/src/Page/ViewMailing.php
+++ b/src/Page/ViewMailing.php
@@ -39,23 +39,8 @@ class ViewMailing {
    */
   public function run() {
     $mailingID = \CRM_Utils_Request::retrieve('id', 'Int', \CRM_Core_DAO::$_nullObject, TRUE);
-    $result = civicrm_api3('Mailing', 'preview', [
-      'id' => $mailingID,
-    ]);
-    $mailing = \CRM_Utils_Array::value('values', $result);
-    if (isset($mailing['body_html']) && empty($_GET['text'])) {
-      $header = 'text/html; charset=utf-8';
-      $content = $mailing['body_html'];
-    }
-    else {
-      $header = 'text/plain; charset=utf-8';
-      $content = $mailing['body_text'];
-    }
-    \CRM_Utils_System::setTitle($mailing['subject']);
-    if (\CRM_Utils_Array::value('snippet', $_GET) === 'json') {
-      \CRM_Core_Page_AJAX::returnJsonResponse($content);
-    }
-    \CRM_Utils_System::setHttpHeader('Content-Type', $header);
+    $mailingView = new \CRM_Mailing_Page_View();
+    $content = $mailingView->run($mailingID);

     print $content;
     \CRM_Utils_System::civiExit();
jitendrapurohit commented 5 years ago

Can you all confirm if https://github.com/civicrm/org.civicrm.flexmailer/pull/33 fixes the problem for anonymous users?

Thanks.

agilewarealok commented 5 years ago

@jitendrapurohit No it still throws following error

CiviCRM_API3_Exception: "A fatal error was triggered: One of parameters (value: ) is not of the type Positive"
org.civicrm.flexmailer/src/Page/ViewMailing.php(43): civicrm_api3("Mailing", "preview", (Array:2))
francescbassas commented 5 years ago

I have applied the changes of #33 and the DB Syntax error described here it has been fixed. Thanks @jitendrapurohit

pradpnayak commented 5 years ago

Hi All,

We also facing similar issue pointed by @agilewarealok. Looks like Mailing.preview expects contact_id in api params to build the tokens and the code only passes mailing id. This does work for logged in user since if contact id is missing it grabs contact id of logged in user but incase of anonymous user it sets to null.

I don't see any reason why we can't use patch from here. @agilewarealok would it be possible for you to send a PR for the fix?

seamuslee001 commented 5 years ago

@agilewarealok can you see if https://github.com/civicrm/civicrm-core/pull/13956/files fixes your preview issue also ping @pradpnayak

agilewarealok commented 5 years ago

@seamuslee001 Unfortunately it does not fix the issue.

agilewarealok commented 5 years ago

@pradpnayak Created PR for the fix, https://github.com/civicrm/org.civicrm.flexmailer/pull/34

totten commented 5 years ago

Part 1 - Background

(lcdservices) +1 for commenting out the alterMenu hook mod. reverting to the core civicrm mailing view works fine.

( agilewarealok ) Any particular reason we have used mailing.preview API for viewing an email?

There is a reason why this is overridden, and the reason is still valid today.

To see this, it helps to consider the field civicrm_mailing.template_type. This field generally contains the word traditional, which means that the email-rendering should follow the, eh, traditional semantics. (Which means several things -- CIVICRM_MAIL_SMARTY is respected. CiviMail tokens are evaluated first, and then Smarty tokens are evaluated on the output. It's impossible for the Smarty compiler to meaningfully use it's compilation cache. CSS handling gets funky. Et al.) We could make the system generally easier to grok, more performant, etc by modifying those semantics. (Ex: Re-doing the Smarty integration in a Normal-And-Not-Crazy-Way and/or upgrading Smarty and/or switching to Twig or Mustache, ad nauseum.) But doing so would also break tons of existing data/workflows (e.g. live sites have message-templates and re-usable mailings which are encoded to the traditional semantics).

Changing the semantics of the email templating language requires a long transition - which means supporting both old+new semantics for a period of time. The following facilitates this:

civicrm-core's implementation of civicrm/mailing/view is hard-coded to always use traditional semantics. So if you open a mailing based on other semantics, it will be misinterpreted. That's pretty subtle when you test against mailings with template_type=mosaico (because the semantics of mosaico and traditional are broadly similar), but there would be problems. (Ex: Some installs will mishandle CSS. Ex: The token-alias notation will malfunction.) The problems would be more visceral with formats that feature more substantive differences (eg using Twig or Mustache or Pug or Markdown).

Part 2 - Resolving this bug

So... the approach #34 may run OK (r-run) in a basic case, but from my POV it's regressive (mishandling non-traditional template-types).

The approach in @jitendrapurohit's #33 seems more sound and consistent. 👍 Merged it.

Part 3 - Improving architecture

Zooming out, I agree it would be good to get rid of the alterMenu override for civicrm/mailing/view - but the code/approach in flexmailer's version is better. It uses an API which is supported by old+new delivery engines and which is amenable to unit-tests (like Civi\FlexMailer\MailingPreviewTest).

The following would be a reasonable way to get rid of the menu override:

totten commented 5 years ago

Erg, re-opening. Upstream CRM_Mailing_Page_View has good bits around contact-ID's for anonymous users, and Civi\FlexMailer\Page\ViewMailing has good bits around using APIs. Need to synthesize these better for a proper fix...

(Update for clarification)

At time of writing, the fix in #33 resolves the crash if flexmailer has substantive responsibility for rendering. (Which would be true of mosaico mailings, and it would be true if you customize the setting flexmailer_traditional.) However, the default behavior for traditional mailings is to delegate back to core via civicrm_api3_mailing_preview(). The proximate issue is that this API seems to be crashing in absence of $params['contact_id'].... but this is really just a symptom that flexmailer's implementation civicrm/mailing/view doesn't handle several identity+permission use-cases that are in upstream's implementation.

Which brings us back to the earlier point -- that it would be best to make the civicrm/mailing/view override unnecessary. That requires combining some good bits from CRM_Mailing_Page_View and Civi\FlexMailer\Page\ViewMailing.

Upperholme commented 5 years ago

Not sure why this issue is marked as closed? I've just updated to the latest Flexmailer release and I'm still unable to view the mailing unless logged in.

Upperholme commented 5 years ago

Need to be using not only up to date releases of Flexmailer and Mosaico, but also CiviCRM, and then the issue is resolved for me at least. Thanks.