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.

Sign up for GitHub

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

825 Add assignment end date (resign_date) to UI #832

Merged
merged 12 commits into from
Sep 27, 2020
Merged

825 Add assignment end date (resign_date) to UI #832

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
143ae51
Add resign date to edit, add, and officer views
egfrank Sep 16, 2020
d96964b
Add tests
egfrank Sep 17, 2020
501e159
Add validation for assignment form
egfrank Sep 17, 2020
a633ba1
Rewrite officer badge tests into assignment tests
egfrank Sep 17, 2020
4303ef5
change view so add assignment error handling is persisted through reload
egfrank Sep 17, 2020
34b145e
Formatting changes for flake8
egfrank Sep 17, 2020
436ca75
Change render to redirect on validation error for adding assignment
egfrank Sep 27, 2020
bd011ad
Change unknown resign date to blank resign date
egfrank Sep 27, 2020
ba24f4c
Change tests to make more exact
egfrank Sep 27, 2020
8816908
Check for flake8
egfrank Sep 27, 2020
5d0b5ea
Merge branch 'develop' into 825_assignment_end_date
abandoned-prototype Sep 27, 2020
39fcf10
Merge branch 'develop' into 825_assignment_end_date
abandoned-prototype Sep 27, 2020
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
7 changes: 7 additions & 0 deletions OpenOversight/app/main/forms.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@ def validate_money(form, field):
raise ValidationError('Invalid monetary value')


def validate_end_date(form, field):
if form.data["star_date"] and field.data:
if form.data["star_date"] > field.data:
raise ValidationError('End date must come after start date.')


class HumintContribution(Form):
photo = FileField(
'image', validators=[FileRequired(message='There was no file!'),
Expand Down Expand Up @@ -97,6 +103,7 @@ class AssignmentForm(Form):
query_factory=unit_choices, get_label='descrip',
allow_blank=True, blank_text=u'None')
star_date = DateField('Assignment start date', validators=[Optional()])
resign_date = DateField('Assignment end date', validators=[Optional(), validate_end_date])


class SalaryForm(Form):
Expand Down
4 changes: 3 additions & 1 deletion OpenOversight/app/main/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,9 @@ def add_assignment(officer_id):
abort(403)
else:
current_app.logger.info(form.errors)
return redirect(url_for('main.officer_profile', officer_id=officer_id))
flash("Error: " + str(form.errors))

return redirect(url_for('main.officer_profile', officer_id=officer_id))


@main.route('/officer/<int:officer_id>/assignment/<int:assignment_id>',
Expand Down
1 change: 1 addition & 0 deletions OpenOversight/app/templates/edit_assignment.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ <h1>Edit Officer Assignment</h1>
{{ wtf.form_field(form.unit) }}
<h4><small><a href="{{ url_for( 'main.add_unit' )}}">Don't see your unit? Add one!</a></small></h4>
{{ wtf.form_field(form.star_date) }}
{{ wtf.form_field(form.resign_date) }}
<input class="btn btn-primary btn-lg" type="submit" value="Edit" />
</form>
<br>
Expand Down
22 changes: 20 additions & 2 deletions OpenOversight/app/templates/officer.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,8 @@ <h3>Assignment History</h3>
<th><b>Job Title</b></th>
<th><b>Badge No.</b></th>
<th><b>Unit</b></th>
<th><b>Assignment Date</b></th>
<th><b>Start Date</b></th>
<th><b>End Date</b></th>
{% if current_user.is_administrator
or (current_user.is_area_coordinator and current_user.ac_department_id == officer.department_id) %}
<th><b>Edit</b></th>
Expand All @@ -182,12 +183,16 @@ <h3>Assignment History</h3>
<td>{{ assignment.job.job_title }}</td>
<td>{{ assignment.star_no }}</td>
<td>{% if assignment.unit_id %}{{ assignment.unit.descrip }}{% endif %}</td>
<td>{% if assignment.star_date: %}
<td>{% if assignment.star_date %}
{{ assignment.star_date }}
{% else %}
Unknown
{% endif %}
</td>
<td>{% if assignment.resign_date %}
{{ assignment.resign_date }}
{% endif %}
</td>
<td>
{% if current_user.is_administrator
or (current_user.is_area_coordinator and current_user.ac_department_id == officer.department_id) %}
Expand All @@ -211,6 +216,10 @@ <h3>Assignment History</h3>
Unknown
{% endif %}
</td>
<td>{% if assignment.resign_date %}
{{ assignment.resign_date }}
{% endif %}
</td>
<td>
{% if current_user.is_administrator
or (current_user.is_area_coordinator and current_user.ac_department_id == officer.department_id) %}
Expand Down Expand Up @@ -270,6 +279,15 @@ <h3>Add Assignment<small> Admin only</small></h3>
</td>
</tr>

<tr>
<td><b>End date of new assignment:</b></td>
<td>{{ form.resign_date }}
{% for error in form.resign_date.errors %}
<p><span style="color: red;">[{{ error }}]</span></p>
{% endfor %}
</td>
</tr>

<tr>
<td>
<input type="submit" value="Add Assignment" name="add-assignment" class="btn-sm btn-primary"/>
Expand Down
5 changes: 3 additions & 2 deletions OpenOversight/app/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ def dept_choices():


def add_new_assignment(officer_id, form):
# Resign date should be null
if form.unit.data:
unit_id = form.unit.data.id
else:
Expand All @@ -99,7 +98,8 @@ def add_new_assignment(officer_id, form):
star_no=form.star_no.data,
job_id=job.id,
unit_id=unit_id,
star_date=form.star_date.data)
star_date=form.star_date.data,
resign_date=form.resign_date.data)
db.session.add(new_assignment)
db.session.commit()

