From c09ac162c3b024ffba517a4922898eac456a04e0 Mon Sep 17 00:00:00 2001 From: Ivan Bochkarev Date: Wed, 29 Jul 2026 16:32:52 +0600 Subject: [PATCH] fix(mgr): restore newsletter grid after create (#114) Drop JOIN/subquery SQL from newsletter getlist; enrich rows in prepareRow so MySQL cannot break the mgr grid. Align getlist/get ACL with view_sendex, defer grid refresh after create, and return strict true from beforeSet on MODX 3. --- _build/build.config.php | 2 +- .../sendex/js/mgr/widgets/newsletters.grid.js | 5 +- core/components/sendex/docs/changelog.txt | 5 ++ .../sendex/sxnewsletterlistquery.class.php | 39 ++++++------ .../mgr/newsletter/create.class.php | 16 ++--- .../processors/mgr/newsletter/get.class.php | 9 +++ .../mgr/newsletter/getlist.class.php | 21 ++++--- tests/Unit/NewsletterGetListQueryTest.php | 63 ++++++++++++++----- 8 files changed, 104 insertions(+), 56 deletions(-) diff --git a/_build/build.config.php b/_build/build.config.php index 68ca801..dba7212 100644 --- a/_build/build.config.php +++ b/_build/build.config.php @@ -4,7 +4,7 @@ define('PKG_NAME', 'Sendex'); define('PKG_NAME_LOWER', strtolower(PKG_NAME)); -define('PKG_VERSION', '2.0.1'); +define('PKG_VERSION', '2.0.2'); define('PKG_RELEASE', 'pl'); define('PKG_AUTO_INSTALL', true); define('PKG_NAMESPACE_PATH', '{core_path}components/' . PKG_NAME_LOWER . '/'); diff --git a/assets/components/sendex/js/mgr/widgets/newsletters.grid.js b/assets/components/sendex/js/mgr/widgets/newsletters.grid.js index e464be9..0e8cbe2 100644 --- a/assets/components/sendex/js/mgr/widgets/newsletters.grid.js +++ b/assets/components/sendex/js/mgr/widgets/newsletters.grid.js @@ -118,7 +118,10 @@ Ext.extend(Sendex.grid.Newsletters,MODx.grid.Grid,Ext.apply({ this.windows.createNewsletter = MODx.load({ xtype: 'sendex-window-newsletter-create' ,listeners: { - 'success': {fn:function() { this.refresh(); },scope:this} + 'success': {fn:function() { + var grid = this; + window.setTimeout(function() { grid.refresh(); }, 50); + },scope:this} } }); } diff --git a/core/components/sendex/docs/changelog.txt b/core/components/sendex/docs/changelog.txt index 7e33989..8756c03 100644 --- a/core/components/sendex/docs/changelog.txt +++ b/core/components/sendex/docs/changelog.txt @@ -5,6 +5,11 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [2.0.2-pl] - 2026-07-29 + +### Fixed +- [#114] Mgr newsletter create still showed endless «Загружается…» after 2.0.1: `getlist` no longer uses JOIN/subquery SQL (subscriber count and template name are added in `prepareRow`); grid refresh after save is deferred so the create window can close first; `getlist`/`get` accept `view_sendex` as well as `view_document`; create `beforeSet()` returns strict `true` for MODX 3. + ## [2.0.1-pl] - 2026-07-29 ### Fixed diff --git a/core/components/sendex/model/sendex/sxnewsletterlistquery.class.php b/core/components/sendex/model/sendex/sxnewsletterlistquery.class.php index 02ff0ee..bc3baaf 100644 --- a/core/components/sendex/model/sendex/sxnewsletterlistquery.class.php +++ b/core/components/sendex/model/sendex/sxnewsletterlistquery.class.php @@ -1,7 +1,8 @@ leftJoin('modTemplate', 'Template'); - $query->select($modx->getSelectColumns($classKey, $classKey)); - $query->select($modx->getSelectColumns('modTemplate', 'Template', '', array('templatename'))); + $newsletterId = isset($array['id']) ? (int) $array['id'] : 0; + $array['subscribers'] = $newsletterId > 0 + ? (int) $modx->getCount('sxSubscriber', array('newsletter_id' => $newsletterId)) + : 0; - $subscriberTable = $modx->getTableName('sxSubscriber'); - $query->select(sprintf( - '(SELECT COUNT(*) FROM %s AS `sxSubCnt` WHERE `sxSubCnt`.`newsletter_id` = %s.id) AS `subscribers`', - $subscriberTable, - $classKey - )); + $array['templatename'] = ''; + $templateId = isset($array['template']) ? (int) $array['template'] : 0; + if ($templateId > 0) { + $template = $modx->getObject('modTemplate', $templateId); + if ($template) { + $array['templatename'] = (string) $template->get('templatename'); + } + } - return $query; + return $array; } } diff --git a/core/components/sendex/processors/mgr/newsletter/create.class.php b/core/components/sendex/processors/mgr/newsletter/create.class.php index dd8b40f..b0f3b61 100644 --- a/core/components/sendex/processors/mgr/newsletter/create.class.php +++ b/core/components/sendex/processors/mgr/newsletter/create.class.php @@ -39,20 +39,14 @@ public function beforeSet() } } + if ($this->hasErrors()) { + return false; + } + $active = $this->getProperty('active'); $this->setProperty('active', !empty($active) && $active != 'false'); - return !$this->hasErrors(); - } - - /** - * Return a plain array so ExtJS always gets JSON it can parse (MODX 3). - * - * @return array - */ - public function cleanup() - { - return $this->success('', $this->object->toArray()); + return true; } } diff --git a/core/components/sendex/processors/mgr/newsletter/get.class.php b/core/components/sendex/processors/mgr/newsletter/get.class.php index 1664857..92753c1 100644 --- a/core/components/sendex/processors/mgr/newsletter/get.class.php +++ b/core/components/sendex/processors/mgr/newsletter/get.class.php @@ -10,6 +10,15 @@ class sxNewsletterGetProcessor extends modObjectGetProcessor public $classKey = 'sxNewsletter'; public $languageTopics = array('sendex:default'); public $permission = 'view_document'; + + /** + * @return bool + */ + public function checkPermissions() + { + return $this->modx->hasPermission('view_sendex') + || $this->modx->hasPermission('view_document'); + } } return 'sxNewsletterGetProcessor'; diff --git a/core/components/sendex/processors/mgr/newsletter/getlist.class.php b/core/components/sendex/processors/mgr/newsletter/getlist.class.php index a7b0610..c70d176 100644 --- a/core/components/sendex/processors/mgr/newsletter/getlist.class.php +++ b/core/components/sendex/processors/mgr/newsletter/getlist.class.php @@ -14,16 +14,14 @@ class sxNewsletterGetListProcessor extends modObjectGetListProcessor public $permission = 'view_document'; /** - * @param xPDOQuery $c + * Match mgr controller ACL: Sendex menu may grant view_sendex without view_document. * - * @return xPDOQuery + * @return bool */ - public function prepareQueryBeforeCount(xPDOQuery $c) + public function checkPermissions() { - return sxNewsletterListQuery::applyFilters($c, array( - 'query' => $this->getProperty('query'), - 'combo' => $this->getProperty('combo'), - )); + return $this->modx->hasPermission('view_sendex') + || $this->modx->hasPermission('view_document'); } /** @@ -31,9 +29,12 @@ public function prepareQueryBeforeCount(xPDOQuery $c) * * @return xPDOQuery */ - public function prepareQueryAfterCount(xPDOQuery $c) + public function prepareQueryBeforeCount(xPDOQuery $c) { - return sxNewsletterListQuery::applyListSelects($this->modx, $c, $this->classKey); + return sxNewsletterListQuery::applyFilters($c, array( + 'query' => $this->getProperty('query'), + 'combo' => $this->getProperty('combo'), + )); } /** @@ -43,7 +44,7 @@ public function prepareQueryAfterCount(xPDOQuery $c) */ public function prepareRow(xPDOObject $object) { - $array = $object->toArray(); + $array = sxNewsletterListQuery::enrichRow($this->modx, $object->toArray()); $array['actions'] = array(); // Update diff --git a/tests/Unit/NewsletterGetListQueryTest.php b/tests/Unit/NewsletterGetListQueryTest.php index 4cd583a..645a3c6 100644 --- a/tests/Unit/NewsletterGetListQueryTest.php +++ b/tests/Unit/NewsletterGetListQueryTest.php @@ -29,34 +29,69 @@ public function testApplyFiltersDoesNotJoinOrGroup() $this->assertSame(1, $query->where['active']); } - public function testApplyListSelectsAddsTemplateJoinAndSubscriberSubquery() + public function testEnrichRowAddsSubscriberCountAndTemplateName() { - $query = new FakeQuery('sxNewsletter'); + $this->modx->newsletters[] = new sxNewsletter($this->modx); + $this->modx->newsletters[0]->fromArray(array( + 'id' => 5, + 'name' => 'Weekly', + 'template' => 2, + )); - sxNewsletterListQuery::applyListSelects($this->modx, $query, 'sxNewsletter'); + $subscriber = new sxSubscriber($this->modx); + $subscriber->fromArray(array('id' => 1, 'newsletter_id' => 5)); + $this->modx->subscribers[] = $subscriber; - $this->assertCount(1, $query->joins); - $this->assertSame('modTemplate', $query->joins[0][0]); - $this->assertNull($query->groupby); - $this->assertGreaterThanOrEqual(3, count($query->selects)); - $subscriberSelect = end($query->selects); - $this->assertStringContainsString('SELECT COUNT(*)', $subscriberSelect); - $this->assertStringContainsString('newsletter_id', $subscriberSelect); - $this->assertStringNotContainsString('GROUP BY', strtoupper(implode(' ', $query->selects))); + $subscriber = new sxSubscriber($this->modx); + $subscriber->fromArray(array('id' => 2, 'newsletter_id' => 5)); + $this->modx->subscribers[] = $subscriber; + + $template = new class { + /** @var array */ + private $data = array('templatename' => 'Mail layout'); + + /** + * @param string $key + * @return string|null + */ + public function get($key) + { + return isset($this->data[$key]) ? $this->data[$key] : null; + } + }; + $this->modx->templates[2] = $template; + + $row = sxNewsletterListQuery::enrichRow($this->modx, array( + 'id' => 5, + 'template' => 2, + )); + + $this->assertSame(2, $row['subscribers']); + $this->assertSame('Mail layout', $row['templatename']); } - public function testGetListProcessorUsesAfterCountForAggregates() + public function testGetListProcessorUsesFiltersOnlyBeforeCount() { $source = file_get_contents( dirname(__DIR__, 2) . '/core/components/sendex/processors/mgr/newsletter/getlist.class.php' ); - $this->assertStringContainsString('prepareQueryAfterCount', $source); - $this->assertStringContainsString('sxNewsletterListQuery::applyListSelects', $source); $this->assertStringContainsString('sxNewsletterListQuery::applyFilters', $source); + $this->assertStringContainsString('sxNewsletterListQuery::enrichRow', $source); + $this->assertStringNotContainsString('prepareQueryAfterCount', $source); $this->assertStringNotContainsString('groupby', $this->beforeCountBody($source)); } + public function testGetListProcessorAllowsViewSendexPermission() + { + $source = file_get_contents( + dirname(__DIR__, 2) . '/core/components/sendex/processors/mgr/newsletter/getlist.class.php' + ); + + $this->assertStringContainsString("hasPermission('view_sendex')", $source); + $this->assertStringContainsString("hasPermission('view_document')", $source); + } + /** * @param string $source * @return string