[Event Request] GB Codeunit 950 Time Sheet Management.

[LewisHyett]

Why do you need this change?

GB Codeunit 950 Time Sheet Management.

proecdure CheckAccPeriod

Please add an OnBefore event to the procedure CheckAccPeriod to allow us to bypass this check. We want to allow the users to reopen and resubmit a timesheet dated in a closed period.

Describe the request

    procedure CheckAccPeriod(Date: Date)
    var
        AccountingPeriod: Record "Accounting Period";
        Ishandled: Boolean;
    begin
         OnBeforeCheckAccPeriod(Date, IsHandled);
        if IsHandled then
            exit;

        if AccountingPeriod.IsEmpty() then
            exit;
        AccountingPeriod.SetFilter("Starting Date", '..%1', Date);
        AccountingPeriod.FindLast();
        AccountingPeriod.TestField(Closed, false);
    end;

[IntegrationEvent(false,false)]
local procedure OnBeforeCheckAccPeriod(DateToCheck: Date; var IsHandled: Boolean)
begin
end;

Internal work item: AB#596439

[nikolakukrika]

@LewisHyett This is rather bad. First we are skipping validation which is not good. Why do we need to skip the validation?
Could we get a new method to reopen and resubmit a timesheet instead of IsHandled event?

General feedback on IsHandled events is:
Using IsHandled events is making the codebase harder to maintain and is introducing fragile solutions.
The main problem of IsHandled pattern is that it is disabling large sections of code, which can easily break other implementations that expect athat the code will run. Often other events are not raised.
Any changes that are done to the code that is skipped, will be ignored by the code that is using the IsHandled pattern, we often see that this would lead to unpredictable side-effects, because new code did not run.
Another problem of the event is if multiple extensions are trying to subscribe to IsHandled event. By pattern definition - only a single extension can handle the event, other extensions should exit, the code below should be the first line:
if IsHandled then
exit;
Lastly when we are refactoring code, it is difficult to maintain the exact logic that the extension using these events is expecting. The name of the event is not clear; thus we do not know in which scenarios the event is used. If the solution depends on very specific code to run before the event is raised, moving the event breaks the logic, which happens occasionally.
Can you please restructure the code to use different events instead of the handled event? Introducing a regular event in a different place is much better than turning off code by IsHandled event, because we will run existing code. Regular event will not turn off the code, multiple subscribers will be able to subscribe, and it will have a good name with a clear purpose. The best is if you could implement proper extensibility, but this could be a lot of work.
If none of this is possible, IsHandled event should turn off as little code as possible.

[JesperSchulz]

Thanks for reporting this. We agree, and we’ll publish a fix asap, either in an update for the current version or in the next major release. Please do not reply to this, as we do not monitor closed issues. If you have follow-up questions or requests, please create a new issue where you reference this one.

Build ID: .