Expand All @@ -117,6 +117,7 @@ def edit_existing_assignment(assignment, form):

assignment.unit_id = officer_unit
assignment.star_date = form.star_date.data
assignment.resign_date = form.resign_date.data
db.session.add(assignment)
db.session.commit()
return assignment
Expand Down
157 changes: 139 additions & 18 deletions OpenOversight/tests/routes/test_officer_and_department.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,13 +99,18 @@ def test_ac_cannot_access_admin_on_non_dept_officer_profile(mockdata, client, se
assert 'Admin only' not in rv.data.decode('utf-8')


def test_admin_can_add_officer_badge_number(mockdata, client, session):
def test_admin_can_add_assignment(mockdata, client, session):
with current_app.test_request_context():
login_admin(client)

officer = Officer.query.filter_by(id=3).one()
job = Job.query.filter_by(department_id=officer.department_id, job_title='Police Officer').one()
form = AssignmentForm(star_no='1234', job_title=job.id)
form = AssignmentForm(
star_no='1234',
job_title=job.id,
star_date=date(2019, 1, 1),
resign_date=date(2019, 12, 31),
)

rv = client.post(
url_for('main.add_assignment', officer_id=3),
Expand All @@ -114,15 +119,48 @@ def test_admin_can_add_officer_badge_number(mockdata, client, session):
)

assert 'Added new assignment' in rv.data.decode('utf-8')
assert '2019-01-01' in rv.data.decode('utf-8')
assert '2019-12-31' in rv.data.decode('utf-8')
Comment on lines +122 to +123
Copy link
Collaborator

Choose a reason for hiding this comment

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

instead of testing the return values I would prefer a test that checks the database for the entry. For example something like this:

assignment = Assignment.query.filter_by(star_no='1234', officer_id=officer.id).first()
assert assignment is not None
assert assignment.star_date == date(2019, 1, 1)
assert assignment.resign_date == date(2019, 12, 31)

I know we don't do this in a lot of the existing tests, but its gradual improvements I think

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea I definitely agree testing the db is also necessary. I think it makes sense to keep the assert '2019-01-01' in rv.data.decode('utf-8') in as well, since that's effectively testing the template code, which was also changed in this PR. Do you agree?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree, good point

assignment = Assignment.query.filter_by(
star_no='1234', job_id=job.id).first()
assert assignment.star_date == date(2019, 1, 1)
assert assignment.resign_date == date(2019, 12, 31)


def test_ac_can_add_officer_badge_number_in_their_dept(mockdata, client, session):
def test_admin_add_assignment_validation_error(mockdata, client, session):
with current_app.test_request_context():
login_ac(client)
login_admin(client)
officer = Officer.query.filter_by(id=3).one()
job = Job.query.filter_by(department_id=officer.department_id, job_title='Police Officer').one()
form = AssignmentForm(
star_no='1234',
job_title=job.id,
star_date=date(2020, 1, 1),
resign_date=date(2019, 12, 31),
)

rv = client.post(
url_for('main.add_assignment', officer_id=3),
data=form.data,
follow_redirects=True
)

assert "End date must come after start date." in rv.data.decode('utf-8')
assignments = Assignment.query.filter_by(star_no='1234', job_id=job.id).scalar()
assert assignments is None


def test_ac_can_add_assignment_in_their_dept(mockdata, client, session):
with current_app.test_request_context():
login_ac(client)
officer = Officer.query.filter_by(department_id=AC_DEPT).first()
job = Job.query.filter_by(department_id=officer.department_id, job_title='Police Officer').one()
form = AssignmentForm(star_no='S1234', job_title=job.id)
form = AssignmentForm(
star_no='S1234',
job_title=job.id,
star_date=date(2019, 1, 1),
resign_date=date(2019, 12, 31),
)

rv = client.post(
url_for('main.add_assignment', officer_id=officer.id),
Expand All @@ -131,13 +169,17 @@ def test_ac_can_add_officer_badge_number_in_their_dept(mockdata, client, session
)

assert 'Added new assignment' in rv.data.decode('utf-8')
assert '2019-01-01' in rv.data.decode('utf-8')
assert '2019-12-31' in rv.data.decode('utf-8')

# test that assignment exists in database
officer = Officer.query.filter(Officer.assignments.any(star_no='S1234')).first()
assert officer is not None
assignment = Assignment.query.filter_by(
star_no='S1234', officer_id=officer.id).first()
assert assignment.star_date == date(2019, 1, 1)
assert assignment.resign_date == date(2019, 12, 31)


def test_ac_cannot_add_non_dept_officer_badge(mockdata, client, session):
def test_ac_cannot_add_non_dept_assignment(mockdata, client, session):
with current_app.test_request_context():
login_ac(client)

Expand All @@ -152,17 +194,23 @@ def test_ac_cannot_add_non_dept_officer_badge(mockdata, client, session):
)

assert rv.status_code == 403
assignments = Assignment.query.filter_by(
star_no='1234', job_id=job.id).scalar()
assert assignments is None


def test_admin_can_edit_officer_badge_number(mockdata, client, session):
def test_admin_can_edit_assignment(mockdata, client, session):
with current_app.test_request_context():
login_admin(client)

# Remove existing assignments
Assignment.query.filter_by(officer_id=3).delete()
officer = Officer.query.filter_by(id=3).one()
job = Job.query.filter_by(department_id=officer.department_id, job_title='Police Officer').one()
form = AssignmentForm(star_no='1234', job_title=job.id)
form = AssignmentForm(
star_no='1234',
job_title=job.id,
star_date=date(2019, 1, 1),
resign_date=date(2019, 12, 31),
)

rv = client.post(
url_for('main.add_assignment', officer_id=3),
Expand All @@ -172,9 +220,20 @@ def test_admin_can_edit_officer_badge_number(mockdata, client, session):

assert 'Added new assignment' in rv.data.decode('utf-8')
assert '<td>1234</td>' in rv.data.decode('utf-8')
assert '2019-01-01' in rv.data.decode('utf-8')
assert '2019-12-31' in rv.data.decode('utf-8')

assignment = Assignment.query.filter_by(star_no='1234', job_id=job.id).first()
assert assignment.star_date == date(2019, 1, 1)
assert assignment.resign_date == date(2019, 12, 31)

job = Job.query.filter_by(department_id=officer.department_id, job_title='Commander').one()
form = AssignmentForm(star_no='12345', job_title=job.id)
form = AssignmentForm(
star_no='12345',
job_title=job.id,
star_date=date(2019, 2, 1),
resign_date=date(2019, 11, 30)
)
officer = Officer.query.filter_by(id=3).one()

rv = client.post(
Expand All @@ -186,18 +245,66 @@ def test_admin_can_edit_officer_badge_number(mockdata, client, session):

assert 'Edited officer assignment' in rv.data.decode('utf-8')
assert '<td>12345</td>' in rv.data.decode('utf-8')
assert officer.assignments[0].star_no == '12345'
assert '2019-02-01' in rv.data.decode('utf-8')
assert '2019-11-30' in rv.data.decode('utf-8')

assignment = Assignment.query.filter_by(star_no='12345', job_id=job.id).first()
assert assignment.star_date == date(2019, 2, 1)
assert assignment.resign_date == date(2019, 11, 30)


def test_admin_edit_assignment_validation_error(mockdata, client, session):
with current_app.test_request_context():
login_admin(client)

# Remove existing assignments
officer = Officer.query.filter_by(id=3).one()
job = Job.query.filter_by(department_id=officer.department_id, job_title='Police Officer').one()
form = AssignmentForm(
star_no='1234',
job_title=job.id,
star_date=date(2019, 1, 1),
resign_date=date(2019, 12, 31),
)

rv = client.post(
url_for('main.add_assignment', officer_id=3),
data=form.data,
follow_redirects=True
)

def test_ac_can_edit_officer_in_their_dept_badge_number(mockdata, client, session):
# Attempt to set resign date to a date before the start date
form = AssignmentForm(
resign_date=date(2018, 12, 31)
)
officer = Officer.query.filter_by(id=3).one()

rv = client.post(
url_for('main.edit_assignment', officer_id=officer.id,
assignment_id=officer.assignments[0].id),
data=form.data,
follow_redirects=True
)
assignment = Assignment.query.filter_by(star_no='1234', job_id=job.id).first()
assert "End date must come after start date." in rv.data.decode('utf-8')
assert assignment.star_date == date(2019, 1, 1)
assert assignment.resign_date == date(2019, 12, 31)


def test_ac_can_edit_officer_in_their_dept_assignment(mockdata, client, session):
with current_app.test_request_context():
login_ac(client)

star_no = '1234'
new_star_no = '12345'
officer = Officer.query.filter_by(department_id=AC_DEPT).first()
job = Job.query.filter_by(department_id=officer.department_id, job_title='Police Officer').one()
form = AssignmentForm(star_no=star_no, job_title=job.id)
form = AssignmentForm(
star_no=star_no,
job_title=job.id,
star_date=date(2019, 1, 1),
resign_date=date(2019, 12, 31),
)

# Remove existing assignments
Assignment.query.filter_by(officer_id=officer.id).delete()
Expand All @@ -209,10 +316,20 @@ def test_ac_can_edit_officer_in_their_dept_badge_number(mockdata, client, sessio
)
assert 'Added new assignment' in rv.data.decode('utf-8')
assert '<td>{}</td>'.format(star_no) in rv.data.decode('utf-8')
assert '2019-01-01' in rv.data.decode('utf-8')
assert '2019-12-31' in rv.data.decode('utf-8')
assert officer.assignments[0].star_no == star_no
assert officer.assignments[0].star_date == date(2019, 1, 1)
assert officer.assignments[0].resign_date == date(2019, 12, 31)

officer = Officer.query.filter_by(id=officer.id).one()
job = Job.query.filter_by(department_id=officer.department_id, job_title='Commander').one()
form = AssignmentForm(star_no=new_star_no, job_title=job.id)
form = AssignmentForm(
star_no=new_star_no,
job_title=job.id,
star_date=date(2019, 2, 1),
resign_date=date(2019, 11, 30),
)

rv = client.post(
url_for('main.edit_assignment', officer_id=officer.id,
Expand All @@ -223,10 +340,14 @@ def test_ac_can_edit_officer_in_their_dept_badge_number(mockdata, client, sessio

assert 'Edited officer assignment' in rv.data.decode('utf-8')
assert '<td>{}</td>'.format(new_star_no) in rv.data.decode('utf-8')
assert '2019-02-01' in rv.data.decode('utf-8')
assert '2019-11-30' in rv.data.decode('utf-8')
assert officer.assignments[0].star_no == new_star_no
assert officer.assignments[0].star_date == date(2019, 2, 1)
assert officer.assignments[0].resign_date == date(2019, 11, 30)


def test_ac_cannot_edit_officer_outside_their_dept_badge_number(mockdata, client, session):
def test_ac_cannot_edit_assignment_outside_their_dept(mockdata, client, session):
with current_app.test_request_context():
login_admin(client)

Expand Down