Handle property migration write conflicts #412

[joelvdavies]

Description

See #412. Adds a new WriteConflictError, added function to handle these for transactions and used this in the property migrations.

To test the function you can use the following scripts updated from those in the issue:

test1.py

import time

from inventory_management_system_api.core.custom_object_id import CustomObjectId
from inventory_management_system_api.core.database import get_database, start_session_transaction

database = get_database()


script_name = "test1"
catalogue_item_id = "6745eb4d7f23b95d5f050c70"


session = None

with start_session_transaction("testing") as session:
    database.catalogue_items.update_one(
        {"_id": CustomObjectId(catalogue_item_id)}, {"$set": {"count": None}}, session=session
    )

    count = database.items.count_documents({}, session=session)
    print(f"Count before insert ({script_name}): ", count)

    database.items.insert_one({"test": "hello"}, session=session)

    count = database.items.count_documents({}, session=session)
    print(f"Count after insert ({script_name}): ", count)

    time.sleep(10)

    database.catalogue_items.update_one(
        {"_id": CustomObjectId(catalogue_item_id)}, {"$set": {"count": count}}, session=session
    )

count = database.items.count_documents({})
print(f"Count after ({script_name}): ", count)

test2.py

from inventory_management_system_api.core.custom_object_id import CustomObjectId
from inventory_management_system_api.core.database import get_database, start_session_transaction

database = get_database()


script_name = "test2"
catalogue_item_id = "6745eb4d7f23b95d5f050c70"


# session = None

with start_session_transaction("testing") as session:
    database.catalogue_items.update_one(
        {"_id": CustomObjectId(catalogue_item_id)}, {"$set": {"count": None}}, session=session
    )

    count = database.items.count_documents({}, session=session)
    print(f"Count before insert ({script_name}): ", count)

    database.items.insert_one({"test": "hello"}, session=session)

    count = database.items.count_documents({}, session=session)
    print(f"Count after insert ({script_name}): ", count)

    database.catalogue_items.update_one(
        {"_id": CustomObjectId(catalogue_item_id)}, {"$set": {"count": count}}, session=session
    )

count = database.items.count_documents({})
print(f"Count after ({script_name}): ", count)

It is not easily possible to test it inside the actual api itself which is why there isn't any e2e tests. The final error message gets displayed in the front end currently as

going to while updating property when updating.

Testing instructions

Add a set up instructions describing how the reviewer should test the code

• Review code
• Check Actions build
• Review changes to test coverage

Agile board tracking

Closes #412

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.91%. Comparing base (2930356) to head (2274dcb).
Report is 13 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #433      +/-   ##
===========================================
+ Coverage    97.89%   97.91%   +0.01%     
===========================================
  Files           48       48              
  Lines         1712     1723      +11     
===========================================
+ Hits          1676     1687      +11     
  Misses          36       36              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[asuresh-code]

Looks good, just have a few minor docstring comments to make. I also had a general question about the issue itself.

The issue is meant to handle when a property migration fails; is this an issue related to our migration scripts, or generally when users add/edit properties? Also, why has it only been an issue for catalogue category properties, and not other entities?

[joelvdavies]

Looks good, just have a few minor docstring comments to make. I also had a general question about the issue itself.

The issue is meant to handle when a property migration fails; is this an issue related to our migration scripts, or generally when users add/edit properties? Also, why has it only been an issue for catalogue category properties, and not other entities?

This is specifically for adding or editing properties at the catalogue category level. We tend to keep calling them migrations as that's how we thought of them when we added them, but they are different to the migrations scripts. The property migration here refers to the propagation of new properties/changes to their name down through their catalogue items and items, whereas the migration scripts are a one time change we make to how the database stores the data itself.