Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error when setting table_name with inherit=True #1195

Closed
muneeb706 opened this issue Jun 24, 2023 · 0 comments · Fixed by #1196
Closed

Error when setting table_name with inherit=True #1195

muneeb706 opened this issue Jun 24, 2023 · 0 comments · Fixed by #1196

Comments

@muneeb706
Copy link
Member

muneeb706 commented Jun 24, 2023

Problem Statement

When table_name is set with inherit=True in base model then error is thrown when makemigrations command is entered.

For example:

from django.db import models
from simple_history.models import HistoricalRecords

class BaseModel(models.Model):
    created_date = models.DateTimeField(auto_now_add=True)
    last_updated_date = models.DateTimeField(auto_now=True)
    history = HistoricalRecords(inherit=True, table_name='base_model_history')
  
class ChildModel(BaseModel):
    title = models.CharField(max_length=255, blank=False, null=False)

Following error is thrown with above code:

(models.E028) db_table 'base_model_history' is used by multiple models: inherit.HistoricalBaseModel, inherit.HistoricalChildModel.

Describe the solution you'd like

Cause of Problem

As per my understanding, after looking at the source code, I found that Meta.db_table property of model is updated when table_name property is set. In case, when inherit=True, self.table_name still points to table_name set in the base model when create_history_model method is invoked for the child model. This logic is implemented in line 301-302 in create_history_model method.

Possible Solution

inherited flag is sent as an argument to create_history_model method, this can be used in the condition where table_name is checked. Condition at line 301 will look like this:
if not inherited and self.table_name is not None:

If it can not be done, then this behavior should be mentioned explicitly in the documentation.

Describe alternatives you've considered

Overridden create_history_model method to update table_name when inherited is set to True and table_name is set:

from django.db import models
from simple_history.models import HistoricalRecords


class CustomHistoricalRecords(HistoricalRecords):
    def create_history_model(self, model, inherited):
        if inherited and self.table_name:
            self.table_name=f"{model._meta.app_label}_{self.get_history_model_name(model)}"
        history_model = super().create_history_model(model, inherited)
        return history_model

class BaseModel(models.Model):
    created_date = models.DateTimeField(auto_now_add=True)
    last_updated_date = models.DateTimeField(auto_now=True)
    history = CustomHistoricalRecords(inherit=True, table_name="base_model_history")
  
class ChildModel(BaseModel):
    title = models.CharField(max_length=255, blank=False, null=False)

Additional context

I found this issue while looking for the solution of #1174

@muneeb706 muneeb706 changed the title db_table is used by multiple models error when setting table_name with inherit=True Error when setting table_name with inherit=True Jun 24, 2023
muneeb706 added a commit to muneeb706/django-simple-history that referenced this issue Jun 24, 2023
Error was thrown when table_name is set with inherit=True in base model.

- add not inherited check

jazzband#1195
ddabble added a commit that referenced this issue Aug 15, 2023
* fix(simple_histor.models): Custom history table name

Error was thrown when table_name is set with inherit=True in base model.

- add not inherited check

#1195

* fix(docs): Custom history table name

- authors and changes docs updated

* merge(master): Merged latest changes from master

* merge(master): Resolved conflicts

* merge(master): Resolved conflicts

* merge(master): Resolved conflicts

* merge(master): Resolved conflicts

* fix(simple_histor.models): Custom history table name

- unit test added

* Small improvement of the test for #1196

Now also compares the table name of the inherited (base) model, so that
it's easier to see that the history table name is not inherited.

* Added note on table_name inheritance to docs

---------

Co-authored-by: Anders <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant