Api improvements (#180)

* work around possible Django migration bugs

* cleaner solution that still seems to work

* update forms and tests for email-based users

* forgot to remove some spurious prints

* Add the requirements from fkprocess to venv so inotify is loaded

* oops, accidentally added a log file

* these changes seem unoffensive enough and let the local version work by default

* turns out you need to do this for the user model too

* Minimal upgrade to Django 2.2 that also passes tests

* run 0002 non-atomically for backwards compat

* basic user auth working now

* basic authentication is working

* further incremental tweaks to everything

* clean up database migrations

* add atomic = False to original 0002 migration

* make foreign key on_deletes more permissive

* retrofit migrations

Makefile

@@ -1,4 +1,4 @@
-pyreqs=packages/fkweb/requirements-dev.txt packages/fkupload/requirements.txt
+pyreqs=packages/fkweb/requirements-dev.txt packages/fkupload/requirements.txt packages/fkprocess/requirements.txt
 
 env = env/bin/activate
 ifneq ("$(wildcard $(env))","")

packages/fkweb/agenda/views.py

@@ -5,7 +5,7 @@
 
 from django.conf import settings
 from django.core.paginator import Paginator
-from django.core.urlresolvers import reverse
+from django.urls import reverse
 from django.forms import ModelForm
 from django.http import HttpResponseForbidden
 from django.shortcuts import redirect
@@ -69,7 +69,7 @@ def get(self, request, form = None):
 
 class ManageVideoList(TemplateView):
   def get(self, request):
-    if not request.user.is_authenticated():
+    if not request.user.is_authenticated:
       return redirect('/login/?next=%s' % request.path)
     context = {}
     context["title"] = _('My videos')
@@ -155,7 +155,7 @@ def get_form(self, request, data=None, initial={}, form=None, instance=None):
 
 class ManageVideoNew(AbstractVideoFormView):
   def get(self, request, form=None):
-    if not request.user.is_authenticated() or not request.user.is_superuser:
+    if not request.user.is_authenticated or not request.user.is_superuser:
       return redirect('/login/?next=%s' % request.path)
     initial = {}
     form = self.get_form(request, initial=initial, form=form)
@@ -166,7 +166,7 @@ def get(self, request, form=None):
     return render(request, 'agenda/manage_video_new.html', context)
 
   def post(self, request):
-    if not request.user.is_authenticated() or not request.user.is_superuser:
+    if not request.user.is_authenticated or not request.user.is_superuser:
       return redirect('/login/?next=%s' % request.path)
     if request.user.is_superuser:
       video = Video()
@@ -183,15 +183,15 @@ def post(self, request):
     return self.get(request, form=form)
 
 def allowed_to_edit(video, user):
-  return (user.is_authenticated()
+  return (user.is_authenticated
           and ((video.organization
                 and video.organization.members.filter(pk=user.id).exists())
                or user.is_superuser))
 
 class ManageVideoEdit(AbstractVideoFormView):
   Form = VideoFormForUsers
   def get(self, request, id=None, form=None):
-    if not request.user.is_authenticated():
+    if not request.user.is_authenticated:
       return redirect('/login/?next=%s' % request.path)
     video = Video.objects.get(id=id)
     if not allowed_to_edit(video, request.user):
@@ -207,7 +207,7 @@ def get(self, request, id=None, form=None):
     return render(request, 'agenda/manage_video_new.html', context)
 
   def post(self, request, id):
-    if not request.user.is_authenticated():
+    if not request.user.is_authenticated:
       return redirect('/login/?next=%s' % request.path)
     video = Video.objects.get(id=id)
     if not allowed_to_edit(video, request.user):

packages/fkweb/fk/fixtures/test.yaml

@@ -1,25 +1,25 @@
 - model: fk.user
   pk: 1
   fields:
-    email: nuug_user
+    email: nuug_user@fake.com
     is_active: true
     password: "pbkdf2_sha256$10000$HHCakMLbFW70$+otoYf+1VFEcfoYKbhQOA6/JZrgq+4aTpohtzT+/pf8="  # password = test
 - model: fk.user
   pk: 2
   fields:
-    email: dummy_user
+    email: dummy_user@fake.com
     is_active: true
     password: "pbkdf2_sha256$10000$HHCakMLbFW70$+otoYf+1VFEcfoYKbhQOA6/JZrgq+4aTpohtzT+/pf8="  # password = test
 - model: fk.user
   pk: 3
   fields:
-    email: noorg_user
+    email: noorg_user@fake.com
     is_active: true
     password: "pbkdf2_sha256$10000$HHCakMLbFW70$+otoYf+1VFEcfoYKbhQOA6/JZrgq+4aTpohtzT+/pf8="  # password = test
 - model: fk.user
   pk: 4
   fields:
-    email: staff_user
+    email: staff_user@fake.com
     is_active: true
     is_superuser: true
     password: "pbkdf2_sha256$10000$HHCakMLbFW70$+otoYf+1VFEcfoYKbhQOA6/JZrgq+4aTpohtzT+/pf8="  # password = test

packages/fkweb/fk/forms.py

@@ -10,7 +10,7 @@ class UserForm(forms.ModelForm):
     pass
     class Meta:
         model = User
-        fields = ['first_name', 'last_name', 'email']
+        fields = ['first_name', 'last_name']
 
 
 class UserChangeForm(forms.ModelForm):
@@ -22,7 +22,7 @@ class UserChangeForm(forms.ModelForm):
 
     class Meta:
         model = User
-        fields = ('email', 'password', 'date_of_birth', 'is_active', 'is_superuser')
+        fields = ('password', 'date_of_birth', 'is_active', 'is_superuser')
 
     def clean_password(self):
         # Regardless of what the user provides, return the initial value.

packages/fkweb/fk/migrations/0002_rename_all_custom_tables_to_default.py

@@ -6,6 +6,7 @@
 
 
 class Migration(migrations.Migration):
+    atomic = False
 
     dependencies = [
         ('fk', '0001_initial'),

packages/fkweb/fk/migrations/0006_add_foreign_key_constraints.py

@@ -0,0 +1,55 @@
+# Generated by Django 2.2 on 2020-01-25 10:45
+
+from django.conf import settings
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('fk', '0005_user_rm_old_fields'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='asrun',
+            name='video',
+            field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, to='fk.Video'),
+        ),
+        migrations.AlterField(
+            model_name='scheduleitem',
+            name='video',
+            field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, to='fk.Video'),
+        ),
+        migrations.AlterField(
+            model_name='schedulepurpose',
+            name='organization',
+            field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, to='fk.Organization'),
+        ),
+        migrations.AlterField(
+            model_name='video',
+            name='editor',
+            field=models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, to=settings.AUTH_USER_MODEL),
+        ),
+        migrations.AlterField(
+            model_name='video',
+            name='organization',
+            field=models.ForeignKey(help_text='Organization for video', null=True, on_delete=django.db.models.deletion.PROTECT, to='fk.Organization'),
+        ),
+        migrations.AlterField(
+            model_name='videofile',
+            name='format',
+            field=models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, to='fk.FileFormat'),
+        ),
+        migrations.AlterField(
+            model_name='videofile',
+            name='video',
+            field=models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, to='fk.Video'),
+        ),
+        migrations.AlterField(
+            model_name='weeklyslot',
+            name='purpose',
+            field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, to='fk.SchedulePurpose'),
+        ),
+    ]

