search

manywho / service-box

A plugin for ManyWho that allows integration of flows with Box
MIT License
0 stars 1 forks source link

Removing child object #30

Open josecollazzi opened 7 years ago

josecollazzi commented 7 years ago

The current behavior looks like a pre-optimization, removing child object will do the user experience easier:

1) don't need to handle child objects 2) don't need to partial populate objects and then keep track of which properties are populated during the flow (a common scenario we populated Parent Folder.ID but not the name of the folder) 3) it keep consistency with SQL-service (we return foreign key instead of related tables)

josecollazzi commented 7 years ago

@jonjomckay could you please review?

jonjomckay commented 7 years ago

@josecollazzi are you sure we should do this? What's the purpose of removing objects in favour of scalars? If using objects is hard to use, then the tooling should be improved, not neuter the services to fit :)

josecollazzi commented 7 years ago

@jonjomckay Yes, I am sure. (I agree it should be 100% supported in the system but that is another topic) I don't see a really positive impact on the final user to have a child object at this point. This can also be improved in the future adding child object if needed without making breaking changes to the service, so it seems pretty clear to me. Do I have thumbs up from your side?

jonjomckay commented 7 years ago

@josecollazzi I'm not sure - not sending objects back means that users have to have a whole bunch of extra load elements now if they want to find the parent folder of a file for example. If people were manually typing IDs to find stuff into their flows then it would be fine, but they're not - they're normally selecting a file from a table and then creating a task from it, and that selection is an object not a scalar. I think this change will actually increase the cognitive load for a builder, when we should be decreasing it (i.e. the only thing I see this solving is filtering, so we should add a "filter by object with ID" option or something in the tooling instead). It's also a breaking change to the service's API, so we'd need to support yet another version (i.e. version 4) :)

josecollazzi commented 7 years ago

@jonjomckay E.g. Approve a contract in the folder "folder-contract", the manager search for the folder with the name "folder-contract", then list all the files in that folder, but with pagination so it uses a load. Then he selects a file (he got an object file). then create a task (with the file id) and subject "approved contract".

What do you think of having both IDs and child object, and we analyze how is it used? That will not break the current ("staging") service, and we can take a decision based on real data. Also, it will bring the advantage of knowing using the debugger if a child object is null or it hasn't been load (the case of "Child ID" is not null and a "Child" is null). Currently, there is no concept of lazy-load so the builder needs to "remember" if the child object has been loaded.

jonjomckay commented 7 years ago

@josecollazzi sure - I think having both the object and the ID is fine for now. Maybe you can talk with @matt-manywho about having better filtering options in the tooling now too, as it's not so friendly to use objects there currently