safwanrahman / django-webpush

Web Push Notification Package for Django
GNU General Public License v3.0
371 stars 107 forks source link

Migrations problems with Django 3 #110

Closed Trafitto closed 2 years ago

Trafitto commented 2 years ago

Hi, I am using this package with Django 3, and I have noticed that doing python manage.py makemigrations creates me a migration that adds the ids to the models. The migration that is created is called 0003_auto_xxxxxxx_xxx.py and has a dependency on 0002_auto_20190603_0005.py ignoring the migration 0003_subscriptioninfo_user_agent.py. I fixed this problem in my fork #111

safwanrahman commented 2 years ago

Closed by #111

tyilo commented 2 years ago

After this fix and running ./manage.py makemigrations Django generates the following migration in my project:

# Generated by Django 3.2.16 on 2022-11-22 20:21

from django.db import migrations, models

class Migration(migrations.Migration):

    dependencies = [
        ('webpush', '0004_auto_20220831_1500'),
    ]

    operations = [
        migrations.AlterField(
            model_name='group',
            name='id',
            field=models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID'),
        ),
        migrations.AlterField(
            model_name='pushinformation',
            name='id',
            field=models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID'),
        ),
        migrations.AlterField(
            model_name='subscriptioninfo',
            name='id',
            field=models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID'),
        ),
    ]

I think it was a mistake to change the fields to BigAutoField as done in the pull request.

satoon101 commented 1 year ago

I noticed the same thing as @tyilo. Instead of using BigAutoField, it should respect the app's DEFAULT_AUTO_FIELD setting. The whole reason that @Trafitto had to create a migration in the first place is likely because they were using BigAutoField as the DEFAULT_AUTO_FIELD.

satoon101 commented 1 year ago

Actually, looking at this stack overflow answer, you should be able to add an apps.py and make a default there.

satoon101 commented 1 year ago

I believe that #122 should fix this issue for everyone going forward.