packages/fkweb/fk/migrations/0007_work_around_table_name_mishigas.py

@@ -0,0 +1,54 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.26 on 2019-11-17 21:03
+from __future__ import unicode_literals
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+    atomic = False
+
+    dependencies = [
+        ('fk', '0006_add_foreign_key_constraints'),
+    ]
+
+    operations = [
+        # By calling RenameModel, for some reason Django no longer
+        # fails to update the ForeignKey references in the schema.
+        migrations.RenameModel('User', 'UserNew'),
+        migrations.AlterModelTable(
+            name='UserNew',
+            table=None,
+        ),
+        migrations.RenameModel('UserNew', 'User'),
+        migrations.AlterModelOptions(
+            name='user',
+            options={'verbose_name': 'user', 'verbose_name_plural': 'users'},
+        ),
+        migrations.AlterModelTable(
+            name='category',
+            table=None,
+        ),
+        migrations.RenameModel('FileFormat', 'FileFormatNew'),
+        migrations.AlterModelTable(
+            name='fileformatnew',
+            table=None,
+        ),
+        migrations.RenameModel('FileFormatNew', 'FileFormat'),
+        migrations.RenameModel('Organization', 'OrganizationNew'),
+        migrations.AlterModelTable(
+            name='organizationnew',
+            table=None,
+        ),
+        migrations.RenameModel('OrganizationNew', 'Organization'),
+        migrations.AlterModelTable(
+            name='scheduleitem',
+            table=None,
+        ),
+        migrations.RenameModel('Video', 'VideoNew'),
+        migrations.AlterModelTable(
+            name='videonew',
+            table=None,
+        ),
+        migrations.RenameModel('VideoNew', 'Video'),
+    ]

