CancelEdits not cleaning up related object references

[markwhitfeld]

Scenario:
Given I have a new Item with a specified existing ItemType
So, the Item now exists in the ItemType.Items relationship collection
When I cancel edits on the new Item object
Then the Item should no longer exist in the ItemType.Items relstionship collection

This scenario fails on the Final step.

[markwhitfeld]

Hi @peter-wiles

I logged a Habanero issue yesterday that I need to resolve quite quickly for IUA.
I have given them a workaround for the specific issue they had, but I’m not confident that it won’t manifest in other places.

Issue: #59

Scenario:
Given I have a new Item with a specified existing ItemType
So, the Item now exists in the ItemType.Items relationship collection
When I cancel edits on the new Item object
Then the Item should no longer exist in the ItemType.Items relationship collection

This scenario fails on the Final step.

Basically, what is happening is that when you call CancelEdits the properties are set back to what they were but the related object’s relationship back to the cancelled object does not remove the object.

I have reproduced it with a test, but there are a few ways I can go to resolve this:

1. I could add code the Single Relationship cancel edits to fix the opposing relationship if necessary
   1. The SingleRelationship currently does nothing in it’s CancelEdits method
      1. I would make this clean up the relationship
   2. The SingleRelationship doesn’t currently return as IsDirty if its FK prop is dirty…
      1. (it is only dirty if the related object props are dirty, if the related object is new or deleted or if it is an aggregation/composition type relationship)
      2. It makes sense to me that this should return that the relationship is dirty if the local FK prop is dirty or if the _relatedBo doesn’t match the FK.
      3. The Relationships.CancelEdits happens after the properties are Restored, so the cancel on the relationships may need to move to before the prop Restores.
   3. This feels like a good place to fix this, but I am worried about other side effects of changing when the Single Relationship is dirty
2. I could alternatively add logic to the “RelKey.RelatedPropValueChanged” event inside SingleRelationship
   1. i.e. when the prop gets reverted/changed it fixes the linked object relationship (if hydrated).
   2. This feels like it would be the more general way to solve it, and it would also have other benefits like:
      1. when you change the FK prop directly and the reference to the _relatedBo is currently ‘hydrated’, it could ‘unhydrate’ until you access the related bo again.

What do you think?
Any other ideas?

My current failing test is this ( on TestBusinessObject):

        [Test]
        public void Test_CancelEdit_ShouldCleanupRelationships_BUGFIX_GitHub59()
        {
            //---------------Set up test pack-------------------
            ClassDef.ClassDefs.Clear();
            IClassDef orgClassDef = OrganisationTestBO.LoadDefaultClassDef();
            orgClassDef.RelationshipDefCol["ContactPeople"].TimeOut = 10000;
            ContactPersonTestBO.LoadClassDefOrganisationTestBORelationship_MultipleReverse();

            var organisation = OrganisationTestBO.CreateSavedOrganisation();
            organisation.Save();

            var contactPerson = ContactPersonTestBO.CreateUnsavedContactPerson("Contact", "Person");
            contactPerson.Organisation = organisation;

            var relationshipBackToBo = (MultipleRelationship<ContactPersonTestBO>)organisation.Relationships["ContactPeople"];

            //---------------Assert Precondition----------------
            Assert.IsTrue(contactPerson.Status.IsNew);
            Assert.IsTrue(contactPerson.Status.IsDirty);
            Assert.IsTrue(contactPerson.Status.IsEditing);
            Assert.That(relationshipBackToBo.BusinessObjectCollection, Contains.Item(contactPerson));
            //---------------Execute Test ----------------------
            contactPerson.CancelEdits();
            //---------------Test Result -----------------------
            Assert.That(relationshipBackToBo.BusinessObjectCollection, Is.Empty);
        }

[peter-wiles]

Hi Mark,

I think I prefer the event handler approach, it seems cleaner than special code in the CancelEdits, will probably handle more cases and sounds fairly simple. I'm not able to look in depth at the code at the moment but out of the two options I like that more.

