Skip to content

Commit

Permalink
fix: stabilize makemigrations when SITE_ID != 1 (#34787)
Browse files Browse the repository at this point in the history
Some models in third_party_auth used settings.SITE_ID as a field
default, which caused Django to say migrations were out of sync whenever
settings.SITE_ID happened to be anything other than 1 for any developer:

    Your models in app(s): 'third_party_auth' have changes that are not
    yet reflected in a migration, and so won't be applied. Run
    'manage.py makemigrations' to make new migrations, and then re-run
    'manage.py migrate' to apply them.

This could easily happen if a developer is testing out site
configuration or site-specific theming and ends up with a SITE_ID other
than 1.

The fix, inspired by a StackOverflow answer [1], is to simply create
a wrapper function for the dynamic default value. The wrapper function,
rather than the current value of SITE_ID, will be serialized to the
migraiton file.

This commit includes a migration file, but from a database perspective,
the migration is a no-op.

[1] https://stackoverflow.com/a/12654998
  • Loading branch information
kdmccormick authored May 22, 2024
1 parent 1162614 commit ccf2b75
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 2 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# Generated by Django 4.2.13 on 2024-05-13 19:22

import common.djangoapps.third_party_auth.models
from django.db import migrations, models
import django.db.models.deletion


class Migration(migrations.Migration):

dependencies = [
('sites', '0002_alter_domain_unique'),
('third_party_auth', '0012_alter_ltiproviderconfig_site_and_more'),
]

operations = [
migrations.AlterField(
model_name='ltiproviderconfig',
name='site',
field=models.ForeignKey(default=common.djangoapps.third_party_auth.models._get_site_id_from_settings, help_text='The Site that this provider configuration belongs to.', on_delete=django.db.models.deletion.CASCADE, related_name='%(class)ss', to='sites.site'),
),
migrations.AlterField(
model_name='oauth2providerconfig',
name='site',
field=models.ForeignKey(default=common.djangoapps.third_party_auth.models._get_site_id_from_settings, help_text='The Site that this provider configuration belongs to.', on_delete=django.db.models.deletion.CASCADE, related_name='%(class)ss', to='sites.site'),
),
migrations.AlterField(
model_name='samlconfiguration',
name='site',
field=models.ForeignKey(default=common.djangoapps.third_party_auth.models._get_site_id_from_settings, help_text='The Site that this SAML configuration belongs to.', on_delete=django.db.models.deletion.CASCADE, related_name='%(class)ss', to='sites.site'),
),
migrations.AlterField(
model_name='samlproviderconfig',
name='site',
field=models.ForeignKey(default=common.djangoapps.third_party_auth.models._get_site_id_from_settings, help_text='The Site that this provider configuration belongs to.', on_delete=django.db.models.deletion.CASCADE, related_name='%(class)ss', to='sites.site'),
),
]
22 changes: 20 additions & 2 deletions common/djangoapps/third_party_auth/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,24 @@ def __str__(self):
)


def _get_site_id_from_settings() -> int:
"""
Simply return SITE_ID from settings.
We define this function so the current SITE_ID can be used as the default value on a
couple fields in this module. We can't use `settings.SITE_ID` directly in the model class,
because then the migration file will contain whatever the value of SITE_ID was when the
migration was created. This value is usually 1, but in the event that a developer is
running a non-default site, they would get a value like 2 or 3, which will result in
a dirty migration state. By defining this wrapper function, the name
`_get_site_id_from_settings` will be serialized to the migration file as the default,
regardless of what any developer's active SITE_ID is.
Reference: https://docs.djangoproject.com/en/dev/ref/models/fields/#default
"""
return settings.SITE_ID


class ProviderConfig(ConfigurationModel):
"""
Abstract Base Class for configuring a third_party_auth provider
Expand Down Expand Up @@ -143,7 +161,7 @@ class ProviderConfig(ConfigurationModel):
)
site = models.ForeignKey(
Site,
default=settings.SITE_ID,
default=_get_site_id_from_settings,
related_name='%(class)ss',
help_text=_(
'The Site that this provider configuration belongs to.'
Expand Down Expand Up @@ -455,7 +473,7 @@ class SAMLConfiguration(ConfigurationModel):
KEY_FIELDS = ('site_id', 'slug')
site = models.ForeignKey(
Site,
default=settings.SITE_ID,
default=_get_site_id_from_settings,
related_name='%(class)ss',
help_text=_(
'The Site that this SAML configuration belongs to.'
Expand Down

0 comments on commit ccf2b75

Please sign in to comment.