[Projects] Fix prebuild association #5054
Conversation
|
/approve no-issue |
AlexTugarev
force-pushed
the
at/fix-project-prebuilds
branch
from
e4f5b78
to
d721e40
Compare
| if (project) { | ||
| project.cloneUrl = repository.clone_url; | ||
| await this.projectDB.storeProject(project); | ||
| const oldName = (ctx.payload as any)?.changes?.repository?.name?.from; |
There was a problem hiding this comment.
This is undocumented stuff, I'm afraid. The event payload contains:
"changes": { "repository": { "name": { "from": "test-repo-123" } } }
To implement this in a robust way, we'd need to store repository.id to the project. next to the cloneUrl.
There was a problem hiding this comment.
makes sense. maybe put this as a TODO into the code
AlexTugarev
changed the title
|
@AlexTugarev This will probably resolve #5065, right? |
gtsiolis
added
component: dashboard
feature: prebuilds
type: bug
| if (project) { | ||
| project.cloneUrl = repository.clone_url; | ||
| await this.projectDB.storeProject(project); | ||
| const oldName = (ctx.payload as any)?.changes?.repository?.name?.from; |
There was a problem hiding this comment.
makes sense. maybe put this as a TODO into the code
| @@ -329,11 +334,11 @@ export class GithubApp { | |||
| return this.env.hostUrl.with({ pathname: '/button/open-in-gitpod.svg' }).toString(); | |||
| } | |||
|
|
|||
| protected async findInstallationOwner(installationId: number): Promise<{user: User, project?: Project} | undefined> { | |||
| protected async findInstallationOwner(installationId: number, cloneURL: string): Promise<{user: User, project?: Project} | undefined> { | |||
There was a problem hiding this comment.
Should this rather be two methods? (findUser(installationId) and findPRoject(cloneUrl))
Maybe at least name the method findInstallationOwnerAndProject
There was a problem hiding this comment.
both make sense. will do!
|
/hold |
AlexTugarev
force-pushed
the
at/fix-project-prebuilds
branch
from
d721e40
to
e4d4917
Compare
|
LGTM label has been added. Git tree hash: c635928c608ca7b8e977fd7c129cd12805dc860c
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: AlexTugarev, svenefftinge Associated issue requirement bypassed by: AlexTugarev The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/hold cancel |
This fixes a serious bug in the association of projects to prebuilds which are triggered on push events. Replacing all
findProjectByInstallationIdbyfindProjectByCloneUrl, because an installation can be used to create multiple projects this cannot be unambiguous.