search

microsoft / PowerPlatform-DataverseServiceClient

Code Replica for Microsoft.PowerPlatform.Dataverse.Client and supporting nuget packages.
MIT License
285 stars 52 forks source link

Refactor ServiceClient extension methods to IOrganizationService* extension methods #325

Closed jordimontana82 closed 2 years ago

jordimontana82 commented 2 years ago

Hi !

I've seen many extension methods against the ServiceClient class which have business logic, like the following

Most of them seem follow the same pattern, they reset the last error and make sure the client is not null, then perform some logic.

Could maybe these extension methods refactored into a a common place so they could be then be used as IOrganizationService* extension methods?

This would allow us who do heavy unit testing and that we rely mostly on the interfaces for mocking not having to depend on the client to use these methods, and would allow us to use them on the server side too (plugins etc).

MattB-msft commented 2 years ago

Thanks for you feedback and recommendation @jordimontana82 , The extensions you mentioned are fairly tightly integrated to the behaviors of the ServiceClient and do not lend themselves well to be refactored into IOrganizationService/Async based extensions.

That said, we are considering a new set of extensions based on internal and external feedback from the community that we would like to provide as separate packages in conjunction with other work being done in PAC CLI.

Also, we have consider adding an interface to the ServiceClient directly to help folks Mocking the client and intended to provide such an interface as part of the DI work we are doing with it.

It would be helpful if you could tell us which of the existing extensions you are using to help inform future decisions. (We do not collect telemetry on your use of this lib, only that the lib itself is being used)

jordimontana82 commented 2 years ago

Thank you @MattB-msft !

Also, we have consider adding an interface to the ServiceClient directly to help folks Mocking the client and intended to provide such an interface as part of the DI work we are doing with it.

Looking forward to that interface!

It would be helpful if you could tell us which of the existing extensions you are using to help inform future decisions. (We do not collect telemetry on your use of this lib, only that the lib itself is being used)

Several orgs using the latest version of FakeXrmEasy were using several methods on the ServiceClient that do not exist in IOrgService* interfaces. I don't have the details of which ones yet, but will get back to you

jordimontana82 commented 2 years ago

Hi @MattB-msft,

I have an update on this.

These would be methods like CloseIncident or CloseOpportunity. They have a fair bit of logic which I suppose it makes it easier to use them directly (i.e. CloseOpportunity is in charge of calling several messages depending on the status code).

https://github.com/microsoft/PowerPlatform-DataverseServiceClient/blob/254952f3a781f1ab363c56f1a43c4d9b49652fed/src/GeneralTools/DataverseClient/Extensions/DynamicsExtension/CdsServiceClientExtensions.cs#L198

https://github.com/microsoft/PowerPlatform-DataverseServiceClient/blob/254952f3a781f1ab363c56f1a43c4d9b49652fed/src/GeneralTools/DataverseClient/Extensions/DynamicsExtension/CdsServiceClientExtensions.cs#L113

I suppose these are one of the messages you're considering moving to dedicated extension packages (i.e. messages for Sales in package, and so on)? For example, only devs who work on an implementation that have the Sales app would need access to this functionality...)

MattB-msft commented 2 years ago

I am going to move this over to the Discussions area... I would be interesting to see what sort of extension methods folks would like us to provide