From a6f7f11693e7d1cb59e3e1392c8c70486cc8d0e1 Mon Sep 17 00:00:00 2001 From: Mike Kamermans Date: Mon, 5 Feb 2018 12:42:58 -0800 Subject: [PATCH] Extended profile (#274) - add extended profile fields with serialization toggle - changed `/myprofile` PUT handling to prevent updates for data that is not exposed. --- pulseapi/profiles/admin.py | 19 ++- .../migrations/0013_auto_20180130_0953.py | 72 ++++++++++ .../migrations/0014_bootstrap_relations.py | 39 ++++++ .../migrations/0015_auto_20180201_1152.py | 46 +++++++ .../migrations/0016_auto_20180201_1257.py | 31 +++++ pulseapi/profiles/models.py | 126 ++++++++++++++++-- pulseapi/profiles/serializers.py | 54 +++++++- pulseapi/profiles/test_views.py | 75 ++++++++++- pulseapi/profiles/urls.py | 6 +- pulseapi/profiles/views.py | 10 +- pulseapi/tests.py | 11 ++ 11 files changed, 467 insertions(+), 22 deletions(-) create mode 100644 pulseapi/profiles/migrations/0013_auto_20180130_0953.py create mode 100644 pulseapi/profiles/migrations/0014_bootstrap_relations.py create mode 100644 pulseapi/profiles/migrations/0015_auto_20180201_1152.py create mode 100644 pulseapi/profiles/migrations/0016_auto_20180201_1257.py diff --git a/pulseapi/profiles/admin.py b/pulseapi/profiles/admin.py index fdf9098..f6881b2 100644 --- a/pulseapi/profiles/admin.py +++ b/pulseapi/profiles/admin.py @@ -1,7 +1,14 @@ from django.contrib import admin from django.utils.html import format_html from pulseapi.utility.get_admin_url import get_admin_url -from .models import Location, UserProfile, UserBookmarks +from .models import ( + Location, + ProfileType, + ProgramType, + ProgramYear, + UserProfile, + UserBookmarks, +) class LocationInline(admin.TabularInline): @@ -33,6 +40,12 @@ class UserProfileAdmin(admin.ModelAdmin): 'linkedin', 'github', 'website', + 'enable_extended_information', + 'profile_type', + 'program_type', + 'program_year', + 'affiliation', + 'user_bio_long', ) readonly_fields = ( @@ -72,3 +85,7 @@ class UserBookmarksAdmin(admin.ModelAdmin): admin.site.register(UserProfile, UserProfileAdmin) admin.site.register(UserBookmarks, UserBookmarksAdmin) + +admin.site.register(ProfileType) +admin.site.register(ProgramType) +admin.site.register(ProgramYear) diff --git a/pulseapi/profiles/migrations/0013_auto_20180130_0953.py b/pulseapi/profiles/migrations/0013_auto_20180130_0953.py new file mode 100644 index 0000000..8a75e59 --- /dev/null +++ b/pulseapi/profiles/migrations/0013_auto_20180130_0953.py @@ -0,0 +1,72 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.10.3 on 2018-01-30 17:53 +from __future__ import unicode_literals + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('profiles', '0012_remove_userprofile_user'), + ] + + operations = [ + migrations.CreateModel( + name='ProfileType', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('value', models.CharField(max_length=50)), + ], + ), + migrations.CreateModel( + name='ProgramType', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('value', models.CharField(max_length=150)), + ], + ), + migrations.CreateModel( + name='ProgramYear', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('value', models.CharField(max_length=25)), + ], + ), + migrations.AddField( + model_name='userprofile', + name='affiliation', + field=models.CharField(blank=True, max_length=200), + ), + migrations.AddField( + model_name='userprofile', + name='enable_extended_information', + field=models.BooleanField(default=False), + ), + migrations.AddField( + model_name='userprofile', + name='user_bio_long', + field=models.CharField(blank=True, max_length=4096), + ), + migrations.AlterField( + model_name='userprofile', + name='user_bio', + field=models.CharField(blank=True, max_length=212), + ), + migrations.AddField( + model_name='userprofile', + name='profile_type', + field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.CASCADE, to='profiles.ProfileType'), + ), + migrations.AddField( + model_name='userprofile', + name='program_type', + field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.CASCADE, to='profiles.ProgramType'), + ), + migrations.AddField( + model_name='userprofile', + name='program_year', + field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.CASCADE, to='profiles.ProgramYear'), + ), + ] diff --git a/pulseapi/profiles/migrations/0014_bootstrap_relations.py b/pulseapi/profiles/migrations/0014_bootstrap_relations.py new file mode 100644 index 0000000..84085bd --- /dev/null +++ b/pulseapi/profiles/migrations/0014_bootstrap_relations.py @@ -0,0 +1,39 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.10.3 on 2018-01-30 01:08 +from __future__ import unicode_literals + +from django.db import migrations + + +def setup_default_values(apps, schema_editor): + ProfileType = apps.get_model('profiles', 'ProfileType') + ProfileType.objects.get_or_create(value='plain') + ProfileType.objects.get_or_create(value='staff') + ProfileType.objects.get_or_create(value='fellow') + ProfileType.objects.get_or_create(value='board member') + ProfileType.objects.get_or_create(value='grantee') + + ProgramType = apps.get_model('profiles', 'ProgramType') + ProgramType.objects.get_or_create(value='senior fellow') + ProgramType.objects.get_or_create(value='science fellow') + ProgramType.objects.get_or_create(value='open web fellow') + ProgramType.objects.get_or_create(value='tech policy fellow') + ProgramType.objects.get_or_create(value='media fellow') + + ProgramYear = apps.get_model('profiles', 'ProgramYear') + ProgramYear.objects.get_or_create(value='2015') + ProgramYear.objects.get_or_create(value='2016') + ProgramYear.objects.get_or_create(value='2017') + ProgramYear.objects.get_or_create(value='2018') + ProgramYear.objects.get_or_create(value='2019') + + +class Migration(migrations.Migration): + + dependencies = [ + ('profiles', '0013_auto_20180130_0953'), + ] + + operations = [ + migrations.RunPython(setup_default_values), + ] diff --git a/pulseapi/profiles/migrations/0015_auto_20180201_1152.py b/pulseapi/profiles/migrations/0015_auto_20180201_1152.py new file mode 100644 index 0000000..50e0a58 --- /dev/null +++ b/pulseapi/profiles/migrations/0015_auto_20180201_1152.py @@ -0,0 +1,46 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.10.3 on 2018-02-01 19:52 +from __future__ import unicode_literals + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('profiles', '0014_bootstrap_relations'), + ] + + operations = [ + migrations.AlterField( + model_name='profiletype', + name='value', + field=models.CharField(max_length=50, unique=True), + ), + migrations.AlterField( + model_name='programtype', + name='value', + field=models.CharField(max_length=150, unique=True), + ), + migrations.AlterField( + model_name='programyear', + name='value', + field=models.CharField(max_length=25, unique=True), + ), + migrations.AlterField( + model_name='userprofile', + name='profile_type', + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, to='profiles.ProfileType'), + ), + migrations.AlterField( + model_name='userprofile', + name='program_type', + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, to='profiles.ProgramType'), + ), + migrations.AlterField( + model_name='userprofile', + name='program_year', + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, to='profiles.ProgramYear'), + ), + ] diff --git a/pulseapi/profiles/migrations/0016_auto_20180201_1257.py b/pulseapi/profiles/migrations/0016_auto_20180201_1257.py new file mode 100644 index 0000000..5e8e7ca --- /dev/null +++ b/pulseapi/profiles/migrations/0016_auto_20180201_1257.py @@ -0,0 +1,31 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.10.3 on 2018-02-01 20:57 +from __future__ import unicode_literals + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('profiles', '0015_auto_20180201_1152'), + ] + + operations = [ + migrations.AlterField( + model_name='userprofile', + name='profile_type', + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, to='profiles.ProfileType'), + ), + migrations.AlterField( + model_name='userprofile', + name='program_type', + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, to='profiles.ProgramType'), + ), + migrations.AlterField( + model_name='userprofile', + name='program_year', + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, to='profiles.ProgramYear'), + ), + ] diff --git a/pulseapi/profiles/models.py b/pulseapi/profiles/models.py index 8664eb7..774c70f 100644 --- a/pulseapi/profiles/models.py +++ b/pulseapi/profiles/models.py @@ -59,6 +59,66 @@ class Location(models.Model): ) +class ProfileType(models.Model): + """ + See https://github.com/mozilla/network-pulse/issues/657 + + Values that should exist (handled via migration): + + - plain + - staff + - fellow + - board member + - grantee + """ + value = models.CharField( + max_length=50, + unique=True + ) + + def get_default_profile_type(): + (default, _) = ProfileType.objects.get_or_create(value='plain') + return default + + def __str__(self): + return self.value + + +class ProgramType(models.Model): + """ + See https://github.com/mozilla/network-pulse/issues/657 + + These values are determined by pulse API administrators + (tech policy fellowship, mozfest speaker, etc) + """ + value = models.CharField( + max_length=150, + unique=True + ) + + def __str__(self): + return self.value + + +class ProgramYear(models.Model): + """ + See https://github.com/mozilla/network-pulse/issues/657 + + You'd think this would be 4 characters, but a "year" is + not a calendar year, so the year could just as easily + be "summer 2017" or "Q2 2016 - Q1 2018", so this is the + same kind of simple value model that the profile and + program types use. + """ + value = models.CharField( + max_length=25, + unique=True + ) + + def __str__(self): + return self.value + + class UserProfile(models.Model): """ This class houses all user profile information, @@ -74,12 +134,6 @@ class UserProfile(models.Model): default=False ) - # A tweet-style user bio - user_bio = models.CharField( - max_length=140, - blank=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 @@ -119,10 +173,11 @@ class UserProfile(models.Model): # We provide an easy accessor to the profile's user because # accessing the reverse relation (using related_name) can throw - # a RelatedObjectDoesNotExist exception for orphan profiles. This - # allows us to return None instead. - # We however cannot use this accessor as a lookup field in querysets - # because it is not an actual field. + # a RelatedObjectDoesNotExist exception for orphan profiles. + # This allows us to return None instead. + # + # Note: we cannot use this accessor as a lookup field in querysets + # because it is not an actual field. @property def user(self): # We do not import EmailUser directly so that we don't end up with @@ -207,6 +262,57 @@ class UserProfile(models.Model): blank=True ) + # --- extended information --- + + enable_extended_information = models.BooleanField( + default=False + ) + + profile_type = models.ForeignKey( + 'profiles.ProfileType', + null=True, + blank=True, + on_delete=models.SET_NULL + # default is handled in save() + ) + + program_type = models.ForeignKey( + 'profiles.ProgramType', + null=True, + blank=True, + on_delete=models.SET_NULL + ) + + program_year = models.ForeignKey( + 'profiles.ProgramYear', + null=True, + blank=True, + on_delete=models.SET_NULL + ) + + # Free form affiliation information + affiliation = models.CharField( + max_length=200, + blank=True + ) + + # A tweet-style user bio + user_bio = models.CharField( + max_length=212, + blank=True + ) + + # A long-form user bio + user_bio_long = models.CharField( + max_length=4096, + blank=True + ) + + def save(self, *args, **kwargs): + if self.profile_type is None: + self.profile_type = ProfileType.get_default_profile_type() + super(UserProfile, self).save(*args, **kwargs) + def __str__(self): if self.user is None: return 'orphan profile' diff --git a/pulseapi/profiles/serializers.py b/pulseapi/profiles/serializers.py index 3f39314..6015a88 100644 --- a/pulseapi/profiles/serializers.py +++ b/pulseapi/profiles/serializers.py @@ -9,6 +9,15 @@ from pulseapi.creators.models import OrderedCreatorRecord from pulseapi.entries.serializers import EntrySerializer +# Helper function to remove a value from a dictionary +# by key, removing the key itself as well. +def remove_key(data, key): + try: + del data[key] + except: + pass + + class UserBookmarksSerializer(serializers.ModelSerializer): """ Serializes a {user,entry,when} bookmark. @@ -24,12 +33,28 @@ class UserBookmarksSerializer(serializers.ModelSerializer): class UserProfileSerializer(serializers.ModelSerializer): """ Serializes a user profile. + + Note that the following fields should only show up when + the 'enable_extended_information' flag is set to True: + + - user_bio_long + - program_type + - program_year + - affiliation """ - user_bio = serializers.CharField( - max_length=140, - required=False, - allow_blank=True, - ) + + def __init__(self, instance=None, *args, **kwargs): + super().__init__(instance, *args, **kwargs) + if instance is not None: + if instance.enable_extended_information is False: + self.fields.pop('user_bio_long') + self.fields.pop('program_type') + self.fields.pop('program_year') + self.fields.pop('affiliation') + # Whether this flag is set or not, it should not + # end up in the actual serialized profile data. + self.fields.pop('enable_extended_information') + custom_name = serializers.CharField( max_length=70, required=False, @@ -66,6 +91,25 @@ class UserProfileSerializer(serializers.ModelSerializer): allow_blank=True, ) + user_bio = serializers.CharField( + max_length=140, + required=False, + allow_blank=True, + ) + + profile_type = serializers.StringRelatedField() + program_type = serializers.StringRelatedField() + program_year = serializers.StringRelatedField() + + def update(self, instance, validated_data): + if instance.enable_extended_information is False: + remove_key(validated_data, 'user_bio_long') + remove_key(validated_data, 'program_type') + remove_key(validated_data, 'program_year') + remove_key(validated_data, 'affiliation') + remove_key(validated_data, 'enable_extended_information') + return super(UserProfileSerializer, self).update(instance, validated_data) + class Meta: """ Meta class. Because diff --git a/pulseapi/profiles/test_views.py b/pulseapi/profiles/test_views.py index 4b76a00..f331940 100644 --- a/pulseapi/profiles/test_views.py +++ b/pulseapi/profiles/test_views.py @@ -2,6 +2,8 @@ import json from django.core.urlresolvers import reverse +from .models import UserProfile, ProfileType, ProgramType, ProgramYear + from pulseapi.tests import PulseMemberTestCase from pulseapi.entries.serializers import EntrySerializer from pulseapi.creators.models import OrderedCreatorRecord @@ -30,5 +32,76 @@ class TestProfileView(PulseMemberTestCase): ) created_entries = [EntrySerializer(x.entry).data for x in entry_creators] - self.assertEqual(entriesjson['created_entries'], created_entries) + + # make sure extended profile data does not show + self.assertEqual('program_type' in entriesjson, False) + + def test_extended_profile_data(self): + (profile, created) = UserProfile.objects.get_or_create(related_user=self.user) + profile.enable_extended_information = True + test_program = ProgramType.objects.all().first() + profile.program_type = test_program + profile.save() + + profile_url = reverse('profile', kwargs={'pk': profile.id}) + + # extended profile data should show in API responses + response = self.client.get(profile_url) + entriesjson = json.loads(str(response.content, 'utf-8')) + self.assertEqual('program_type' in entriesjson, True) + self.assertEqual(entriesjson['program_type'], test_program.value) + + def test_updating_extended_profile_data(self): + (profile, created) = UserProfile.objects.get_or_create(related_user=self.user) + profile.enable_extended_information = True + profile.program_type = ProgramType.objects.all().first() + profile.save() + + profile_url = reverse('myprofile') + + # authentication is absolutely required + self.client.logout() + response = self.client.put(profile_url, json.dumps({'affiliation': 'Mozilla'})) + self.assertEqual(response.status_code, 403) + + # with authentication, updates should work + self.client.force_login(user=self.user) + response = self.client.put(profile_url, json.dumps({'affiliation': 'Mozilla'})) + profile.refresh_from_db() + self.assertEqual(profile.affiliation, 'Mozilla') + + response = self.client.get(profile_url) + entriesjson = json.loads(str(response.content, 'utf-8')) + self.assertEqual('affiliation' in entriesjson, True) + self.assertEqual(entriesjson['affiliation'], 'Mozilla') + + def test_updating_disabled_extended_profile_data(self): + (profile, created) = UserProfile.objects.get_or_create(related_user=self.user) + profile.enable_extended_information = False + profile.affiliation = 'untouched' + profile.save() + + profile_url = reverse('myprofile') + + # With authentication, "updates" should work, but + # enable_extened_information=False should prevent + # an update from occurring. + self.client.put(profile_url, {'affiliation': 'Mozilla'}) + profile.refresh_from_db() + self.assertEqual(profile.affiliation, 'untouched') + + def test_profile_type_uniqueness(self): + # as found in the bootstrap migration: + (profile, created) = ProfileType.objects.get_or_create(value='plain') + self.assertEqual(created, False) + + def test_program_type_uniqueness(self): + # as found in the bootstrap migration: + (profile, created) = ProgramType.objects.get_or_create(value='senior fellow') + self.assertEqual(created, False) + + def test_program_year_uniqueness(self): + # as found in the bootstrap migration: + (profile, created) = ProgramYear.objects.get_or_create(value='2018') + self.assertEqual(created, False) diff --git a/pulseapi/profiles/urls.py b/pulseapi/profiles/urls.py index 2dbe3a1..b99fc33 100644 --- a/pulseapi/profiles/urls.py +++ b/pulseapi/profiles/urls.py @@ -1,6 +1,7 @@ from django.conf.urls import url from pulseapi.profiles.views import ( + # UserProfileAPIView, # see note below. UserProfilePublicAPIView, UserProfilePublicSelfAPIView, ) @@ -15,5 +16,8 @@ urlpatterns = [ r'^me/', UserProfilePublicSelfAPIView.as_view(), name='profile_self', - ) + ), + # note that there is also a /myprofile route + # defined in the root urls.py which connects + # to the UserProfileAPIView class. ] diff --git a/pulseapi/profiles/views.py b/pulseapi/profiles/views.py index 5bed483..57c9a7c 100644 --- a/pulseapi/profiles/views.py +++ b/pulseapi/profiles/views.py @@ -1,9 +1,9 @@ import base64 + from django.core.files.base import ContentFile from django.shortcuts import get_object_or_404 from rest_framework import permissions -from rest_framework.decorators import detail_route from rest_framework.generics import RetrieveAPIView, RetrieveUpdateAPIView from pulseapi.profiles.models import UserProfile @@ -34,13 +34,13 @@ class UserProfileAPIView(RetrieveUpdateAPIView): permissions.IsAuthenticated, IsProfileOwner ) + serializer_class = UserProfileSerializer def get_object(self): user = self.request.user return get_object_or_404(UserProfile, related_user=user) - @detail_route(methods=['put']) def put(self, request, *args, **kwargs): ''' If there is a thumbnail, and it was sent as part of an @@ -52,14 +52,16 @@ class UserProfileAPIView(RetrieveUpdateAPIView): much mutually exclusive patterns. A try/pass make far more sense. ''' + payload = request.data + try: - thumbnail = request.data['thumbnail'] + thumbnail = payload['thumbnail'] # do we actually need to repack as ContentFile? if thumbnail['name'] and thumbnail['base64']: name = thumbnail['name'] encdata = thumbnail['base64'] proxy = ContentFile(base64.b64decode(encdata), name=name) - request.data['thumbnail'] = proxy + payload['thumbnail'] = proxy except: pass diff --git a/pulseapi/tests.py b/pulseapi/tests.py index 61722d1..4d2e48c 100644 --- a/pulseapi/tests.py +++ b/pulseapi/tests.py @@ -102,6 +102,17 @@ class JSONDefaultClient(Client): **extra ) + def put(self, path, data=None, content_type=CONTENT_TYPE_JSON, + follow=False, secure=False, **extra): + return super(JSONDefaultClient, self).put( + path, + data=data, + content_type=content_type, + follow=follow, + secure=secure, + **extra + ) + def create_logged_in_user(test, name, email, password="password1234"): test.name = name