diff --git a/dist/feca_mailshots_plugin-1.1.11.zip b/dist/feca_mailshots_plugin-1.1.11.zip new file mode 100644 index 0000000..6c77957 Binary files /dev/null and b/dist/feca_mailshots_plugin-1.1.11.zip differ diff --git a/feca_mailshots_plugin/feca_mailshots_plugin.php b/feca_mailshots_plugin/feca_mailshots_plugin.php index 5630078..8cd9918 100644 --- a/feca_mailshots_plugin/feca_mailshots_plugin.php +++ b/feca_mailshots_plugin/feca_mailshots_plugin.php @@ -3,7 +3,7 @@ * Plugin Name: FECA Mailshots * Plugin URI: https://fenedge.co.uk/ * Description: FECA mailshots plugin. - * Version: 1.1.10 + * Version: 1.1.11 * Requires at least: 6.0 * Requires PHP: 7.4 * Author: FECA diff --git a/feca_mailshots_plugin/src/Admin/DataSourcesAdminPage.php b/feca_mailshots_plugin/src/Admin/DataSourcesAdminPage.php index 065546b..2633784 100644 --- a/feca_mailshots_plugin/src/Admin/DataSourcesAdminPage.php +++ b/feca_mailshots_plugin/src/Admin/DataSourcesAdminPage.php @@ -322,7 +322,7 @@ final class DataSourcesAdminPage echo 'var isDirty=false;'; echo 'function confirmDiscard(){if(!isDirty){return true;}return window.confirm("You have unsaved changes. Close without saving?");}'; echo 'if(editorForm){editorForm.querySelectorAll("input,select,textarea").forEach(function(el){el.addEventListener("input",function(){isDirty=true;});el.addEventListener("change",function(){isDirty=true;});});editorForm.addEventListener("submit",function(){isDirty=false;});}'; - echo 'var filters=[["selected-renewal","Renewal is selected","none"],["pending-renewal","Renewal is pending","none"],["primary-contact","Contact is primary","none"],["fen1-contact","Contact is FEN1","none"],["member-or-affiliate-or-parish-council","Account is member/affiliate/parish council","none"],["account-has-article-in-issue","Account has article in issue","issue"],["selected","Advertiser is selected","none"],["issue","Issue is","issue"],["pending-invoice","Invoice is pending","none"],["selected-invoice","Invoice is selected","none"],["invoice-ids","Invoice ID is one of","ids"]];'; + echo 'var filters=[["selected-renewal","Renewal is selected","none"],["pending-renewal","Renewal is pending","none"],["primary-contact","Contact is primary","none"],["fen1-contact","Contact is FEN1","none"],["member-account","Account is member","none"],["affiliate-account","Account is affiliate","none"],["member-or-affiliate-or-parish-council","Account is member/affiliate/parish council","none"],["account-has-article-in-issue","Account has article in issue","issue"],["selected","Advertiser is selected","none"],["issue","Issue is","issue"],["pending-invoice","Invoice is pending","none"],["selected-invoice","Invoice is selected","none"],["invoice-ids","Invoice ID is one of","ids"]];'; echo 'function selectedSources(){var selected={};builtChecks.forEach(function(c){if(c.checked){selected[c.value]=true;}});customSources.forEach(function(v){selected[v]=true;});var s=[];sourceOrder.forEach(function(src){if(selected[src]&&s.indexOf(src)===-1){s.push(src);}});builtChecks.forEach(function(c){if(c.checked&&s.indexOf(c.value)===-1){s.push(c.value);}});customSources.forEach(function(v){if(s.indexOf(v)===-1){s.push(v);}});return s;}'; echo 'function noteSourceSelected(src){if(src&&sourceOrder.indexOf(src)===-1){sourceOrder.push(src);}}'; echo 'function noteSourceDeselected(src){sourceOrder=sourceOrder.filter(function(v){return v!==src;});}'; diff --git a/feca_mailshots_plugin/src/Application/DslCompiler.php b/feca_mailshots_plugin/src/Application/DslCompiler.php index fd07724..286ccc2 100644 --- a/feca_mailshots_plugin/src/Application/DslCompiler.php +++ b/feca_mailshots_plugin/src/Application/DslCompiler.php @@ -186,6 +186,12 @@ final class DslCompiler if ($name === 'primary-contact') { return $this->alias('contacts') . '.`is_contact_1` = 1'; } + if ($name === 'member-account') { + return $this->compileAccountTypeSlugFilter('member'); + } + if ($name === 'affiliate-account') { + return $this->compileAccountTypeSlugFilter('affiliate'); + } if ($name === 'selected') { return 'COALESCE(' . $this->alias('advertisers') . '.`Selected`, 0) <> 0'; } @@ -220,6 +226,11 @@ final class DslCompiler throw new AppError('dsl_compile', 'Unknown filter', ['filter' => $name]); } + private function compileAccountTypeSlugFilter(string $slug): string + { + return $this->alias('accounts') . '.`account_type_id` IN (SELECT id FROM `picklist_account_type` WHERE LOWER(TRIM(COALESCE(`slug`, \'\'))) = \'' . $slug . '\')'; + } + /** @return array{string,string} */ private function rewriteJoinRef(string $left, string $right): array { diff --git a/feca_mailshots_plugin/src/Application/DslValidator.php b/feca_mailshots_plugin/src/Application/DslValidator.php index c389a1f..537756b 100644 --- a/feca_mailshots_plugin/src/Application/DslValidator.php +++ b/feca_mailshots_plugin/src/Application/DslValidator.php @@ -16,6 +16,8 @@ final class DslValidator 'pending-renewal' => ['renewals'], 'primary-contact' => ['contacts'], 'fen1-contact' => ['contacts'], + 'member-account' => ['accounts'], + 'affiliate-account' => ['accounts'], 'selected' => ['advertisers'], 'issue' => ['advertisers|ads|pages|articles|issues|invoices'], 'pending-invoice' => ['invoices'], @@ -156,7 +158,7 @@ final class DslValidator { $name = (string) ($predicate['name'] ?? ''); $args = is_array($predicate['args'] ?? null) ? $predicate['args'] : []; - if (in_array($name, ['selected-renewal', 'pending-renewal', 'primary-contact', 'fen1-contact', 'selected', 'pending-invoice', 'selected-invoice', 'member-or-affiliate-or-parish-council'], true)) { + if (in_array($name, ['selected-renewal', 'pending-renewal', 'primary-contact', 'fen1-contact', 'member-account', 'affiliate-account', 'selected', 'pending-invoice', 'selected-invoice', 'member-or-affiliate-or-parish-council'], true)) { if ($args !== []) { $errors[] = sprintf('Filter %s does not take arguments', $name); } diff --git a/feca_mailshots_plugin/src/Domain/DslParser.php b/feca_mailshots_plugin/src/Domain/DslParser.php index bd43281..56291dd 100644 --- a/feca_mailshots_plugin/src/Domain/DslParser.php +++ b/feca_mailshots_plugin/src/Domain/DslParser.php @@ -264,6 +264,8 @@ final class DslParser 'selected', 'primary-contact', 'fen1-contact', + 'member-account', + 'affiliate-account', 'pending-invoice', 'selected-invoice', 'member-or-affiliate-or-parish-council', diff --git a/feca_mailshots_plugin/src/Repository/AttachmentRepository.php b/feca_mailshots_plugin/src/Repository/AttachmentRepository.php index 2cccdab..c2525a1 100644 --- a/feca_mailshots_plugin/src/Repository/AttachmentRepository.php +++ b/feca_mailshots_plugin/src/Repository/AttachmentRepository.php @@ -69,7 +69,7 @@ final class AttachmentRepository if ($name === '') { return null; } - $sql = 'SELECT id, name, file_name, mime_type, TO_BASE64(file_bytes) AS file_bytes_b64, created_at, updated_at + $sql = 'SELECT id, name, file_name, mime_type, file_bytes, created_at, updated_at FROM mailshot_attachments WHERE LOWER(name) = LOWER(:name) LIMIT 1'; @@ -79,8 +79,7 @@ final class AttachmentRepository if ($row === false) { return null; } - $row['file_bytes'] = $this->decodeBase64Blob((string) ($row['file_bytes_b64'] ?? '')); - unset($row['file_bytes_b64']); + $row['file_bytes'] = (string) ($row['file_bytes'] ?? ''); $mimeType = trim((string) ($row['mime_type'] ?? '')); if ($mimeType === '') { $mimeType = $this->inferMimeTypeFromFilename((string) ($row['file_name'] ?? '')); @@ -92,18 +91,6 @@ final class AttachmentRepository return $row; } - private function decodeBase64Blob(string $value): string - { - if ($value === '') { - return ''; - } - $decoded = base64_decode($value, true); - if (!is_string($decoded)) { - throw new \RuntimeException('Failed to decode attachment blob.'); - } - return $decoded; - } - private function updateMimeType(int $id, string $mimeType): void { if ($id <= 0 || trim($mimeType) === '') { diff --git a/feca_mailshots_plugin/src/Repository/PdfAssetRepository.php b/feca_mailshots_plugin/src/Repository/PdfAssetRepository.php index 6b764ae..001827a 100644 --- a/feca_mailshots_plugin/src/Repository/PdfAssetRepository.php +++ b/feca_mailshots_plugin/src/Repository/PdfAssetRepository.php @@ -30,7 +30,7 @@ final class PdfAssetRepository if ($name === '') { return null; } - $sql = 'SELECT id, name, file_name, mime_type, TO_BASE64(file_bytes) AS file_bytes_b64, width_mm, height_mm, justification + $sql = 'SELECT id, name, file_name, mime_type, file_bytes, width_mm, height_mm, justification FROM mailshot_pdf_assets WHERE LOWER(name) = LOWER(:name) LIMIT 1'; @@ -40,8 +40,7 @@ final class PdfAssetRepository if ($row === false) { return null; } - $row['file_bytes'] = $this->decodeBase64Blob((string) ($row['file_bytes_b64'] ?? '')); - unset($row['file_bytes_b64']); + $row['file_bytes'] = (string) ($row['file_bytes'] ?? ''); return $row; } @@ -83,15 +82,4 @@ final class PdfAssetRepository $stmt->execute(['id' => $id]); } - private function decodeBase64Blob(string $value): string - { - if ($value === '') { - return ''; - } - $decoded = base64_decode($value, true); - if (!is_string($decoded)) { - throw new \RuntimeException('Failed to decode PDF asset blob.'); - } - return $decoded; - } } diff --git a/requirements/mailshot_data_source.md b/requirements/mailshot_data_source.md index 850f9fd..3ac3559 100644 --- a/requirements/mailshot_data_source.md +++ b/requirements/mailshot_data_source.md @@ -95,6 +95,8 @@ filter_name = "selected-renewal" | "invoice-ids" | "fen1-contact" | "primary-contact" + | "member-account" + | "affiliate-account" | "member-or-affiliate-or-parish-council" ; field_ref = source_ref , "." , field_name ; @@ -135,11 +137,13 @@ digit = "0"…"9" ; * `where` applies after source composition. * `not` negates only the next predicate/group. * Comparison operators (`=`, `!=`, `contains`, `starts-with`, and `ends-with`) treat a database `NULL` field value as an empty string. This makes a negated comparison the complete logical inverse of its positive form; for example, `not (source.field contains 'text')` includes rows where `source.field` is `NULL`. -* Predefined predicates are `selected-renewal`, `pending-renewal`, `selected`, `issue`, `pending-invoice`, `selected-invoice`, `invoice-ids`, `fen1-contact`, `primary-contact`, `member-or-affiliate-or-parish-council`, and `account-has-article-in-issue`. +* Predefined predicates are `selected-renewal`, `pending-renewal`, `selected`, `issue`, `pending-invoice`, `selected-invoice`, `invoice-ids`, `fen1-contact`, `primary-contact`, `member-account`, `affiliate-account`, `member-or-affiliate-or-parish-council`, and `account-has-article-in-issue`. * `renewals` is a built-in source mapped to membership renewal rows. * `pending-renewal` applies only when source set includes `renewals` and means `renewals.status = 'pending'`. * `selected-renewal` applies only when source set includes `renewals` and means `renewals.selected = true`. * `primary-contact` applies only when source set includes `contacts` and means `contacts.is_contact_1` is truthy. +* `member-account` applies only when source set includes `accounts` and means the account type picklist slug is `member`. +* `affiliate-account` applies only when source set includes `accounts` and means the account type picklist slug is `affiliate`. * FEN editorial, advertising, and invoice data must be exposed as built-in sources, not custom table references, using the source names below. * These built-in sources are derived from the sibling `../feca2-app/server/src/lib/mailshotDsl.js` implementation, except that `articles` must now be promoted to a first-class built-in source. In the sibling implementation, `Articles` is used by an issue-scoped article filter but is not listed as a standalone built-in source. * FEN built-in source table mappings: @@ -235,6 +239,8 @@ No implicit join behavior is allowed: * `contacts where contacts.Last contains 'smith'` * `contacts and accounts where accounts.Type = 'Member' and contacts.FENContact1 = true` * `accounts where member-or-affiliate-or-parish-council` +* `accounts where member-account` +* `accounts where affiliate-account` * `renewals where pending-renewal` * `renewals where selected-renewal` * `renewals and accounts and contacts where pending-renewal` @@ -258,6 +264,8 @@ No implicit join behavior is allowed: * `accounts where pending-renewal` (invalid: filter requires `renewals` source) * `accounts where fen1-contact` (invalid: filter requires `contacts` source) * `accounts where primary-contact` (invalid: filter requires `contacts` source) +* `contacts where member-account` (invalid: filter requires `accounts` source) +* `contacts where affiliate-account` (invalid: filter requires `accounts` source) * `ads where selected` (invalid: filter requires `advertisers` source) * `advertisers where pending-invoice` (invalid: filter requires `invoices` source) * `pages where issue` (invalid: `issue` requires exactly one numeric issue ID) diff --git a/tests/e2e/specs/data-sources.spec.mjs b/tests/e2e/specs/data-sources.spec.mjs index 4efd2af..3a2b3ea 100644 --- a/tests/e2e/specs/data-sources.spec.mjs +++ b/tests/e2e/specs/data-sources.spec.mjs @@ -184,3 +184,38 @@ test('data sources: DSL builder round-trip preserves FEN built-in filters', asyn } } }); + +test('data sources: DSL builder round-trip preserves account type filters', async ({ page, request }) => { + const uniq = `${Date.now()}_${Math.floor(Math.random() * 100000)}`; + const dsName = `e2e_ds_account_type_roundtrip_${uniq}`; + const dsl = 'accounts where member-account and not (affiliate-account)'; + + try { + await ensureDataSource(request, dsName, dsl); + + await page.goto(adminPath('feca-mailshot-data-sources')); + await expect(page.getByRole('heading', { name: 'Mailshot Data Sources' })).toBeVisible(); + + const row = page.locator('tr', { + has: page.getByRole('cell', { name: dsName }) + }).first(); + await expect(row).toBeVisible(); + await row.getByRole('link', { name: 'Edit' }).click(); + + await expect(page.locator('#ds-editor-modal')).toBeVisible(); + await expect(page.locator('#ds_dsl')).toHaveValue(dsl); + + await page.locator('#ds-build-dsl-open').click(); + await expect(page.locator('#ds-builder-modal')).toBeVisible(); + await expect(page.locator('#ds-builder-output')).toHaveValue(dsl); + + await page.locator('#ds-builder-apply').click(); + await expect(page.locator('#ds_dsl')).toHaveValue(dsl); + } finally { + try { + await cleanupByNames(request, { dataSourceNames: [dsName] }); + } catch { + // best effort + } + } +}); diff --git a/tests/integration/test_attachment_blob_roundtrip.php b/tests/integration/test_attachment_blob_roundtrip.php new file mode 100644 index 0000000..c082122 --- /dev/null +++ b/tests/integration/test_attachment_blob_roundtrip.php @@ -0,0 +1,68 @@ + '127.0.0.1', + 'MYSQL_PORT' => (string) (getenv('MYSQL_TUNNEL_LOCAL_PORT') ?: '13306'), + 'MYSQL_USER' => Env::require('REMOTE_MYSQL_USER'), + 'MYSQL_PASSWORD' => Env::require('REMOTE_MYSQL_PASSWORD'), + 'MAILSHOTS_REMOTE_MYSQL_DB' => Env::require('MAILSHOTS_REMOTE_MYSQL_DB'), + 'MEMBERS_REMOTE_MYSQL_DB' => Env::require('MEMBERS_REMOTE_MYSQL_DB'), + 'FEN_REMOTE_MYSQL_DB' => Env::require('FEN_REMOTE_MYSQL_DB'), +]; + +$router = new PdoDatabaseRouter($dbConfig); +$repo = new AttachmentRepository($router); +$service = new AttachmentService($repo); +$name = 'blob_roundtrip_' . gmdate('Ymd_His') . '_' . bin2hex(random_bytes(3)); +$id = null; + +$payload = "%PDF-1.7\n" . random_bytes(1536 * 1024) . "\n%%EOF"; + +try { + $saved = $service->save(null, [ + 'name' => $name, + 'file_name' => 'roundtrip.pdf', + 'mime_type' => 'application/pdf', + 'file_bytes_base64' => base64_encode($payload), + ]); + if (($saved['ok'] ?? false) !== true) { + fwrite(STDERR, 'Failed to save large attachment: ' . json_encode($saved) . "\n"); + exit(1); + } + $id = (int) ($saved['id'] ?? 0); + + $reloaded = $repo->findByName($name); + if (!is_array($reloaded)) { + fwrite(STDERR, "Large attachment was not found after save\n"); + exit(1); + } + $bytes = (string) ($reloaded['file_bytes'] ?? ''); + if (strlen($bytes) !== strlen($payload)) { + fwrite(STDERR, 'Large attachment byte length changed: expected ' . strlen($payload) . ', got ' . strlen($bytes) . "\n"); + exit(1); + } + if (hash('sha256', $bytes) !== hash('sha256', $payload)) { + fwrite(STDERR, "Large attachment bytes changed after DB round-trip\n"); + exit(1); + } +} finally { + if ($id !== null && $id > 0) { + try { + $repo->delete($id); + } catch (Throwable $e) { + } + } +} + +echo "Attachment blob round-trip regression test passed\n"; diff --git a/tests/unit/test_dsl_accounts_picklists.php b/tests/unit/test_dsl_accounts_picklists.php index 13b1aaa..d92d889 100644 --- a/tests/unit/test_dsl_accounts_picklists.php +++ b/tests/unit/test_dsl_accounts_picklists.php @@ -6,6 +6,7 @@ require_once dirname(__DIR__, 2) . '/feca_mailshots_plugin/src/autoload.php'; require_once __DIR__ . '/FakeMetadataProvider.php'; use FecaMailshots\Application\DslCompiler; +use FecaMailshots\Application\DslValidator; use FecaMailshots\Domain\DslParser; use FecaMailshots\Tests\Unit\FakeMetadataProvider; @@ -16,6 +17,7 @@ $metadata = new FakeMetadataProvider([ $parser = new DslParser(); $compiler = new DslCompiler($metadata); +$validator = new DslValidator($metadata); $ast = $parser->parse("accounts and contacts where accounts.account_type_id = 1"); $compiled = $compiler->compile($ast); @@ -33,4 +35,35 @@ if (strpos($sql, "COALESCE(s_accounts.`account_type_id`, '') = ?") === false) { fwrite(STDERR, "Expected predicate accounts.account_type_id to compile against physical column\n"); exit(1); } + +$memberAst = $parser->parse('accounts where member-account'); +$memberValidation = $validator->validate($memberAst); +if (($memberValidation['errors'] ?? []) !== []) { + fwrite(STDERR, "Expected member-account to validate on accounts: " . json_encode($memberValidation['errors']) . "\n"); + exit(1); +} +$memberSql = (string) ($compiler->compile($memberAst)['sql'] ?? ''); +foreach ([ + 's_accounts.`account_type_id` IN (SELECT id FROM `picklist_account_type`', + "LOWER(TRIM(COALESCE(`slug`, ''))) = 'member'", +] as $needle) { + if (strpos($memberSql, $needle) === false) { + fwrite(STDERR, "Expected member-account SQL to contain {$needle}\n{$memberSql}\n"); + exit(1); + } +} + +$affiliateAst = $parser->parse('accounts where affiliate-account'); +$affiliateSql = (string) ($compiler->compile($affiliateAst)['sql'] ?? ''); +if (strpos($affiliateSql, "LOWER(TRIM(COALESCE(`slug`, ''))) = 'affiliate'") === false) { + fwrite(STDERR, "Expected affiliate-account SQL to filter by affiliate slug\n{$affiliateSql}\n"); + exit(1); +} + +$invalidValidation = $validator->validate($parser->parse('contacts where member-account')); +if (!in_array('Filter member-account requires source accounts', $invalidValidation['errors'] ?? [], true)) { + fwrite(STDERR, "Expected member-account to require accounts source: " . json_encode($invalidValidation['errors'] ?? []) . "\n"); + exit(1); +} + echo "DSL accounts physical-schema regression test passed\n"; diff --git a/tests/unit/test_wordpress_smtp_sender.php b/tests/unit/test_wordpress_smtp_sender.php index c0e7547..c9675f4 100644 --- a/tests/unit/test_wordpress_smtp_sender.php +++ b/tests/unit/test_wordpress_smtp_sender.php @@ -122,6 +122,7 @@ require_once dirname(__DIR__, 2) . '/feca_mailshots_plugin/src/autoload.php'; use FecaMailshots\Infrastructure\WordPressSmtpSender; $sender = new WordPressSmtpSender(); +$binaryPdfBytes = "%PDF-1.7\n" . "\x00\x01\x80\xff" . "\n%%EOF"; $result = $sender->send( [ 'smtp_host' => 'smtp.example.org', @@ -141,7 +142,7 @@ $result = $sender->send( [[ 'filename' => 'notice.pdf', 'mime_type' => 'application/pdf', - 'content_bytes' => '%PDF fixture', + 'content_bytes' => $binaryPdfBytes, ]] ); @@ -174,6 +175,10 @@ if ($mailer->stringAttachments === [] || $mailer->stringAttachments[0]['filename fwrite(STDERR, "In-memory attachment was not added to WordPress mailer\n"); exit(1); } +if (($mailer->stringAttachments[0]['bytes'] ?? '') !== $binaryPdfBytes) { + fwrite(STDERR, "Binary attachment bytes should be passed to PHPMailer unchanged\n"); + exit(1); +} if (($GLOBALS['feca_test_actions']['phpmailer_init'] ?? []) !== []) { fwrite(STDERR, "phpmailer_init hook should be removed after send\n"); exit(1);