nyaruka / floweditor

Graphical flow editor for the TextIt platform.
Other
71 stars 39 forks source link

SetContactAttrib: doesn't show new fields on initial #246

Closed ericnewcomer closed 6 years ago

ericnewcomer commented 6 years ago

Repro:

Should initialize the select widget with the new field, but it's blank

ycleptkellan commented 6 years ago

This and #223 are related in my mind. I believe the issue is that the list of options SelectSearch returns from its loadOptions method doesn't include the new field (passed as the initial prop). If that's the case we would need to update contactFields state to include the newly created field on save and pass SelectSearch contactFields state through its localSearchOptions prop, right? AttribElement already reads from contactFields state and passes its value to SelectSearch. One question I had when looking into this is what's the best way to confirm the new field is in fact new when saving the action from SetContactAttribForm?

ericnewcomer commented 6 years ago

Yup, I agree these are totally related. We need to put the extra fields and groups, etc in redux and include them when searching or initializing these widgets.

ycleptkellan commented 6 years ago

So then we’d need to mark them as ‘new’/local and save them off to our store when the action is opened (if the definition contains a field we haven’t seen) and saved (if created in SetContactAttribForm)?

ericnewcomer commented 6 years ago

I think maybe it makes more sense to say the editor is not responsible for creating new fields, just letting them be referenced consistently within the flow editing session. Then when the flow is saved or ran or whatever, they get created at the other end as needed. Curious if @nicpottier or @rowanseymour have an opinion there.

rowanseymour commented 6 years ago

In the past we've talked about the option of manual saving which might have implications here - i.e. you should be able to make flow changes, define new contact fields... but we don't save anything on the server until you explicitly save.

ericnewcomer commented 6 years ago

Not sure we want explicit saves, but don't think this question depends on that decision. It's more, when are groups and fields created? I would say it's done at flow definition saving (implicitly or explicitly) and not as part of the asset endpoint.

ycleptkellan commented 6 years ago

@rowanseymour what’s the motivation behind manual saving?

@ericnewcomer agree that they should be created when the FlowNode is saved, even if we’re seeing the field/group for the first time when the definition loads. My concern is how we’d go about determining the field/group is new. I assume we’d need to make that determination when the form is opened (if we haven’t seen the field/group before) and when the node is saved (if the field/group was created in AttribElement or GroupsElement) so we can merge local fields/groups with the list of assets SelectSearch loads. So how would we make that determination? A network request on open/save?

rowanseymour commented 6 years ago

@ycleptkellan it would be about reducing the number of flow revisions and giving users some control over when their changes become published and part of the live flow

ericnewcomer commented 6 years ago

@ycleptkellan no, I'm saying there is no logic at all in the flow editor for creating fields (except for local referencing pre-save). That's something that is determined by the service that we are embedded in (ie, RapidPro). When they get a flow definition to save, they need to create things they doesn't recognize at that point. At least that's the question I'm posing.

ycleptkellan commented 6 years ago

@rowanseymour gotcha, thanks. Seems to me that sorting out the logic for saving new fields to our Redux store is a logical first step for that work and this issue.

@ericnewcomer my assumption was that this issue is about when and how to save new fields to our Redux store, is that not right?

ericnewcomer commented 6 years ago

Yes, just making sure we aren't conflating with updating the asset store with newly created fields which is something that I think will be happening outside of the editor's responsibilities.

ycleptkellan commented 6 years ago

Ya, same page there. By network request I meant GET requests to the fields/groups endpoints so that we can check for the presence of the new field/group on the account and update the Redux store accordingly. I imagine that’s the bit we need to sort out as SelectSearch is the only component requesting those assets. What were you thinking there?

ericnewcomer commented 6 years ago

Fixed in #272