From f9cb5e645b8792528a483a18b9d1939b1513389d Mon Sep 17 00:00:00 2001 From: Roman Zimmermann Date: Fri, 29 Jun 2018 10:38:08 +0200 Subject: [PATCH 1/3] Fix copying filters for e2t messages and conditional redirects. This fixes a bug where filters were removed from one message/redirect when they were copied to another. --- campaignion_action/src/Redirects/Filter.php | 3 +- campaignion_action/src/Redirects/Redirect.php | 2 +- .../tests/Redirects/RedirectTest.php | 32 +++++++++++++++++ campaignion_email_to_target/src/Filter.php | 4 ++- .../src/MessageTemplate.php | 2 +- .../tests/MessageTemplateTest.php | 35 +++++++++++++++++++ 6 files changed, 73 insertions(+), 5 deletions(-) diff --git a/campaignion_action/src/Redirects/Filter.php b/campaignion_action/src/Redirects/Filter.php index e1d8f1b60..5ed43700e 100644 --- a/campaignion_action/src/Redirects/Filter.php +++ b/campaignion_action/src/Redirects/Filter.php @@ -29,9 +29,8 @@ class Filter extends Model { * Data representing the filter. */ public static function fromArray(array $data) { - $data += ['id' => NULL, 'weight' => 0]; + $data += ['weight' => 0]; $filter = new static(); - $filter->id = $data['id']; $filter->setData($data); return $filter; } diff --git a/campaignion_action/src/Redirects/Redirect.php b/campaignion_action/src/Redirects/Redirect.php index 9c2e8e26b..8b8a552b5 100644 --- a/campaignion_action/src/Redirects/Redirect.php +++ b/campaignion_action/src/Redirects/Redirect.php @@ -73,7 +73,7 @@ public function setFilters(array $new_filters) { foreach ($new_filters as $nf) { if ($nf instanceof Filter) { - $f = $nf; + $f = $nf->redirect_id == $this->id ? $nf : clone $nf; } else { // Reuse filter objects if 'id' is passed and found. diff --git a/campaignion_action/tests/Redirects/RedirectTest.php b/campaignion_action/tests/Redirects/RedirectTest.php index 2519d781c..c401bedbd 100644 --- a/campaignion_action/tests/Redirects/RedirectTest.php +++ b/campaignion_action/tests/Redirects/RedirectTest.php @@ -69,6 +69,38 @@ public function testRedirectCloning() { $this->assertEqual(1, $t1->filters[0]->config['value']); } + /** + * Test copying a filter from one redirect template to another. + */ + public function testCopyFilter() { + $data = [ + 'id' => 42, + 'label' => 'A', + 'destination' => 'node/50', + 'filters' => [['type' => 'test', 'value' => 1]], + ]; + $redirect_a = new Redirect($data); + $redirect_a->filters[0]->id = 42; + + $data = [ + 'id' => 43, + 'label' => 'B', + 'destination' => 'node/50', + 'filters' => [], + ]; + $redirect_b = new Redirect($data); + + // Copy as array. + $redirect_b->setFilters([$redirect_a->filters[0]->toArray()]); + $this->assertEquals(42, $redirect_a->filters[0]->id); + $this->assertNotEquals(42, $redirect_b->filters[0]->id); + + // Copy as filter object. + $redirect_b->setFilters([$redirect_a->filters[0]]); + $this->assertEquals(42, $redirect_a->filters[0]->id); + $this->assertNotEquals(42, $redirect_b->filters[0]->id); + } + /** * Test checkFilters without any filters configured (ie. default redirect). */ diff --git a/campaignion_email_to_target/src/Filter.php b/campaignion_email_to_target/src/Filter.php index d17d7a2a5..f956abdb5 100644 --- a/campaignion_email_to_target/src/Filter.php +++ b/campaignion_email_to_target/src/Filter.php @@ -19,7 +19,9 @@ class Filter extends Model { public $config = []; public static function fromArray($data) { - return new static($data); + $f = new static(); + $f->setData($data); + return $f; } public function __construct($data = array(), $new = TRUE) { diff --git a/campaignion_email_to_target/src/MessageTemplate.php b/campaignion_email_to_target/src/MessageTemplate.php index 758cb3c67..18ed10d1d 100644 --- a/campaignion_email_to_target/src/MessageTemplate.php +++ b/campaignion_email_to_target/src/MessageTemplate.php @@ -55,7 +55,7 @@ public function setFilters($new_filters) { foreach ($new_filters as $nf) { if ($nf instanceof Filter) { - $f = $nf; + $f = $nf->message_id == $this->id ? $nf : clone $nf; } else { // Reuse filter objects if 'id' is passed and found. diff --git a/campaignion_email_to_target/tests/MessageTemplateTest.php b/campaignion_email_to_target/tests/MessageTemplateTest.php index 1a589b415..5f6181468 100644 --- a/campaignion_email_to_target/tests/MessageTemplateTest.php +++ b/campaignion_email_to_target/tests/MessageTemplateTest.php @@ -66,4 +66,39 @@ public function testMessageCloning() { $this->assertEqual(1, $t1->filters[0]->config['value']); } + /** + * Test copying a filter from one message template to another. + */ + public function testCopyFilter() { + $data = [ + 'id' => 42, + 'type' => 'message', + 'label' => 'A', + 'filters' => [[ + 'type' => 'test', + 'config' => ['value' => 1], + ]], + ]; + $template_a = new MessageTemplate($data); + $template_a->filters[0]->id = 42; + + $data = [ + 'id' => 43, + 'type' => 'message', + 'label' => 'B', + 'filters' => [], + ]; + $template_b = new MessageTemplate($data); + + // Copy as array. + $template_b->setFilters([$template_a->filters[0]->toArray()]); + $this->assertEquals(42, $template_a->filters[0]->id); + $this->assertNotEquals(42, $template_b->filters[0]->id); + + // Copy as filter object. + $template_b->setFilters([$template_a->filters[0]]); + $this->assertEquals(42, $template_a->filters[0]->id); + $this->assertNotEquals(42, $template_b->filters[0]->id); + } + } From fb31fa9d63a8d94b7237e354a3099d07258b59e8 Mon Sep 17 00:00:00 2001 From: Katharina Zwinger Date: Tue, 17 Jul 2018 17:30:07 +0200 Subject: [PATCH 2/3] =?UTF-8?q?[email=5Fto=5Ftarget]=20message=20app:=20do?= =?UTF-8?q?n=E2=80=99t=20duplicate=20filter=20ids?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../messages_app/package.json | 1 + .../src/components/SpecDialog.vue | 1 + .../test/unit/specs/components.spec.js | 44 +++++++++++++++++++ .../messages_app/yarn.lock | 6 +++ 4 files changed, 52 insertions(+) create mode 100644 campaignion_email_to_target/messages_app/test/unit/specs/components.spec.js diff --git a/campaignion_email_to_target/messages_app/package.json b/campaignion_email_to_target/messages_app/package.json index 1ef0eef9d..6ddeb2518 100644 --- a/campaignion_email_to_target/messages_app/package.json +++ b/campaignion_email_to_target/messages_app/package.json @@ -21,6 +21,7 @@ "lodash.omit": "^4.5.0" }, "devDependencies": { + "@vue/test-utils": "^1.0.0-beta.20", "autoprefixer": "^6.7.2", "babel-core": "^6.22.1", "babel-eslint": "^7.1.1", diff --git a/campaignion_email_to_target/messages_app/src/components/SpecDialog.vue b/campaignion_email_to_target/messages_app/src/components/SpecDialog.vue index 5fcb1728f..9061f76f1 100644 --- a/campaignion_email_to_target/messages_app/src/components/SpecDialog.vue +++ b/campaignion_email_to_target/messages_app/src/components/SpecDialog.vue @@ -175,6 +175,7 @@ export default { this.$bus.$on('duplicateSpec', index => { const duplicate = clone(this.specs[index]) duplicate.id = emptySpec(duplicate.type).id + duplicate.filters.forEach((filter) => { filter.id = null }) duplicate.label = Drupal.t('Copy of @messageName', {'@messageName': duplicate.label}) this.currentSpec = duplicate this.$store.commit('editNewSpec') diff --git a/campaignion_email_to_target/messages_app/test/unit/specs/components.spec.js b/campaignion_email_to_target/messages_app/test/unit/specs/components.spec.js new file mode 100644 index 000000000..4f0346ef0 --- /dev/null +++ b/campaignion_email_to_target/messages_app/test/unit/specs/components.spec.js @@ -0,0 +1,44 @@ +import mutations from '@/store/mutations' +import initialState from '@/store/state' +import testData from '../fixtures/example-data' +import Vue from 'vue' +import { shallowMount } from '@vue/test-utils' +import SpecDialog from '@/components/SpecDialog' + +describe('components', function () { + var state + + beforeEach(function () { + state = Object.assign({}, initialState) + mutations.initializeData(state, testData) + }) + + describe('SpecDialog', () => { + Vue.config.ignoredElements = [ + 'el-dialog', 'el-button', 'el-dropdown', 'el-dropdown-menu', 'el-dropdown-item' + ] + const bus = new Vue() + + it('duplicates a spec', () => { + const wrapper = shallowMount(SpecDialog, { + mocks: { + $store: { + state: state, + commit: function () {} + }, + $bus: bus + } + }) + var s = state.specs[0] + bus.$emit('duplicateSpec', 0) + + expect(wrapper.vm.currentSpec.label).to.equal(`Copy of ${s.label}`) + expect(wrapper.vm.currentSpec.type).to.equal(s.type) + expect(wrapper.vm.currentSpec.message).to.deep.equal(s.message) + expect(wrapper.vm.currentSpec.id, 'id should be resetted').to.be.null + for (let filter of wrapper.vm.currentSpec.filters) { + expect(filter.id, 'filter ids should be resetted').to.be.null + } + }) + }) +}) diff --git a/campaignion_email_to_target/messages_app/yarn.lock b/campaignion_email_to_target/messages_app/yarn.lock index 214d39cbc..a8f85ae4f 100644 --- a/campaignion_email_to_target/messages_app/yarn.lock +++ b/campaignion_email_to_target/messages_app/yarn.lock @@ -2,6 +2,12 @@ # yarn lockfile v1 +"@vue/test-utils@^1.0.0-beta.20": + version "1.0.0-beta.20" + resolved "https://registry.yarnpkg.com/@vue/test-utils/-/test-utils-1.0.0-beta.20.tgz#ef4505341b802f3de1c06b3cb8651378c87371fa" + dependencies: + lodash "^4.17.4" + abbrev@1, abbrev@1.0.x: version "1.0.9" resolved "https://registry.yarnpkg.com/abbrev/-/abbrev-1.0.9.tgz#91b4792588a7738c25f35dd6f63752a2f8776135" From f7a3cf3eb4f33b09037cfe016f1126a0986e7240 Mon Sep 17 00:00:00 2001 From: Katharina Zwinger Date: Mon, 1 Oct 2018 13:49:10 +0200 Subject: [PATCH 3/3] =?UTF-8?q?[wizard]=20redirect=20app:=20don=E2=80=99t?= =?UTF-8?q?=20duplicate=20filter=20ids?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../redirects_app/src/components/RedirectDialog.vue | 1 + 1 file changed, 1 insertion(+) diff --git a/campaignion_wizard/redirects_app/src/components/RedirectDialog.vue b/campaignion_wizard/redirects_app/src/components/RedirectDialog.vue index 53737755d..458597782 100644 --- a/campaignion_wizard/redirects_app/src/components/RedirectDialog.vue +++ b/campaignion_wizard/redirects_app/src/components/RedirectDialog.vue @@ -176,6 +176,7 @@ export default { this.$root.$on('duplicateRedirect', index => { const duplicate = clone(this.redirects[index]) duplicate.id = emptyRedirect().id + duplicate.filters.forEach((filter) => { filter.id = null }) duplicate.label = Drupal.t('Copy of @redirectLabel', {'@redirectLabel': duplicate.label}) this.currentRedirect = duplicate this.$store.commit('editNewRedirect')