Feat: Add compliance period column to transaction table - 1719 #1799
Conversation
| ) | ||
| description = Column( | ||
| String, | ||
| primary_key=True, |
There was a problem hiding this comment.
I don't think this needs to be a primary_key?
There was a problem hiding this comment.
I noticed that the unique index in the migration is a combination of transaction_id, description, and transaction_type, which is why I included it in the model. Should I remove it from the migration as well?
op.execute(
"""
CREATE UNIQUE INDEX mv_transaction_aggregate_unique_idx
ON mv_transaction_aggregate (
transaction_id,
description,
transaction_type
);
"""
)There was a problem hiding this comment.
Yes I don't think its required to have description as a third primary element. transaction_type and transaction_id will be unique enough.
There was a problem hiding this comment.
Thanks, Alex, for reviewing my code! I've removed the description as a primary key in both the migration and model.
|
This looks good, just repoint it to out latest release branch release-1.0.0 |
hamed-valiollahi
force-pushed
the
feat/hamed-add-compliance-period-1719
branch
from
0fe42c5 to
87aec8d
Compare
Sure, I've repointed it to our latest release branch, |
This PR adds a 'Compliance period' column to the Transaction table for both IDIR and BCeID users.
Closes #1719