fix: Make speaker email nullable (#7469)

Co-authored-by: Areeb Jamal <jamal.areeb@gmail.com>

app/api/helpers/custom_forms.py

@@ -29,7 +29,7 @@ def get_schema(form_fields):
     return type('DynamicSchema', (marshmallow.Schema,), attrs)
 
 
-def validate_custom_form_constraints(form, obj, relationship_fields):
+def validate_custom_form_constraints(form, obj, excluded):
     form_fields = CustomForms.query.filter_by(
         form=form,
         event_id=obj.event_id,
@@ -38,7 +38,7 @@ def validate_custom_form_constraints(form, obj, relationship_fields):
     required_form_fields = filter(lambda field: field.is_required, form_fields)
     missing_required_fields = []
     for field in required_form_fields:
-        if field.identifier in relationship_fields:
+        if field.identifier in excluded:
             continue
         if not field.is_complex:
             if not getattr(obj, field.identifier):
@@ -66,11 +66,13 @@ def validate_custom_form_constraints(form, obj, relationship_fields):
         return data if data else None
 
 
-def validate_custom_form_constraints_request(form, schema, obj, data):
+def validate_custom_form_constraints_request(form, schema, obj, data, excluded=[]):
     new_obj = type(obj)(**object_as_dict(obj))
     relationship_fields = get_relationships(schema)
     for key, value in data.items():
         if hasattr(new_obj, key) and key not in relationship_fields:
             setattr(new_obj, key, value)
 
-    return validate_custom_form_constraints(form, new_obj, relationship_fields)
+    return validate_custom_form_constraints(
+        form, new_obj, set(relationship_fields.keys()) | set(excluded)
+    )

app/api/speakers.py

@@ -5,7 +5,7 @@
 from app.api.bootstrap import api
 from app.api.helpers.custom_forms import validate_custom_form_constraints_request
 from app.api.helpers.db import get_count, safe_query_kwargs, save_to_db
-from app.api.helpers.errors import ForbiddenError
+from app.api.helpers.errors import ForbiddenError, UnprocessableEntityError
 from app.api.helpers.permission_manager import has_access, is_logged_in
 from app.api.helpers.permissions import jwt_required
 from app.api.helpers.query import event_query
@@ -20,6 +20,23 @@
 from app.models.user import User
 
 
+def check_email_override(data, event_id):
+    is_organizer = has_access('is_organizer', event_id=event_id)
+    if data.get('is_email_overridden') and not is_organizer:
+        raise ForbiddenError(
+            {'pointer': '/data/attributes/is_email_overridden'},
+            'Organizer access required to override email',
+        )
+    if not data.get('is_email_overridden') and is_organizer and not data.get('email'):
+        data['email'] = current_user.email
+    elif data.get('is_email_overridden') and is_organizer and not data.get('email'):
+        data['email'] = None
+    if not is_organizer and not data.get('email'):
+        raise UnprocessableEntityError(
+            {'pointer': '/data/attributes/email'}, 'Email is required for speaker'
+        )
+
+
 class SpeakerListPost(ResourceList):
     """
     List and create speakers
@@ -54,8 +71,10 @@ def before_post(self, args, kwargs, data=None):
         ):
             raise ForbiddenError({'pointer': ''}, "Speakers are disabled for this Event")
 
+        check_email_override(data, data['event'])
+
         if (
-            not data.get('is_email_overridden')
+            data.get('email')
             and get_count(
                 db.session.query(Speaker).filter_by(
                     event_id=int(data['event']), email=data['email'], deleted_at=None
@@ -66,18 +85,7 @@ def before_post(self, args, kwargs, data=None):
             raise ForbiddenError(
                 {'pointer': ''}, 'Speaker with this Email ID already exists'
             )
-        is_organizer = has_access('is_organizer', event_id=data['event'])
-        if data.get('is_email_overridden') and not is_organizer:
-            raise ForbiddenError(
-                {'pointer': 'data/attributes/is_email_overridden'},
-                'Organizer access required to override email',
-            )
-        if not data.get('is_email_overridden') and is_organizer and not data.get('email'):
-            data['email'] = current_user.email
-        if not is_organizer and not data.get('email'):
-            raise ForbiddenError(
-                {'pointer': '/data/email'}, 'Email is required for speaker'
-            )
+
         if 'sessions' in data:
             session_ids = data['sessions']
             for session_id in session_ids:
@@ -87,8 +95,12 @@ def before_post(self, args, kwargs, data=None):
                         f"Session: {session_id} not found",
                     )
 
+        excluded = []
+        if not data.get('email'):
+            # Don't check requirement of email if overriden
+            excluded = ['email']
         data['complex_field_values'] = validate_custom_form_constraints_request(
-            'speaker', self.schema, Speaker(event_id=data['event']), data
+            'speaker', self.schema, Speaker(event_id=data['event']), data, excluded
         )
 
     def after_create_object(self, speaker, data, view_kwargs):
@@ -182,22 +194,14 @@ def before_update_object(self, speaker, data, view_kwargs):
         if data.get('photo_url') and data['photo_url'] != speaker.photo_url:
             start_image_resizing_tasks(speaker, data['photo_url'])
 
-        if data.get('is_email_overridden') and not has_access(
-            'is_organizer', event_id=speaker.event_id
-        ):
-            raise ForbiddenError(
-                {'pointer': 'data/attributes/is_email_overridden'},
-                'Organizer access required to override email',
-            )
-        if (
-            not data.get('is_email_overridden')
-            and has_access('is_organizer', event_id=speaker.event_id)
-            and not data.get('email')
-        ):
-            data['email'] = current_user.email
+        check_email_override(data, speaker.event_id)
 
+        excluded = []
+        if not data.get('email'):
+            # Don't check requirement of email if overriden
+            excluded = ['email']
         data['complex_field_values'] = validate_custom_form_constraints_request(
-            'speaker', self.resource.schema, speaker, data
+            'speaker', self.resource.schema, speaker, data, excluded
         )
 
     def after_patch(self, result):

app/models/speaker.py

@@ -16,7 +16,7 @@ class Speaker(SoftDeletionModel):
     short_biography = db.Column(db.Text)
     long_biography = db.Column(db.Text)
     speaking_experience = db.Column(db.Text)
-    email = db.Column(db.String, nullable=False)
+    email = db.Column(db.String)
     mobile = db.Column(db.String)
     website = db.Column(db.String)
     twitter = db.Column(db.String)

migrations/versions/rev-2020-11-26-14:16:30-060a77f9a1ea_.py

@@ -0,0 +1,32 @@
+"""empty message
+
+Revision ID: 060a77f9a1ea
+Revises: 4642459983c0
+Create Date: 2020-11-26 14:16:30.201379
+
+"""
+
+from alembic import op
+import sqlalchemy as sa
+import sqlalchemy_utils
+
+
+# revision identifiers, used by Alembic.
+revision = '060a77f9a1ea'
+down_revision = '4642459983c0'
+
+
+def upgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    op.alter_column('speaker', 'email',
+               existing_type=sa.VARCHAR(),
+               nullable=True)
+    # ### end Alembic commands ###
+
+
+def downgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    op.alter_column('speaker', 'email',
+               existing_type=sa.VARCHAR(),
+               nullable=False)
+    # ### end Alembic commands ###