Refactoring links forms:

- add New Link/Video button to officer profiles - add edit and delete buttons to links on officer profiles - add LinksApi for basic CRUD operations on links, including html templates - remove links subform from edit officer page - change user_id to creator_id in links table - add optional officer_id to links table - remove officer_links table - add links to officer profiles in test data
lucyparsons · Apr 25, 2019 · 9115b1f · 9115b1f
1 parent c0623cf
commit 9115b1f
Show file tree

Hide file tree

Showing 12 changed files with 241 additions and 48 deletions.
diff --git a/OpenOversight/app/main/forms.py b/OpenOversight/app/main/forms.py
@@ -150,7 +150,7 @@ class LinkForm(Form):
         choices=LINK_CHOICES,
         default='',
         validators=[AnyOf(allowed_values(LINK_CHOICES))])
-    user_id = HiddenField(validators=[Required(message='Not a valid user ID')])
+    creator_id = HiddenField(validators=[Required(message='Not a valid user ID')])
 
     def validate(self):
         success = super(LinkForm, self).validate()
@@ -162,6 +162,11 @@ def validate(self):
         return success
 
 
+class OfficerLinkForm(LinkForm):
+    officer_id = HiddenField(validators=[Required(message='Not a valid officer ID')])
+    submit = SubmitField(label='Submit')
+
+
 class BaseTextForm(Form):
     text_contents = TextAreaField()
     description = "This information about the officer will be attributed to your username."
@@ -256,12 +261,6 @@ class EditOfficerForm(Form):
         validators=[Optional()],
         query_factory=dept_choices,
         get_label='name')
-    links = FieldList(FormField(
-        LinkForm,
-        widget=FormFieldWidget()),
-        description='Links to articles about or videos of the officer.',
-        min_entries=1,
-        widget=BootstrapListWidget())
     submit = SubmitField(label='Update')
 
 

diff --git a/OpenOversight/app/main/model_view.py b/OpenOversight/app/main/model_view.py
@@ -101,7 +101,7 @@ def delete(self, obj_id):
         if request.method == 'POST':
             db.session.delete(obj)
             db.session.commit()
-
+            flash('{} successfully deleted!'.format(self.model_name))
             return self.get_post_delete_url()
 
         return render_template('{}_delete.html'.format(self.model_name), obj=obj)

diff --git a/OpenOversight/app/main/views.py b/OpenOversight/app/main/views.py
@@ -21,12 +21,12 @@
                      ac_can_edit_officer, add_department_query, add_unit_query,
                      create_incident, get_or_create, replace_list,
                      set_dynamic_default, create_note, get_uploaded_cropped_image,