I do agree that a change in the fk prop (which is thus a change in the related object itself) should make the relationship dirty. I don't think this is too dangerous, but I can see why you're worried. I think that change would be ok to make. Not sure what should happen for a one-to-one relationship though.

Peter

---

From: markwhitfeldmailto:[email protected]
Sent: ý2014-ý11-ý27 14:27
To: Chillisoft/habaneromailto:[email protected]
Cc: Peter Wilesmailto:[email protected]
Subject: Re: [habanero] CancelEdits not cleaning up related object references (#59)

Hi @peter-wileshttps://github.com/peter-wiles

I logged a Habanero issue yesterday that I need to resolve quite quickly for IUA.
I have given them a workaround for the specific issue they had, but I’m not confident that it won’t manifest in other places.

Issue: #59#59

Scenario:
Given I have a new Item with a specified existing ItemType
So, the Item now exists in the ItemType.Items relationship collection
When I cancel edits on the new Item object
Then the Item should no longer exist in the ItemType.Items relationship collection

This scenario fails on the Final step.

Basically, what is happening is that when you call CancelEdits the properties are set back to what they were but the related object’s relationship back to the cancelled object does not remove the object.

I have reproduced it with a test, but there are a few ways I can go to resolve this:

1. I could add code the Single Relationship cancel edits to fix the opposing relationship if necessary

• The SingleRelationship currently does nothing in it’s CancelEdits method
  - I would make this clean up the relationship
• The SingleRelationship doesn’t currently return as IsDirty if its FK prop is dirty…
  - (it is only dirty if the related object props are dirty, if the related object is new or deleted or if it is an aggregation/composition type relationship)
  - It makes sense to me that this should return that the relationship is dirty if the local FK prop is dirty or if the _relatedBo doesn’t match the FK.
  - The Relationships.CancelEdits happens after the properties are Restored, so the cancel on the relationships may need to move to before the prop Restores.
• This feels like a good place to fix this, but I am worried about other side effects of changing when the Single Relationship is dirty

2. I could alternatively add logic to the “RelKey.RelatedPropValueChanged” event inside SingleRelationship

• i.e. when the prop gets reverted/changed it fixes the linked object relationship (if hydrated).
• This feels like it would be the more general way to solve it, and it would also have other benefits like:
  - when you change the FK prop directly and the reference to the _relatedBo is currently ‘hydrated’, it could ‘unhydrate’ until you access the related bo again.

What do you think?
Any other ideas?

My current failing test is this ( on TestBusinessObject):

    [Test]
    public void Test_CancelEdit_ShouldCleanupRelationships_BUGFIX_GitHub59()
    {
        //---------------Set up test pack-------------------
        ClassDef.ClassDefs.Clear();
        IClassDef orgClassDef = OrganisationTestBO.LoadDefaultClassDef();
        orgClassDef.RelationshipDefCol["ContactPeople"].TimeOut = 10000;
        ContactPersonTestBO.LoadClassDefOrganisationTestBORelationship_MultipleReverse();

        var organisation = OrganisationTestBO.CreateSavedOrganisation();
        organisation.Save();

        var contactPerson = ContactPersonTestBO.CreateUnsavedContactPerson("Contact", "Person");
        contactPerson.Organisation = organisation;

        var relationshipBackToBo = (MultipleRelationship<ContactPersonTestBO>)organisation.Relationships["ContactPeople"];

        //---------------Assert Precondition----------------
        Assert.IsTrue(contactPerson.Status.IsNew);
        Assert.IsTrue(contactPerson.Status.IsDirty);
        Assert.IsTrue(contactPerson.Status.IsEditing);
        Assert.That(relationshipBackToBo.BusinessObjectCollection, Contains.Item(contactPerson));
        //---------------Execute Test ----------------------
        contactPerson.CancelEdits();
        //---------------Test Result -----------------------
        Assert.That(relationshipBackToBo.BusinessObjectCollection, Is.Empty);
    }

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/59#issuecomment-64772070.