verbb / formie

The most user-friendly forms plugin for Craft CMS.
Other
94 stars 70 forks source link

Microsoft Dynamics 365 CRM: Created by field and query #1579

Closed jamesmacwhite closed 1 day ago

jamesmacwhite commented 11 months ago

Describe the bug

I noticed the recent inclusion of the Created By field. The query for pulling system users might need to be reviewed or possibly able to be customised because of the large amount of system users for certain organisations.

The case we have is technically all Azure Active Directory accounts can be returned as a system user, this doesn't however take into account if they actually have a Dynamics 365 CRM sales enterprise licence or indeed an activated user account in Dynamics 365. As a large organisation this would return thousands of accounts. The query is limited to 100 but because it's done in A-Z, it's basically just the first 100 names beginning with A.

One way I found to reduce the returned results is using a couple of additional parameters:

https://learn.microsoft.com/en-us/power-apps/developer/data-platform/webapi/reference/systemuser?view=dataverse-latest

invitestatuscode - Value of 4 (confirms the presence of a sales licence I think) isdisabled - Value of 0 (enabled) or false.

This is actually something I ended up doing with the EVENT_MODIFY_TARGET_SCHEMAS event to modify certain queries for specific usage cases like this, however as the created by field is using a seperate query it bypasses that.

Not sure what your thoughts are?

Steps to reproduce

N/A

Form settings

Craft CMS version

3.9.4

Plugin version

1.6.34

Multi-site?

No

Additional context

No response

engram-design commented 11 months ago

Yeah I knew something like this would probably come back to bite me in https://github.com/verbb/formie/issues/1567

I think those quick query edits are a great idea. I was going to try and make this part of the schema function and editable with that event, but it got a little messy.

jamesmacwhite commented 11 months ago

@engram-design No worries, no two Dynamics 365 systems are the same, it becomes difficult to have things work across everyone. We are a large organisation with thousands of user accounts, we are likely the exception to the common rule!

Quick solution is modify the query with defaults, or add another event to modify the query return data before it is run similar to the target schemas event, but perhaps needs to be specific just for this purpose. As we use an extended integration we can modify this how we see fit but I've also been mindful trying to commit back where possible, unless it really doesn't make sense when it gets down into specific business logic. We may be on the border of this being the case here too.

I haven't found a conclusive query that just fetches all activated users with Dynamics 365 licences, the additional parameters have been what's got me about as close to that so far, but it's probably still not accurate.

We have just under 70 licences assigned to system users, I'll see if I can revisit a query that matches this.

engram-design commented 11 months ago

I’ll ensure at least your suggestions are included - it’s a good start!

jamesmacwhite commented 11 months ago

Thanks. I am not entirely sure if the Web API is capable of quering all licenced users accurately to be honest. I've seen some questions around it, with different results.

The obvious islicensed value returns true for most users, so that's helpful. That's why the invitestatuscode is at least half way to getting a much more accurate number but I don't think that is right either.

Equally userlicensetype and caltype all seem to vary despite all our users having the same license in our Active Directory. I feel this is classic Microsoft at this point.

Edit: caltype is unreliable apparently for cloud based solutions

License type of user. This is used only in the on-premises version of the product. Online licenses are managed through Microsoft 365 Office Portal

engram-design commented 11 months ago

Typical Dynamics it seems! Just want to check https://github.com/verbb/formie/commit/d0799456fece1e7a9ddb832fd9ca87163766cfe0 is working for you?

jamesmacwhite commented 11 months ago

It works better, but I've found further issues invitestatuscode, it isn't reliable either. I'm concluding there is no a specific query that will get you the list of CRM enabled users with licences through the Web API (I've been at it for like two hours), it is just too variable and differs between on premise and cloud with the whole licence situation anyway. Even when getting the accurate list by locking it to system user GUID values of licenced users, I looked at the various data fields and there's differences on all areas like invitestatuscode, licensetype etc, no consistency to have a reliable query. Fun.

The alternative approach for Dynamics 365 that's slightly more involved but does at least limit the scope would be to fetch the systemusers entity, but then use a relation ($expand) on something like Teams and pull in a team group. In the case of our environment, we have a default team group for all CRM users, which everyone is assigned to. This technically still includes users that don't have a CRM licence assigned (I checked our Azure Active Directory) but better than 1000+ users! Ended up being about 80 odd users, were it should be 60. This however can be remidied by simply updating the CRM environment in Power Platform Admin Center to remove those users from the group which do not have a licence.

Here's a stupidly more complicated API query but gets me about as close as I can.

{{webapiurl}}systemusers?$select=fullname,systemuserid&$expand=teammembership_association($select=teamid;$filter=(teamid eq 'af0fb2d8-452f-eb11-a813-0022483fa49d'))&$filter=(isdisabled eq false and islicensed eq true and azurestate eq 0) and (teammembership_association/any(o1:(o1/teamid eq 'af0fb2d8-452f-eb11-a813-0022483fa49d')))&$orderby=fullname asc

