From d1acb58f2e4b094c04d5461d63ca24dabc23e4ff Mon Sep 17 00:00:00 2001 From: tobes Date: Mon, 19 Nov 2012 16:14:15 +0000 Subject: [PATCH 1/6] fix sa warning in tests/functional/api/test_activity.py:TestActivity.test_create_group --- ckan/logic/action/get.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/ckan/logic/action/get.py b/ckan/logic/action/get.py index a6776216aff..146cd899b86 100644 --- a/ckan/logic/action/get.py +++ b/ckan/logic/action/get.py @@ -1794,8 +1794,11 @@ def group_activity_list(context, data_dict): # Get the group's activities. query = model.Session.query(model.Activity) - query = query.filter(_or_(model.Activity.object_id == group_id, - model.Activity.object_id.in_(dataset_ids))) + if dataset_ids: + query = query.filter(_or_(model.Activity.object_id == group_id, + model.Activity.object_id.in_(dataset_ids))) + else: + query = query.filter(model.Activity.object_id == group_id) query = query.order_by(_desc(model.Activity.timestamp)) query = query.limit(15) activity_objects = query.all() From 0326af359aaf6a85eb0f047f7113aca34e9b7365 Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Tue, 20 Nov 2012 14:47:36 +0100 Subject: [PATCH 2/6] PEP8 --- ckan/tests/functional/api/test_dashboard.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ckan/tests/functional/api/test_dashboard.py b/ckan/tests/functional/api/test_dashboard.py index 5c5cbd8443f..087acfb9c48 100644 --- a/ckan/tests/functional/api/test_dashboard.py +++ b/ckan/tests/functional/api/test_dashboard.py @@ -196,10 +196,11 @@ def test_06_mark_new_activities_as_read(self): def test_07_maximum_number_of_new_activities(self): '''Test that the new activities count does not go higher than 15, even if there are more than 15 new activities from the user's followers.''' - for n in range(0,20): + for n in range(0, 20): notes = "Updated {n} times".format(n=n) params = json.dumps({'name': 'warandpeace', 'notes': notes}) - response = self.app.post('/api/action/package_update', params=params, + response = self.app.post('/api/action/package_update', + params=params, extra_environ={'Authorization': str(self.joeadmin['apikey'])}) assert response.json['success'] is True assert self.dashboard_new_activities_count(self.new_user) == 15 From 611b87ad94feaca192c09b18c05b03a4b92720d4 Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Tue, 20 Nov 2012 14:50:59 +0100 Subject: [PATCH 3/6] Fix a crash in dashboard_activity_list auth Fix the dashboard_activity_list auth function to not crash when no user is logged in, and add some tests for this case. --- ckan/logic/auth/get.py | 2 +- ckan/tests/functional/api/test_dashboard.py | 50 ++++++++++++--------- 2 files changed, 31 insertions(+), 21 deletions(-) diff --git a/ckan/logic/auth/get.py b/ckan/logic/auth/get.py index cc3c20f88fe..51174f89b26 100644 --- a/ckan/logic/auth/get.py +++ b/ckan/logic/auth/get.py @@ -192,7 +192,7 @@ def get_site_user(context, data_dict): def dashboard_activity_list(context, data_dict): - if 'user' in context: + if context.get('user'): return {'success': True} else: return {'success': False, diff --git a/ckan/tests/functional/api/test_dashboard.py b/ckan/tests/functional/api/test_dashboard.py index 087acfb9c48..1d7c291c200 100644 --- a/ckan/tests/functional/api/test_dashboard.py +++ b/ckan/tests/functional/api/test_dashboard.py @@ -46,27 +46,32 @@ def setup_class(cls): def teardown_class(cls): ckan.model.repo.rebuild_db() + def post(self, action, params=None, apikey=None, status=200): + '''Post to the CKAN API and return the result.''' + if params is None: + params = {} + params = json.dumps(params) + response = self.app.post('/api/action/{0}'.format(action), + params=params, + extra_environ={'Authorization': str(apikey)}, + status=status) + if status in (200,): + assert response.json['success'] is True + return response.json['result'] + else: + assert response.json['success'] is False + return response.json['error'] + def dashboard_new_activities_count(self, user): '''Return the given user's new activities count from the CKAN API.''' - params = json.dumps({}) - response = self.app.post('/api/action/dashboard_new_activities_count', - params=params, - extra_environ={'Authorization': str(user['apikey'])}) - assert response.json['success'] is True - new_activities_count = response.json['result'] - return new_activities_count + return self.post('dashboard_new_activities_count', + apikey=user['apikey']) def dashboard_activity_list(self, user): '''Return the given user's dashboard activity list from the CKAN API. ''' - params = json.dumps({}) - response = self.app.post('/api/action/dashboard_activity_list', - params=params, - extra_environ={'Authorization': str(user['apikey'])}) - assert response.json['success'] is True - activity_list = response.json['result'] - return activity_list + return self.post('dashboard_activity_list', apikey=user['apikey']) def dashboard_new_activities(self, user): '''Return the activities from the user's dashboard activity stream @@ -75,12 +80,17 @@ def dashboard_new_activities(self, user): return [activity for activity in activity_list if activity['is_new']] def dashboard_mark_all_new_activities_as_old(self, user): - params = json.dumps({}) - response = self.app.post( - '/api/action/dashboard_mark_all_new_activities_as_old', - params=params, - extra_environ={'Authorization': str(user['apikey'])}) - assert response.json['success'] is True + self.post('dashboard_mark_all_new_activities_as_old', + apikey=user['apikey']) + + def test_00_dashboard_activity_list_not_logged_in(self): + self.post('dashboard_activity_list', status=403) + + def test_00_dashboard_new_activities_count_not_logged_in(self): + self.post('dashboard_new_activities_count', status=403) + + def test_00_dashboard_mark_new_activities_not_logged_in(self): + self.post('dashboard_mark_all_new_activities_as_old', status=403) def test_01_new_activities_count_for_new_user(self): '''Test that a newly registered user's new activities count is 0.''' From 9c5f314efcccc90f1980ce8a0ea807f81de17a38 Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Tue, 20 Nov 2012 14:59:36 +0100 Subject: [PATCH 4/6] Add a couple of FIXMEs that should be done after 2939-orgs is merged --- ckan/logic/auth/get.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ckan/logic/auth/get.py b/ckan/logic/auth/get.py index cc3c20f88fe..a83e2464d4d 100644 --- a/ckan/logic/auth/get.py +++ b/ckan/logic/auth/get.py @@ -200,10 +200,14 @@ def dashboard_activity_list(context, data_dict): def dashboard_new_activities_count(context, data_dict): + # FIXME: This should go through check_access() not call is_authorized() + # directly, but wait until 2939-orgs is merged before fixing this. return ckan.new_authz.is_authorized('dashboard_activity_list', context, data_dict) def dashboard_mark_all_new_activities_as_old(context, data_dict): + # FIXME: This should go through check_access() not call is_authorized() + # directly, but wait until 2939-orgs is merged before fixing this. return ckan.new_authz.is_authorized('dashboard_activity_list', context, data_dict) From a6392b5ba64e5fa91fffeefc28e0974a028a3dda Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Tue, 20 Nov 2012 15:12:33 +0100 Subject: [PATCH 5/6] Add a FIXME for an unhandled case --- ckan/logic/auth/get.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ckan/logic/auth/get.py b/ckan/logic/auth/get.py index 51174f89b26..4058c70b9ca 100644 --- a/ckan/logic/auth/get.py +++ b/ckan/logic/auth/get.py @@ -192,6 +192,8 @@ def get_site_user(context, data_dict): def dashboard_activity_list(context, data_dict): + # FIXME: context['user'] could be an IP address but that case is not + # handled here. Maybe add an auth helper function like is_logged_in(). if context.get('user'): return {'success': True} else: From 9ed808b81c908775b9c6e7b9c570707cdc97d5d1 Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Wed, 21 Nov 2012 14:46:58 +0100 Subject: [PATCH 6/6] Add a step to the source install docs It seems it's sometimes necessary to deactivate and reactivate your virtualenv after installing everything, to get the copy of paster right. --- doc/install-from-source.rst | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/doc/install-from-source.rst b/doc/install-from-source.rst index 74ba4334b79..172c3ae31b3 100644 --- a/doc/install-from-source.rst +++ b/doc/install-from-source.rst @@ -66,6 +66,13 @@ c. Install the Python modules that CKAN requires into your virtualenv:: pip install -r ~/pyenv/src/ckan/pip-requirements.txt +d. Deactivate and reactivate your virtualenv, to make sure you're using the + virtualenv's copies of commands like ``paster`` rather than any system-wide + installed copies:: + + deactivate + . ~/pyenv/bin/activate + 3. Setup a PostgreSQL database ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~