CLIxIndia-Dev / MiTiRobot

A chatbot for Telegram
MIT License
0 stars 1 forks source link

Getting the "root" element? #6

Open coleshaw opened 7 years ago

coleshaw commented 7 years ago

So you'll have to educate me on this, but I'm unclear if this code does what you want it to (~lines 92-95):

        top_level_elements = Element.objects.filter(level=0) # this is a string not an element object
        for x in top_level_elements:
            if str.lower(x.name) == "start":
                element = x

Are you trying to get the first start Element, or the last one, assigned to the element variable? If you really start supporting multiple dialog roots, won't you have to change this code to account for multiple start Elements? (and how would you do that? At the very least, you might need a break statement here, or otherwise grab the first one back from the queryset).

This also seems like an opportunity to break this code out into its own method and gave it a clarifying name. And of course, add a set of tests (when there are no elements named start, 1 element, multiple, etc.), depending on what is the current use case you're supposed to be supporting.

Right now I think the on_chat_message method breaks if there are no elements named start (line 295, user.last_node=element might throw an exception), but maybe that's not a possible state to be in?

coleshaw commented 7 years ago

Or, if you're sticking with one dialog tree for now, just add in a TODO comment to refactor that section, when you need to support multiple dialog trees.