kvesteri / sqlalchemy-continuum

Versioning extension for SQLAlchemy.
BSD 3-Clause "New" or "Revised" License
568 stars 128 forks source link

Changeset doesn't gather changes in many-to-many relationships #343

Open ErikvdVen opened 8 months ago

ErikvdVen commented 8 months ago

I've a many to many relationship in my current application, which looks pretty much like the example I posted at the end of this issue. It all works great and SQLAlchemy Continuum nicely tracks all changes, except for gathering the changeset accross these relationships.

It does keep track of the changes nicely in the database, however: if I change a tag on a bank-transaction, the version table of the junction table bank_transaction_tags_version gets an extra record with the same transaction_id as the transaction_id of the version table of bank_transaction_version. And this is always the case, if I only change a tag, the bank_transaction_version still gets another record with the same transaction_id as the record in bank_transaction_tag_version, but obviously only the relationship has changed and nothing to the record in bank_transaction

So, my issue is that, when printing the changeset of the BankTransaction class it does show the changeset of that specific table but it ignores all relationships. Example:

bank_transaction = db.query(BankTransaction).first()

 # this prints a list with two tag objects
print(bank_transaction.tags)
# Output:
# [<models.tag.Tag object at 0x7f85cf609350>, <models.tag.Tag object at 0x7f85cf609450>]

for version in bank_transaction.versions:
    # This only prints the changeset of the main object
    print(version.changeset)

    # This is always empty
    print(version.tags)

So version.changeset is also returning an empty object, when there was only a change with respect to the tags, ie. I only added or removed a tag to the object.

This is how the classes look like:

class BankTransaction(Base):
    """Model which contains all bank transactions from all bank accounts"""
    __versioned__ = {}
    __tablename__ = "bank_transactions"
    __table_args__ = (UniqueConstraint("hash", name="unique_bank_transaction_hash"),)

    id = Column(Integer, primary_key=True)

    user_id = Column(Integer, ForeignKey("users.id"))
    date = Column(Date())

    # Account where transaction goes to or comes from
    offset_iban = Column(String(30))
    offset_name = Column(String(256))

    # Where money goes to, or initiating party
    final_party = Column(String(256))

    # Credit or debit amount
    amount = Column(Float())

    currency = Column(String(10))
    description = Column(Text())

    # Dates records created/updated
    created_on = Column(DateTime, server_default=func.now())
    updated_on = Column(
        DateTime, server_default=func.now(), onupdate=func.current_timestamp()
    )

    # Hash we create to keep track of unique records
    hash = Column(String(256))

    # Many-to-Many relationships
    tags = relationship(
        "Tag", secondary=bank_transaction_tag_mapping, back_populates="bank_transactions"
    )

class Tag(Base):
    __versioned__ = {}
    __tablename__ = "tags"

    # fields here
    id = Column(Integer, primary_key=True)
    name = Column(String(256))
    alias = Column(String(256))
    color = Column(String(256))
    description = Column(String(256))

    # Many-to-Many relationships
    bank_transactions = relationship(
        "BankTransaction", secondary=bank_transaction_tag_mapping, back_populates="tags"
    )

# Many to many relationship for tags and bank_transactions
bank_transaction_tag_mapping = Table(
    "bank_transaction_tags",
    Base.metadata,
    Column("bank_transaction_id", ForeignKey("bank_transactions.id"), primary_key=True),
    Column("tag_id", ForeignKey("tags.id"), primary_key=True),
)

Is there a way to get this working?

ErikvdVen commented 8 months ago

The longer I thought about it, the more it made sense it's probably not possible to make the relation with the right non-version table. Especially because in terms of tags, not every tag is avaiable in the version table and such. How I fixed it now:

# apps.members.models
from sqlalchemy import Column, ForeignKey, Table

from database import Base

# Many to many relationship for tags and bank_transactions
bank_transaction_tag_mapping = Table(
    "bank_transaction_tags",
    Base.metadata,
    Column("bank_transaction_id", ForeignKey("bank_transactions.id"), primary_key=True),
    Column("tag_id", ForeignKey("tags.id"), primary_key=True),
)

# Many to many relationship for users and accounts
user_account_mapping = Table(
    "user_accounts",
    Base.metadata,
    Column("user_id", ForeignKey("users.id"), primary_key=True),
    Column("account_id", ForeignKey("accounts.id"), primary_key=True),
)

class BankTransactionTag(Base):
    __versioned__ = {}
    __table__ = bank_transaction_tag_mapping
    __mapper_args__ = {"primary_key": [
        bank_transaction_tag_mapping.c.bank_transaction_id, 
        bank_transaction_tag_mapping.c.tag_id
    ]}

class UserAccounts(Base):
    __versioned__ = {}
    __table__ = user_account_mapping
    __mapper_args__ = {"primary_key": [
        user_account_mapping.c.user_id, 
        user_account_mapping.c.account_id
    ]}

So I actually created classes of the many-to-many relationships, so I'm able to get the version_class of those junction table. This way it's possible to gather the actual changes on the many-to-many relationships by manually building the relationship:

versioned_banktransaction = version_class(BankTransaction)
    versioned_banktransactiontag = version_class(BankTransactionTag)
    results = ( 
        db.query(versioned_banktransaction, versioned_banktransactiontag, Tag)
            .join(versioned_banktransactiontag, 
                  and_(
                        versioned_banktransaction.id == versioned_banktransactiontag.bank_transaction_id,
                        versioned_banktransaction.transaction_id == versioned_banktransactiontag.transaction_id,
                  )
            )
            .join(Tag, versioned_banktransactiontag.tag_id == Tag.id).all()
    )

    bank_transaction_tag_mapping = {}
    for bank_transaction, versioned_banktransactiontag, tag in results:
        if bank_transaction.transaction_id not in bank_transaction_tag_mapping:
            bank_transaction_tag_mapping[bank_transaction.transaction_id] = {}
            bank_transaction_tag_mapping[bank_transaction.transaction_id]["bank_transaction"] = bank_transaction
            bank_transaction_tag_mapping[bank_transaction.transaction_id]["tags"] = []
        bank_transaction_tag_mapping[bank_transaction.transaction_id]["tags"].append({
            "id": versioned_banktransactiontag.tag_id,
            "operation_type": versioned_banktransactiontag.operation_type,
            "name": tag.name
        })

This is just a messy solution for quickly testing the solution.

If there is a better solution, I'd love to hear it, else this can be closed.

b4sen commented 8 months ago

I had the same issue, turns out this line doesn't return the relationships, so I used these lines to get the missing columns and exclude the viewonly relationships.

Here is my code:

def _get_changeset(version: VersionClassBase, versioned_relationship_keys: list[str] | None = None):
    data = {}
    keys = inspect(version.__class__).columns.keys()

    if versioned_relationship_keys is not None:
        keys.extend(versioned_relationship_keys)

    previous_version = version.previous

    for key in keys:
        if is_internal_column(version, key):
            continue

        if not previous_version:
            old = None
        else:
            old = getattr(previous_version, key)
        new = getattr(version, key)
        if old != new:
            data[key] = [old, new]

    manager = get_versioning_manager(version)
    manager.plugins.after_construct_changeset(version, data)

    return data
    versioned_column_keys = [prop.key for prop in versioned_column_properties(obj)]
    versioned_relationship_keys = [
        prop.key
        for prop in versioned_relationships(obj, versioned_column_keys)
        if not prop.viewonly
    ]

I don't know if this is the correct solution, but works for me.

EDIT: I didn't need to create those extra classes, just used my association tables