fix(lead): reload contact before updating links

[barredterra]

Contact might have changed since it was created. In this case the user saw an error message ("contact has been modified") and was unable to save the lead.

The error only shows when you have hooks on Contact that modify Contact.

[codecov]

Codecov Report

Merging #29966 (3726914) into develop (785fcca) will increase coverage by 18.91%.
The diff coverage is 100.00%.

❗ Current head 3726914 differs from pull request most recent head 5de2ba5. Consider uploading reports for the commit 5de2ba5 to get more accurate results

@@             Coverage Diff              @@
##           develop   #29966       +/-   ##
============================================
+ Coverage    40.68%   59.60%   +18.91%     
============================================
  Files         1083     1109       +26     
  Lines        68280    69168      +888     
============================================
+ Hits         27780    41227    +13447     
+ Misses       40500    27941    -12559     

Impacted Files	Coverage Δ
erpnext/crm/doctype/lead/lead.py	71.92% <100.00%> (+3.60%)	⬆️
...eport/item_variant_details/item_variant_details.py	31.32% <0.00%> (-53.02%)	⬇️
.../report/delayed_item_report/delayed_item_report.py	60.78% <0.00%> (-35.30%)	⬇️
...t/selling/doctype/product_bundle/product_bundle.py	58.33% <0.00%> (-25.01%)	⬇️
...rial_no_valuation/incorrect_serial_no_valuation.py	85.96% <0.00%> (-10.53%)	⬇️
.../report/stock_projected_qty/stock_projected_qty.py	86.84% <0.00%> (-2.64%)	⬇️
erpnext/stock/report/stock_ledger/stock_ledger.py	82.11% <0.00%> (-2.44%)	⬇️
...saction/incorrect_balance_qty_after_transaction.py	88.37% <0.00%> (-2.33%)	⬇️
...eorder_level/itemwise_recommended_reorder_level.py	92.45% <0.00%> (-1.89%)	⬇️
...rpnext/stock/report/stock_balance/stock_balance.py	79.16% <0.00%> (-0.60%)	⬇️
... and 383 more

[ankush]

The error only shows when you have hooks on Contact that modify Contact.

Can you describe this in a bit more detail? or share what your hook is doing?

Hooks shouldn't be triggering .save which is the only way to cause "has been modified" error. 😅

Use db_set instead if calling from after-save hooks.

[barredterra]

@ankush Lead inserts a Contact at one point in time (and stores a reference to the object). This causes other hooks on Contact to fire, which might modify the contact on the database level.
Later, Lead modifies the same Contact object from before (without reloading) and tries to save it again. If meanwhile the database state has changed, this produces an error.

We can avoid the error by reloading the contact before updating it.

Produces Error

class Lead(Document):
    def before_insert(self):
        self.contact_doc = frappe.get_doc({
            "first_name": "Original",
            # ...
        }).insert()

    def after_insert(self):
        self.contact_doc.first_name = "I'm Second"
        self.save()

class Contact(Document):
    def on_update(self):
        frappe.db.set_value(self.doctype, self.name, "fist_name", "I'm First")

Works

class Lead(Document):
    def before_insert(self):
        self.contact_doc = frappe.get_doc({
            "first_name": "Original",
            # ...
        }).insert()

    def after_insert(self):
        self.contact_doc.reload()
        self.contact_doc.first_name = "I'm Second"
        self.save()

class Contact(Document):
    def on_update(self):
        frappe.db.set_value(self.doctype, self.name, "fist_name", "I'm First")

[ankush]

@barredterra thanks for the explanation. This is very conflicting indeed.

.reload() will ignore any other changes done in lead.py (while there aren't any right now, there could be in the future. 😐 Not sure what's the best/safe solution here.

The simplest way to avoid this would be not to set update_modified=False in Contact hooks. That way "this document is modified" validation won't trigger.

cc: @ruchamahabal / @anupamvs

[ankush]

@barredterra perhaps it would be better to move to reload right next to insert?

class Lead(Document):
    def before_insert(self):
        self.contact_doc = frappe.get_doc({
            "first_name": "Original",
            # ...
        }).insert()
        self.contact_doc.reload()

    def after_insert(self):
        self.contact_doc.first_name = "I'm Second"
        self.save()

class Contact(Document):
    def on_update(self):
        frappe.db.set_value(self.doctype, self.name, "fist_name", "I'm First")