Skip to content

Commit

Permalink
Merge pull request #11 from sjaensch/load-csv-use-post
Browse files Browse the repository at this point in the history
Use POST for loading CSV, document why we're using GET for the other tasks
  • Loading branch information
rockdog authored Feb 16, 2017
2 parents 12f6f85 + 744cf2b commit 4996eb3
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 3 deletions.
2 changes: 1 addition & 1 deletion tests/views/task_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ def test_load_employees_from_s3(self, mock_load_employees): # noqa

@mock.patch('logic.employee.load_employees_from_csv', autospec=True)
def test_load_employees_from_csv(self, mock_load_employees_from_csv): # noqa
response = self.app.get('/tasks/employees/load/csv')
response = self.app.post('/tasks/employees/load/csv')

self.assertEqual(response.status_int, 200)
mock_load_employees_from_csv.assert_called_with()
6 changes: 5 additions & 1 deletion views/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,22 @@
from models import Love


# All tasks that are to be executed by cron need to use HTTP GET
# see https://cloud.google.com/appengine/docs/python/config/cron
@app.route('/tasks/employees/load/s3', methods=['GET'])
def load_employees_from_s3():
logic.employee.load_employees()
return Response(status=200)


@app.route('/tasks/employees/load/csv', methods=['GET'])
# This task has a web UI to trigger it, so let's use POST
@app.route('/tasks/employees/load/csv', methods=['POST'])
def load_employees_from_csv():
logic.employee.load_employees_from_csv()
return Response(status=200)


# One-off tasks are much easier to trigger using GET
@app.route('/tasks/employees/combine', methods=['GET'])
def combine_employees():
old_username, new_username = request.args['old'], request.args['new']
Expand Down
2 changes: 1 addition & 1 deletion views/web.py
Original file line number Diff line number Diff line change
Expand Up @@ -327,5 +327,5 @@ def import_employees_form():
@admin_required
def import_employees():
flash('We started importing employee data in the background. Refresh the page to see it.', 'info')
taskqueue.add(url='/tasks/employees/load/csv', method='GET')
taskqueue.add(url='/tasks/employees/load/csv')
return redirect(url_for('employees'))

0 comments on commit 4996eb3

Please sign in to comment.