DavidFeldhoff / al-codeactions

MIT License
17 stars 8 forks source link

Refactor Message to Notification #147

Open StefanMaron opened 1 year ago

StefanMaron commented 1 year ago

Hi David!

I just realized that I could make far more use of Notifcations and that it often makes sense to use a notification instead of a message. That way the flow in BC can better for the user, as he does not get interrupted by message so often.

What do you think about the idea to refactor A message into a notification.

This:

Message('Some message to show to the user');

To this:

MyNotification.Message('Some message to show to the user');
MyNotification.Send();

When a label already is in place, that should be used obviously.

waldo1001 commented 1 year ago

May be also add a unique, hardcoded notificationid when you're at it ;-)

StefanMaron commented 1 year ago

May be also add a unique, hardcoded notificationid when you're at it ;-)

Is there any benefit to have an ID there?

waldo1001 commented 1 year ago

Hm, may be not in this case. An ID will be assigned anyway (in memory). Only interesting to be recalled (or to make sure the same is not being added multiple times)

DavidFeldhoff commented 1 year ago

An ID could also be good if you want to turn off the notification, but then it would become a bit more complex. Probably too much, right? Or would you like to have at least the possibility to have the code afterwards like this?

var
    TheMessageTxt: Label 'Some message to show to the user';

procedure ExistingProc()
begin
    // [some other existing code]
    CallNotification();
    // [some other existing code]
end;

local procedure CallNotification()
var
    MyNotifications: Record "My Notifications";
begin
    if MyNotifications.IsEnabled(GetNotificationId()) then begin
        MyNotification.Id := GetNotificationId();
        MyNotification.Message(TheMessageTxt);
        MyNotification.Send();
    end;
end;

local procedure GetNotificationId(): Guid
begin
    exit('5dd5a7df-14cc-49b6-9d16-b34066c06f05');
end;

[EventSubscriber(ObjectType::Page, Page::"My Notifications", 'OnInitializingNotificationWithDefaultState', '', false, false)]
local procedure OnInitializingNotificationWithDefaultState()
var
    MyNotifications: Record "My Notifications";
    DescriptionTxt: Label 'Text inserted by snippet';
begin
    MyNotifications.InsertDefault(GetNotificationId(), TheMessageTxt, DescriptionTxt, true);
end;
DavidFeldhoff commented 1 year ago

It could be two codeactions like:

  1. Convert to Notification
  2. Convert to optional Notification
StefanMaron commented 1 year ago

I like to have both posibilities, the full example and the very minimalistic one :)

DavidFeldhoff commented 1 year ago

But at least the "Convert to optional notification" would only work in codeunits right? Because only there I'm able to create event subscribers..

StefanMaron commented 1 year ago

I mean, you could still create it so its there to be copied somewhere else, right?

DavidFeldhoff commented 1 year ago

Yes, would be possible, but then "suddenly" after a codeaction your solution isn't compilable anymore right? I can imagine that a few people are then a bit confused. If I'd do it like that, I'd at least put a comment to it or something like that

StefanMaron commented 1 year ago

Or you could also only offer the complex scenario if the code already is withing a codeunit :)

DavidFeldhoff commented 1 year ago

Short update: haven't started with this yet, as I'm a bit pushing the other two extensions currently (ATDD.TestScriptor and Azure DevOps Simplify). Will come to this at a later time