Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Event Request] Codeunit 99000817 "Manu. Print Report".ConvertUsage #27658

Open
fridrichovsky opened this issue Nov 20, 2024 · 3 comments
Open
Labels
event-request Request for adding an event Integration GitHub request for Integration area

Comments

@fridrichovsky
Copy link
Contributor

fridrichovsky commented Nov 20, 2024

Describe the request

Please add new event

    local procedure ConvertUsage(Usage: Option M1,M2,M3,M4): Enum "Report Selection Usage"
	var
	    ResultReportSelectionUsage: Enum "Report Selection Usage";
    begin
        case Usage of
            Usage::M1:
                exit(ReportSelections.Usage::M1);
            Usage::M2:
                exit(ReportSelections.Usage::M2);
            Usage::M3:
                exit(ReportSelections.Usage::M3);
            Usage::M4:
                exit(ReportSelections.Usage::M4)
            //---------------------------------------------------OnConvertUsageOnUsageElse:BEGIN
            else begin
                OnConvertUsageOnUsageElse(Usage,ResultReportSelectionUsage);
                exit(ResultReportSelectionUsage);
            end;
            //---------------------------------------------------OnConvertUsageOnUsageElse:END
        end;
    end;

    //---------------------------------------------------OnConvertUsageOnUsageElse:BEGIN
    [IntegrationEvent(true, false)]
    local procedure OnConvertUsageOnUsageElse(Usage: Option; var ResultReportSelectionUsage: Enum "Report Selection Usage")
    begin
    end;
    //---------------------------------------------------OnConvertUsageOnUsageElse:END

Additional context

We need option run different usage and different report selection usage.
Internal work item: AB#558147

@JesperSchulz JesperSchulz added event-request Request for adding an event Integration GitHub request for Integration area labels Nov 21, 2024
@nikolakukrika
Copy link
Contributor

The signature of the method needs to be changed too. We should drop the option definition in the code, and replace it with numbers. That would indicate to the code that it can be more options than 4. Option definition in method definition is wrong, we should delete all of these.

@fridrichovsky
Copy link
Contributor Author

hello @nikolakukrika, I agree, but I do not know if we can suggest this kind of change. If you use integer it is OK for us. Enum with extensible=true can be too.
Thank you

@nikolakukrika
Copy link
Contributor

@fridrichovsky You can ask additional things, the point is to get a good design in place. My comment was more for the team that is processing requests. Defining Options inline is bad. If it is not possible to avoid an option as a parameter in the method signature, then it must not contain any member definitions. The risk is that we are duplicating the definition, it will be hard to keep all the definitions up to date if one of the values changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
event-request Request for adding an event Integration GitHub request for Integration area
Projects
None yet
Development

No branches or pull requests

3 participants