Skip to content
This repository has been archived by the owner on Dec 10, 2021. It is now read-only.

RND-38 email sign up form #222

Merged
merged 42 commits into from
May 25, 2016
Merged

Conversation

mrtstaunton
Copy link
Contributor

@mrtstaunton mrtstaunton commented May 9, 2016

Fixes https://jira.comicrelief.com/browse/RND-38

Changes proposed in this pull request

  • Provide a skeleton multi-step form code base.
  • Initially provide a way to subscribe users via email address
  • Send data to (configurable) RabbitMQ queues. See data contract in google doc

Requirements.

  • When is send the data be able to send a custom message
  • Validate that fields are not empty
  • Show an error message in the form when is not send to the queue
  • Customizable Rabbitmq

Tom Staunton and others added 6 commits May 4, 2016 12:17
@pvhee
Copy link
Contributor

pvhee commented May 10, 2016

@stauntont I've added the data contract document above which specifies what to send to rabbit queue

@Saphyel
Copy link
Contributor

Saphyel commented May 10, 2016

@stauntont you have here also the eddirect PR

@mrtstaunton
Copy link
Contributor Author

@Saphyel Damn it! I'll remove.

description: Comic Relief Multi Step Form
core: 8.x
package: Comic Relief
type: module
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think maybe is a good idea add a dependdency to the module rabbitmq? or cr_rabbitmq ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Saphyel why do we need this? with queues we have a nice abstraction between the type of queues. we just need to add $settings['queue_default'] = 'queue.rabbitmq'; to use rabbit queues for all db. queues

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am thinking though that we need to give this module a better name, "Comic Relief Multi Step Form" is quite generic as it's really doing the Email signup two-step form

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see your point, you are right

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's a good name for it? Just "Comic Relief Email Signup Form"?

Copy link
Contributor

@pvhee pvhee May 11, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's dealing with email signup mostly, so something like cr_email_signup would do I guess?

The previous D7 module is called cr_marketing_prefs but that's not a great name I think

@pvhee
Copy link
Contributor

pvhee commented May 17, 2016

@Saphyel @stauntont I'd like to see some tests as part of this setup - could we come up with a plan for implementing these tests? We'd need to setup the block somewhere + have rabbitmq in the testing environment as well, and then with behat we could write @javascript tests (to run AJAX requests).

@pvhee
Copy link
Contributor

pvhee commented May 17, 2016

@stauntont did a quick review, with the first AJAX submit we don't seem to get a message returned like "Thanks! Your first email will be with you shortly" - how would we handle this?

Also, I'm not too sure we can theme this properly as discussed previously, maybe @gusliedke you could have a look at htis branch and enable cr_email_signup and see if you see any issues?

$queue_message = array_merge($this->skeletonMessage, $append_message);

// TODO: Move to config/default.
$queue_name = 'queue1';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe @Saphyel you could help out with making all these bits configurable? \cc @stauntont

@Saphyel
Copy link
Contributor

Saphyel commented May 17, 2016

@pvhee we don't need javascript in the test, we only need 2 behat tests... and about if is really send to the queue I can't answer to that...


