Skip to content

Feature/improve audit log#632

Merged
vincent-olivert-riera merged 2 commits intoline:masterfrom
hoangpn:feature/improve_audit_log
Aug 13, 2025
Merged

Feature/improve audit log#632
vincent-olivert-riera merged 2 commits intoline:masterfrom
hoangpn:feature/improve_audit_log

Conversation

@hoangpn
Copy link
Contributor

@hoangpn hoangpn commented Aug 12, 2025

The detail is on each commit's decription. 🙇

@hoangpn hoangpn force-pushed the feature/improve_audit_log branch from ccdef83 to a0aa4b0 Compare August 12, 2025 14:27
Copy link
Contributor

@vincent-olivert-riera vincent-olivert-riera left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hoangpn , review done.

The code suggestions I wanted to make were a little bit complicated, so I decided to create patches instead.

Please apply them and test the app. Check the DB contents are as expected.

If everything is OK, feel free to squash the commits appropriately.

return ""

@staticmethod
def find_parent_object(obj):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think specifying _object is redundant. And also, get is more appropriate than find in this case.

I would name this get_parent, as simple as that.

Later in the code, there are also a few more redundant things like:

  • instance_parent_object -> parent
  • old_parent_object -> old_parent

Please apply this patch (git am <that file>):
0001-fixup-Modify-the-Audit-model-to-improve-the-Audit-Lo.PATCH

name="promgen_aud_parent__4cbb33_idx",
),
),
]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should be able to create the new columns and populate them in the same migration file.

Please apply this patch:
0002-fixup-Modify-the-Audit-model-to-improve-the-Audit-Lo.PATCH

("promgen", "0030_audit_parent_content_type_id_audit_parent_object_id_and_more"),
]

operations = [migrations.RunPython(extract_data_and_populate_parent_object)]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we populate the columns in the other migration file, this one becomes unnecessary.
Please remove it:
0003-Revert-Add-a-migration-script-for-migrate-old-audit-.PATCH

We have added two fields, 'parent_content_type_id' and 'parent_object_id', to the
Audit model to represent the parent object of each object on each entry log. This will
help reduce the dependency of Audit Logs lookups on data from other models, thereby
allowing for the retrieval of Audit Logs for child objects even if they have been
deleted from other tables. Additionally, using only the promgen_audit table to query the
Audit Logs will improve data retrieval performance.
We also have added a migration script to extract the parent object from the "data" field,
which is a longtext field with dict string value, and set that parent object to the
"parent_content_type_id" and the "parent_object_id" fields of each Audit record.
@hoangpn hoangpn force-pushed the feature/improve_audit_log branch from a0aa4b0 to 8503d0b Compare August 13, 2025 07:07
With the addition of two new columns in commit 714e1fc, querying audit logs for a
specific Service or Project and its child objects has become simpler and better.
We have updated the query_set in the AuditList class so that queries are performed only
on the Audit model. The result is still returning audit logs for both the main object
and its child objects, even if the child objects have been deleted.
@hoangpn hoangpn force-pushed the feature/improve_audit_log branch from 8503d0b to 77f2aab Compare August 13, 2025 07:18
@vincent-olivert-riera vincent-olivert-riera merged commit 69fc635 into line:master Aug 13, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants