include tasks in view

[snewcomer]

WIP

So this is a first stab at including tasks in the task_list view and would remove the need for this line taskLists.forEach((taskList) => get(taskList, 'tasks').reload()); in the tasks index route.

A couple of notes:

1. This is using a separate TaskIncludedView with just the attrs needed for the tasks index route. Also note the has_github_pull_request and overall_status is not in the attrs. I can add back, but wasn't seeing it on the index route.
2. This should just work on the frontend. Further refactors to the tasks index route and task detail route will follow.

Progress on: #1156

[snewcomer]

Maybe better to call this TaskSlimView.

[begedin]

Not a bad idea. Ideally, I would love if we could just specify what to include and where.

Possibly even the Ember client would do it, so we don't have to worry about this.

[snewcomer]

kind of like graphql? I would love that. I worked on a project and eventually there was 5 different views for the same resource, each giving you a different subset of data. It was slightly hard to manage.

We implemented the request on the ember side, but eventually the logic got complex and the various views were easier to manage.

[snewcomer]

This task now includes all relevant relationships I believe that are needed for the task index route.

[begedin]

@joshsmith @snewcomer and I talked about this in Slack for a bit, but I'm not sure how up to date you are.

Right now, our task list on ember is effectively doing a manual include of a lot of stuff, firing off a bunch of requests before rendering the page.

We're acting as if it's all async, but really, almost none of it is.

This would do on the API what Ember is trying to do on its own - include everything it needs and get it as a single request.

How do you feel about it?

[begedin]

Not a bad idea. Ideally, I would love if we could just specify what to include and where.

Possibly even the Ember client would do it, so we don't have to worry about this.

[joshsmith]

@begedin is this something we can talk about sync some soon? I'd like to actually step through this a little more explicitly to wrap my head around it.

[begedin]

@joshsmith Posted a comment which we could conver into an issue at #1349 (comment) but I would agree that this is wort standup time.

[joshsmith]

@begedin wasn't our conclusion that this is necessary to do, we just need clarity on what the client is going to need so we can design our API more intentionally?

[joshsmith]

kind of like graphql? I would love that. I worked on a project and eventually there was 5 different views for the same resource, each giving you a different subset of data. It was slightly hard to manage.

In fact, JSON API provides mechanisms to do exactly this: render subsets of data based on what the client requests.

[begedin]

@begedin wasn't our conclusion that this is necessary to do, we just need clarity on what the client is going to need so we can design our API more intentionally?

Including conversation parts in conversation is easier to merge ahead of time, since it doesn't alter much in our architecture, so I would say #1349 is good to go for now, as something we do before adding support for explicit includes.

However, this one is harder to decide on, since it involves adding new subtypes of views, which is an architecture change. It still seems to be an improvement in performance, but I'm not sure what else it entails.