packages/fkweb/fk/models.py

@@ -7,8 +7,9 @@
 import pytz
 from django.conf import settings
 from django.contrib.auth.models import AbstractBaseUser, BaseUserManager
+from django.contrib.auth import get_user_model
 from django.core.exceptions import ObjectDoesNotExist
-from django.core.urlresolvers import reverse
+from django.urls import reverse
 from django.db import models
 from django.db.models.signals import post_save
 from django.utils import timezone
@@ -161,8 +162,9 @@ def __str__(self):
 class VideoFile(models.Model):
     id = models.AutoField(primary_key=True)
     # uploader = models.ForeignKey(User) # Not migrated
-    video = models.ForeignKey("Video")
-    format = models.ForeignKey("FileFormat")
+    # Until we have some way of handling file deletion, I'm erring on the side of caution
+    video = models.ForeignKey("Video", on_delete=models.PROTECT)
+    format = models.ForeignKey("FileFormat", on_delete=models.PROTECT)
     filename = models.CharField(max_length=256)
     old_filename = models.CharField(max_length=256, default='', blank=True)
     # source = video = models.ForeignKey("VideoFile")
@@ -233,7 +235,7 @@ class Video(models.Model):
     # Code for editors' internal use
     # production_code = models.CharField(null=True,max_length=255)
     categories = models.ManyToManyField(Category)
-    editor = models.ForeignKey(settings.AUTH_USER_MODEL)
+    editor = models.ForeignKey(get_user_model(), on_delete=models.PROTECT)
     has_tono_records = models.BooleanField(default=False)
     is_filler = models.BooleanField('Play automatically?',
                                     help_text = 'You still have the editorial responsibility.  Only affect videos from members.',
@@ -263,7 +265,8 @@ class Video(models.Model):
         default=25000,
         help_text='Framerate of master video in thousands / second')
     organization = models.ForeignKey(
-        Organization, null=True, help_text='Organization for video')
+        Organization, null=True, help_text='Organization for video',
+        on_delete=models.PROTECT)
     ref_url = models.CharField(
         blank=True, max_length=1024, help_text='URL for reference')
     duration = models.DurationField(blank=True, default=datetime.timedelta(0))
@@ -430,7 +433,7 @@ class Scheduleitem(models.Model):
 
     id = models.AutoField(primary_key=True)
     default_name = models.CharField(max_length=255, blank=True)
-    video = models.ForeignKey(Video, null=True, blank=True)
+    video = models.ForeignKey(Video, null=True, blank=True, on_delete=models.SET_NULL)
     schedulereason = models.IntegerField(blank=True, choices=SCHEDULE_REASONS)
     starttime = models.DateTimeField()
     duration = models.DurationField()
@@ -473,7 +476,7 @@ class SchedulePurpose(models.Model):
     strategy = models.CharField(max_length=32, choices=STRATEGY)
 
     # You probably need one of these depending on type and strategy
-    organization = models.ForeignKey(Organization, blank=True, null=True)
+    organization = models.ForeignKey(Organization, blank=True, null=True, on_delete=models.SET_NULL)
     direct_videos = models.ManyToManyField(Video, blank=True)
 
     class Meta:
