Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactoring links forms #646

Merged
merged 15 commits into from
Sep 27, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion OpenOversight/app/csv_imports.py
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ def _handle_links_csv(
"link_type",
"description",
"author",
"user_id",
"creator_id",
"officer_ids",
"incident_ids",
],
Expand Down
13 changes: 6 additions & 7 deletions OpenOversight/app/main/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ class LinkForm(Form):
choices=LINK_CHOICES,
default='',
validators=[AnyOf(allowed_values(LINK_CHOICES))])
user_id = HiddenField(validators=[DataRequired(message='Not a valid user ID')])
creator_id = HiddenField(validators=[DataRequired(message='Not a valid user ID')])

def validate(self):
success = super(LinkForm, self).validate()
Expand All @@ -166,6 +166,11 @@ def validate(self):
return success


class OfficerLinkForm(LinkForm):
officer_id = HiddenField(validators=[DataRequired(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."
Expand Down Expand Up @@ -259,12 +264,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')


Expand Down
2 changes: 1 addition & 1 deletion OpenOversight/app/main/model_view.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,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)
Expand Down
110 changes: 101 additions & 9 deletions OpenOversight/app/main/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,10 @@
crop_image, create_incident, get_or_create, dept_choices,
upload_image_to_s3_and_store_in_db)


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, AGE_CHOICES
from ..models import (db, Image, User, Face, Officer, Assignment, Department,
Expand Down Expand Up @@ -587,7 +586,7 @@ def add_officer():
jsloads = ['js/dynamic_lists.js', 'js/add_officer.js']
form = AddOfficerForm()
for link in form.links:
link.user_id.data = current_user.get_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)
Expand Down Expand Up @@ -616,9 +615,6 @@ def edit_officer(officer_id):
jsloads = ['js/dynamic_lists.js']
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.get_id()

if request.method == 'GET':
if officer.race is None:
Expand Down Expand Up @@ -1148,7 +1144,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.get_id()
link.creator_id.data = current_user.id
return form

def get_edit_form(self, obj):
Expand All @@ -1158,10 +1154,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.get_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
Expand Down Expand Up @@ -1336,3 +1332,99 @@ def dispatch_request(self, *args, **kwargs):
'/officer/<int:officer_id>/description/<int:obj_id>/delete',
view_func=description_view,
methods=['GET', 'POST'])


class OfficerLinkApi(ModelView):
'''This API only applies to links attached to officer profiles, not links attached to incidents'''

model = Link
model_name = 'link'
form = OfficerLinkForm
department_check = True

@property
def officer(self):
if not hasattr(self, '_officer'):
self._officer = db.session.query(Officer).filter_by(id=self.officer_id).one()
return self._officer

@login_required
@ac_or_admin_required
def new(self, form=None):
if not current_user.is_administrator and current_user.ac_department_id != self.officer.department_id:
abort(403)
if not form:
form = self.get_new_form()
if hasattr(form, 'creator_id') and not form.creator_id.data:
form.creator_id.data = current_user.get_id()

if form.validate_on_submit():
Copy link
Contributor

@fritzdavenport fritzdavenport Aug 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add an else to catch if there are errors and log them (or anything other than die silently?), in the spirit of #786 ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ModelView class it inherits from is written in the same way, so I wouldn't want to break that pattern here without doing it elsewhere, to keep things consistent. And I think it would be best to come to consensus on a solution for #786 and apply it in a separate PR.

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)
self.officer.links.append(link)
db.session.add(link)
db.session.commit()
flash('{} created!'.format(self.model_name))
return self.get_redirect_url(obj_id=link.id)

return render_template('{}_new.html'.format(self.model_name), form=form)

@login_required
@ac_or_admin_required
def delete(self, obj_id):
obj = self.model.query.get_or_404(obj_id)
if not current_user.is_administrator and current_user.ac_department_id != self.get_department_id(obj):
abort(403)

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, officer_id=self.officer_id)

def get_new_form(self):
form = self.form()
form.officer_id.data = self.officer_id
return form

def get_edit_form(self, obj):
form = self.form(obj=obj)
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 get_department_id(self, obj):
return self.officer.department_id

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(OfficerLinkApi, self).dispatch_request(*args, **kwargs)


main.add_url_rule(
'/officer/<int:officer_id>/link/new',
view_func=OfficerLinkApi.as_view('link_api_new'),
methods=['GET', 'POST'])
main.add_url_rule(
'/officer/<int:officer_id>/link/<int:obj_id>/edit',
view_func=OfficerLinkApi.as_view('link_api_edit'),
methods=['GET', 'POST'])
main.add_url_rule(
'/officer/<int:officer_id>/link/<int:obj_id>/delete',
view_func=OfficerLinkApi.as_view('link_api_delete'),
methods=['GET', 'POST'])
6 changes: 3 additions & 3 deletions OpenOversight/app/model_imports.py
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ def create_link_from_dict(data: Dict[str, Any], force_id: bool = False) -> Link:
link_type=validate_choice(data.get("link_type"), choices.LINK_CHOICES),
description=parse_str(data.get("description"), None),
author=parse_str(data.get("author"), None),
user_id=parse_int(data.get("user_id")),
creator_id=parse_int(data.get("creator_id")),
)

if force_id and data.get("id"):
Expand All @@ -221,8 +221,8 @@ def update_link_from_dict(data: Dict[str, Any], link: Link) -> Link:
link.description = parse_str(data.get("description"), None)
if "author" in data:
link.author = parse_str(data.get("author"), None)
if "user_id" in data:
link.user_id = parse_int(data.get("user_id"))
if "creator_id" in data:
link.creator_id = parse_int(data.get("creator_id"))
if "officers" in data:
link.officers = data.get("officers") or []
if "incidents" in data:
Expand Down
4 changes: 2 additions & 2 deletions OpenOversight/app/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -345,8 +345,8 @@ class Link(BaseModel):
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)

@validates('url')
def validate_url(self, key, url):
Expand Down
1 change: 0 additions & 1 deletion OpenOversight/app/templates/edit_officer.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
32 changes: 32 additions & 0 deletions OpenOversight/app/templates/link_delete.html
Original file line number Diff line number Diff line change
@@ -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=officer_id) }}" method="post">
<button class='btn btn-danger' type="submit">Delete</button>
</form>
</p>
</div>
{% endblock content %}
10 changes: 10 additions & 0 deletions OpenOversight/app/templates/link_edit.html
Original file line number Diff line number Diff line change
@@ -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 %}
10 changes: 10 additions & 0 deletions OpenOversight/app/templates/link_new.html
Original file line number Diff line number Diff line change
@@ -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 %}
61 changes: 56 additions & 5 deletions OpenOversight/app/templates/partials/links_and_videos_row.html
Original file line number Diff line number Diff line change
@@ -1,12 +1,28 @@
{% if obj.links|length > 0 or current_user.is_administrator
or (current_user.is_area_coordinator and current_user.ac_department_id == obj.department_id) %}
<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 %}
Expand All @@ -20,8 +36,14 @@ <h3>Links</h3>
</li>
{% endfor %}
</ul>
</div>
{% endif %}
{% endfor %}
{% if officer and (current_user.is_administrator
or (current_user.is_area_coordinator and current_user.ac_department_id == officer.department_id)) %}
<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>
Expand All @@ -32,6 +54,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>
Expand All @@ -58,6 +94,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 %}
Expand All @@ -75,3 +125,4 @@ <h3>Other videos</h3>
{% endif %}
{% endfor %}
</div> {# end row #}
{% endif %}
Loading