Opus10 / django-pghistory

Track historical events to Django models using Postgres triggers.
https://django-pghistory.readthedocs.io
BSD 3-Clause "New" or "Revised" License
330 stars 41 forks source link

Errors when createing or deleting child models #161

Closed xaitec closed 5 days ago

xaitec commented 1 week ago

I have some concrete inheritance models that i am using but am unable to create any new objects. When i try to save i get the following message:

ProgrammingError: record "new" has no field "acting" CONTEXT: SQL statement "INSERT INTO "employee_services_allowancesummaryevent" ("acting", "active", "allowance_type", "allowance_value", "appraisal_state", "approval_date", "assumption_date", "continuous", "created", "created_by_id", "employee_id", "employment_type", "employmentsummary_ptr_id", "end", "id", "justification_id", "modified", "notes", "percentage_of_salary_scale", "pgh_context_id", "pgh_created_at", "pgh_label", "pgh_obj_id", "position_id", "post_id", "report_to_id", "salary_scale_id", "start", "summary_document", "summary_type", "verified_by_id", "verified_date") VALUES (NEW."acting", NEW."active", NEW."allowance_type", NEW."allowance_value", NEW."appraisal_state", NEW."approval_date", NEW."assumption_date", NEW."continuous", NEW."created", NEW."created_by_id", NEW."employee_id", NEW."employment_type", NEW."employmentsummary_ptr_id", NEW."end", NEW."id", NEW."justification_id", NEW."modified", NEW."notes", NEW."percentage_of_salary_scale", _pgh_attach_context(), NOW(), 'insert', NEW."employmentsummary_ptr_id", NEW."position_id", NEW."post_id", NEW."report_to_id", NEW."salary_scale_id", NEW."start", NEW."summary_document", NEW."summary_type", NEW."verified_by_id", NEW."verified_date")" PL/pgSQL function pgtrigger_insert_insert_2f834() line 11 at SQL statement

I assume that the "acting" field shows as it is the first field alphabetically.

I also tested deleting and got the following: ProgrammingError: record "old" has no field "acting" CONTEXT: SQL statement "INSERT INTO "employee_services_actingsummaryevent" ("acting", "active", "appraisal_state", "approval_date", "assumption_date", "continuous", "created", "created_by_id", "employee_id", "employment_type", "employmentsummary_ptr_id", "end", "id", "justification_id", "modified", "pgh_context_id", "pgh_created_at", "pgh_label", "pgh_obj_id", "position_id", "post_id", "salary_scale_id", "start", "summary_document", "summary_type", "verified_by_id", "verified_date") VALUES (OLD."acting", OLD."active", OLD."appraisal_state", OLD."approval_date", OLD."assumption_date", OLD."continuous", OLD."created", OLD."created_by_id", OLD."employee_id", OLD."employment_type", OLD."employmentsummary_ptr_id", OLD."end", OLD."id", OLD."justification_id", OLD."modified", _pgh_attach_context(), NOW(), 'delete', OLD."employmentsummary_ptr_id", OLD."position_id", OLD."post_id", OLD."salary_scale_id", OLD."start", OLD."summary_document", OLD."summary_type", OLD."verified_by_id", OLD."verified_date")" PL/pgSQL function pgtrigger_delete_delete_b7fdb() line 11 at SQL statement

Also I get no problems updateing existing models.

I would like to add that this only happens for child models, the parent works as expected.

wesleykendall commented 1 week ago

@xaitec is it possible to provide an example parent/child model so I can reproduce this behavior?

Also, is it the parent or child that has pghistory.track on it?

pghistory doesn't have the greatest support for concrete inheritance, but this seems like a bug

xaitec commented 1 week ago

Its a big dump but this is a parent and child, i did have pghistor.trac() on both with custom related names.

Would it be easier to switch to an abstract type inheritance to use pghistory?

