Problem/Motivation

Dialogflow has changed completely the way entities are exposed and managed through the API V2.

Proposed resolution

We need to align with the changes, if we want to keep providing this feature.

Remaining tasks

  1. Cry
  2. Wipe the tears
  3. Suggest - not require - the google/cloud-dialogflow package on our composer
  4. Use the library to sync entities
  5. Update documentation, mainly about authentication as it's crazy.

Release notes snippet

TBD

Comments

gambry created an issue. See original summary.

larowlan’s picture

Update documentation, mainly about authentication as it's crazy.

😂

gambry’s picture

Assigned: Unassigned » gambry

Yeah dude. :(

Working on this BTW.

gambry’s picture

Status: Active » Needs work
StatusFileSize
new9.83 KB

This is the first shape. I got it working locally, and it didn't require a lot of code... which is good.
It still requires tests, so uploading the patch just to show progress and give a workaround to whoever needs to move to V2 entities push soon.

Also: currently the google/cloud-dialogflow package is on `required-dev` and `suggested`. In this way if you don't need the entities push handler you can avoid downloading shit-ton of google code.

gambry’s picture

Testing this is even worse. Google library uses a lot of static methods, which don't play nice with Prophecy.

I'll give it another go tomorrow.

larowlan’s picture

This is looking nice!

gambry’s picture

Status: Needs work » Needs review
StatusFileSize
new0 bytes
new15.3 KB

Updated handler and initial tests.

Tests only covers the saveConfiguration() for now. Testing google EntityTypeClient has been a bit of a pain, but finally I've got there.
We can improve tests later on, when we have more feedback from users using V2 entities push.

Of this work the only thing I'm not 100% sure are the two finally {$client->close()}. I wanted to close the connection to save resources, but if $client will be called again after being closed gRPC library doesn't fail nice and instead segfaults brillianty leaving users without a clue. However currently they are harmless so I'm happy with having those in.

gambry’s picture

StatusFileSize
new19.78 KB

Ooops. Empty patch. Re-uploading.

gambry’s picture

StatusFileSize
new19.81 KB

Fixing coding standard issues + require-dev chatbot_api in order to be available during tests.

larowlan’s picture

+++ b/composer.json
@@ -12,5 +12,15 @@
+    "drupal/chatbot_api": "^2.0",
+    "google/cloud-dialogflow": "^0.10"

fwiw I think you have to commit these to HEAD first in order for CI to work properly, that and a combination of entries in test_dependencies in info.yml (in my experience)

Looks solid - nice work!

gambry’s picture

@larowlan this is definitely true for test_dependencies in info.yml (documentation mentions this) but the docs are unclear for require-dev in composer.json alternative.

So I tried and apparently it works if changes to composer.json are in the patch, see the console output extract below:

15:24:45 - Installing google/cloud-dialogflow (v0.10.0): Downloading (100%)

and indeed tests against #9 do have ApiAiPushHandlerTest::testSaveConfiguration()

So, all looks good!

  • gambry authored fc5214a on 8.x-2.x
    Issue #3082765 by gambry, larowlan: Support Dialoglow V2 entities push/...
gambry’s picture

Issue summary: View changes
Status: Needs review » Needs work

This is now merged in. I'll update the documentation today, and get a RC release out.

Setting NW to track the documentation task.

  • gambry authored f9857c8 on 8.x-2.x
    Issue #3082765 by gambry: Support Dialoglow V2 entities push/sync (...
gambry’s picture

Status: Needs work » Fixed

Done with some documentation in the README.txt file. I'll point to this in the module description page.
Thanks!

Status: Fixed » Closed (fixed)

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