Refactor changelog triggers to use rust code

[jmbrunskill]

The suggestion

Currently we use database triggers to record when changes are made to records that we need to sync. This has some advantages (particularly that manual database interventions will be automatically captured), however it also has disadvantages such as the need to re-create triggers when some kinds of database changes are made, making it harder to see where and when triggers are used, and work arounds such as db manipulation to skip changelog for db migrations.

A new pattern was established with the asset sync work, that would allow the rust code to manage the changelog entries using the Upsert Trait.

Example use case

Currently we have an Upsert Trait like this used for sync. We should remove the upsert_sync function from this trait and refactor the associated _row.rs file to insert the changelog record.

pub trait Upsert: DebugTrait {
    // TODO:(Long term) DELETE THIS TRAIT METHOD AND REMOVE TRIGGERS?
    fn upsert_sync(&self, con: &StorageConnection) -> Result<(), RepositoryError>;

    fn upsert(&self, con: &StorageConnection) -> Result<Option<i64>, RepositoryError> {
        self.upsert_sync(con)?;
        // When not using triggers to create changelog records, this is where you may want to implement changelog logic
        // This function should return the id of the changelog record created...
        Ok(None)
    }

    // Test only
    fn assert_upserted(&self, con: &StorageConnection);
    // Test only, can be used to drill down to concrete type (see test below)
    // also casting to any must be implemented by concrete type to be able to downcast to it
    // This is needed for integration test (where test record is generated for inventory adjustment, but id is not know until site is created)
    fn as_mut_any(&mut self) -> Option<&mut dyn Any> {
        None
    }
}

Example code to inserting changelog in rust code.

   pub fn upsert_one(&self, asset_row: &AssetRow) -> Result<i64, RepositoryError> {
        self._upsert_one(asset_row)?;
        self.insert_changelog(
            asset_row.id.to_owned(),
            RowActionType::Upsert,
            Some(asset_row.clone()),
        )
    }

    fn insert_changelog(
        &self,
        asset_id: String,
        action: RowActionType,
        row: Option<AssetRow>,
    ) -> Result<i64, RepositoryError> {
        let row = ChangeLogInsertRow {
            table_name: ChangelogTableName::Asset,
            record_id: asset_id,
            row_action: action,
            store_id: row.map(|r| r.store_id).unwrap_or(None),
            ..Default::default()
        };
        ChangelogRepository::new(&self.connection).insert(&row)
    }

Why should we invest time in this?

Certain types of migrations such as Name & Store merge have required re-creating all triggers in the system which creates a high risk of something being missed. Rust's code checking will ensure Allow more flexibility and logic to be managed around when and how changelog entries are created (e.g. related entries might need to be referenced to find the associated name, or store) Makes code most consistent and maintainable.

Are there any risks associated with this change?

I think it should ultimately reduce risk, but the risks of the change would be removing changelog triggers but not creating the associated insert_changelog functions. We also have a risk that manual changes to the database (most likely at central) wouldn't automatically be synced as we'd need a changelog record to trigger sync

How much effort is required?

Quite a bit as there's a lot of tables with change log that need to be considered and some testing would be required to make sure things are working correctly. Maybe 2-3 days?

Agreed Solution

[andreievg]

One issue with invoice_lines, requisition_lines, but should be solvable (needs name_id, which come from related record, but when we update we usually have that record in memory)

[andreievg]

Can potentially also use this refactor as an opportunity to remove differences in upsert methods (postgres version should work for sqlite now)