Fix performance issues

[joshsmith]

What's in this PR?

Some fixes for API performance issues.

• Remove preloads from views and write explicitly in controllers
• Reduce id filter repetition
• Ensure id query filters are run on every controller that has an index action
• Add missing indexes

[begedin]

This is a great change overall.

I especially like the move of preloads to controllers. It's more explicit and something that's generally recommended in phoenix.

Just a potential concern with removing tasks from a project, but should be fine otherwise.

[begedin]

Did you check how this works with ember. Do we access project.tasks on the client end anywhere? I seem to remember this missing causing problems for us, but the problem may have gone away due to introducing task lists.

[joshsmith]

This is breaking things in Ember. Even worse, I wanted to be able to specify the tasks index to load only open tasks by default, but this appears to break functionality if the tasks are not included on the project.

There's some work to be done here figuring out the best way forward.

[joshsmith]

Do you agree with breaking out the preloads into the model @begedin?

[begedin]

@joshsmith If you mean, into the controller, then yeah, I absolutely do. Can't find a link for it, but pretty sure that's the recommended approach for phoenix

[begedin]

To update myself, as I said on slack, nut sure the model module is the best place to put them, but we need them and I don't have a better idea, so I'm fine with placing them there.

Seconds later we agreed the view is a good spot.

[joshsmith]

AuthToken

• value

Category

• slug

Comment

• user
• task
• github_id

DonationGoal:

• current (unique to project)
• project_id

GithubAppInstallation

• github_id (unique)
• user_id
• project_id

GithubComment

• github_id (unique)
• github_issue_id

GithubEvent

• github_delivery_id
• status

GithubIssue

• github_id (unique)
• github_repo_id
• github_pull_request_id

GithubPullRequest

• github_id
• github_repo_id

GithubRepo

• github_id (unique)
• github_app_installation_id (used by InstallationRepositories event but not sure it should be)

OrganizationGithubAppInstallation

• organization_id
• github_app_installation_id

These should probably be made a unique pair?

OrgnanizationInvite

• code (unique)
• email

Organization

• approved
• owner_id
• slug (unique)

Preview

• user_id

Project has:

• slug (unique)
• approved
• organization_id

This has a github_repo and github_owner – what are these?

StripeX:

• id_from_stripe

StripeConnectSubscription, StripePlatformCustomer

• user_id

StripeExternalAccount

• id_from_stripe (unique)

StripeFileUpload

• id_from_stripe (unique)

Task

• github_issue_id
• github_repo_id
• project_id
• task_list_id
• number
• order
• archived
• status

TaskList has:

• inbox
• project_id
• order

User:

• github_id (unique)
• email (unique)

[joshsmith]

Additionally, it doesn't look like we sort tasks by order, which is maybe a lost opportunity since that's expected on the front-end.

[joshsmith]

Should OrganizationGithubAppInstallation have a unique pair of organization_id and github_app_installation_id?

Project has a github_repo and github_owner – what are these?

[begedin]

Should OrganizationGithubAppInstallation have a unique pair of organization_id and github_app_installation_id?

I think no. Consider this case.

On my github account, I have 2 repos, intended to be two separate projects in two organizations. I install the CC app once and then from each project's installation tab, can see that installation and connect it to that organization.

It's a rare case, but I think it is possible.

Project has a github_repo and github_owner – what are these?

Probably remainder from before extracting project_github_repo or possibly even github_repo. We may have added them way early in this milestone.

[joshsmith]

Probably remainder from before extracting project_github_repo or possibly even github_repo. We may have added them way early in this milestone.

Can you add an issue for this?

[begedin]

Looks good. I see you decided to keep preloads in the controller. Works for me and makes sense, considering that's where they are used from.