diff --git a/src/server/db/migrations/004_add_unique_constraint_pull_requests.ts b/src/server/db/migrations/004_add_unique_constraint_pull_requests.ts new file mode 100644 index 00000000..a1a3ee96 --- /dev/null +++ b/src/server/db/migrations/004_add_unique_constraint_pull_requests.ts @@ -0,0 +1,29 @@ +/** + * Copyright 2025 GoodRx, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Knex } from 'knex'; + +export async function up(knex: Knex): Promise { + await knex.schema.alterTable('pull_requests', (table) => { + table.unique(['githubPullRequestId']); + }); +} + +export async function down(knex: Knex): Promise { + await knex.schema.alterTable('pull_requests', (table) => { + table.dropUnique(['githubPullRequestId']); + }); +} diff --git a/src/server/services/pullRequest.ts b/src/server/services/pullRequest.ts index c0ed3492..766b9287 100644 --- a/src/server/services/pullRequest.ts +++ b/src/server/services/pullRequest.ts @@ -47,23 +47,15 @@ export default class PullRequestService extends BaseService { * @returns Pull request model */ async findOrCreatePullRequest(repository: Repository, githubPullRequestId: number, options: PullRequestOptions) { - const { title, status, number: pullRequestNumber, fullName, deployOnUpdate } = options; + const { title, status, number: pullRequestNumber, fullName, deployOnUpdate, githubLogin } = options; let pullRequest = await this.db.models.PullRequest.findOne({ repositoryId: repository.id, githubPullRequestId, }); - if (pullRequest != null) { - if (pullRequest.githubLogin == null) { - await pullRequest.$query().patch({ githubLogin: options?.githubLogin }); - } - if (status === 'open' && !pullRequest.deployOnUpdate) { - await pullRequest.$query().patch({ - deployOnUpdate, - }); - } - } else { + if (!pullRequest) { + // If not found, try to create new one try { pullRequest = await this.db.models.PullRequest.create({ githubPullRequestId, @@ -71,31 +63,46 @@ export default class PullRequestService extends BaseService { deployOnUpdate, githubLogin: options.githubLogin, branchName: options.branch, - }).catch((error) => { - logger.error(`[REPO]${options.fullName} [PR#]${options.number} ${error}`); - return null; }); - } catch (e) { - logger.error(`[REPO]${repository.fullName} [PR NUM]${options.number}: ${e}`); - - // If there is more than 1 entry, pick one to return. - if (e instanceof UniqueViolationError) { + } catch (error) { + if (error instanceof UniqueViolationError) { + logger.info( + `[REPO]${fullName} [PR#]${pullRequestNumber} Pull request already exists, fetching existing record` + ); pullRequest = await this.db.models.PullRequest.findOne({ repositoryId: repository.id, githubPullRequestId, }); + + if (!pullRequest) { + // should never happen, but just in case + throw new Error( + `Failed to find pull request after unique violation for repo ${repository.id}, PR ${githubPullRequestId}` + ); + } } else { - throw e; + logger.error(`[REPO]${fullName} [PR#]${pullRequestNumber} Failed to create pull request: ${error}`); + throw error; } } } - await pullRequest.$query().patch({ + const updates: any = { title, status, pullRequestNumber, fullName, - }); + }; + + if (pullRequest.githubLogin == null && githubLogin) { + updates.githubLogin = githubLogin; + } + + if (status === 'open' && !pullRequest.deployOnUpdate && deployOnUpdate) { + updates.deployOnUpdate = deployOnUpdate; + } + + await pullRequest.$query().patch(updates); pullRequest.$setRelated('repository', repository); return pullRequest;