Closed (fixed)
Project:
Chatbot API
Version:
8.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Feature request
Assigned:
Reporter:
Created:
25 Aug 2017 at 23:43 UTC
Updated:
28 Sep 2017 at 21:45 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
gambryFrom my understanding Alexa entities definition can be done only from their UI, while Api.AI let you define entities either via UI or pushing to the entities endpoint.
Additionally developers seem to be able to use "on-the-fly entities" through webhook slot filling in Intents. If this is true and final result is the same as pushing values to the Entity, it may be easier and quicker to implement this one with api_ai_webhook rather then connecting to the Api.AI REST endpoint.
Either way this is an important feature request, and it's a shame Alexa doesn't have this feature.
@larowlan have you had a look if same can be done for Wit?
Comment #3
larowlanhttps://wit.ai/docs/http/20170307#post--entities-link Yep, Wit.ai supports it.
Comment #4
larowlanSo here's my wip.
Architecture is as follows:
To do:
Comment #5
larowlandone for today
Comment #6
larowlanto do:
- edit form submit handler
- make ui test pass (complete the form - TDD)
- config dependencies test
- config dependencies
Comment #7
gambry@larowlan thanks for the amazing work. Some services you use are new to me, so I'm sure this is going to be for me a great training exercise.
I like the architecture. I struggled at the begging with why you were pushing all entries every time, then I noticed this is the way Api.AI expects data in its /entries endpoint(s). That differs from - for instance - Wit, where you need to POST a single value for each HTTP request.
But I'm happy with the way you did, it's the best approach against any scenario.
One question I need to ask is how QueryHandler plugins work, in terms of what the logic for which plugin will be selected for execution is. I can see there are
isEnabled()andapplies()methods, which should control if the query is valid for the current collection, but I don't see those two used anywhere.To me looks like at the moment all run. Is that correct?
Anyway the idea is brilliant, as I'm assuming QueryHandler let developers add complex queries/filtering like 'All articles with "Foo" value in the "bar" field' OR "children of 'Parent' taxonomy term'.
I'm going to do some test to better understand the UI and UX, but generally I'm happy with the work done so far.
Thanks again.
Comment #8
larowlanYep, the
appliesisn't yet implemented, but will be for the UIi.e. limiting query handlers based on the entity type.
The form still needs to be finished - that's what I'm working on now - hopefully it will make a bit more sense once that is done
Comment #9
larowlanThis is ready for review now, working nicely on my local environment/api.ai account
Comment #10
larowlanComment #11
gambryHi @larowlan.
All looks very good, thank you for the hard work.
There are just few coding standard issues we should fix before commit, but really no major thing.
The only open question is if
isEnabled()methods are of any use or not. I don't see they been called within their own plugin managers and in the Plugin system itself.After an user save an Entity we need to explain what's going on. If first sync is going to happen after creating/editing an entity we should let the user know. Then when the entity is changed we should also flag actual sync will actually happen on cron run.
But I'm generally happy, UX/UI is friendly and fluid so feel free to make these minor changes and commit.
Comment #12
larowlanAh yeah - I changed the way the plugins were stored on the config entity, I filter out the disabled ones - so the isEnabled isn't needed.
Will remove on Monday
Comment #13
larowlanI didn't get to this hoping to get to it tomorrow
Comment #14
larowlanRemoved the
isEnabledmethod from the base class and the interface.Added some additional information regarding cron etc to the messages/UI
Comment #17
larowlanProper interdiff this time
And a fix for test groups
Comment #18
gambryLooks good. More than happy to have this in!
Thanks for your work.
Comment #22
gambryRemoving/Hiding previous files.
Comment #24
larowlanCommitted and pushed to 8.x-1.x
Added @gambry in credits for all the reviews
Thanks! excited to get this in.
Will update project page to mention it.