$queue_message = array(
'transSourceURL' => \Drupal::service('path.current')->getPath(),
'transSource' => "{$this->skeletonMessage['campaign']}_[Device]_ESU_[PageElementSource]",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doing some minor testing around the paragraph type we are going to use to ipliment this and ive noticed that on both submits im getting Notice: Array to string conversion in Drupal\cr_email_signup\Form\SignUp->queueEmail() (line 112 of /var/www/campaign/profiles/cr/modules/custom/cr_email_signup/src/Form/SignUp.php).

I've not got the queue set up locally so this needs to be taken into account.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where are you seeing that @bimsonz ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry i totally forgot to write that part!

For the ajax form, within the the db logs and for the post submit, with in drupal set message that appears on screen

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could well be my local set up but worth flagging!, full stack trace below

Notice: Array to string conversion in Drupal\cr_email_signup\Form\SignUp->submitForm() (line 130 of profiles/cr/modules/custom/cr_email_signup/src/Form/SignUp.php).
Drupal\cr_email_signup\Form\SignUp->submitForm(Array, Object)
call_user_func_array(Array, Array) (Line: 111)
Drupal\Core\Form\FormSubmitter->executeSubmitHandlers(Array, Object) (Line: 51)
Drupal\Core\Form\FormSubmitter->doSubmitForm(Array, Object) (Line: 583)
Drupal\Core\Form\FormBuilder->processForm('cr_email_signup_form', Array, Object) (Line: 314)
Drupal\Core\Form\FormBuilder->buildForm('cr_email_signup_form', Object) (Line: 212)
Drupal\Core\Form\FormBuilder->getForm('Drupal\cr_email_signup\Form\SignUp') (Line: 27)
Drupal\cr_email_signup\Plugin\Block\SignUpBlock->build() (Line: 202)
Drupal\block\BlockViewBuilder::preRender(Array)
call_user_func('Drupal\block\BlockViewBuilder::preRender', Array) (Line: 381)
Drupal\Core\Render\Renderer->doRender(Array, ) (Line: 195)
Drupal\Core\Render\Renderer->render(Array) (Line: 462)
Drupal\Core\Template\TwigExtension->escapeFilter(Object, Array, 'html', NULL, 1) (Line: 82)
__TwigTemplate_dda4a20940358d56763a954eb1f4ffede836067b85220cd010f9bfa1a51b9c11->doDisplay(Array, Array) (Line: 381)
Twig_Template->displayWithErrorHandling(Array, Array) (Line: 355)
Twig_Template->display(Array) (Line: 366)
Twig_Template->render(Array) (Line: 64)
twig_render_template('profiles/cr/themes/custom/campaign_base/templates/field/field.html.twig', Array) (Line: 384)
Drupal\Core\Theme\ThemeManager->render('field', Array) (Line: 435)
Drupal\Core\Render\Renderer->doRender(Array) (Line: 448)
Drupal\Core\Render\Renderer->doRender(Array, ) (Line: 195)
Drupal\Core\Render\Renderer->render(Array) (Line: 462)
Drupal\Core\Template\TwigExtension->escapeFilter(Object, Array, 'html', NULL, 1) (Line: 52)
__TwigTemplate_81bd87afca7d780ded9715138f56d0b45ed31241827e49a934258aa62abd7422->doDisplay(Array, Array) (Line: 381)
Twig_Template->displayWithErrorHandling(Array, Array) (Line: 355)
Twig_Template->display(Array) (Line: 366)
Twig_Template->render(Array) (Line: 64)
twig_render_template('profiles/cr/modules/contrib/paragraphs/templates/paragraph.html.twig', Array) (Line: 384)
Drupal\Core\Theme\ThemeManager->render('paragraph', Array) (Line: 435)
Drupal\Core\Render\Renderer->doRender(Array, ) (Line: 195)
Drupal\Core\Render\Renderer->render(Array) (Line: 462)
Drupal\Core\Template\TwigExtension->escapeFilter(Object, Array, 'html', NULL, 1) (Line: 65)
__TwigTemplate_dda4a20940358d56763a954eb1f4ffede836067b85220cd010f9bfa1a51b9c11->doDisplay(Array, Array) (Line: 381)
Twig_Template->displayWithErrorHandling(Array, Array) (Line: 355)
Twig_Template->display(Array) (Line: 366)
Twig_Template->render(Array) (Line: 64)
twig_render_template('profiles/cr/themes/custom/campaign_base/templates/field/field.html.twig', Array) (Line: 384)
Drupal\Core\Theme\ThemeManager->render('field', Array) (Line: 435)
Drupal\Core\Render\Renderer->doRender(Array) (Line: 448)
Drupal\Core\Render\Renderer->doRender(Array, ) (Line: 195)
Drupal\Core\Render\Renderer->render(Array) (Line: 462)
Drupal\Core\Template\TwigExtension->escapeFilter(Object, Array, 'html', NULL, 1) (Line: 83)
__TwigTemplate_4ff8aef3affff0b412978c6dd35c12b1adf09552e7cdb4118bfc54eaaaccb438->block_content(Array, Array) (Line: 167)
Twig_Template->displayBlock('content', Array, Array) (Line: 72)
__TwigTemplate_4ff8aef3affff0b412978c6dd35c12b1adf09552e7cdb4118bfc54eaaaccb438->doDisplay(Array, Array) (Line: 381)
Twig_Template->displayWithErrorHandling(Array, Array) (Line: 355)
Twig_Template->display(Array) (Line: 366)
Twig_Template->render(Array) (Line: 64)
twig_render_template('core/themes/classy/templates/block/block.html.twig', Array) (Line: 384)
Drupal\Core\Theme\ThemeManager->render('block', Array) (Line: 435)
Drupal\Core\Render\Renderer->doRender(Array) (Line: 448)
Drupal\Core\Render\Renderer->doRender(Array, ) (Line: 195)
Drupal\Core\Render\Renderer->render(Array) (Line: 462)
Drupal\Core\Template\TwigExtension->escapeFilter(Object, Array, 'html', NULL, 1) (Line: 67)
__TwigTemplate_e04751ec69df6e06bac4d475b36991cb478802b96866eccbdda17e97341b32ce->doDisplay(Array, Array) (Line: 381)
Twig_Template->displayWithErrorHandling(Array, Array) (Line: 355)
Twig_Template->display(Array) (Line: 366)
Twig_Template->render(Array) (Line: 64)
twig_render_template('profiles/cr/modules/custom/cr_layout/layouts/fivefullregions/fivefullregions.html.twig', Array) (Line: 384)
Drupal\Core\Theme\ThemeManager->render('fivefullregions', Array) (Line: 435)
Drupal\Core\Render\Renderer->doRender(Array, ) (Line: 195)
Drupal\Core\Render\Renderer->render(Array) (Line: 462)
Drupal\Core\Template\TwigExtension->escapeFilter(Object, Array, 'html', NULL, 1) (Line: 92)
__TwigTemplate_14bed27ac71a9baaeca201a63d61b2eb0d482c4c051487b242275c36028c31c0->doDisplay(Array, Array) (Line: 381)
Twig_Template->displayWithErrorHandling(Array, Array) (Line: 355)
Twig_Template->display(Array) (Line: 366)
Twig_Template->render(Array) (Line: 64)
twig_render_template('profiles/cr/modules/contrib/panelizer/templates/panelizer-view-mode.html.twig', Array) (Line: 384)
Drupal\Core\Theme\ThemeManager->render('panelizer_view_mode', Array) (Line: 435)
Drupal\Core\Render\Renderer->doRender(Array, ) (Line: 195)
Drupal\Core\Render\Renderer->render(Array, ) (Line: 226)
Drupal\Core\Render\MainContent\HtmlRenderer->Drupal\Core\Render\MainContent\{closure}() (Line: 574)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 227)
Drupal\Core\Render\MainContent\HtmlRenderer->prepare(Array, Object, Object) (Line: 117)
Drupal\Core\Render\MainContent\HtmlRenderer->renderResponse(Array, Object, Object) (Line: 90)
Drupal\Core\EventSubscriber\MainContentViewSubscriber->onViewRenderArray(Object, 'kernel.view', Object) (Line: 111)
Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch('kernel.view', Object) (Line: 144)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 62)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 57)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 98)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 77)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 50)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 628)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

@bimsonz
Copy link
Contributor

bimsonz commented May 20, 2016

#255 Just mentioning this here so its referenced

@bimsonz
Copy link
Contributor

bimsonz commented May 24, 2016

Not sure on the status of this but Travis is failing due to coding standards checks here: https://travis-ci.com/comicrelief/campaign/builds/25765095#L1203

Tom Staunton and others added 2 commits May 24, 2016 15:14
parent::validateForm($form, $form_state);
}

return $form;
Copy link
Contributor

@Saphyel Saphyel May 24, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stauntont Take a look to examples/fapi_example/src/Form/BuildDemo.php
You don't need to return anything or use the parent

Also you can play with:

      '#states' => array(
        // Hide the additional settings when this email is disabled.
        'invisible' => array(
          'input[name="user_mail_status_activated_notify"]' => array('checked' => FALSE),
        ),
      )

and play with different ajaxSubmitStepN rather than unset things, check steps and etc..

@bimsonz bimsonz merged commit fcf1010 into develop May 25, 2016
@bimsonz bimsonz deleted the feature/RND-38_email_data_capture_form branch May 25, 2016 14:48
@bimsonz bimsonz added this to the RND17 June milestone May 26, 2016
@pvhee pvhee mentioned this pull request Jan 5, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants