apex-enterprise-patterns / fflib-apex-common

Common Apex Library supporting Apex Enterprise Patterns and much more!
BSD 3-Clause "New" or "Revised" License
913 stars 517 forks source link

UnitOfWork: RegisterDirty with new parent relationship not working #221

Open calemcme opened 5 years ago

calemcme commented 5 years ago

The issue: UnitOfWork does not resolve the relationship at update. Here is the problem description:

Use "registerDirty" with a relationship to a parent record which will be inserted (updated?). Method to use: public void registerDirty(SObject record, Schema.sObjectField relatedToParentField, SObject relatedToParentRecord)

Example: create a new transaction "tranNew" record, set the new transaction Id in an existing record "updTran". Code snippet: mUow.registerNew(tranNew); mUow.registerDirty(updTran, Transactionc.NewTransactionc, tranNew);

After UnitOfWork is committed, the parent field is not filled in record "updTran". The implementation of "updateDmlByType" does not have relationship resolution:

private void updateDmlByType()
{
    for (Schema.SObjectType sObjectType : m_sObjectTypes)
    {
                m_dml.dmlUpdate(m_dirtyMapByType.get(sObjectType.getDescribe().getName()).values());
    }
}

It should resolve the relationship before update. The updated implementation will have "m_relationships.get(sObjectType.getDescribe().getName()).resolve();" before update is performed.

private void updateDmlByType()
{
    for (Schema.SObjectType sObjectType : m_sObjectTypes)
    {
                m_relationships.get(sObjectType.getDescribe().getName()).resolve();
                m_dml.dmlUpdate(m_dirtyMapByType.get(sObjectType.getDescribe().getName()).values());
    }
}

The modification is similar to the resolution applied in method "private void insertDmlByType()".

private void insertDmlByType()
{
    for (Schema.SObjectType sObjectType : m_sObjectTypes)
    {
                m_relationships.get(sObjectType.getDescribe().getName()).resolve();
                m_dml.dmlInsert(m_newListByType.get(sObjectType.getDescribe().getName()));
    }
}
calemcme commented 5 years ago

The resolution added in "updateDmlByType" may cause an issue if a resolution was resolved at "insertDmlByType". An additional test is necessary to avoid this case.

public void resolve()
{
        // check relationship Id due to update use.
        if (this.Record.get(this.RelatedToField)==null) {
                this.Record.put(this.RelatedToField, this.RelatedTo.Id);
        }
}
cnaccio commented 5 years ago

Hey @calemcme, we just ran into the same issue today, and was fortunate enough to find your solution mentioned above. I thought it was strange that registerDirty had an additional method signature which appeared to support updating a child, and linking it to a new parent that would be created during the same unit of work; I was surprised when this didn't work as expected.

Your solution above appears to have solved the problem, so thanks for that! However, I wanted to know if you've discovered any other/downstream issues with the changes above? Just let me know; thanks!

P.S. I would think these changes should be merged into the repo. @afawcett Would it make sense to open a PR for this change/fix?

calemcme commented 5 years ago

Hey @cnaccio, we have not found issues with our use cases based on the changes above. Likewise, if you run into issues in your cases please let me know.

afawcett commented 4 years ago

Hmmm flagging this as a bug for now.

ImJohnMDaniel commented 1 year ago

Interesting. I just got another report of this issue from @jprichter.

@dsurfleet, Am I correct that your fix is not currently part of an open PR? We are intending to reopen this issue again and start with your proposed fix.

dsurfleet commented 1 year ago

@ImJohnMDaniel - I believe you are correct. Been awhile and can't remember :)