Skip to content

Commit

Permalink
Fix bug in getActiveResidentCount
Browse files Browse the repository at this point in the history
  • Loading branch information
@kronosapiens
kronosapiens committed Jul 15, 2023
1 parent 891af76 commit 603b918
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 69 deletions.
1 change: 1 addition & 0 deletions migrations/20200329100540_ChoreBreak.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ exports.up = function(knex, Promise) {
return knex.schema.createTable('ChoreBreak', function(t) {
t.increments('id').unsigned().primary();
t.timestamps(useTimestamps = true, defaultToNow = true, useCamelCase = true);
t.string('houseId').references('House.slackId').notNull();
t.string('residentId').references('Resident.slackId').notNull();
t.date('startDate').notNull();
t.date('endDate').notNull();
Expand Down
2 changes: 1 addition & 1 deletion src/bolt/chores.app.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ app.view('chores-break-callback', async ({ ack, body }) => {
await common.postEphemeral(app, choresOauth, choresChannel, residentId, text);
} else {
// Record the break
await Chores.addChoreBreak(residentId, breakStart, breakEnd);
await Chores.addChoreBreak(houseId, residentId, breakStart, breakEnd);
const text = `<@${residentId}> is taking a *${breakDays}-day* break ` +
`starting ${breakStart.toDateString()} :beach_with_umbrella:\n` +
`_${circumstance}_`;
Expand Down
30 changes: 16 additions & 14 deletions src/core/chores.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,8 @@ exports.updateChoreValues = async function (houseId, updateTime) {
// If we've updated in the last interval, short-circuit execution
if (intervalScalar === 0) { return Promise.resolve([]); }

const [ residentCount ] = await exports.getActiveResidentCount(houseId, updateTime);
const updateScalar = (residentCount.count * pointsPerResident) * intervalScalar * inflationFactor;
const residentCount = await exports.getActiveResidentCount(houseId, updateTime);
const updateScalar = (residentCount * pointsPerResident) * intervalScalar * inflationFactor;
const choreRankings = await exports.getCurrentChoreRankings(houseId);

const choreValues = choreRankings.map(chore => {
Expand All @@ -157,7 +157,7 @@ exports.updateChoreValues = async function (houseId, updateTime) {
valuedAt: updateTime,
value: chore.ranking * updateScalar,
ranking: chore.ranking,
residents: residentCount.count
residents: residentCount
};
});

Expand Down Expand Up @@ -258,9 +258,9 @@ exports.getAllChorePoints = async function (claimedBy, startTime, endTime) {

// Chore Breaks

exports.addChoreBreak = async function (residentId, startDate, endDate) {
exports.addChoreBreak = async function (houseId, residentId, startDate, endDate) {
return db('ChoreBreak')
.insert({ residentId, startDate, endDate })
.insert({ houseId, residentId, startDate, endDate })
.returning('*');
};

Expand All @@ -270,16 +270,18 @@ exports.deleteChoreBreak = async function (choreBreakId) {
.del();
};

exports.getChoreBreaks = async function (houseId, now) {
return db('ChoreBreak')
.where({ houseId })
.where('startDate', '<=', now)
.where('endDate', '>', now)
.returning('*');
};

exports.getActiveResidentCount = async function (houseId, now) {
return db('Resident')
.fullOuterJoin('ChoreBreak', 'Resident.slackId', 'ChoreBreak.residentId')
.where('Resident.houseId', houseId)
.where('Resident.active', true)
.where(function () { // Want residents NOT currently on a break
this.where('ChoreBreak.startDate', '>', now).orWhereNull('ChoreBreak.startDate')
.orWhere('ChoreBreak.endDate', '<=', now).orWhereNull('ChoreBreak.endDate');
})
.countDistinct('Resident.slackId');
const residents = await Admin.getResidents(houseId);
const choreBreaks = await exports.getChoreBreaks(houseId, now);
return residents.length - (new Set(choreBreaks.map(cb => cb.residentId))).size;
};

exports.getActiveResidentPercentage = async function (residentId, now) {
Expand Down
126 changes: 72 additions & 54 deletions test/chores.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -541,9 +541,9 @@ describe('Chores', async () => {
await Chores.claimChore(HOUSE, restock.id, RESIDENT3, feb1);

// Everyone takes half the month off
await Chores.addChoreBreak(RESIDENT1, feb1, feb15);
await Chores.addChoreBreak(RESIDENT2, feb1, feb15);
await Chores.addChoreBreak(RESIDENT3, feb1, feb15);
await Chores.addChoreBreak(HOUSE, RESIDENT1, feb1, feb15);
await Chores.addChoreBreak(HOUSE, RESIDENT2, feb1, feb15);
await Chores.addChoreBreak(HOUSE, RESIDENT3, feb1, feb15);

let penalty;
const penaltyTime = new Date(getNextMonthStart(feb1).getTime() + penaltyDelay);
Expand Down Expand Up @@ -596,63 +596,81 @@ describe('Chores', async () => {
await Admin.addResident(HOUSE, RESIDENT2, now);
});

it('can add a chore break', async () => {
let breakCount;
[ breakCount ] = await db('ChoreBreak').count('*');
expect(parseInt(breakCount.count)).to.equal(0);
it('can add, query, and delete chore breaks', async () => {
const oneDay = new Date(now.getTime() + 1 * DAY);
const twoDays = new Date(now.getTime() + 2 * DAY);

await Chores.addChoreBreak(RESIDENT1, now, challengeEnd);
let choreBreaks;
choreBreaks = await Chores.getChoreBreaks(HOUSE, now);
expect(choreBreaks.length).to.equal(0);

[ breakCount ] = await db('ChoreBreak').count('*');
expect(parseInt(breakCount.count)).to.equal(1);
});

it('can delete a chore break', async () => {
let breakCount;
[ breakCount ] = await db('ChoreBreak').count('*');
expect(parseInt(breakCount.count)).to.equal(0);

const [ choreBreak ] = await Chores.addChoreBreak(RESIDENT1, now, challengeEnd);
await Chores.addChoreBreak(HOUSE, RESIDENT1, now, oneDay);
choreBreaks = await Chores.getChoreBreaks(HOUSE, now);
expect(choreBreaks.length).to.equal(1);

[ breakCount ] = await db('ChoreBreak').count('*');
expect(parseInt(breakCount.count)).to.equal(1);
await Chores.deleteChoreBreak(choreBreaks[0].id);
choreBreaks = await Chores.getChoreBreaks(HOUSE, now);
expect(choreBreaks.length).to.equal(0);

await Chores.deleteChoreBreak(choreBreak.id);

[ breakCount ] = await db('ChoreBreak').count('*');
expect(parseInt(breakCount.count)).to.equal(0);
await Chores.addChoreBreak(HOUSE, RESIDENT2, now, oneDay);
await Chores.addChoreBreak(HOUSE, RESIDENT2, now, twoDays);
choreBreaks = await Chores.getChoreBreaks(HOUSE, now);
expect(choreBreaks.length).to.equal(2);
choreBreaks = await Chores.getChoreBreaks(HOUSE, oneDay);
expect(choreBreaks.length).to.equal(1);
choreBreaks = await Chores.getChoreBreaks(HOUSE, twoDays);
expect(choreBreaks.length).to.equal(0);
});

it('can exclude inactive residents from the chore valuing', async () => {
await Admin.addResident(HOUSE, RESIDENT3, now);
await Admin.addResident(HOUSE, RESIDENT4, now);

const later = new Date(now.getTime() + 2 * DAY);
const oneDay = new Date(now.getTime() + 1 * DAY);
const twoDays = new Date(now.getTime() + 2 * DAY);
const oneWeek = new Date(now.getTime() + 7 * DAY);
const twoWeeks = new Date(now.getTime() + 14 * DAY);
const lastMonth = new Date(now.getTime() - 35 * DAY);
const nextMonth = new Date(now.getTime() + 35 * DAY);
const twoMonths = new Date(now.getTime() + 60 * DAY);

let residentCount;
[ residentCount ] = await Chores.getActiveResidentCount(HOUSE, soon);
expect(parseInt(residentCount.count)).to.equal(3);
residentCount = await Chores.getActiveResidentCount(HOUSE, now);
expect(residentCount).to.equal(4);

await Chores.addChoreBreak(RESIDENT1, now, challengeEnd);
// Will exclude inactive residents
await Admin.deleteResident(HOUSE, RESIDENT4);
residentCount = await Chores.getActiveResidentCount(HOUSE, now);
expect(residentCount).to.equal(3);

[ residentCount ] = await Chores.getActiveResidentCount(HOUSE, soon);
expect(parseInt(residentCount.count)).to.equal(2);
// Will count active breaks
await Chores.addChoreBreak(HOUSE, RESIDENT1, now, twoDays);
residentCount = await Chores.getActiveResidentCount(HOUSE, now);
expect(residentCount).to.equal(2);

// After the break ends
[ residentCount ] = await Chores.getActiveResidentCount(HOUSE, later);
expect(parseInt(residentCount.count)).to.equal(3);
// Can handle overlapping breaks
await Chores.addChoreBreak(HOUSE, RESIDENT1, now, oneDay);
residentCount = await Chores.getActiveResidentCount(HOUSE, now);
expect(residentCount).to.equal(2);

// Will also exclude if inactive
await Admin.deleteResident(HOUSE, RESIDENT3);
[ residentCount ] = await Chores.getActiveResidentCount(HOUSE, later);
expect(parseInt(residentCount.count)).to.equal(2);
// Can handle new breaks by the same user
await Chores.addChoreBreak(HOUSE, RESIDENT1, oneWeek, twoWeeks);
residentCount = await Chores.getActiveResidentCount(HOUSE, oneWeek);
expect(residentCount).to.equal(2);

// Will also exclude if break extends across months
const lastMonth = new Date(now.getTime() - 35 * DAY);
const nextMonth = new Date(now.getTime() + 35 * DAY);
await Chores.addChoreBreak(RESIDENT2, lastMonth, nextMonth);
await Chores.addChoreBreak(HOUSE, RESIDENT2, lastMonth, nextMonth);
residentCount = await Chores.getActiveResidentCount(HOUSE, now);
expect(residentCount).to.equal(1);

// Will not count breaks in the past
residentCount = await Chores.getActiveResidentCount(HOUSE, twoMonths);
expect(residentCount).to.equal(3);

[ residentCount ] = await Chores.getActiveResidentCount(HOUSE, later);
expect(parseInt(residentCount.count)).to.equal(1);
// Will not count breaks in the future
await Chores.addChoreBreak(HOUSE, RESIDENT3, oneDay, oneWeek);
residentCount = await Chores.getActiveResidentCount(HOUSE, now);
expect(residentCount).to.equal(1);
});

it('can return the percent of the period a resident is not on break', async () => {
Expand All @@ -670,27 +688,27 @@ describe('Chores', async () => {
expect(activeDays).to.equal(1);

// Take the first week off
await Chores.addChoreBreak(RESIDENT1, feb1, feb8);
await Chores.addChoreBreak(HOUSE, RESIDENT1, feb1, feb8);
activeDays = await Chores.getActiveResidentPercentage(RESIDENT1, feb1);
expect(activeDays).to.equal(0.75);

// Take the third week off
await Chores.addChoreBreak(RESIDENT1, feb15, feb22);
await Chores.addChoreBreak(HOUSE, RESIDENT1, feb15, feb22);
activeDays = await Chores.getActiveResidentPercentage(RESIDENT1, feb1);
expect(activeDays).to.equal(0.5);

// Take time off next month, has no effect
await Chores.addChoreBreak(RESIDENT1, mar1, mar15);
await Chores.addChoreBreak(HOUSE, RESIDENT1, mar1, mar15);
activeDays = await Chores.getActiveResidentPercentage(RESIDENT1, feb1);
expect(activeDays).to.equal(0.5);

// Take the first two weeks off, this break overlaps with the first break
await Chores.addChoreBreak(RESIDENT1, feb1, feb15);
await Chores.addChoreBreak(HOUSE, RESIDENT1, feb1, feb15);
activeDays = await Chores.getActiveResidentPercentage(RESIDENT1, feb1);
expect(activeDays).to.equal(0.25);

// Take the last week off, this break stretches into the next month
await Chores.addChoreBreak(RESIDENT1, feb22, mar8);
await Chores.addChoreBreak(HOUSE, RESIDENT1, feb22, mar8);
activeDays = await Chores.getActiveResidentPercentage(RESIDENT1, feb1);
expect(activeDays).to.equal(0.0);
});
Expand All @@ -705,12 +723,12 @@ describe('Chores', async () => {
let activeDays;

// Overlap last and first weeks
await Chores.addChoreBreak(RESIDENT1, jan25, feb8);
await Chores.addChoreBreak(HOUSE, RESIDENT1, jan25, feb8);
activeDays = await Chores.getActiveResidentPercentage(RESIDENT1, feb1);
expect(activeDays).to.equal(0.75);

// Overlap last and first weeks
await Chores.addChoreBreak(RESIDENT1, feb22, mar8);
await Chores.addChoreBreak(HOUSE, RESIDENT1, feb22, mar8);
activeDays = await Chores.getActiveResidentPercentage(RESIDENT1, feb1);
expect(activeDays).to.equal(0.5);
});
Expand All @@ -721,7 +739,7 @@ describe('Chores', async () => {
const jan25 = new Date(feb1.getTime() - 7 * DAY);

// Add a six-week break
await Chores.addChoreBreak(RESIDENT1, jan25, mar8);
await Chores.addChoreBreak(HOUSE, RESIDENT1, jan25, mar8);
const activeDays = await Chores.getActiveResidentPercentage(RESIDENT1, feb1);
expect(activeDays).to.equal(0);
});
Expand All @@ -742,7 +760,7 @@ describe('Chores', async () => {
expect(activeDays).to.equal(1);

// Add a six-week break from feb into april (6 day break)
await Chores.addChoreBreak(RESIDENT1, feb15, apr7);
await Chores.addChoreBreak(HOUSE, RESIDENT1, feb15, apr7);
activeDays = await Chores.getActiveResidentPercentage(RESIDENT1, feb15);
expect(activeDays).to.equal(0.5);

Expand All @@ -753,12 +771,12 @@ describe('Chores', async () => {
expect(activeDays).to.equal(0.8);

// Add a week-long break mid-april (12 day break)
await Chores.addChoreBreak(RESIDENT1, apr10, apr22);
await Chores.addChoreBreak(HOUSE, RESIDENT1, apr10, apr22);
activeDays = await Chores.getActiveResidentPercentage(RESIDENT1, apr1);
expect(activeDays).to.equal(0.4);

// Add a two-week break spanning april and may (6 day break)
await Chores.addChoreBreak(RESIDENT1, apr25, may5);
await Chores.addChoreBreak(HOUSE, RESIDENT1, apr25, may5);
activeDays = await Chores.getActiveResidentPercentage(RESIDENT1, apr1);
expect(activeDays).to.equal(0.2);
});
Expand All @@ -778,7 +796,7 @@ describe('Chores', async () => {
expect(activeDays).to.equal(0.75);

// Can combine with regular breaks
await Chores.addChoreBreak(RESIDENT3, feb22, mar1);
await Chores.addChoreBreak(HOUSE, RESIDENT3, feb22, mar1);
activeDays = await Chores.getActiveResidentPercentage(RESIDENT3, feb1);
expect(activeDays).to.equal(0.5);
});
Expand Down

0 comments on commit 603b918

Please sign in to comment.