Move bookmarks from EmailUser to UserProfile (#171)

This PR effects the following steps:
- step 1: create a profiles app
- step 2: associate user profiles for each existing user
- intermediate step: code refactors while making sure to keep passing tests
- step 3: create parallel bookmark data by copying from users to asscoaited profiles
- step 4: rebing everything to profile.UserBookmarks
- step 5: finally, removed the bookmark code from the user model

pulseapi/entries/serializers.py

@@ -7,8 +7,7 @@ from pulseapi.tags.models import Tag
 from pulseapi.issues.models import Issue
 from pulseapi.helptypes.models import HelpType
 from pulseapi.creators.models import Creator
-from pulseapi.users.models import EmailUser, UserBookmarks
-from pulseapi.users.serializers import UserBookmarksSerializer
+from pulseapi.profiles.models import UserProfile
 
 class CreatableSlugRelatedField(serializers.SlugRelatedField):
     """
@@ -97,7 +96,8 @@ class EntrySerializer(serializers.ModelSerializer):
         if hasattr(request, 'user'):
             user = request.user
             if user.is_authenticated():
-                res = instance.bookmarked_by.filter(user=user)
+                profile = UserProfile.objects.get(user=user)
+                res = instance.bookmarked_by.filter(profile=profile)
                 return res.count() > 0
 
         return False

pulseapi/entries/views.py

@@ -21,7 +21,8 @@ from pulseapi.entries.serializers import (
     EntrySerializer,
     ModerationStateSerializer
 )
-from pulseapi.users.models import EmailUser, UserBookmarks
+from pulseapi.users.models import EmailUser
+from pulseapi.profiles.models import UserProfile, UserBookmarks
 
 from pulseapi.utility.userpermissions import is_staff_address
 
@@ -37,6 +38,7 @@ def toggle_bookmark(request, entryid):
 
     if user.is_authenticated():
         entry = None
+        profile = UserProfile.objects.get(user=user)
 
         # find the entry for this id
         try:
@@ -45,7 +47,7 @@ def toggle_bookmark(request, entryid):
             return Response("No such entry", status=status.HTTP_404_NOT_FOUND)
 
         # find out if there is already a {user,entry,(timestamp)} triple
-        bookmarks = entry.bookmarked_by.filter(user=user)
+        bookmarks = entry.bookmarked_by.filter(profile=profile)
         exists = bookmarks.count() > 0
 
         # if there is a bookmark, remove it. Otherwise, make one.
@@ -53,7 +55,7 @@ def toggle_bookmark(request, entryid):
             for bookmark in bookmarks:
                 bookmark.delete()
         else:
-            bookmark = UserBookmarks(entry=entry, user=user)
+            bookmark = UserBookmarks(entry=entry, profile=profile)
             bookmark.save()
 
         return Response("Toggled bookmark.", status=status.HTTP_204_NO_CONTENT)
@@ -225,7 +227,8 @@ class BookmarkedEntries(ListAPIView):
         if user.is_authenticated() is False:
             return Entry.objects.none()
 
-        bookmarks = UserBookmarks.objects.filter(user=user)
+        profile = UserProfile.objects.get(user=user)
+        bookmarks = UserBookmarks.objects.filter(profile=profile)
         return Entry.objects.filter(bookmarked_by__in=bookmarks).order_by('-bookmarked_by__timestamp')
 
     # When people POST to this route, we want to do some
@@ -254,12 +257,13 @@ class BookmarkedEntries(ListAPIView):
                     return
 
                 # find out if there is already a {user,entry,(timestamp)} triple
-                bookmarks = entry.bookmarked_by.filter(user=user)
+                profile = UserProfile.objects.get(user=user)
+                bookmarks = entry.bookmarked_by.filter(profile=profile)
                 exists = bookmarks.count() > 0
 
                 # make a bookmark if there isn't one already
                 if exists is False:
-                    bookmark = UserBookmarks(entry=entry, user=user)
+                    bookmark = UserBookmarks(entry=entry, profile=profile)
                     bookmark.save()
 
             if ids is not None and user.is_authenticated():

pulseapi/profiles/admin.py

@@ -0,0 +1,19 @@
+from django.contrib import admin
+from .models import UserProfile, UserBookmarks
+
+class UserProfileAdmin(admin.ModelAdmin):
+    """
+    Show the profile-associated user.
+    """
+    fields = ('user',)
+    readonly_fields = ('user',)
+
+class UserBookmarksAdmin(admin.ModelAdmin):
+	"""
+	...
+	"""
+	fields = ('entry', 'profile', 'timestamp')
+	readonly_fields = ('entry', 'profile', 'timestamp')
+
+admin.site.register(UserProfile, UserProfileAdmin)
+admin.site.register(UserBookmarks, UserBookmarksAdmin)

pulseapi/profiles/apps.py

@@ -0,0 +1,5 @@
+from django.apps import AppConfig
+
+
+class UsersConfig(AppConfig):
+    name = 'user profiles'

pulseapi/profiles/migrations/0001_initial.py

@@ -0,0 +1,25 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.10.3 on 2017-08-03 20:30
+from __future__ import unicode_literals
+
+from django.conf import settings
+from django.db import migrations, models
+import django.db.models.deletion
+
+class Migration(migrations.Migration):
+
+    initial = True
+
+    dependencies = [
+        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='UserProfile',
+            fields=[
+                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+                ('user', models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='profile', to=settings.AUTH_USER_MODEL)),
+            ],
+        ),
+    ]

pulseapi/profiles/migrations/0002_auto_20170803_1333.py

@@ -0,0 +1,26 @@
+from django.db import migrations
+
+from pulseapi.users.models import EmailUser
+from pulseapi.profiles.models import UserProfile
+
+def ensure_user_profiles(app, schema_editor):
+    """
+    The only thing this function does is ensure that for every user
+    of the system, we have an associated user profile, even if it
+    does absolutely nothing (yet).
+    """
+    users = EmailUser.objects.all()
+    for user in users:
+        (profile,created) = UserProfile.objects.get_or_create(user=user)
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('profiles', '0001_initial'),
+        ('users', '0004_auto_20170616_1131'),
+    ]
+
+    operations = [
+        migrations.RunPython(ensure_user_profiles),
+    ]

pulseapi/profiles/migrations/0003_userbookmarks.py

@@ -0,0 +1,28 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.10.3 on 2017-08-04 18:28
+from __future__ import unicode_literals
+
+from django.conf import settings
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
+        ('entries', '0013_entry_help_types'),
+        ('profiles', '0002_auto_20170803_1333'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='UserBookmarks',
+            fields=[
+                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+                ('timestamp', models.DateTimeField(auto_now=True)),
+                ('entry', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='bookmarked_by', to='entries.Entry')),
+                ('user', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='bookmark_entries_from_profile', to=settings.AUTH_USER_MODEL)),
+            ],
+        ),
+    ]

pulseapi/profiles/migrations/0004_userbookmarks_profile.py

@@ -0,0 +1,21 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.10.3 on 2017-08-04 18:40
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('profiles', '0003_userbookmarks'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='userbookmarks',
+            name='profile',
+            field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.CASCADE, related_name='bookmark_entries_from_profile', to='profiles.UserProfile'),
+        ),
+    ]

pulseapi/profiles/migrations/0005_copy_user_bookmarks.py

@@ -0,0 +1,41 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.10.3 on 2017-08-04 18:40
+from __future__ import unicode_literals
+
+from django.db import migrations
+from pulseapi import users, profiles
+
+
+def copy_bookmarks_from_user_to_prolfies(apps, schema_editor):
+    EmailUser = apps.get_model('users', 'EmailUser')
+    ProfileUserBookmarks = apps.get_model('profiles', 'UserBookmarks')
+    UserProfile = apps.get_model('profiles', 'UserProfile')
+
+    current_bookmarks = apps.get_model('users', 'UserBookmarks').objects.all()
+    for bookmark in current_bookmarks:
+        user = EmailUser.objects.get(id=bookmark.user.id)
+        profile = UserProfile.objects.get(user=user)
+
+        (nbm, created) = ProfileUserBookmarks.objects.get_or_create(
+            user=user,
+            profile=profile,
+            entry=bookmark.entry,
+            timestamp=bookmark.timestamp
+        )
+
+        if created is True:
+            msg = "created a parallel bookmark between user:{u} and entry:{e}"
+            msg = msg.format(u=user.id, e=bookmark.entry.id)
+            print(msg)
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('profiles', '0004_userbookmarks_profile'),
+        ('users', '0004_auto_20170616_1131'),
+    ]
+
+    operations = [
+        migrations.RunPython(copy_bookmarks_from_user_to_prolfies),
+    ]

pulseapi/profiles/migrations/0006_cleanup_due_to_bookmark_rebind_in_users_emailuser.py

@@ -0,0 +1,22 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.10.3 on 2017-08-04 19:49
+from __future__ import unicode_literals
+
+from django.conf import settings
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('profiles', '0005_copy_user_bookmarks'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='userbookmarks',
+            name='user',
+            field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='bookmark_entries_from_user', to=settings.AUTH_USER_MODEL),
+        ),
+    ]

pulseapi/profiles/migrations/0007_auto_20170804_1303.py

@@ -0,0 +1,36 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.10.3 on 2017-08-04 20:03
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('entries', '0013_entry_help_types'),
+        ('profiles', '0006_cleanup_due_to_bookmark_rebind_in_users_emailuser'),
+    ]
+
+    operations = [
+        migrations.RemoveField(
+            model_name='userbookmarks',
+            name='user',
+        ),
+        migrations.AddField(
+            model_name='userprofile',
+            name='bookmarks',
+            field=models.ManyToManyField(through='profiles.UserBookmarks', to='entries.Entry'),
+        ),
+        migrations.AlterField(
+            model_name='userbookmarks',
+            name='entry',
+            field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='bookmarked_by', to='entries.Entry'),
+        ),
+        migrations.AlterField(
+            model_name='userbookmarks',
+            name='profile',
+            field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.CASCADE, related_name='bookmarks_from', to='profiles.UserProfile'),
+        ),
+    ]

pulseapi/profiles/models.py

@@ -0,0 +1,64 @@
+from django.db import models
+
+class UserProfile(models.Model):
+    """
+    This class houses all user profile information,
+    such as real name, social media information,
+    bookmarks on the site, etc.
+    """
+
+    user = models.ForeignKey(
+        'users.EmailUser',
+        on_delete=models.SET_NULL,
+        related_name='profile',
+        null=True
+    )
+
+    # "user X bookmarked entry Y" is a many to many relation,
+    # for which we also want to know *when* a user bookmarked
+    # a specific entry. As such, we use a helper class that
+    # tracks this relation as well as the time it's created.
+    bookmarks = models.ManyToManyField(
+        'entries.Entry',
+        through='profiles.UserBookmarks'
+    )
+
+    def __str__(self):
+        return 'profile for {}'.format(self.user.email)
+   
+    class Meta:
+        verbose_name = "Profile"
+
+
+class UserBookmarks(models.Model):
+    """
+    This class is used to link users and entries through a
+    "bookmark" relation. One user can bookmark many entries,
+    and one entry can have bookmarks from many users.
+    """
+    entry = models.ForeignKey(
+        'entries.Entry',
+        on_delete=models.CASCADE,
+        related_name='bookmarked_by'
+    )
+
+    profile = models.ForeignKey(
+        'profiles.UserProfile',
+        on_delete=models.CASCADE,
+        related_name='bookmarks_from',
+        null=True
+    )
+
+    timestamp = models.DateTimeField(
+        auto_now=True,
+    )
+
+    def __str__(self):
+        return 'bookmark for "{e}" by [{p}]'.format(
+            e=self.entry,
+            p=self.profile.id
+        )
+
+    class Meta:
+        verbose_name = "Bookmarks"
+        verbose_name_plural = "Bookmarks"

pulseapi/profiles/serializers.py

@@ -0,0 +1,14 @@
+"""Serialize the models"""
+from rest_framework import serializers
+from pulseapi.users.models import UserBookmarks
+
+class UserBookmarksSerializer(serializers.ModelSerializer):
+    """
+    Serializes a {user,entry,when} bookmark.
+    """
+
+    class Meta:
+        """
+        Meta class. Again: because
+        """
+        model = UserBookmarks

pulseapi/profiles/urls.py

@@ -0,0 +1,5 @@
+from django.conf.urls import url
+
+from . import views
+
+urlpatterns = []

pulseapi/profiles/views.py

@@ -0,0 +1 @@
+from django.conf import settings

pulseapi/settings.py

@@ -66,6 +66,7 @@ INSTALLED_APPS = [
     'pulseapi.issues',
     'pulseapi.helptypes',
     'pulseapi.users',
+    'pulseapi.profiles',
     'pulseapi.creators',
 ]
 

pulseapi/tests.py

@@ -59,7 +59,7 @@ def create_logged_in_user(test, name, email, password="password1234"):
 
     # create use instance
     User = EmailUser
-    user = User.objects.create(name=name, email=email, password=password)
+    user = User.objects.create_user(name=name, email=email, password=password)
     user.save()
 
     # make sure this user is in the staff group, too

pulseapi/users/admin.py

@@ -3,26 +3,18 @@ Admin setings for EmailUser app
 """
 from django.contrib import admin
 from django.contrib.auth.models import Group
-from .models import EmailUser
+from django.utils.html import format_html
 
-class UserBookmarksInline(admin.TabularInline):
-    """
-    We need an inline widget before we can do anything
-    with the user/entry bookmark data.
-    """
-    model = EmailUser.bookmarks.through
-    verbose_name = 'UserBookmarks'
+from .models import EmailUser
+from pulseapi.profiles.models import UserProfile, UserBookmarks
 
 
 class EmailUserAdmin(admin.ModelAdmin):
     """
     Show a list of entries a user has submitted in the EmailUser Admin app
     """
-    fields = ('password', 'last_login', 'email', 'name', 'entries','bookmarks', 'is_staff', 'is_superuser')
-    readonly_fields = ('entries','bookmarks')
-
-    # this allows us to create/edit/delete/etc bookmarks:
-    inlines = [ UserBookmarksInline ]
+    fields = ('password', 'last_login', 'email', 'name', 'is_staff', 'is_superuser', 'profile', 'entries','bookmarks', )
+    readonly_fields = ('entries','bookmarks','profile')
 
     def entries(self, instance):
         """
@@ -30,6 +22,26 @@ class EmailUserAdmin(admin.ModelAdmin):
         """
         return ", ".join([str(entry) for entry in instance.entries.all()])
 
+    def profile(self, instance):
+        """
+        Link to this user's profile
+        """
+        profile = UserProfile.objects.get(user=instance)
+
+        html = '<a href="/admin/profiles/userprofile/{id}/change/">Click here for this user\'s profile</a>'.format(
+            id=profile.id,
+        )
+
+        return format_html(html)
+
+    def bookmarks(self, instance):
+        """
+        Show all bookmarked entries as a string of titles. In the future we should make them links.
+        """
+        profile = UserProfile.objects.get(user=instance)
+        return ", ".join([str(bookmark.entry) for bookmark in profile.bookmarks])
+
+    profile.short_description = 'User profile'
 
 admin.site.register(EmailUser, EmailUserAdmin)
 

pulseapi/users/migrations/0005_rebind_userbookmarks_from_profile.py

@@ -0,0 +1,31 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.10.3 on 2017-08-04 19:49
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('users', '0004_auto_20170616_1131'),
+    ]
+
+    operations = [
+        migrations.RemoveField(
+            model_name='userbookmarks',
+            name='entry',
+        ),
+        migrations.RemoveField(
+            model_name='userbookmarks',
+            name='user',
+        ),
+        migrations.AlterField(
+            model_name='emailuser',
+            name='bookmarks',
+            field=models.ManyToManyField(related_name='bookmark_by_profile', through='profiles.UserBookmarks', to='entries.Entry'),
+        ),
+        migrations.DeleteModel(
+            name='UserBookmarks',
+        ),
+    ]