class EmploymentSummary(TimeFramedModel, TimeStampedModel):
    class EmploymentType(models.TextChoices):
        DAILY = "Daily"
        MONTHLY = "Monthly"
        SESSIONAL = "Sessional"

    class TransferLocations(models.TextChoices):
        MOH = "M.o.H"
        ERHA = "E.R.H.A."
        NCRHA = "N.C.R.H.A."
        SWRHA = "S.W.R.H.A."
        TRHA = "T.R.H.A."

    class SummaryType(models.TextChoices):
        ACTING = "Acting"
        ALLOWANCE = "Allowance"
        ASSUMPTION = "Assumption"
        CASUAL_EMPLOYMENT = "Casual Employment", "Casual"
        CONTRACT = "Contract", "Contract"
        LEAVE_RELIEF = "Leave Relief", "Leave"
        PERMANENT = "Permanent"
        PROMOTION = "Promotion"
        SESSIONAL = "Sessional"
        TEMPORARY = "Temporary Period", "Temporary"
        TRANSFER = "Transfer"

    class AppraisalState(models.IntegerChoices):
        NO_APPRAISAL_REQUIRED = 1
        APPRAISAL_RECEIVED = 2
        AWAITING_APPRAISAL = 3

    id = models.UUIDField(default=uuid.uuid4, editable=False, primary_key=True)
    employee = models.ForeignKey(
        "employee_services.employee",
        on_delete=models.CASCADE,
        help_text=_("Name of employee."),
        related_name="employment_summary",
    )
    post = models.ForeignKey(
        "establishment.post",
        on_delete=models.SET_NULL,
        null=True,
        related_name="summary_history",
    )
    position = models.ForeignKey(
        "establishment.position",
        on_delete=models.RESTRICT,
        related_name="employment_summary",
        null=True,
        blank=True,
    )
    approval_date = models.DateField(
        help_text=_("Date the duty was approved."), null=True, blank=True
    )
    employment_type = models.CharField(choices=EmploymentType.choices, max_length=10)
    justification = models.ForeignKey(
        "establishment.BoardJustification",
        on_delete=models.RESTRICT,
        null=True,
        blank=True,
        help_text="Reason for summary.",
    )
    acting = models.BooleanField(default=False)
    salary_scale = models.ForeignKey(
        SalaryScale, on_delete=models.RESTRICT, null=True, blank=True
    )
    summary_type = models.CharField(
        max_length=17, choices=SummaryType.choices, editable=False
    )
    summary_document = models.FileField(
        upload_to=summary_directory,
        null=True,
        blank=True,
    )
    appraisal_state = models.SmallIntegerField(
        choices=AppraisalState.choices,
        default=AppraisalState.NO_APPRAISAL_REQUIRED,
    )
    active = models.BooleanField(default=False, editable=False)
    created_by = models.ForeignKey(
        "accounts.HrProfile",
        on_delete=models.SET_NULL,
        null=True,
        blank=True,
        related_name="employment_summaries_created",
    )
    # Verification
    verified_by = models.ForeignKey(
        "accounts.HrProfile",
        on_delete=models.SET_NULL,
        null=True,
        blank=True,
        related_name="employment_summaries",
    )
    verified_date = models.DateField(null=True, blank=True)

class ActingSummary(EmploymentSummary):
    assumption_date = models.DateField(help_text="Date the employee assumed duty.")
    continuous = models.BooleanField(
        default=False, help_text=_("Is the period continuous?")
    )
wesleykendall commented 1 week ago

Let me see if I can reproduce this. I have a feeling that this PR https://github.com/Opus10/django-pghistory/pull/36 I closed may have solved some of these issues.

For concrete inheritance, there are multiple tables managed underneath. The snapshots for ActingSummary will only contain the parent ID, id, assumption_date, and continuous. If one makes changes to both parent/child at once, you'd have to rely on context tracking to understand the full change across two tracking tables.

If this is going to be problematic for your use case, abstract modeling/inheritance would be better since the table for ActingSummary would include all fields and not rely on a foreign key to a parent table.

FYI it's likely I won't be able to check out this issue until tomorrow. Will report back

xaitec commented 1 week ago

I cant yet say if it will be a problem to implement as i'm not entirely sure how to implement it.

Tomorrow is fine to look into it as I am not on a time constraint.

wesleykendall commented 5 days ago

I made a simple test case to reproduce this in pghistory. Seems to only matter when tracking all fields on concrete child models. Should be fixed in 3.4.2. Let me know if you have any more issues!