GitHub App: faster migration page #12186
Conversation
| This includes all projects that are connected to the old GitHub OAuth App, | ||
| where the user has admin permissions and the GitHub App is installed. | ||
| """ | ||
| return any(_get_projects_missing_migration(user)) |
There was a problem hiding this comment.
It seems that _get_projects_missing_missing_migration and get_valid_projects_missing_migration are the same -- in particular based on its docstring.
However, one returns all the projects and the other only returns the projects that has_installation and is_admin. Can we update the docstring to reflect this?
There is some confusion because we are talking about "admin" permission on the Project object but also "admin" permission on the GitHub repository. We should clarify what we refer to on each case.
There was a problem hiding this comment.
It would be good to test that installation_target_groups, migration_targets and has_projects_pending_migration are not present on the steps where they are not needed. Something like:
assert migration_targets not in context
should be enough, I think.
For some things, we are returning lists of objects, so no lazy iteration is possible for those. So I'm loading those only when the proper step needs them.