pulseapi/users/migrations/0006_remove_emailuser_bookmarks.py

@@ -0,0 +1,19 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.10.3 on 2017-08-04 20:03
+from __future__ import unicode_literals
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('users', '0005_rebind_userbookmarks_from_profile'),
+    ]
+
+    operations = [
+        migrations.RemoveField(
+            model_name='emailuser',
+            name='bookmarks',
+        ),
+    ]

pulseapi/users/models.py

@@ -5,6 +5,8 @@ from django.contrib.auth.models import (
     AbstractBaseUser,
     PermissionsMixin
 )
+from pulseapi.profiles.models import UserProfile
+
 
 class EmailUserManager(BaseUserManager):
     def create_user(self, name, email, password=None):
@@ -23,6 +25,11 @@ class EmailUserManager(BaseUserManager):
         )
         user.set_password(password)
         user.save()
+
+        # Ensure that new users get a user profile associated
+        # with them, even though it'll be empty by default.
+        UserProfile.objects.get_or_create(user=user)
+
         return user
 
     def create_superuser(self, name, email, password):
@@ -54,16 +61,6 @@ class EmailUser(AbstractBaseUser, PermissionsMixin):
         verbose_name="this user counts as django::staff",
     )
 
-    # "user X bookmarked entry Y" is a many to many relation,
-    # for which we also want to know *when* a user bookmarked
-    # a specific entry. As such, we use a helper class that
-    # tracks this relation as well as the time it's created.
-    bookmarks = models.ManyToManyField(
-        'entries.Entry',
-        through='UserBookmarks',
-        related_name='bookmark_by'
-    )
-
     USERNAME_FIELD = 'email'
     REQUIRED_FIELDS = ['name']
 
