Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 38 additions & 9 deletions __tests__/unit/validators/approvals.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -637,8 +637,8 @@ test('returns proper error when team provided is not found', async () => {
}

let validation = await approval.processValidate(createMockContext(5, reviewList, null, null, false), settings, [])
expect(validation.validations.length).toBe(1)
expect(validation.validations[0].description).toBe('approval: userA required')
expect(validation.validations.length).toBe(2)
expect(validation.validations[1].description).toBe('approval: userA required')
})

describe('required.owners ', () => {
Expand Down Expand Up @@ -679,9 +679,9 @@ describe('required.owners ', () => {
Teams.extractTeamMembers = jest.fn().mockReturnValue(['userB'])

let validation = await approval.processValidate(createMockContext(5, reviewList, null, null, false), settings)
expect(validation.validations.length).toBe(1)
expect(validation.validations.length).toBe(2)
expect(validation.status).toBe('fail')
expect(validation.validations[0].description).toBe('approval: userA required')
expect(validation.validations[1].description).toBe('approval: userA required')
})

test('limit.users option works properly', async () => {
Expand Down Expand Up @@ -716,9 +716,9 @@ describe('required.owners ', () => {

let validation = await approval.processValidate(createMockContext(5, reviewList, null, null, false), settings)

expect(validation.validations.length).toBe(1)
expect(validation.validations.length).toBe(2)
expect(validation.status).toBe('fail')
expect(validation.validations[0].description).toBe('approval count is less than "2"')
expect(validation.validations[1].description).toBe('approval count is less than "2"')

settings = {
do: 'approval',
Expand All @@ -732,7 +732,7 @@ describe('required.owners ', () => {

validation = await approval.processValidate(createMockContext(5, reviewList, null, null, false), settings)

expect(validation.validations.length).toBe(1)
expect(validation.validations.length).toBe(2)
expect(validation.status).toBe('pass')
})

Expand Down Expand Up @@ -769,9 +769,38 @@ describe('required.owners ', () => {
Owners.process = jest.fn().mockReturnValue(['userB'])
let validation = await approval.processValidate(createMockContext(5, reviewList, null, null, false), settings)

expect(validation.validations.length).toBe(1)
expect(validation.validations.length).toBe(2)
expect(validation.status).toBe('fail')
expect(validation.validations[0].description).toBe('approval: userA required')
expect(validation.validations[1].description).toBe('approval: userA required')
})

test('limit option will create an "info" validation', async () => {
const approval = new Approval()

const reviewList = [
{
user: {
login: 'userA'
},
state: 'APPROVED',
submitted_at: Date.now()
}
]

const settings = {
do: 'approval',
limit: {
owners: true
}
}

Owners.process = jest.fn().mockReturnValue(['userB'])
let validation = await approval.processValidate(createMockContext(5, reviewList, null, null, false), settings)

expect(validation.validations.length).toBe(1)
expect(validation.status).toBe('pass')
expect(validation.validations[0].status).toBe('info')
expect(validation.validations[0].description).toBe('Only approvals from following sources are counted')
})
})

Expand Down
1 change: 1 addition & 0 deletions docs/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ CHANGELOG
| February 25, 2021 : feat: add feature to limit approval to a list of users `#509 <https://github.com/mergeability/mergeable/issues/509>`_
| February 25, 2021 : fix: minor bugs `#508 <https://github.com/mergeability/mergeable/pull/508>`_
| February 25, 2021 : fix: Correct use of cache env
| February 22, 2021 : feat: add a info panel for 'limit' option in `approval` validator `#509 <https://github.com/mergeability/mergeable/issues/509#issuecomment-783346365>`_
| February 21, 2021 : feat: `name` sub-option for `repository` filter
| February 18, 2021 : fix: Scheduler support `#499 <https://github.com/mergeability/mergeable/issues/499>`_
| February 12, 2021 : feat: Implemented redis as a dependency to the helm-chart
Expand Down
2 changes: 2 additions & 0 deletions lib/actions/handlebars/populateTemplate.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ handlebars.registerHelper('statusIcon', function (str) {
return ':x:'
case 'error':
return ':heavy_exclamation_mark:'
case 'info':
return ':information_source:'
default:
return `Unknown Status given: ${str}`
}
Expand Down
6 changes: 5 additions & 1 deletion lib/validators/approvals.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ class Approvals extends Validator {

let approvedReviewers = findReviewersByState(reviews, 'approved')

let output = []
// if limit is provided, we only filter the approved Reviewers to members of the teams provided
if (limitOption) {
let owners = []
Expand All @@ -99,12 +100,15 @@ class Approvals extends Validator {
}

teamMembers = _.union(teamMembers, owners)
let validatorContext = { name: 'approvals' }
if (limitOption.users) teamMembers = _.union(teamMembers, limitOption.users)

approvedReviewers = _.intersection(approvedReviewers, teamMembers)
output.push(constructOutput(validatorContext, approvedReviewers, limitOption, {status: 'info', description: `Only approvals from following sources are counted`}))
}

let output = await this.processOptions(validationSettings, approvedReviewers)
const optionProcessed = await this.processOptions(validationSettings, approvedReviewers)
output = [...output, ...optionProcessed]

if (blockOption && blockOption.changes_requested) {
output.push(processBlockOption(blockOption, reviews))
Expand Down