-                     create_description, filter_by_form)
+                     create_description, filter_by_form, create_link)
 
 from .forms import (FindOfficerForm, FindOfficerIDForm, AddUnitForm,
                     FaceTag, AssignmentForm, DepartmentForm, AddOfficerForm,
                     EditOfficerForm, IncidentForm, TextForm, EditTextForm,
-                    AddImageForm, EditDepartmentForm, BrowseForm, SalaryForm)
+                    AddImageForm, EditDepartmentForm, BrowseForm, SalaryForm, OfficerLinkForm)
 from .model_view import ModelView
 from .choices import GENDER_CHOICES, RACE_CHOICES, RANK_CHOICES, AGE_CHOICES
 from ..models import (db, Image, User, Face, Officer, Assignment, Department,
@@ -416,7 +416,7 @@ def list_officer(department_id, page=1, from_search=False, race='Not Sure', gend
 def add_officer():
     form = AddOfficerForm()
     for link in form.links:
-        link.user_id.data = current_user.id
+        link.creator_id.data = current_user.id
     add_unit_query(form, current_user)
     add_department_query(form, current_user)
     set_dynamic_default(form.department, current_user.dept_pref_rel)
@@ -443,9 +443,6 @@ def add_officer():
 def edit_officer(officer_id):
     officer = Officer.query.filter_by(id=officer_id).one()
     form = EditOfficerForm(obj=officer)
-    for link in form.links:
-        if not link.user_id.data:
-            link.user_id.data = current_user.id
 
     if current_user.is_area_coordinator and not current_user.is_administrator:
         if not ac_can_edit_officer(officer, current_user):
@@ -854,7 +851,7 @@ def get_new_form(self):
             form.officers[0].oo_id.data = request.args.get('officer_id')
 
         for link in form.links:
-            link.user_id.data = current_user.id
+            link.creator_id.data = current_user.id
         return form
 
     def get_edit_form(self, obj):
@@ -864,10 +861,10 @@ def get_edit_form(self, obj):
         no_links = len(obj.links)
         no_officers = len(obj.officers)
         for link in form.links:
-            if link.user_id.data:
+            if link.creator_id.data:
                 continue
             else:
-                link.user_id.data = current_user.id
+                link.creator_id.data = current_user.id
 
         for officer_idx, officer in enumerate(obj.officers):
             form.officers[officer_idx].oo_id.data = officer.id
@@ -1030,3 +1027,45 @@ def dispatch_request(self, *args, **kwargs):
     '/officer/<int:officer_id>/description/<int:obj_id>/delete',
     view_func=description_view,
     methods=['GET', 'POST'])
+
+
+class LinkApi(ModelView):
+    model = Link
+    model_name = 'link'
+    form = OfficerLinkForm
+    department_check = True
+    create_function = create_link
+
+    def get_new_form(self):
+        form = self.form()
+        form.officer_id.data = self.officer_id
+        return form
+
+    def get_redirect_url(self, *args, **kwargs):
+        return redirect(url_for('main.officer_profile', officer_id=self.officer_id))
+
+    def get_post_delete_url(self, *args, **kwargs):
+        return self.get_redirect_url()
+
+    def dispatch_request(self, *args, **kwargs):
+        if 'officer_id' in kwargs:
+            officer = Officer.query.get_or_404(kwargs['officer_id'])
+            self.officer_id = kwargs.pop('officer_id')
+            self.department_id = officer.department_id
+        return super(LinkApi, self).dispatch_request(*args, **kwargs)
+
+
+# This API only applies to links attached to officer profiles, not links
+# attached to incidents.
+main.add_url_rule(
+    '/officer/<int:officer_id>/link/new',
+    view_func=LinkApi.as_view('link_api_new'),
+    methods=['GET', 'POST'])
+main.add_url_rule(
+    '/officer/<int:officer_id>/link/<int:obj_id>/edit',
+    view_func=LinkApi.as_view('link_api_edit'),
+    methods=['GET', 'POST'])
+main.add_url_rule(
+    '/officer/<int:officer_id>/link/<int:obj_id>/delete',
+    view_func=LinkApi.as_view('link_api_delete'),
+    methods=['GET', 'POST'])
diff --git a/OpenOversight/app/models.py b/OpenOversight/app/models.py
@@ -18,10 +18,6 @@
 db = SQLAlchemy()
 
 
-officer_links = db.Table('officer_links',
-                         db.Column('officer_id', db.Integer, db.ForeignKey('officers.id'), primary_key=True),
-                         db.Column('link_id', db.Integer, db.ForeignKey('links.id'), primary_key=True))
-
 officer_incidents = db.Table('officer_incidents',
                              db.Column('officer_id', db.Integer, db.ForeignKey('officers.id'), primary_key=True),
                              db.Column('incident_id', db.Integer, db.ForeignKey('incidents.id'), primary_key=True))
@@ -80,12 +76,7 @@ class Officer(db.Model):
     department_id = db.Column(db.Integer, db.ForeignKey('departments.id'))
     department = db.relationship('Department', backref='officers')
     unique_internal_identifier = db.Column(db.String(50), index=True, unique=True, nullable=True)
-    # we don't expect to pull up officers via link often so we make it lazy.
-    links = db.relationship(
-        'Link',
-        secondary=officer_links,
-        lazy='subquery',
-        backref=db.backref('officers', lazy=True))
+    links = db.relationship('Link', back_populates='officer')
     notes = db.relationship('Note', back_populates='officer', order_by='Note.date_created')
     descriptions = db.relationship('Description', back_populates='officer', order_by='Description.date_created')
     salaries = db.relationship('Salary', back_populates='officer', order_by='Salary.year.desc()')
@@ -303,8 +294,10 @@ class Link(db.Model):
     link_type = db.Column(db.String(100), index=True)
     description = db.Column(db.Text(), nullable=True)
     author = db.Column(db.String(255), nullable=True)
-    user_id = db.Column(db.Integer, db.ForeignKey('users.id'))
-    user = db.relationship('User', backref='links', lazy=True)
+    creator_id = db.Column(db.Integer, db.ForeignKey('users.id', ondelete='SET NULL'))
+    creator = db.relationship('User', backref='links', lazy=True)
+    officer_id = db.Column(db.Integer, db.ForeignKey('officers.id', ondelete='CASCADE'), nullable=True)
+    officer = db.relationship('Officer', back_populates='links')
 
     @validates('url')
     def validate_url(self, key, url):

diff --git a/OpenOversight/app/templates/edit_officer.html b/OpenOversight/app/templates/edit_officer.html
@@ -22,7 +22,6 @@ <h1>Edit Officer</h1>
         {{ wtf.form_field(form.birth_year) }}
         {{ wtf.form_field(form.unique_internal_identifier) }}
         {{ wtf.form_field(form.department) }}
-        {% include "partials/links_subform.html" %}
         {{ wtf.form_field(form.submit, id="submit", button_map={'submit':'primary'}) }}
     </form>
     <br>

diff --git a/OpenOversight/app/templates/link_delete.html b/OpenOversight/app/templates/link_delete.html
@@ -0,0 +1,32 @@
+{% extends "base.html" %}
+
+{% block content %}
+	<div class="container theme-showcase" role="main">
+
+		<div class="page-header">
+		    <h1>
+		    	Delete Link for officer {{ obj.officer_id }}
+		    </h1>
+		    <p>
+					<a href="{{ obj.url }}">{{ obj.title or obj.url }}</a>
+					{% if obj.description or obj.author %}
+						<div>
+							{% if obj.description %}
+									{{ obj.description }}
+							{% endif %}
+							{% if obj.author %}
+								{% if obj.description %}- {% endif %}<em>{{ obj.author }}</em>
+							{% endif %}
+						</div>
+					{% endif %}
+		    </p>
+		</div>
+		<p class="lead">
+			Are you sure you want to delete this link?
+			This cannot be undone.
+			<form action="{{ url_for('main.link_api_delete', obj_id=obj.id, officer_id=obj.officer_id) }}" method="post">
+				<button class='btn btn-danger' type="submit">Delete</button>
+			</form>
+		</p>
+	</div>
+{% endblock content %}
diff --git a/OpenOversight/app/templates/link_edit.html b/OpenOversight/app/templates/link_edit.html
@@ -0,0 +1,10 @@
+{% extends "form.html" %}
+
+{% block page_title %}
+	Update Link
+{% endblock page_title %}
+
+{% block form %}
+	<p>For officer with OOID {{ form.officer_id.data }}.</p>
+	{{ super() }}
+{% endblock form %}
diff --git a/OpenOversight/app/templates/link_new.html b/OpenOversight/app/templates/link_new.html
@@ -0,0 +1,10 @@
+{% extends 'form.html' %}
+
+{% block page_title %}
+	New Link
+{% endblock page_title %}
+
+{% block form %}
+  <p>For officer with OOID {{ form.officer_id.data }}.</p>
+	{{ super() }}
+{% endblock form %}
diff --git a/OpenOversight/app/templates/partials/links_and_videos_row.html b/OpenOversight/app/templates/partials/links_and_videos_row.html
@@ -1,12 +1,26 @@
 <div class='row'>
-	{% for type, list in obj.links|groupby('link_type') %}
-		{% if type == 'link' %}
-		<div class="col-sm-12 col-md-6">
-			<h3>Links</h3>
+	<div class="col-sm-12 col-md-6">
+		<h3>Links</h3>
+		{% for type, list in obj.links|groupby('link_type') %}
+			{% if type == 'link' %}
 			<ul class='list-group'>
 				{% for link in list %}
 					<li class='list-group-item'>
 						<a href="{{ link.url }}">{{ link.title or link.url }}</a>
+						{% if officer and (current_user.is_administrator
+						  or (current_user.is_area_coordinator and current_user.ac_department_id == officer.department_id)
+							or link.creator_id == current_user.id) %}
+							<a href="{{ url_for('main.link_api_edit', officer_id=officer.id,
+											 obj_id=link.id) }}">
+							<span class='sr-only'>Edit</span>
+							<i class="fa fa-pencil-square-o" aria-hidden="true"></i>
+							</a>
+							<a href="{{ url_for('main.link_api_delete', officer_id=officer.id,
+											 obj_id=link.id) }}">
+							<span class='sr-only'>Delete</span>
+							<i class="fa fa-trash-o" aria-hidden="true"></i>
+							</a>
+						{% endif %}
 						{% if link.description or link.author %}
 							<div>
 								{% if link.description %}