@@ -86,26 +83,3 @@ class EmailUser(AbstractBaseUser, PermissionsMixin):
 
     def __str__(self):
         return self.toString()
-
-
-class UserBookmarks(models.Model):
-    """
-    This class is used to link users and entries through a
-    "bookmark" relation. One user can bookmark many entries,
-    and one entry can have bookmarks from many users.
-    """
-    entry = models.ForeignKey(
-        'entries.Entry',
-        on_delete=models.CASCADE,
-        related_name='bookmarked_by'
-    )
-
-    user = models.ForeignKey(
-        EmailUser,
-        on_delete=models.CASCADE,
-        related_name='bookmark_entries'
-    )
-
-    timestamp = models.DateTimeField(
-        auto_now=True,
-    )

pulseapi/users/serializers.py

@@ -1,23 +1,9 @@
 """Serialize the models"""
 from rest_framework import serializers
 
-from pulseapi.users.models import (
-    EmailUser,
-    UserBookmarks,
-)
+from pulseapi.users.models import EmailUser
 
 
-class UserBookmarksSerializer(serializers.ModelSerializer):
-    """
-    Serializes a {user,entry,when} bookmark.
-    """
-
-    class Meta:
-        """
-        Meta class. Again: because
-        """
-        model = UserBookmarks
-
 class EmailUserSerializer(serializers.ModelSerializer):
     """
     Serializes an EmailUser...