DavidFeldhoff / al-codeactions

MIT License
17 stars 8 forks source link

OnBefore / OnAfter publisher vs Rec / xRec #127

Closed fvet closed 2 years ago

fvet commented 2 years ago

@DavidFeldhoff I'd like to get your opinion on how to manage 'Rec' when working with integrationevents. Do you set the IncludeSender parameter to true to have access to the Rec? Or do you include 'Rec' as extra function parameter of the integrationevent.

 [IntegrationEvent(true,false)]
    local procedure OnBeforeCopyInstructions(ShipmentInstructionType: enum "NVT Shipment Instruction Type"; var IsHandled: Boolean)
    begin
    end;

vs

 [IntegrationEvent(false,false)]
    local procedure OnBeforeCopyInstructions(ShipmentInstructionType: enum "NVT Shipment Instruction Type"; var Shipment : record "NVT Shipment"; var IsHandled: Boolean)
    begin
    end;

Same for xRec. Do you sometimes pass xRec to see the old value vs the new value? (We've quit some legacy to use this to test old vs new values, although not 100% sure if all works as expected, reading https://www.youtube.com/channel/UCcJCTa4PvbOQj50y_HJSxXw and https://docs.microsoft.com/en-us/dynamics-nav/xrec-and-currfieldno)

Based on the above, I was thinking of adding the option to allow Rec (or xRec) as parameter suggestions for the creation of publishers.

DavidFeldhoff commented 2 years ago

Hi Frederic, I'll look into the links next week, but my first opinion on this is that I like it to be explicit in the code. If I see that the Rec/xRec is passed as parameter, I know that something could be changed/needed there. I wouldn't see it, if I do that with the IncludeSender property which I don't like really.. So I think I'll pick it up after my vacation. Cheers, David

DavidFeldhoff commented 2 years ago

Hi Frédéric, I just wanted to pick this up, but I need some more input as I think I didn't understand it entirely correct in my vacation last week. I understand it that way that you would like to have a "sender"-parameter (maybe the variable could be named like the object) in the publisher if you use the Create OnBefore/After-Publisher-Code Action for procedures, right? So that we don't have to use the not really transparent IncludeSender-Property. But as far as I know the current IncludeSender-Property does not hand over the xRec (haven't double-checked).

So do you want to have an IncludeSender-Alternative or do you only want to have the possibility to add Rec and xRec-parameters on publishers of tables and pages (which then should be named Rec and xRec, I guess)?

Here's a link for the IncludeSender-Property to recall how it works fast.

fvet commented 2 years ago

So do you want to have an IncludeSender-Alternative

No.

or do you only want to have the possibility to add Rec and xRec-parameters on publishers of tables and pages (which then should be named Rec and xRec, I guess)

Yes.

Think below screenshot of BC Base App should give you an idea on the kind of publishers we'd like to support.

image

fvet commented 2 years ago

As far as the position, would be good if they (Rec, xRec) were added as the very first procedure parameters, where IsHandledis added as the very last procedure parameter.

DavidFeldhoff commented 2 years ago

Hi Frédéric. I've just released version 1.0.8. With that you have the ability to use Rec and xRec parameters where possible. They are not preselected for now. Is it okay that way? Best regards David

DavidFeldhoff commented 2 years ago

Hi Frédéric. I've just released version 1.0.8. With that you have the ability to use Rec and xRec parameters where possible. They are not preselected for now. Is it okay that way? Best regards David