@@ -20,8 +34,13 @@ <h3>Links</h3>
 					</li>
 				{% endfor %}
 			</ul>
-		</div>
+			{% endif %}
+		{% endfor %}
+		{% if officer %}
+		<a href="{{ url_for('main.link_api_new', officer_id=officer.id) }}" class='btn btn-primary'>New Link/Video</a>
 		{% endif %}
+	</div>
+	{% for type, list in obj.links|groupby('link_type') %}
 		{% if type == 'video' %}
 		<div class="col-sm-12 col-md-6">
 			<h3>Videos</h3>
@@ -32,6 +51,20 @@ <h3>Videos</h3>
 							{% if link.title %}
 								<h5>{{ link.title }}</h5>
 							{% endif %}
+							{% if officer and (current_user.is_administrator
+								or (current_user.is_area_coordinator and current_user.ac_department_id == officer.department_id)
+								or link.creator_id == current_user.id) %}
+								<a href="{{ url_for('main.link_api_edit', officer_id=officer.id,
+												 obj_id=link.id) }}">
+								<span class='sr-only'>Edit</span>
+								<i class="fa fa-pencil-square-o" aria-hidden="true"></i>
+								</a>
+								<a href="{{ url_for('main.link_api_delete', officer_id=officer.id,
+												 obj_id=link.id) }}">
+								<span class='sr-only'>Delete</span>
+								<i class="fa fa-trash-o" aria-hidden="true"></i>
+								</a>
+							{% endif %}
 							<div class='video-container'>
 								<iframe width="560" height="315" src="https://www.youtube.com/embed/{{ link_url }}" frameborder="0" allow="autoplay; encrypted-media" allowfullscreen></iframe>
 							</div>
