Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Payroll: cover cases for moving extra pay from payday into accrued salary #913

Merged
merged 5 commits into from
Jul 9, 2019
Merged
Changes from 4 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: 27 additions & 20 deletions future-apps/payroll/test/contracts/Payroll_payday.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -129,12 +129,20 @@ contract('Payroll payday', ([owner, employee, anyone]) => {
const assertEmployeeIsUpdatedCorrectly = (requestedAmount, expectedRequestedAmount) => {
it('updates the accrued salary and the last payroll date', async () => {
let expectedLastPayrollDate, expectedAccruedSalary
const [previousAccruedSalary, , , previousPayrollDate] = (await payroll.getEmployee(employeeId)).slice(2, 6)
const [employeeSalary, previousAccruedSalary, , , previousPayrollDate] = (await payroll.getEmployee(employeeId)).slice(1, 6)

if (expectedRequestedAmount >= previousAccruedSalary) {
expectedAccruedSalary = bn(0)
if (expectedRequestedAmount.gte(previousAccruedSalary)) {
const remainder = expectedRequestedAmount.minus(previousAccruedSalary)
expectedLastPayrollDate = previousPayrollDate.plus(remainder.div(salary).ceil())
if (remainder.eq(0)) {
expectedAccruedSalary = bn(0)
} else {
// Have remaining salary that needs to be put back into the accrued salary
expectedAccruedSalary =
remainder.lt(employeeSalary)
? salary.minus(remainder)
: expectedAccruedSalary = remainder.mod(employeeSalary)
}
expectedLastPayrollDate = previousPayrollDate.plus(remainder.div(employeeSalary).ceil())
} else {
expectedAccruedSalary = previousAccruedSalary.minus(expectedRequestedAmount).toString()
expectedLastPayrollDate = previousPayrollDate
Expand Down Expand Up @@ -257,7 +265,7 @@ contract('Payroll payday', ([owner, employee, anyone]) => {
await payroll.terminateEmployee(employeeId, await payroll.getTimestampPublic(), { from: owner })
})

if (requestedAmount.eq(0) || requestedAmount === totalOwedAmount) {
if (requestedAmount.eq(0) || requestedAmount.eq(totalOwedAmount)) {
context('when exchange rates are not expired', () => {
assertTransferredAmounts(requestedAmount, expectedRequestedAmount)

Expand Down Expand Up @@ -301,20 +309,13 @@ contract('Payroll payday', ([owner, employee, anyone]) => {

context('when the requested amount is lower than the total owed salary', () => {
context('when the requested amount represents less than a second of the earnings', () => {
const requestedAmount = salary.div(2)
const requestedAmount = salary.div(2).floor()
Copy link
Contributor

@facuspagnuolo facuspagnuolo Jul 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what I noticed yesterday, after turning the base salary to 1 USD per second, these amount calculations became flaky


it('updates the last payroll date by one second', async () => {
const previousLastPayrollDate = (await payroll.getEmployee(employeeId))[5]

await payroll.payday(PAYMENT_TYPES.PAYROLL, requestedAmount, { from })

const currentLastPayrollDate = (await payroll.getEmployee(employeeId))[5]
assert.equal(currentLastPayrollDate.toString(), previousLastPayrollDate.plus(1).toString(), 'last payroll date does not match')
})
itHandlesPayrollProperlyNeverthelessExtrasOwedAmounts(requestedAmount, currentOwedSalary)
})

context('when the requested amount represents more than a second of the earnings', () => {
const requestedAmount = currentOwedSalary.div(2)
const requestedAmount = currentOwedSalary.div(2).ceil()

itHandlesPayrollProperlyNeverthelessExtrasOwedAmounts(requestedAmount, currentOwedSalary)
})
Expand Down Expand Up @@ -385,8 +386,14 @@ contract('Payroll payday', ([owner, employee, anyone]) => {
itHandlesPayrollProperlyNeverthelessExtrasOwedAmounts(requestedAmount, totalOwedSalary)
})

context('when the requested amount is greater than the previous owed salary but lower than the total owed', () => {
const requestedAmount = totalOwedSalary.div(2)
context('when the requested amount is greater than the previous owed salary but greater than one second of additional salary', () => {
sohkai marked this conversation as resolved.
Show resolved Hide resolved
const requestedAmount = previousOwedSalary.plus(salary).minus(1)

itHandlesPayrollProperlyNeverthelessExtrasOwedAmounts(requestedAmount, totalOwedSalary)
})

context('when the requested amount is greater than the previous owed salary but greater than one second of additional salary', () => {
const requestedAmount = previousOwedSalary.plus(salary).plus(1)

itHandlesPayrollProperlyNeverthelessExtrasOwedAmounts(requestedAmount, totalOwedSalary)
})
Expand Down Expand Up @@ -414,7 +421,7 @@ contract('Payroll payday', ([owner, employee, anyone]) => {
})

context('when the requested amount is lower than the previous owed salary', () => {
const requestedAmount = previousOwedSalary.div(2)
const requestedAmount = previousOwedSalary.div(2).floor()

itHandlesPayrollProperlyNeverthelessExtrasOwedAmounts(requestedAmount, previousOwedSalary)
})
Expand Down Expand Up @@ -445,7 +452,7 @@ contract('Payroll payday', ([owner, employee, anyone]) => {
})

context('when the requested amount is lower than the total owed salary', () => {
const requestedAmount = owedSalary.div(2)
const requestedAmount = owedSalary.div(2).floor()

it('reverts', async () => {
await assertRevert(payroll.payday(PAYMENT_TYPES.PAYROLL, requestedAmount, { from }), 'PAYROLL_DISTRIBUTION_NOT_FULL')
Expand Down Expand Up @@ -694,7 +701,7 @@ contract('Payroll payday', ([owner, employee, anyone]) => {
}

const assertEmployeeIsUpdated = requestedAmount => {
it('updates the last payroll date', async () => {
it('updates the employee accounting', async () => {
const timeDiff = 1 // should be bn(requestedAmount).div(salary).ceil() but BN cannot represent such a small number, hardcoding it to 1
const [previousAccruedSalary, , , previousPayrollDate] = (await payroll.getEmployee(employeeId)).slice(2, 6)

Expand Down