The teamid would obviously be specific to this environment and the params like azurestate would only be valid for those using Azure Active Directory, rather than on-premises.

Based on all of this, I would suggest there needs to be a way to override the default query performed, as it's probably going to not be ideal for some environments. Maybe backing off the invitestatuscode and trying use some of the systemuser entity values like isdisabled false, islicensed true etc is a start for a generic query but won't get you a true picture.

Also just to note, the returned values need to be formatted with the entity e.g. systemusers(GUID) at the moment, I think it's just being returned as the GUID without an entity, which will fail when being sent to the API. All lookup fields need to have their entity.

engram-design commented 11 months ago

Thanks for the investigation here, I'm going to keep figuring out if we can do anything else here - maybe go back to the drawing board. Down the track, I really need to figure a way to improve this sort of thing with large datasets.

Good call on the systemusers(GUID) I'll adjust that.

jamesmacwhite commented 11 months ago

I think similar to the default entities query and being able to modify that through events, that's probably the best way. Dynamics 365 stuff gets very opinionated quickly, the out of the box experience is likely a sane default with options to modify or tweak for specific users, I'm not sure you want to get Formie involved with complexity like the above, but if there's a way to modify queries and data returns that caters for advanced use cases, that would be best I think. For me, I see it like this in this order.

  1. Functioning Formie integration for out of the box experience.
  2. Further events available for customisation without forking/extending the integration class.
  3. Custom or extended integration class for advanced or specific business requirements.

Most users will likely never go beyond option 1, having option 2 available helps without having to do too much customising, while still leverage Formie's own functions. Option 3 is for advanced usage, which at this point you are fully adversed in Dynamics 365 Web API things.

I think the modify target schemas approach is likely the way, maybe we just have to create a specific event for system users, given it's specific requirements. I know the systemuser entity is called in the main schemas, so maybe there's some re-use opportunity, but just my thoughts.

For me, the approach we'd take is a custom query to return the systemusers and cache it and just point the return at a service class with a function returning the data. Probably wants a bit of cache, given returning systemusers is a bit more weighty and probably doesn't change that much.

jamesmacwhite commented 11 months ago

To be fair, while a headache, you probably can't send a createdby value anyway, see the entity definition on something like a contact record:

{
            "@odata.type": "#Microsoft.Dynamics.CRM.LookupAttributeMetadata",
            "AttributeType": "Lookup",
            "IsCustomAttribute": false,
            "IsValidForCreate": false,
            "IsValidForUpdate": false,
            "CanBeSecuredForCreate": false,
            "CanBeSecuredForUpdate": false,
            "LogicalName": "createdby",
            "SchemaName": "CreatedBy",
            "MetadataId": "570f0d59-4827-4710-aea7-01bedcaa2f57",
            "DisplayName": {
                "LocalizedLabels": [
                    {
                        "Label": "Created By",
                        "LanguageCode": 1033,
                        "IsManaged": true,
                        "MetadataId": "52d6a218-2341-db11-898a-0007e9e17ebd",
                        "HasChanged": null
                    }
                ],
                "UserLocalizedLabel": {
                    "Label": "Created By",
                    "LanguageCode": 1033,
                    "IsManaged": true,
                    "MetadataId": "52d6a218-2341-db11-898a-0007e9e17ebd",
                    "HasChanged": null
                }
            },
            "RequiredLevel": {
                "Value": "None",
                "CanBeChanged": true,
                "ManagedPropertyLogicalName": "canmodifyrequirementlevelsettings"
            },
            "Settings": []
        },

Create and update are false, you cannot send this data in a payload. It will error. THe created by value is very much tied to ensuring a licenced CRM systemuser it provided, otherwise things will blow up.

engram-design commented 1 day ago

As an update on this, I've been working on improving this in Formie 3 for Craft 5.

Firstly (and unrelated), I've figured out the difference between LogicalName and SchemaName for fields - LogicalName is used in all instances, apart from Lookup (relational) fields, so that's good to know.

For Lookup fields, I've changed the handling by dynamically setting up the schema, instead of it being static. This is because a particular client had lots of custom entities setup, and had minimal module experience to use the events you've added to modify things and add schemas.

Now we look at all Lookup fields, see their targets, and build the "schema" automatically.

You'll notice a special case for systemusers where we now filter based on applicationid eq null and isdisabled eq false. This is better than checking on applicationid after the query, given the limit of 100 is not factored into that. I've left off invitestatuscode for the moment.

This is all now done before the event, so not a breaking change, and you can still completely modify things with the events.

jamesmacwhite commented 1 day ago

Great work! Sounds like it's a bit more flexible in Craft 5! Good find on the LogicalName vs SchemaName difference. I will go back to our extended class and see if its something we can revisit with these changes under Craft 5.