@@ -535,7 +538,7 @@ class WeeklySlot(models.Model):
         (6, _('Sunday')),
     )
 
-    purpose = models.ForeignKey(SchedulePurpose, null=True, blank=True)
+    purpose = models.ForeignKey(SchedulePurpose, null=True, blank=True, on_delete=models.SET_NULL)
     day = models.IntegerField(
         choices=DAY_OF_THE_WEEK,
     )
@@ -598,7 +601,7 @@ class AsRun(TimeStampedModel):
                how long we live streamed a particular URL.
                Can be null (None) if this is 'currently happening'.
     """
-    video = models.ForeignKey(Video, blank=True, null=True)
+    video = models.ForeignKey(Video, blank=True, null=True, on_delete=models.SET_NULL)
     program_name = models.CharField(max_length=160, blank=True, default='')
     playout = models.CharField(max_length=255, blank=True, default='main')
     played_at = models.DateTimeField(blank=True, default=timezone.now)

packages/fkweb/fk/tests.py

@@ -3,7 +3,7 @@
 import datetime
 
 from django.contrib.auth import get_user_model
-from django.core.urlresolvers import reverse
+from django.urls import reverse
 from django.test import TestCase
 from django.utils import timezone
 
@@ -15,7 +15,7 @@ class UserRegistrationTest(TestCase):
 
 
     def setUp(self):
-        self.client.login(email='staff_user', password='test')
+        self.client.login(email='staff_user@fake.com', password='test')
 
 
     def test_user_profile_update(self):
@@ -25,12 +25,13 @@ def test_user_profile_update(self):
             reverse('profile'), {
                 'first_name': 'Firstname',
                 'last_name':  'Lastname',
+        #        'email':  '[email protected]',
                 'country':    'Norway'
             })
-        u = get_user_model().objects.get(email='staff_user')
+        u = get_user_model().objects.get(email='staff_user@fake.com')
         self.assertEqual('Firstname', u.first_name)
         self.assertEqual('Lastname', u.last_name)
-        self.assertEqual('staff_user', u.email)
+        self.assertEqual('staff_user@fake.com', u.email)
         # Uncomment when https://github.com/Frikanalen/frikanalen/issues/77 is fixed
         #self.assertEqual('Norway', u.userprofile.country)
 
@@ -245,7 +246,7 @@ def test_api_scheduleitems_list(self):
             ['00:00:10.010000', '00:01:00'],
             [v['video']['duration'] for v in r.data['results']])
         self.assertEqual(
-            ['nuug_user', 'dummy_user'],
+            ['nuug_user@fake.com', 'dummy_user@fake.com'],
             [v['video']['editor'] for v in r.data['results']])
 
 

packages/fkweb/fk/views.py

@@ -2,7 +2,7 @@
 
 from django import forms
 from django.contrib.auth import login, authenticate
-from django.core.urlresolvers import reverse
+from django.urls import reverse
 from django.http import HttpResponseRedirect
 from django.shortcuts import render
 from django.utils import timezone

packages/fkweb/fkbeta/settings/base.py

@@ -164,7 +164,8 @@
             'loaders': [
                 'django.template.loaders.filesystem.Loader',
                 'django.template.loaders.app_directories.Loader',
-                'django.template.loaders.eggs.Loader',
+                # deprecated in Django 2
+                # 'django.template.loaders.eggs.Loader',
             ],
             # See: https://docs.djangoproject.com/en/dev/ref/settings/#template-context-processors
             'context_processors': [

packages/fkweb/fkbeta/urls.py

@@ -4,7 +4,7 @@
 
 from django.conf.urls import include, url
 from django.contrib import admin
-from django.contrib.auth.views import login, logout
+from django.contrib.auth import login, logout
 from django.contrib.staticfiles.urls import staticfiles_urlpatterns
 
 import agenda.urls
@@ -25,7 +25,7 @@
     url(r'^logout/$', logout, {'next_page': '/'}, name='logout'),
 
     url(r'^create/', include('create.urls')),
-    url(r'^admin/', include(admin.site.urls)),
+    url(r'^admin/', admin.site.urls),
     url(r'^', include('fk.urls')),
 ]