bug - multiple timesheet entries created ¶

CATS-613

Reported on 11/9/21.

background ¶

Multiple timesheet entries are being created for the same shift, which is causing clock in/out and reporting issues. This seems to have only started recently, and while there are 30+ entries where this has occurred, there’s only 2 shifts that are affecting real users - the rest are test shifts, mostly created by Meg. Unsure of root yet.

The ‘real’ affected shifts so far

jacqueline.shen@indevets.com,18938,c9d2dd2b-d26a-4ed8-8ccf-4ce3b0422f25,2021-11-08 09:00:00,2021-11-08 17:00:00,2021-10-25 20:39:30,2021-11-10 16:20:34
jacqueline.shen@indevets.com,18940,c9d2dd2b-d26a-4ed8-8ccf-4ce3b0422f25,2021-11-08 09:00:00,2021-11-08 17:00:00,2021-10-25 20:40:17,2021-11-03 15:46:56
Marisa.Brunetti@IndeVets.com,18937,ea458b62-6d33-4e3f-b84f-07558fd5923a,2021-11-15 10:00:00,2021-11-15 18:00:00,2021-10-25 20:39:24,2021-11-03 15:46:47
Marisa.Brunetti@IndeVets.com,18939,ea458b62-6d33-4e3f-b84f-07558fd5923a,2021-11-15 10:00:00,2021-11-15 18:00:00,2021-10-25 20:39:30,2021-11-03 15:46:47
Marisa.Brunetti@IndeVets.com,18941,ea458b62-6d33-4e3f-b84f-07558fd5923a,2021-11-15 10:00:00,2021-11-15 18:00:00,2021-10-25 20:40:17,2021-11-03 15:46:47

Both ‘bad’ entries related to created timestamps of 2021-10-25 20:39:30 and 2021-10-25 20:40:17

Both of these were created within … 45 seconds of each other.

SQL to find

select u.email, tse.id, tse.shift_id, tse.scheduled_start, tse.scheduled_end,
       tse.created_at, tse.updated_at from timesheetentry tse
           left join timesheet t on tse.timesheet_id = t.id
           left join employees e on t.employee_id = e.id
           left join users u on e.user_id = u.id
    where
        shift_id in (
            select tse.shift_id from timesheetentry tse
                where tse.deleted_at is null
                group by shift_id
                having count(tse.shift_id) > 1
        )
  and tse.deleted_at is null
order by shift_id, created_at

code ¶

There is only one spot in the code where a new timesheet entry record is created - TimesheetService::createTimesheetEntryForShift

    public function createTimesheetEntryForShift(Timesheet $timesheet, Shift $shift): ?TimesheetEntry

This is only called from one spot in the code: TimesheetService::getTimesheetEntryForShift

    public function getTimesheetEntryForShift(
        Shift $shift,
        Timesheet $timesheet = null,
        $createIfEmpty = true
    ): ?TimesheetEntry {

There is a ‘createIfEmpty’ flag which defaults to true, meaning this will create a timesheet entry record. There’s seemingly no way for a timesheet entry for a shift to be created duplicate times.

getTimesheetEntryForShift is called from a few places.

The PayrollAdjustmentController::getAdjustmentsForShift endpoint, called by the timesheet screen, calls this $timesheetEntry = $timesheetService->getTimesheetEntryForShift($shift, null, null);

and… passes a ‘null’ which seems like an unknown value/behaviour for the getTimesheetEntryForShift() code.

ShiftService::canClockIn calls getTimesheetEntryForShift to check if a timesheet entry exists, and if so, checks if the period is closed, and will throw a message back indicating the timeperiod is closed.

TimesheetService::updateTimesheetsInPeriod calls getTimesheetEntryForShift without a false ‘create’ flag.

theories ¶

A. Call to updateTimesheetsInPeriod was made multiple times, and although an entry was created, a transaction had not finished operating, and the second time the updateTimesheetsInPeriod call was made, the logic to detect is a timesheet entry for shift X existed returned false, and a second entry was created.

This is not as likely - if it was the case, we’d have likely seen a lot more of these.

B. PayrollAdjustmentController::getAdjustmentsForShift endpoint was called when looked at the timesheet screen for
to endpoint. These are called during individual user views of the timesheet screen. If someone had triggered multiple views of a user’s timesheet - say, by viewing Dr X, then switching to Dr Y, then back to Dr X again, the calls may have run somewhat in parallel, and the earlier transactions were still ‘locked’, causing subsequent calls to not see the entry from the first transaction, then creating a second one.

In both scenarios, the culprit is some other workload causing enough delay which triggers a subsequent call to the same behaviour (creating a timesheet) which would not see the first timesheet created (because the workload is casing enough delay to prevent the first transaction from finishing).

Given that this only happened 2x to real users under ‘regular’ usage, it’s harder to determine the precise steps which triggered this. Most of the examples we see were Meg creating test shifts for test doctors.

remediation steps ¶

audit timesheet entry ¶

Unfortunately, a root is harder to determine as the ‘timesheetentry’ model is not audited/logged currently.
This needs to be fixed.

manually delete the offending timesheet entries ¶

Need to review existing duplicated timesheet entries and delete the later/unused ones.

remove null flag in endpoint call ¶

PayrollAdjustmentController::getAdjustmentsForShift passes a ‘null’ to the ‘create’ behaviour flag - this should be resolved to true or false - need to review behaviour/code and make determination.