Hey there,
Contact integration is still an @todo in the code for D8. We're looking to use this for our contact for at http://lastcallmedia.com, so I patched the module to support the current state of things. Hope we can get this integrated. Thanks!

CommentFileSizeAuthor
#12 2230641-honeypot_d8_integration-tests_only-12.patch1.53 KBrbayliss
#12 2230641-honeypot_d8_integration-12.patch4.03 KBrbayliss
#8 2230641-honeypot_d8_integration-tests_only-9.patch1.31 KBrbayliss
#8 2230641-honeypot_d8_integration-9.patch3.82 KBrbayliss
#5 2230641-honeypot_d8_integration-tests_only-5.patch1.38 KBrbayliss
#5 2230641-honeypot_d8_integration-5.patch3.95 KBrbayliss
#1 2230641-honeypot_d8_integration-1.patch2.51 KBrbayliss

Comments

rbayliss’s picture

rbayliss commented
Status: Active » Needs review
StatusFileSize
new 2230641-honeypot_d8_integration-1.patch2.51 KB

And the patch.

geerlingguy’s picture

geerlingguy commented

Initial code review looks pretty good; I would really like to add tests around this (at least one, where contact module is enabled, and honeypot field shows up on the form for anonymous user), as contact forms are radically different in D8, and it would be nice to make sure no other changes I start making will break anything.

Also, did the cache tag invalidation change from using Cache to \Drupal::cache() recently? I was just working with Drupal 8.x-alpha10, and it seemed that the way that was working was correct at that point...

Thanks!

rbayliss’s picture

rbayliss commented

Also, did the cache tag invalidation change from using Cache to \Drupal::cache() recently? I was just working with Drupal 8.x-alpha10, and it seemed that the way that was working was correct at that point...

Sorry, forgot to explain that. You're setting the cache with an ID in Drupal::cache()->set(), then try to invalidate it using cache tags. If you want to use cache tags here, you have to add them in the set() call. I discovered that when my changed settings weren't present after saving the Honeypot settings form. I can't really see a reason to use tag invalidation though, unless I'm missing something.

I'll try to get some tests in tomorrow.

geerlingguy’s picture

geerlingguy commented

Thanks! I haven't taken much time to look at how D8 caching works, and I don't think I've even looked at it since alpha 1, so I'm pretty sure you're correct :)

rbayliss’s picture

rbayliss commented
StatusFileSize
new 2230641-honeypot_d8_integration-5.patch3.95 KB
new 2230641-honeypot_d8_integration-tests_only-5.patch1.38 KB

Alright, this time with a test. The test_only version of this patch should fail.

Status: Needs review » Needs work

The last submitted patch, 5: 2230641-honeypot_d8_integration-tests_only-5.patch, failed testing.

The last submitted patch, 5: 2230641-honeypot_d8_integration-5.patch, failed testing.

rbayliss’s picture

rbayliss commented
StatusFileSize
new 2230641-honeypot_d8_integration-9.patch3.82 KB
new 2230641-honeypot_d8_integration-tests_only-9.patch1.31 KB
rbayliss’s picture

rbayliss commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 8: 2230641-honeypot_d8_integration-tests_only-9.patch, failed testing.

The last submitted patch, 8: 2230641-honeypot_d8_integration-9.patch, failed testing.

rbayliss’s picture

rbayliss commented
Status: Needs work » Needs review
StatusFileSize
new 2230641-honeypot_d8_integration-12.patch4.03 KB
new 2230641-honeypot_d8_integration-tests_only-12.patch1.53 KB

If at first you don't succeed...

Status: Needs review » Needs work

The last submitted patch, 12: 2230641-honeypot_d8_integration-tests_only-12.patch, failed testing.

geerlingguy’s picture

geerlingguy commented

I think you can add do-not-test on the second patch to skip it (just FYI). I'll take a look at this later; thanks!

  • Commit a1a0573 on 8.x-1.x authored by rbayliss, committed by geerlingguy: 
    Issue #2230641 by rbayliss: D8 Contact Integration Doesn't Work.
geerlingguy’s picture

geerlingguy commented
Status: Needs work » Fixed

Looks great! Manually tested everything, and poked and prodded it into submission. Thanks for the great and very quick work :)

Only change I made was the string for the test success message; I updated it to 'Honeypot field is added to Contact form.'

rbayliss’s picture

rbayliss commented

Great! Thanks for the fast turnaround.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.