@@ -58,6 +91,20 @@ <h3>Other videos</h3>
 				{% for link in list %}
 					<li class='list-group-item'>
 						<a href="{{ link.url }}">{{ link.title or link.url }}</a>
+						{% if officer and (current_user.is_administrator
+						  or (current_user.is_area_coordinator and current_user.ac_department_id == officer.department_id)
+							or link.creator_id == current_user.id) %}
+							<a href="{{ url_for('main.link_api_edit', officer_id=officer.id,
+											 obj_id=link.id) }}">
+							<span class='sr-only'>Edit</span>
+							<i class="fa fa-pencil-square-o" aria-hidden="true"></i>
+							</a>
+							<a href="{{ url_for('main.link_api_delete', officer_id=officer.id,
+											 obj_id=link.id) }}">
+							<span class='sr-only'>Delete</span>
+							<i class="fa fa-trash-o" aria-hidden="true"></i>
+							</a>
+						{% endif %}
 						{% if link.description or link.author %}
 							<div>
 								{% if link.description %}

diff --git a/OpenOversight/app/utils.py b/OpenOversight/app/utils.py
@@ -161,15 +161,7 @@ def add_officer_profile(form, current_user):
 
 def edit_officer_profile(officer, form):
     for field, data in iteritems(form.data):
-        if field == 'links':
-            for link in data:
-                # don't try to create with a blank string
-                if link['url']:
-                    li, _ = get_or_create(db.session, Link, **link)
-                    if li:
-                        officer.links.append(li)
-        else:
-            setattr(officer, field, data)
+        setattr(officer, field, data)
 
     db.session.add(officer)
     db.session.commit()
@@ -425,6 +417,19 @@ def create_description(self, form):
         date_updated=datetime.datetime.now())
 
 
+def create_link(self, form):
+    link = Link(
+        title=form.title.data,
+        url=form.url.data,
+        link_type=form.link_type.data,
+        description=form.description.data,
+        author=form.author.data,
+        creator_id=form.creator_id.data)
+    if hasattr(form, 'officer_id'):
+        link.officer_id = form.officer_id.data
+    return link
+
+
 def get_uploaded_cropped_image(original_image, crop_data):
     """ Takes an Image object and a cropping tuple (left, upper, right, lower), and returns a new Image object"""