From dca63e897c33c571b184342eb3b70f50c65c204b Mon Sep 17 00:00:00 2001 From: David Read Date: Fri, 21 Dec 2018 16:20:56 +0000 Subject: [PATCH 1/2] PEP8 and tidy This is extracted from https://github.com/ckan/ckan/pull/3972 The only functional change is:

->

which is for consistency with others of type: {% block primary_content_inner %} --- ckan/logic/action/get.py | 50 +++++++++---------- ckan/logic/auth/__init__.py | 3 +- ckan/model/activity.py | 34 +++++++------ ckan/model/package.py | 7 +-- ckan/templates-bs2/group/activity_stream.html | 6 ++- .../organization/activity_stream.html | 6 ++- .../package/snippets/resources.html | 4 +- ckan/templates-bs2/user/activity_stream.html | 6 ++- ckan/templates/group/activity_stream.html | 6 ++- .../organization/activity_stream.html | 6 ++- .../templates/package/snippets/resources.html | 40 +++++++-------- ckan/templates/user/activity_stream.html | 6 ++- ckan/tests/controllers/test_package.py | 2 +- ckan/tests/logic/action/test_get.py | 15 +++--- ckan/views/dataset.py | 8 ++- 15 files changed, 113 insertions(+), 86 deletions(-) diff --git a/ckan/logic/action/get.py b/ckan/logic/action/get.py index 302e6fa3951..b7146136698 100644 --- a/ckan/logic/action/get.py +++ b/ckan/logic/action/get.py @@ -55,7 +55,7 @@ def _filter_activity_by_user(activity_list, users=[]): ''' - Return the given ``activity_list`` with activities from the specified + Return the given ``activity_list`` but with activities from the specified users removed. The users parameters should be a list of ids. A *new* filtered list is returned, the given ``activity_list`` itself is @@ -1017,7 +1017,7 @@ def package_show(context, data_dict): package_dict_validated = False metadata_modified = pkg.metadata_modified.isoformat() search_metadata_modified = search_result['metadata_modified'] - # solr stores less precice datetime, + # solr stores less precise datetime, # truncate to 22 charactors to get good enough match if metadata_modified[:22] != search_metadata_modified[:22]: package_dict = None @@ -2511,10 +2511,10 @@ def user_activity_list(context, data_dict): offset = data_dict.get('offset', 0) limit = data_dict['limit'] # defaulted, limited & made an int by schema - _activity_objects = model.activity.user_activity_list(user.id, limit=limit, - offset=offset) - activity_objects = _filter_activity_by_user(_activity_objects, - _activity_stream_get_filtered_users()) + _activity_objects = model.activity.user_activity_list( + user.id, limit=limit, offset=offset) + activity_objects = _filter_activity_by_user( + _activity_objects, _activity_stream_get_filtered_users()) return model_dictize.activity_list_dictize(activity_objects, context) @@ -2553,10 +2553,10 @@ def package_activity_list(context, data_dict): offset = int(data_dict.get('offset', 0)) limit = data_dict['limit'] # defaulted, limited & made an int by schema - _activity_objects = model.activity.package_activity_list(package.id, - limit=limit, offset=offset) - activity_objects = _filter_activity_by_user(_activity_objects, - _activity_stream_get_filtered_users()) + _activity_objects = model.activity.package_activity_list( + package.id, limit=limit, offset=offset) + activity_objects = _filter_activity_by_user( + _activity_objects, _activity_stream_get_filtered_users()) return model_dictize.activity_list_dictize(activity_objects, context) @@ -2594,10 +2594,10 @@ def group_activity_list(context, data_dict): group_show = logic.get_action('group_show') group_id = group_show(context, {'id': group_id})['id'] - _activity_objects = model.activity.group_activity_list(group_id, - limit=limit, offset=offset) - activity_objects = _filter_activity_by_user(_activity_objects, - _activity_stream_get_filtered_users()) + _activity_objects = model.activity.group_activity_list( + group_id, limit=limit, offset=offset) + activity_objects = _filter_activity_by_user( + _activity_objects, _activity_stream_get_filtered_users()) return model_dictize.activity_list_dictize(activity_objects, context) @@ -2633,10 +2633,10 @@ def organization_activity_list(context, data_dict): org_show = logic.get_action('organization_show') org_id = org_show(context, {'id': org_id})['id'] - _activity_objects = model.activity.group_activity_list(org_id, - limit=limit, offset=offset) - activity_objects = _filter_activity_by_user(_activity_objects, - _activity_stream_get_filtered_users()) + _activity_objects = model.activity.group_activity_list( + org_id, limit=limit, offset=offset) + activity_objects = _filter_activity_by_user( + _activity_objects, _activity_stream_get_filtered_users()) return model_dictize.activity_list_dictize(activity_objects, context) @@ -2664,9 +2664,9 @@ def recently_changed_packages_activity_list(context, data_dict): limit = data_dict['limit'] # defaulted, limited & made an int by schema _activity_objects = model.activity.recently_changed_packages_activity_list( - limit=limit, offset=offset) - activity_objects = _filter_activity_by_user(_activity_objects, - _activity_stream_get_filtered_users()) + limit=limit, offset=offset) + activity_objects = _filter_activity_by_user( + _activity_objects, _activity_stream_get_filtered_users()) return model_dictize.activity_list_dictize(activity_objects, context) @@ -3354,11 +3354,11 @@ def dashboard_activity_list(context, data_dict): # FIXME: Filter out activities whose subject or object the user is not # authorized to read. - _activity_objects = model.activity.dashboard_activity_list(user_id, - limit=limit, offset=offset) + _activity_objects = model.activity.dashboard_activity_list( + user_id, limit=limit, offset=offset) - activity_objects = _filter_activity_by_user(_activity_objects, - _activity_stream_get_filtered_users()) + activity_objects = _filter_activity_by_user( + _activity_objects, _activity_stream_get_filtered_users()) activity_dicts = model_dictize.activity_list_dictize( activity_objects, context) diff --git a/ckan/logic/auth/__init__.py b/ckan/logic/auth/__init__.py index 288a7f8467a..7556f0441de 100644 --- a/ckan/logic/auth/__init__.py +++ b/ckan/logic/auth/__init__.py @@ -9,8 +9,7 @@ def _get_object(context, data_dict, name, class_name): - # return the named item if in the data_dict, or get it from - # model.class_name + # return the named item if in the context, or get it from model.class_name try: return context[name] except KeyError: diff --git a/ckan/model/activity.py b/ckan/model/activity.py index b56d91ecc26..5d8f2845499 100644 --- a/ckan/model/activity.py +++ b/ckan/model/activity.py @@ -45,10 +45,11 @@ Column('data', _types.JsonDictType), ) + class Activity(domain_object.DomainObject): - def __init__(self, user_id, object_id, revision_id, activity_type, - data=None): + def __init__( + self, user_id, object_id, revision_id, activity_type, data=None): self.id = _types.make_uuid() self.timestamp = datetime.datetime.utcnow() self.user_id = user_id @@ -60,12 +61,14 @@ def __init__(self, user_id, object_id, revision_id, activity_type, else: self.data = data + meta.mapper(Activity, activity_table) class ActivityDetail(domain_object.DomainObject): - def __init__(self, activity_id, object_id, object_type, activity_type, + def __init__( + self, activity_id, object_id, object_type, activity_type, data=None): self.activity_id = activity_id self.object_id = object_id @@ -79,11 +82,11 @@ def __init__(self, activity_id, object_id, object_type, activity_type, @classmethod def by_activity_id(cls, activity_id): return ckan.model.Session.query(cls) \ - .filter_by(activity_id = activity_id).all() + .filter_by(activity_id=activity_id).all() -meta.mapper(ActivityDetail, activity_detail_table, properties = { - 'activity':orm.relation ( Activity, backref=orm.backref('activity_detail')) +meta.mapper(ActivityDetail, activity_detail_table, properties={ + 'activity': orm.relation(Activity, backref=orm.backref('activity_detail')) }) @@ -99,6 +102,7 @@ def _activities_limit(q, limit, offset=None): q = q.limit(limit) return q + def _activities_union_all(*qlist): ''' Return union of two or more queries sorted by timestamp, @@ -109,12 +113,14 @@ def _activities_union_all(*qlist): union_all(*[q.subquery().select() for q in qlist]) ).distinct(model.Activity.timestamp) + def _activities_at_offset(q, limit, offset): ''' Return a list of all activities at an offset with a limit. ''' return _activities_limit(q, limit, offset).all() + def _activities_from_user_query(user_id): '''Return an SQLAlchemy query for all activities from user_id.''' import ckan.model as model @@ -158,15 +164,15 @@ def _package_activity_query(package_id): ''' import ckan.model as model - q = model.Session.query(model.Activity) - q = q.filter_by(object_id=package_id) + q = model.Session.query(model.Activity) \ + .filter_by(object_id=package_id) return q def package_activity_list(package_id, limit, offset): '''Return the given dataset (package)'s public activity stream. - Returns all activities about the given dataset, i.e. where the given + Returns all activities about the given dataset, i.e. where the given dataset is the object of the activity, e.g.: "{USER} created the dataset {DATASET}" @@ -216,7 +222,7 @@ def _group_activity_query(group_id): or_( model.Member.group_id == group_id, model.Activity.object_id == group_id - ), + ) ) return q @@ -225,7 +231,7 @@ def _group_activity_query(group_id): def group_activity_list(group_id, limit, offset): '''Return the given group's public activity stream. - Returns all activities where the given group or one of its datasets is the + Returns activities where the given group or one of its datasets is the object of the activity, e.g.: "{USER} updated the group {GROUP}" @@ -237,7 +243,7 @@ def group_activity_list(group_id, limit, offset): return _activities_at_offset(q, limit, offset) -def _activites_from_users_followed_by_user_query(user_id, limit): +def _activities_from_users_followed_by_user_query(user_id, limit): '''Return a query for all activities from users that user_id follows.''' import ckan.model as model @@ -290,7 +296,7 @@ def _activities_from_groups_followed_by_user_query(user_id, limit): def _activities_from_everything_followed_by_user_query(user_id, limit): '''Return a query for all activities from everything user_id follows.''' - q1 = _activites_from_users_followed_by_user_query(user_id, limit) + q1 = _activities_from_users_followed_by_user_query(user_id, limit) q2 = _activities_from_datasets_followed_by_user_query(user_id, limit) q3 = _activities_from_groups_followed_by_user_query(user_id, limit) return _activities_union_all(q1, q2, q3) @@ -330,7 +336,7 @@ def dashboard_activity_list(user_id, limit, offset): return _activities_at_offset(q, limit, offset) def _changed_packages_activity_query(): - '''Return an SQLAlchemyu query for all changed package activities. + '''Return an SQLAlchemy query for all changed package activities. Return a query for all activities with activity_type '*package', e.g. 'new_package', 'changed_package', 'deleted_package'. diff --git a/ckan/model/package.py b/ckan/model/package.py index 3022119b4ba..e267d712f59 100644 --- a/ckan/model/package.py +++ b/ckan/model/package.py @@ -1,11 +1,9 @@ # encoding: utf-8 import datetime -from calendar import timegm import logging -logger = logging.getLogger(__name__) -from sqlalchemy.sql import select, and_, union, or_ +from sqlalchemy.sql import and_, or_ from sqlalchemy import orm from sqlalchemy import types, Column, Table from ckan.common import config @@ -22,6 +20,8 @@ import ckan.lib.maintain as maintain import ckan.lib.dictization as dictization +logger = logging.getLogger(__name__) + __all__ = ['Package', 'package_table', 'package_revision_table', 'PACKAGE_NAME_MAX_LENGTH', 'PACKAGE_NAME_MIN_LENGTH', 'PACKAGE_VERSION_MAX_LENGTH', 'PackageTagRevision', @@ -508,6 +508,7 @@ def get_fields(core_only=False, fields_to_ignore=None): def activity_stream_item(self, activity_type, revision, user_id): import ckan.model import ckan.logic + assert activity_type in ("new", "changed"), ( str(activity_type)) diff --git a/ckan/templates-bs2/group/activity_stream.html b/ckan/templates-bs2/group/activity_stream.html index c7c42dfc902..9aaca718428 100644 --- a/ckan/templates-bs2/group/activity_stream.html +++ b/ckan/templates-bs2/group/activity_stream.html @@ -3,7 +3,11 @@ {% block subtitle %}{{ _('Activity Stream') }} {{ g.template_title_delimiter }} {{ super() }}{% endblock %} {% block primary_content_inner %} -

{% block page_heading %}{{ _('Activity Stream') }}{% endblock %}

+

+ {% block page_heading -%} + {{ _('Activity Stream') }} + {%- endblock %} +

{% block activity_stream %} {{ group_activity_stream | safe }} {% endblock %} diff --git a/ckan/templates-bs2/organization/activity_stream.html b/ckan/templates-bs2/organization/activity_stream.html index f502cc51669..f85fb38b2b8 100644 --- a/ckan/templates-bs2/organization/activity_stream.html +++ b/ckan/templates-bs2/organization/activity_stream.html @@ -3,7 +3,11 @@ {% block subtitle %}{{ _('Activity Stream') }} {{ g.template_title_delimiter }} {{ super() }}{% endblock %} {% block primary_content_inner %} -

{% block page_heading %}{{ _('Activity Stream') }}{% endblock %}

+

+ {% block page_heading -%} + {{ _('Activity Stream') }} + {%- endblock %} +

{% block activity_stream %} {{ c.group_activity_stream | safe }} {% endblock %} diff --git a/ckan/templates-bs2/package/snippets/resources.html b/ckan/templates-bs2/package/snippets/resources.html index afab6202f88..8c55f8ee4c4 100644 --- a/ckan/templates-bs2/package/snippets/resources.html +++ b/ckan/templates-bs2/package/snippets/resources.html @@ -1,10 +1,10 @@ {# -Displays a sidebard module with navigation containing the provided resources. +Displays a sidebar module with navigation containing the provided resources. If no resources are provided then the module will not be displayed. pkg - The package dict that owns the resources. active - The id of the currently displayed resource. -action - The controller action to use (default: 'resource_read'). +action - The resource action to use (default: 'read', meaning route 'resource.read'). Example: diff --git a/ckan/templates-bs2/user/activity_stream.html b/ckan/templates-bs2/user/activity_stream.html index 12fe507c417..9eb39f09724 100644 --- a/ckan/templates-bs2/user/activity_stream.html +++ b/ckan/templates-bs2/user/activity_stream.html @@ -3,7 +3,11 @@ {% block subtitle %}{{ _('Activity Stream') }} {{ g.template_title_delimiter }} {{ super() }}{% endblock %} {% block primary_content_inner %} -

{% block page_heading %}{{ _('Activity Stream') }}{% endblock %}

+

+ {% block page_heading -%} + {{ _('Activity Stream') }} + {%- endblock %} +

{% block activity_stream %} {{ g.user_activity_stream | safe }} {% endblock %} diff --git a/ckan/templates/group/activity_stream.html b/ckan/templates/group/activity_stream.html index c7c42dfc902..9aaca718428 100644 --- a/ckan/templates/group/activity_stream.html +++ b/ckan/templates/group/activity_stream.html @@ -3,7 +3,11 @@ {% block subtitle %}{{ _('Activity Stream') }} {{ g.template_title_delimiter }} {{ super() }}{% endblock %} {% block primary_content_inner %} -

{% block page_heading %}{{ _('Activity Stream') }}{% endblock %}

+

+ {% block page_heading -%} + {{ _('Activity Stream') }} + {%- endblock %} +

{% block activity_stream %} {{ group_activity_stream | safe }} {% endblock %} diff --git a/ckan/templates/organization/activity_stream.html b/ckan/templates/organization/activity_stream.html index 9f909c4c424..b2e52252770 100644 --- a/ckan/templates/organization/activity_stream.html +++ b/ckan/templates/organization/activity_stream.html @@ -3,7 +3,11 @@ {% block subtitle %}{{ _('Activity Stream') }} {{ g.template_title_delimiter }} {{ super() }}{% endblock %} {% block primary_content_inner %} -

{% block page_heading %}{{ _('Activity Stream') }}{% endblock %}

+

+ {% block page_heading -%} + {{ _('Activity Stream') }} + {%- endblock %} +

{% block activity_stream %} {{ group_activity_stream | safe }} {% endblock %} diff --git a/ckan/templates/package/snippets/resources.html b/ckan/templates/package/snippets/resources.html index b2d38174062..09fc977c1c6 100644 --- a/ckan/templates/package/snippets/resources.html +++ b/ckan/templates/package/snippets/resources.html @@ -1,10 +1,10 @@ {# -Displays a sidebard module with navigation containing the provided resources. +Displays a sidebar module with navigation containing the provided resources. If no resources are provided then the module will not be displayed. pkg - The package dict that owns the resources. active - The id of the currently displayed resource. -action - The controller action to use (default: 'resource_read'). +action - The resource action to use (default: 'read', meaning route 'resource.read'). Example: @@ -13,22 +13,22 @@ #} {% set resources = pkg.resources or [] %} {% if resources %} - {% block resources %} -
- {% block resources_inner %} - {% block resources_title %} -

{{ _("Resources") }}

- {% endblock %} - {% block resources_list %} - - {% endblock %} - {% endblock %} -
- {% endblock %} + {% block resources %} +
+ {% block resources_inner %} + {% block resources_title %} +

{{ _("Resources") }}

+ {% endblock %} + {% block resources_list %} + + {% endblock %} + {% endblock %} +
+ {% endblock %} {% endif %} diff --git a/ckan/templates/user/activity_stream.html b/ckan/templates/user/activity_stream.html index 12fe507c417..9eb39f09724 100644 --- a/ckan/templates/user/activity_stream.html +++ b/ckan/templates/user/activity_stream.html @@ -3,7 +3,11 @@ {% block subtitle %}{{ _('Activity Stream') }} {{ g.template_title_delimiter }} {{ super() }}{% endblock %} {% block primary_content_inner %} -

{% block page_heading %}{{ _('Activity Stream') }}{% endblock %}

+

+ {% block page_heading -%} + {{ _('Activity Stream') }} + {%- endblock %} +

{% block activity_stream %} {{ g.user_activity_stream | safe }} {% endblock %} diff --git a/ckan/tests/controllers/test_package.py b/ckan/tests/controllers/test_package.py index 16b54381494..08376acdfb2 100644 --- a/ckan/tests/controllers/test_package.py +++ b/ckan/tests/controllers/test_package.py @@ -1009,7 +1009,7 @@ def setup_class(cls): @classmethod def teardown_class(cls): - p.unload('image_view') + p.unload('image_view') def test_resource_view_create(self): user = factories.User() diff --git a/ckan/tests/logic/action/test_get.py b/ckan/tests/logic/action/test_get.py index 344f2026e4d..fdefd885113 100644 --- a/ckan/tests/logic/action/test_get.py +++ b/ckan/tests/logic/action/test_get.py @@ -1403,6 +1403,13 @@ def test_package_works_without_user_in_context(self): ''' logic.get_action('package_search')({}, dict(q='anything')) + def test_local_parameters_not_supported(self): + nose.tools.assert_raises( + SearchError, + helpers.call_action, + 'package_search', + q='{!child of="content_type:parentDoc"}') + class TestPackageAutocompleteWithDatasetForm(helpers.FunctionalTestBase): @classmethod @@ -1436,14 +1443,6 @@ def test_custom_schema_not_returned(self): eq(query['results'][0]['extras'][0]['key'], 'custom_text') eq(query['results'][0]['extras'][0]['value'], 'foo') - def test_local_parameters_not_supported(self): - - nose.tools.assert_raises( - SearchError, - helpers.call_action, - 'package_search', - q='{!child of="content_type:parentDoc"}') - class TestBadLimitQueryParameters(helpers.FunctionalTestBase): '''test class for #1258 non-int query parameters cause 500 errors diff --git a/ckan/views/dataset.py b/ckan/views/dataset.py index 1d35a4ecd18..7cc7259a80e 100644 --- a/ckan/views/dataset.py +++ b/ckan/views/dataset.py @@ -1014,11 +1014,9 @@ def activity(package_type, id): try: pkg_dict = get_action(u'package_show')(context, data_dict) pkg = context[u'package'] - package_activity_stream = get_action(u'package_activity_list_html')( - context, { - u'id': pkg_dict[u'id'] - } - ) + package_activity_stream = get_action( + u'package_activity_list_html')( + context, {u'id': pkg_dict[u'id']}) dataset_type = pkg_dict[u'type'] or u'dataset' except NotFound: return base.abort(404, _(u'Dataset not found')) From d19484b5fb06a93683df977fb353a73f6d4a7341 Mon Sep 17 00:00:00 2001 From: David Read Date: Fri, 21 Dec 2018 17:17:09 +0000 Subject: [PATCH 2/2] Remove tab --- ckan/templates/package/snippets/resources.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ckan/templates/package/snippets/resources.html b/ckan/templates/package/snippets/resources.html index 09fc977c1c6..2a3ea0a346e 100644 --- a/ckan/templates/package/snippets/resources.html +++ b/ckan/templates/package/snippets/resources.html @@ -13,7 +13,7 @@ #} {% set resources = pkg.resources or [] %} {% if resources %} - {% block resources %} + {% block resources %}
{% block resources_inner %} {% block resources_title %}