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

UnitOfWork: RegisterDirty with new parent relationship not working #221

Open
calemcme opened this issue Apr 10, 2019 · 6 comments
Open

UnitOfWork: RegisterDirty with new parent relationship not working #221

calemcme opened this issue Apr 10, 2019 · 6 comments
Assignees
Labels

Comments

@calemcme
Copy link

calemcme commented Apr 10, 2019

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, Transaction__c.NewTransaction__c, 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
Copy link
Author

calemcme commented Apr 10, 2019

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
Copy link

cnaccio commented May 8, 2019

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
Copy link
Author

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
Copy link
Contributor

Hmmm flagging this as a bug for now.

@afawcett afawcett added the bug label Dec 24, 2019
dsurfleet pushed a commit to dsurfleet/fflib-apex-common that referenced this issue Apr 23, 2021
@ImJohnMDaniel
Copy link
Contributor

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
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants