From 482141e27158074f88d760d9268b45eedcf24094 Mon Sep 17 00:00:00 2001 From: Justin Dalrymple Date: Fri, 27 Aug 2021 18:31:02 -0400 Subject: [PATCH] Update MergeRequestApprovals API to match latest Gitlab API Release (#2035) --- .../src/resources/MergeRequestApprovals.ts | 237 +++++++++++++----- packages/core/src/resources/types.ts | 10 +- .../unit/resources/MergeRequestApprovals.ts | 234 +++++++++-------- 3 files changed, 310 insertions(+), 171 deletions(-) diff --git a/packages/core/src/resources/MergeRequestApprovals.ts b/packages/core/src/resources/MergeRequestApprovals.ts index aa57c216..f822d753 100644 --- a/packages/core/src/resources/MergeRequestApprovals.ts +++ b/packages/core/src/resources/MergeRequestApprovals.ts @@ -1,10 +1,10 @@ import { BaseResource } from '@gitbeaker/requester-utils'; import { BaseRequestOptions, RequestHelper, Sudo } from '../infrastructure'; +import { UserSchema } from './Users'; +import { GroupSchema } from './Groups'; +import { ProtectedBranchSchema } from './ProtectedBranches'; -// TODO: This one is has changed quite a bit, requires a deeper look -// https://docs.gitlab.com/ee/api/merge_request_approvals.html#project-level-mr-approvals - -export interface MergeRequestApprovalSchema extends Record { +export interface ProjectLevelMergeRequestApprovalSchema extends Record { approvals_before_merge: number; reset_approvals_on_push: boolean; disable_overriding_approvers_per_merge_request: boolean; @@ -13,36 +13,126 @@ export interface MergeRequestApprovalSchema extends Record { require_password_to_approve: boolean; } +export interface ApprovedByEntity { + user: Pick; +} + +export interface MergeRequestLevelMergeRequestApprovalSchema extends Record { + id: number; + iid: number; + project_id: number; + title: string; + description: string; + state: string; + created_at: string; + updated_at: string; + merge_status: string; + approvals_required: number; + approvals_left: number; + approved_by?: ApprovedByEntity[]; +} + export type ApprovalRulesRequestOptions = { userIds?: number[]; groupIds?: number[]; protectedBranchIds?: number[]; }; +export interface ApprovalRuleSchema extends Record { + id: number; + name: string; + rule_type: string; + eligible_approvers?: Pick< + UserSchema, + 'name' | 'username' | 'id' | 'state' | 'avatar_url' | 'web_url' + >[]; + approvals_required: number; + users?: Pick[]; + groups?: GroupSchema[]; + contains_hidden_groups: boolean; + overridden: boolean; +} + +export interface ProjectLevelApprovalRuleSchema extends ApprovalRuleSchema { + protected_branches?: ProtectedBranchSchema[]; +} + +export interface MergeRequestLevelApprovalRuleSchema extends ApprovalRuleSchema { + source_rule?: string; +} + export class MergeRequestApprovals extends BaseResource { - addApprovalRule( + configuration( projectId: string | number, - name: string, - approvalsRequired: number, - { - mergerequestIid, - ...options - }: { mergerequestIid?: number } & ApprovalRulesRequestOptions & BaseRequestOptions, + options?: BaseRequestOptions, + ): Promise; + + configuration( + projectId: string | number, + options: { mergerequestIid: number } & BaseRequestOptions, + ): Promise; + + configuration( + projectId: string | number, + { mergerequestIid, ...options }: { mergerequestIid?: number } & BaseRequestOptions = {}, ) { const pId = encodeURIComponent(projectId); - let url: string; if (mergerequestIid) { const mIid = encodeURIComponent(mergerequestIid); - url = `projects/${pId}/merge_requests/${mIid}/approval_rules`; + url = `projects/${pId}/merge_requests/${mIid}/approvals`; } else { - url = `projects/${pId}/approval_rules`; + url = `projects/${pId}/approvals`; } - return RequestHelper.post()(this, url, { name, approvalsRequired, ...options }); + return RequestHelper.get()(this, url, options); } + editConfiguration( + projectId: string | number, + options?: BaseRequestOptions, + ): Promise; + + editConfiguration( + projectId: string | number, + options: { mergerequestIid: number } & BaseRequestOptions, + ): Promise; + + editConfiguration( + projectId: string | number, + { mergerequestIid, ...options }: { mergerequestIid?: number } & BaseRequestOptions = {}, + ) { + const pId = encodeURIComponent(projectId); + let url: string; + + if (mergerequestIid) { + const mIid = encodeURIComponent(mergerequestIid); + + url = `projects/${pId}/merge_requests/${mIid}/approvals`; + } else { + url = `projects/${pId}/approvals`; + } + + return RequestHelper.post()(this, url, options); + } + + approvalRule(projectId: string | number, approvalRuleId: number, options: BaseRequestOptions) { + const [pId, aId] = [projectId, approvalRuleId].map(encodeURIComponent); + + return RequestHelper.get()(this, `projects/${pId}/approval_rules/${aId}`, options); + } + + approvalRules( + projectId: string | number, + options?: BaseRequestOptions, + ): Promise; + + approvalRules( + projectId: string | number, + options: { mergerequestIid: number } & BaseRequestOptions, + ): Promise; + approvalRules( projectId: string | number, { mergerequestIid, ...options }: { mergerequestIid?: number } & BaseRequestOptions = {}, @@ -60,9 +150,28 @@ export class MergeRequestApprovals extends BaseResour return RequestHelper.get()(this, url, options); } - approvals( + addApprovalRule( projectId: string | number, - { mergerequestIid, ...options }: { mergerequestIid?: number } & BaseRequestOptions = {}, + name: string, + approvalsRequired: number, + options?: ApprovalRulesRequestOptions & BaseRequestOptions, + ): Promise; + + addApprovalRule( + projectId: string | number, + name: string, + approvalsRequired: number, + options: { mergerequestIid: number } & ApprovalRulesRequestOptions & BaseRequestOptions, + ): Promise; + + addApprovalRule( + projectId: string | number, + name: string, + approvalsRequired: number, + { + mergerequestIid, + ...options + }: { mergerequestIid?: number } & ApprovalRulesRequestOptions & BaseRequestOptions = {}, ) { const pId = encodeURIComponent(projectId); @@ -70,12 +179,12 @@ export class MergeRequestApprovals extends BaseResour if (mergerequestIid) { const mIid = encodeURIComponent(mergerequestIid); - url = `projects/${pId}/merge_requests/${mIid}/approvals`; + url = `projects/${pId}/merge_requests/${mIid}/approval_rules`; } else { - url = `projects/${pId}/approvals`; + url = `projects/${pId}/approval_rules`; } - return RequestHelper.get()(this, url, options); + return RequestHelper.post()(this, url, { name, approvalsRequired, ...options }); } approvalState( @@ -92,35 +201,21 @@ export class MergeRequestApprovals extends BaseResour ); } - approve( + editApprovalRule( projectId: string | number, - mergerequestIid: number, - options?: { sha?: string } & BaseRequestOptions, - ) { - const [pId, mIid] = [projectId, mergerequestIid].map(encodeURIComponent); + approvalRuleId: number, + name: string, + approvalsRequired: number, + options?: ApprovalRulesRequestOptions & BaseRequestOptions, + ): Promise; - return RequestHelper.post()(this, `projects/${pId}/merge_requests/${mIid}/approve`, options); - } - - approvers( + editApprovalRule( projectId: string | number, - approverIds: number[], - approverGroupIds: (string | number)[], - { mergerequestIid, ...options }: { mergerequestIid?: number } & BaseRequestOptions = {}, - ) { - const pId = encodeURIComponent(projectId); - - let url: string; - - if (mergerequestIid) { - const mIid = encodeURIComponent(mergerequestIid); - url = `projects/${pId}/merge_requests/${mIid}/approvers`; - } else { - url = `projects/${pId}/approvers`; - } - - return RequestHelper.put()(this, url, { approverIds, approverGroupIds, ...options }); - } + approvalRuleId: number, + name: string, + approvalsRequired: number, + options: { mergerequestIid: number } & ApprovalRulesRequestOptions & BaseRequestOptions, + ): Promise; editApprovalRule( projectId: string | number, @@ -146,28 +241,22 @@ export class MergeRequestApprovals extends BaseResour return RequestHelper.put()(this, url, { name, approvalsRequired, ...options }); } - editApprovals( + removeApprovalRule( projectId: string | number, - { mergerequestIid, ...options }: { mergerequestIid?: number } & BaseRequestOptions = {}, - ) { - const pId = encodeURIComponent(projectId); - - let url: string; - - if (mergerequestIid) { - const mIid = encodeURIComponent(mergerequestIid); - url = `projects/${pId}/merge_requests/${mIid}/approvals`; - } else { - url = `projects/${pId}/approvals`; - } - - return RequestHelper.post()(this, url, options); - } + approvalRuleId: number, + options?: Sudo, + ): Promise; removeApprovalRule( projectId: string | number, approvalRuleId: number, - { mergerequestIid, ...options }: { mergerequestIid?: number } & BaseRequestOptions = {}, + options: { mergerequestIid: number } & Sudo, + ): Promise; + + removeApprovalRule( + projectId: string | number, + approvalRuleId: number, + { mergerequestIid }: { mergerequestIid?: number } & Sudo = {}, ) { const [pId, aId] = [projectId, approvalRuleId].map(encodeURIComponent); @@ -180,12 +269,30 @@ export class MergeRequestApprovals extends BaseResour url = `projects/${pId}/approval_rules/${aId}`; } - return RequestHelper.del()(this, url, options); + return RequestHelper.del()(this, url); + } + + approve( + projectId: string | number, + mergerequestIid: number, + options?: { sha?: string } & BaseRequestOptions, + ) { + const [pId, mIid] = [projectId, mergerequestIid].map(encodeURIComponent); + + return RequestHelper.post()( + this, + `projects/${pId}/merge_requests/${mIid}/approve`, + options, + ); } unapprove(projectId: string | number, mergerequestIid: number, options?: Sudo) { const [pId, mIid] = [projectId, mergerequestIid].map(encodeURIComponent); - return RequestHelper.post()(this, `projects/${pId}/merge_requests/${mIid}/unapprove`, options); + return RequestHelper.post()( + this, + `projects/${pId}/merge_requests/${mIid}/unapprove`, + options, + ); } } diff --git a/packages/core/src/resources/types.ts b/packages/core/src/resources/types.ts index 3ef21538..1b61fef1 100644 --- a/packages/core/src/resources/types.ts +++ b/packages/core/src/resources/types.ts @@ -53,7 +53,15 @@ export { DiffSchema, MergeRequestSchema, } from './MergeRequests'; -export { MergeRequestApprovalSchema, ApprovalRulesRequestOptions } from './MergeRequestApprovals'; +export { + ProjectLevelMergeRequestApprovalSchema, + ApprovedByEntity, + MergeRequestLevelMergeRequestApprovalSchema, + ApprovalRulesRequestOptions, + ApprovalRuleSchema, + ProjectLevelApprovalRuleSchema, + MergeRequestLevelApprovalRuleSchema, +} from './MergeRequestApprovals'; export { MergeRequestNoteSchema } from './MergeRequestNotes'; export { PackageSchema, PackageFileSchema } from './Packages'; export { diff --git a/packages/core/test/unit/resources/MergeRequestApprovals.ts b/packages/core/test/unit/resources/MergeRequestApprovals.ts index a460f9ea..6610f91a 100644 --- a/packages/core/test/unit/resources/MergeRequestApprovals.ts +++ b/packages/core/test/unit/resources/MergeRequestApprovals.ts @@ -26,7 +26,102 @@ describe('Instantiating MergeRequestApprovals service', () => { }); }); -describe('MergeRequests.addApprovalRules', () => { +describe('MergeRequestApprovals.configuration', () => { + it('should request GET /projects/:id/approvals without options', async () => { + await service.configuration(3); + + expect(RequestHelper.get()).toHaveBeenCalledWith(service, 'projects/3/approvals', {}); + }); + + it('should request GET /projects/:id/approvals', async () => { + await service.configuration(3, { prop: 4 }); + + expect(RequestHelper.get()).toHaveBeenCalledWith(service, 'projects/3/approvals', { prop: 4 }); + }); + + it('should request GET /projects/:id/merge_requests/:merge_request_iid/approvals when mergerequestIid Id is passed', async () => { + await service.configuration(3, { mergerequestIid: 1, prop: 4 }); + + expect(RequestHelper.get()).toHaveBeenCalledWith( + service, + 'projects/3/merge_requests/1/approvals', + { prop: 4 }, + ); + }); +}); + +describe('MergeRequestApprovals.editConfiguration', () => { + it('should request POST /projects/:id/approvals without options', async () => { + await service.editConfiguration(3); + + expect(RequestHelper.post()).toHaveBeenCalledWith(service, 'projects/3/approvals', {}); + }); + + it('should request POST /projects/:id/approvals', async () => { + await service.editConfiguration(3, { prop: 4 }); + + expect(RequestHelper.post()).toHaveBeenCalledWith(service, 'projects/3/approvals', { prop: 4 }); + }); + + it('should request POST /projects/:id/merge_requests/:merge_request_iid/approvals when mergerequestIid Id is passed', async () => { + await service.editConfiguration(3, { mergerequestIid: 1, prop: 4 }); + + expect(RequestHelper.post()).toHaveBeenCalledWith( + service, + 'projects/3/merge_requests/1/approvals', + { prop: 4 }, + ); + }); +}); + +describe('MergeRequestApprovals.approvalRule', () => { + it('should request GET /projects/:id/approval_rules/:approval_rule_id', async () => { + await service.approvalRule(2, 4, { prop: 3 }); + + expect(RequestHelper.get()).toHaveBeenCalledWith(service, 'projects/2/approval_rules/4', { + prop: 3, + }); + }); +}); + +describe('MergeRequestApprovals.approvalRules', () => { + it('should request GET /projects/:id/approval_rules without options', async () => { + await service.approvalRules(2); + + expect(RequestHelper.get()).toHaveBeenCalledWith(service, 'projects/2/approval_rules', {}); + }); + + it('should request GET /projects/:id/approval_rules', async () => { + await service.approvalRules(2, { prop: 3 }); + + expect(RequestHelper.get()).toHaveBeenCalledWith(service, 'projects/2/approval_rules', { + prop: 3, + }); + }); + + it('should request GET /projects/:id/merge_requests/:merge_request_iid/approval_rules when mergerequestIid is passed', async () => { + await service.approvalRules(2, { mergerequestIid: 1, prop: 3 }); + + expect(RequestHelper.get()).toHaveBeenCalledWith( + service, + 'projects/2/merge_requests/1/approval_rules', + { + prop: 3, + }, + ); + }); +}); + +describe('MergeRequestApprovals.addApprovalRule', () => { + it('should request POST /projects/:id/approval_rules without options', async () => { + await service.addApprovalRule(2, 'Some rule', 5); + + expect(RequestHelper.post()).toHaveBeenCalledWith(service, 'projects/2/approval_rules', { + name: 'Some rule', + approvalsRequired: 5, + }); + }); + it('should request POST /projects/:id/approval_rules', async () => { await service.addApprovalRule(2, 'Some rule', 5, { userIds: [1, 2], @@ -63,92 +158,16 @@ describe('MergeRequests.addApprovalRules', () => { }); }); -describe('MergeRequests.approvals', () => { - it('should request GET /projects/:id/approvals', async () => { - await service.approvals(3); +describe('MergeRequestApprovals.editApprovalRule', () => { + it('should request PUT /projects/:id/approval_rules/:approval_rule_id without options', async () => { + await service.editApprovalRule(2, 30, 'Some rule', 5); - expect(RequestHelper.get()).toHaveBeenCalledWith(service, 'projects/3/approvals', {}); - }); - - it('should request GET /projects/:id/merge_requests/:merge_request_iid/approvals when mergerequestIid Id is passed', async () => { - await service.approvals(3, { mergerequestIid: 1 }); - - expect(RequestHelper.get()).toHaveBeenCalledWith( - service, - 'projects/3/merge_requests/1/approvals', - {}, - ); - }); -}); - -describe('MergeRequests.approvalRules', () => { - it('should request GET /projects/:id/approval_rules', async () => { - await service.approvalRules(2); - - expect(RequestHelper.get()).toHaveBeenCalledWith(service, 'projects/2/approval_rules', {}); - }); - - it('should request GET /projects/:id/merge_requests/:merge_request_iid/approval_rules when mergerequestIid Id is passed', async () => { - await service.approvalRules(2, { mergerequestIid: 3 }); - - expect(RequestHelper.get()).toHaveBeenCalledWith( - service, - 'projects/2/merge_requests/3/approval_rules', - {}, - ); - }); -}); - -describe('MergeRequests.approvalState', () => { - it('should request GET /projects/:id/merge_requests/:merge_request_iid/approval_state', async () => { - await service.approvalState(2, 3); - - expect(RequestHelper.get()).toHaveBeenCalledWith( - service, - 'projects/2/merge_requests/3/approval_state', - undefined, - ); - }); -}); - -describe('MergeRequests.approve', () => { - it('should request POST /projects/:id/merge_requests/:merge_request_iid/approve', async () => { - await service.approve(2, 3); - - expect(RequestHelper.post()).toHaveBeenCalledWith( - service, - 'projects/2/merge_requests/3/approve', - undefined, - ); - }); -}); - -describe('MergeRequests.approvers', () => { - it('should request PUT /projects/:id/approvers', async () => { - await service.approvers(3, [4, 5], [6, 7]); - - expect(RequestHelper.put()).toHaveBeenCalledWith(service, 'projects/3/approvers', { - approverIds: [4, 5], - approverGroupIds: [6, 7], + expect(RequestHelper.put()).toHaveBeenCalledWith(service, 'projects/2/approval_rules/30', { + name: 'Some rule', + approvalsRequired: 5, }); }); - it('should request PUT /projects/:id/merge_requests/:merge_request_iid/approvers when mergerequestIid Id is passed', async () => { - await service.approvers(3, [4, 5], [6, 7], { - approverIds: [4, 5], - approverGroupIds: [6, 7], - mergerequestIid: 1, - }); - - expect(RequestHelper.put()).toHaveBeenCalledWith( - service, - 'projects/3/merge_requests/1/approvers', - { approverIds: [4, 5], approverGroupIds: [6, 7] }, - ); - }); -}); - -describe('MergeRequests.editApprovalRules', () => { it('should request PUT /projects/:id/approval_rules/:approval_rule_id', async () => { await service.editApprovalRule(2, 30, 'Some rule', 5, { userIds: [1, 2], @@ -185,29 +204,11 @@ describe('MergeRequests.editApprovalRules', () => { }); }); -describe('MergeRequests.editApprovals', () => { - it('should request POST /projects/:id/approvals', async () => { - await service.editApprovals(3, { prop: 4 }); - - expect(RequestHelper.post()).toHaveBeenCalledWith(service, 'projects/3/approvals', { prop: 4 }); - }); - - it('should request POST /projects/:id/merge_requests/:merge_request_iid/approvals when mergerequestIid Id is passed', async () => { - await service.editApprovals(3, { mergerequestIid: 1, prop: 4 }); - - expect(RequestHelper.post()).toHaveBeenCalledWith( - service, - 'projects/3/merge_requests/1/approvals', - { prop: 4 }, - ); - }); -}); - -describe('MergeRequests.removeApprovalRules', () => { +describe('MergeRequestApprovals.removeApprovalRule', () => { it('should request DELETE /projects/:id/approval_rules/:approval_rule_id', async () => { await service.removeApprovalRule(2, 30); - expect(RequestHelper.del()).toHaveBeenCalledWith(service, 'projects/2/approval_rules/30', {}); + expect(RequestHelper.del()).toHaveBeenCalledWith(service, 'projects/2/approval_rules/30'); }); it('should request DELETE /projects/:id/merge_requests/:merge_request_iid/approval_rules/:approval_rule_id when mergerequestIid is passed', async () => { @@ -216,12 +217,35 @@ describe('MergeRequests.removeApprovalRules', () => { expect(RequestHelper.del()).toHaveBeenCalledWith( service, 'projects/2/merge_requests/3/approval_rules/30', - {}, ); }); }); -describe('MergeRequests.unapprove', () => { +describe('MergeRequestApprovals.approvalState', () => { + it('should request GET /projects/:id/merge_requests/:merge_request_iid/approval_state', async () => { + await service.approvalState(2, 3); + + expect(RequestHelper.get()).toHaveBeenCalledWith( + service, + 'projects/2/merge_requests/3/approval_state', + undefined, + ); + }); +}); + +describe('MergeRequestApprovals.approve', () => { + it('should request POST /projects/:id/merge_requests/:merge_request_iid/approve', async () => { + await service.approve(2, 3); + + expect(RequestHelper.post()).toHaveBeenCalledWith( + service, + 'projects/2/merge_requests/3/approve', + undefined, + ); + }); +}); + +describe('MergeRequestApprovals.unapprove', () => { it('should request POST /projects/:id/merge_requests/:merge_request_iid/unapprove', async () => { await service.unapprove(2, 3);