diff --git a/README.rst b/README.rst index 2235d807868..97f8f32e205 100644 --- a/README.rst +++ b/README.rst @@ -1,35 +1,45 @@ -CKAN is open-source data hub software. CKAN makes it easy to publish, share and -work with data. It's a data management system that provides a powerful platform -for cataloging, storing and accessing datasets with a rich front-end, full API -(for both data and catalog), visualization tools and more. Read more at -http://ckan.org/. +CKAN: The Open Source Data Portal Software +========================================== - * Installation instructions: see docs at http://docs.ckan.org/ - * Project wiki: http://wiki.ckan.org/ - * Developer mailing list: ckan-dev@lists.okfn.org - * Issue tracker: http://trac.ckan.org/ +**CKAN is the world’s leading open-source data portal platform**. +CKAN makes it easy to publish, share and work with data. It's a data management +system that provides a powerful platform for cataloging, storing and accessing +datasets with a rich front-end, full API (for both data and catalog), visualization +tools and more. Read more at `ckan.org `_. -Building Documentation -====================== -1. Install python-sphinx (>= 1.1) +Installation +------------ -2. Initialize the theme submodule:: +See the `CKAN Documentation `_ for installation instructions. - git submodule init - git submodule update -3. Run the command to build the docs:: +Community +--------- + +* Developer mailing list: `ckan-dev@lists.okfn.org `_ +* Developer IRC channel: #ckan on `irc.freenode.net `_ +* Issue tracker: `trac.ckan.org `_ +* `CKAN tag on StackOverflow `_ + + +Contributing to CKAN +-------------------- + +CKAN is a free software project and code contributions are welcome. +The `For CKAN Developers `_ +section of the documentation explains how to contribute to CKAN or its documentation, +including our **coding standards**. + +The `CKAN Wiki `_ is also open fo contributions. - python setup.py build_sphinx Copying and License -=================== +------------------- This material is copyright (c) 2006-2011 Open Knowledge Foundation. It is open and licensed under the GNU Affero General Public License (AGPL) v3.0 whose full text may be found at: -http://www.fsf.org/licensing/licenses/agpl-3.0.html - +http://www.fsf.org/licensing/licenses/agpl-3.0.html \ No newline at end of file diff --git a/ckan/config/deployment.ini_tmpl b/ckan/config/deployment.ini_tmpl index e048fa3754a..af055959b3a 100644 --- a/ckan/config/deployment.ini_tmpl +++ b/ckan/config/deployment.ini_tmpl @@ -244,6 +244,13 @@ ckan.feeds.author_link = preview.direct = png jpg gif preview.loadable = html htm rdf+xml owl+xml xml n3 n-triples turtle plain atom csv tsv rss txt json +# DEBUGGING + +# ckan.debug_supress_header This option can be set to suppress the debug +# information showing the controller and action recieving the request being +# shown in the header. Note: This info only shows if debug is set to true. +ckan.debug_supress_header = false + ## =================================== ## Extensions diff --git a/ckan/config/routing.py b/ckan/config/routing.py index b6549f525a5..7087bb5b67e 100644 --- a/ckan/config/routing.py +++ b/ckan/config/routing.py @@ -194,6 +194,7 @@ def make_map(): 'history', 'read_ajax', 'history_ajax', + 'activity', 'followers', 'follow', 'unfollow', @@ -265,7 +266,7 @@ def make_map(): # Note: openid users have slashes in their ids, so need the wildcard # in the route. m.connect('/user/activity/{id}', action='activity') - m.connect('/user/dashboard', action='dashboard') + m.connect('/dashboard', action='dashboard') m.connect('/user/follow/{id}', action='follow') m.connect('/user/unfollow/{id}', action='unfollow') m.connect('/user/followers/{id:.*}', action='followers') diff --git a/ckan/controllers/group.py b/ckan/controllers/group.py index d6bffcc8be0..1867079529b 100644 --- a/ckan/controllers/group.py +++ b/ckan/controllers/group.py @@ -342,7 +342,7 @@ def _force_reindex(self, grp): appearing on the read page for the group (as they're connected via the group name)''' group = model.Group.get(grp['name']) - for dataset in group.active_packages().all(): + for dataset in group.packages(): search.rebuild(dataset.name) def _save_edit(self, id, context): diff --git a/ckan/controllers/package.py b/ckan/controllers/package.py index 68a3c89b3e6..d8c9097b562 100644 --- a/ckan/controllers/package.py +++ b/ckan/controllers/package.py @@ -321,13 +321,6 @@ def read(self, id, format='html'): c.current_package_id = c.pkg.id c.related_count = c.pkg.related_count - # Add the package's activity stream (already rendered to HTML) to the - # template context for the package/read.html template to retrieve - # later. - c.package_activity_stream = \ - get_action('package_activity_list_html')( - context, {'id': c.current_package_id}) - PackageSaver().render_package(c.pkg_dict, context) template = self._read_template(package_type) @@ -1243,6 +1236,26 @@ def followers(self, id=None): return render('package/followers.html') + def activity(self, id): + '''Render this package's public activity stream page.''' + + context = {'model': model, 'session': model.Session, + 'user': c.user or c.author, 'for_view': True} + data_dict = {'id': id} + try: + c.pkg_dict = get_action('package_show')(context, data_dict) + c.pkg = context['package'] + c.package_activity_stream = get_action( + 'package_activity_list_html')(context, + {'id': c.pkg_dict['id']}) + c.related_count = c.pkg.related_count + except NotFound: + abort(404, _('Dataset not found')) + except NotAuthorized: + abort(401, _('Unauthorized to read dataset %s') % id) + + return render('package/activity.html') + def resource_embedded_dataviewer(self, id, resource_id, width=500, height=500): """ diff --git a/ckan/controllers/user.py b/ckan/controllers/user.py index a8543938e11..e32d32587cc 100644 --- a/ckan/controllers/user.py +++ b/ckan/controllers/user.py @@ -500,6 +500,13 @@ def dashboard(self, id=None): 'user': c.user or c.author, 'for_view': True} data_dict = {'id': id, 'user_obj': c.userobj} self._setup_template_variables(context, data_dict) + + c.dashboard_activity_stream = h.dashboard_activity_stream(id) + + # Mark the user's new activities as old whenever they view their + # dashboard page. + get_action('dashboard_mark_all_new_activities_as_old')(context, {}) + return render('user/dashboard.html') def follow(self, id): diff --git a/ckan/lib/activity_streams.py b/ckan/lib/activity_streams.py index 457767e5934..8ca1f193079 100644 --- a/ckan/lib/activity_streams.py +++ b/ckan/lib/activity_streams.py @@ -1,4 +1,5 @@ import re +import datetime from pylons.i18n import _ from webhelpers.html import literal @@ -228,10 +229,12 @@ def activity_list_to_html(context, activity_stream): for match in matches: snippet = activity_snippet_functions[match](activity, detail) data[str(match)] = snippet + activity_list.append({'msg': activity_msg, 'type': activity_type.replace(' ', '-').lower(), 'icon': activity_icon, 'data': data, - 'timestamp': activity['timestamp']}) + 'timestamp': activity['timestamp'], + 'is_new': activity.get('is_new', False)}) return literal(base.render('activity_streams/activity_stream_items.html', extra_vars={'activities': activity_list})) diff --git a/ckan/lib/app_globals.py b/ckan/lib/app_globals.py index 92fd5c9469e..bcba9203990 100644 --- a/ckan/lib/app_globals.py +++ b/ckan/lib/app_globals.py @@ -161,5 +161,7 @@ def _init(self): datasets_per_page = int(config.get('ckan.datasets_per_page', '20')) self.datasets_per_page = datasets_per_page + self.debug_supress_header = asbool(config.get('ckan.debug_supress_header', 'false')) + app_globals = _Globals() del _Globals diff --git a/ckan/lib/base.py b/ckan/lib/base.py index edc4264e2f1..904c61a00bd 100644 --- a/ckan/lib/base.py +++ b/ckan/lib/base.py @@ -214,6 +214,16 @@ def __before__(self, action, **params): self._identify_user() i18n.handle_request(request, c) + # If the user is logged in add their number of new activities to the + # template context. + if c.userobj: + from ckan.logic import get_action + new_activities_count = get_action( + 'dashboard_new_activities_count') + context = {'model': model, 'session': model.Session, + 'user': c.user or c.author} + c.new_activities = new_activities_count(context, {}) + def _identify_user(self): ''' Identifies the user using two methods: diff --git a/ckan/lib/cli.py b/ckan/lib/cli.py index 6bb339c042e..d8f43742f42 100644 --- a/ckan/lib/cli.py +++ b/ckan/lib/cli.py @@ -130,6 +130,15 @@ def command(self): if self.verbose: print 'Initialising DB: SUCCESS' elif cmd == 'clean' or cmd == 'drop': + + # remove any *.pyc version files to prevent conflicts + v_path = os.path.join(os.path.dirname(__file__), + '..', 'migration', 'versions', '*.pyc') + import glob + filelist = glob.glob(v_path) + for f in filelist: + os.remove(f) + model.repo.clean_db() search.clear() if self.verbose: diff --git a/ckan/lib/dictization/model_dictize.py b/ckan/lib/dictization/model_dictize.py index 8a08ff50b03..6926e4a3395 100644 --- a/ckan/lib/dictization/model_dictize.py +++ b/ckan/lib/dictization/model_dictize.py @@ -32,7 +32,7 @@ def group_list_dictize(obj_list, context, group_dict['display_name'] = obj.display_name group_dict['packages'] = \ - len(obj.active_packages(with_private=with_private).all()) + len(obj.packages(with_private=with_private)) if context.get('for_view'): for item in plugins.PluginImplementations( diff --git a/ckan/logic/action/get.py b/ckan/logic/action/get.py index 91d3bc41c5a..c85e3a7ece9 100644 --- a/ckan/logic/action/get.py +++ b/ckan/logic/action/get.py @@ -1,6 +1,7 @@ import uuid import logging import json +import datetime from pylons import config from pylons.i18n import _ @@ -749,34 +750,22 @@ def group_package_show(context, data_dict): :rtype: list of dictionaries ''' - model = context["model"] - user = context["user"] - id = _get_or_bust(data_dict, 'id') - limit = data_dict.get("limit") + model = context['model'] + group_id = _get_or_bust(data_dict, 'id') - group = model.Group.get(id) + # FIXME: What if limit is not an int? Schema and validation needed. + limit = data_dict.get('limit') + + group = model.Group.get(group_id) context['group'] = group if group is None: raise NotFound _check_access('group_show', context, data_dict) - query = model.Session.query(model.PackageRevision)\ - .filter(model.PackageRevision.state=='active')\ - .filter(model.PackageRevision.current==True)\ - .join(model.Member, model.Member.table_id==model.PackageRevision.id)\ - .join(model.Group, model.Group.id==model.Member.group_id)\ - .filter_by(id=group.id)\ - .order_by(model.PackageRevision.name) - - if limit: - query = query.limit(limit) - - if context.get('return_query'): - return query - result = [] - for pkg_rev in query.all(): + for pkg_rev in group.packages(limit=limit, + return_query=context.get('return_query')): result.append(model_dictize.package_dictize(pkg_rev, context)) return result @@ -1734,11 +1723,7 @@ def user_activity_list(context, data_dict): _check_access('user_show', context, data_dict) model = context['model'] user_id = _get_or_bust(data_dict, 'id') - query = model.Session.query(model.Activity) - query = query.filter_by(user_id=user_id) - query = query.order_by(_desc(model.Activity.timestamp)) - query = query.limit(15) - activity_objects = query.all() + activity_objects = model.activity.user_activity_list(user_id) return model_dictize.activity_list_dictize(activity_objects, context) def package_activity_list(context, data_dict): @@ -1757,11 +1742,7 @@ def package_activity_list(context, data_dict): _check_access('package_show', context, data_dict) model = context['model'] package_id = _get_or_bust(data_dict, 'id') - query = model.Session.query(model.Activity) - query = query.filter_by(object_id=package_id) - query = query.order_by(_desc(model.Activity.timestamp)) - query = query.limit(15) - activity_objects = query.all() + activity_objects = model.activity.package_activity_list(package_id) return model_dictize.activity_list_dictize(activity_objects, context) def group_activity_list(context, data_dict): @@ -1786,19 +1767,7 @@ def group_activity_list(context, data_dict): group_show = logic.get_action('group_show') group_id = group_show(context, {'id': group_id})['id'] - # Get a list of the IDs of the group's datasets. - group_package_show = logic.get_action('group_package_show') - datasets = group_package_show(context, {'id': group_id}) - dataset_ids = [dataset['id'] for dataset in datasets] - - # 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))) - query = query.order_by(_desc(model.Activity.timestamp)) - query = query.limit(15) - activity_objects = query.all() - + activity_objects = model.activity.group_activity_list(group_id) return model_dictize.activity_list_dictize(activity_objects, context) def recently_changed_packages_activity_list(context, data_dict): @@ -1810,11 +1779,7 @@ def recently_changed_packages_activity_list(context, data_dict): # FIXME: Filter out activities whose subject or object the user is not # authorized to read. model = context['model'] - query = model.Session.query(model.Activity) - query = query.filter(model.Activity.activity_type.endswith('package')) - query = query.order_by(_desc(model.Activity.timestamp)) - query = query.limit(15) - activity_objects = query.all() + activity_objects = model.activity.recently_changed_packages_activity_list() return model_dictize.activity_list_dictize(activity_objects, context) def activity_detail_list(context, data_dict): @@ -2199,45 +2164,51 @@ def group_followee_list(context, data_dict): def dashboard_activity_list(context, data_dict): - '''Return the dashboard activity stream of the given user. + '''Return the authorized user's dashboard activity stream. - :param id: the id or name of the user - :type id: string + Unlike the activity dictionaries returned by other *_activity_list actions, + these activity dictionaries have an extra boolean value with key 'is_new' + that tells you whether the activity happened since the user last viewed her + dashboard ('is_new': True) or not ('is_new': False). - :rtype: list of dictionaries + The user's own activities are always marked 'is_new': False. + + :rtype: list of activity dictionaries ''' - # FIXME: Filter out activities whose subject or object the user is not - # authorized to read. - model = context['model'] - user_id = _get_or_bust(data_dict, 'id') + _check_access('dashboard_activity_list', context, data_dict) - activity_query = model.Session.query(model.Activity) - user_followees_query = activity_query.join(model.UserFollowingUser, model.UserFollowingUser.object_id == model.Activity.user_id) - dataset_followees_query = activity_query.join(model.UserFollowingDataset, model.UserFollowingDataset.object_id == model.Activity.object_id) + model = context['model'] + user_id = model.User.get(context['user']).id - from_user_query = activity_query.filter(model.Activity.user_id==user_id) - about_user_query = activity_query.filter(model.Activity.object_id==user_id) - user_followees_query = user_followees_query.filter(model.UserFollowingUser.follower_id==user_id) - dataset_followees_query = dataset_followees_query.filter(model.UserFollowingDataset.follower_id==user_id) + # FIXME: Filter out activities whose subject or object the user is not + # authorized to read. + activity_objects = model.activity.dashboard_activity_list(user_id) + + activity_dicts = model_dictize.activity_list_dictize( + activity_objects, context) + + # Mark the new (not yet seen by user) activities. + strptime = datetime.datetime.strptime + fmt = '%Y-%m-%dT%H:%M:%S.%f' + last_viewed = model.Dashboard.get_activity_stream_last_viewed(user_id) + for activity in activity_dicts: + if activity['user_id'] == user_id: + # Never mark the user's own activities as new. + activity['is_new'] = False + else: + activity['is_new'] = (strptime(activity['timestamp'], fmt) + > last_viewed) - query = from_user_query.union(about_user_query).union( - user_followees_query).union(dataset_followees_query) - query = query.order_by(_desc(model.Activity.timestamp)) - query = query.limit(15) - activity_objects = query.all() + return activity_dicts - return model_dictize.activity_list_dictize(activity_objects, context) def dashboard_activity_list_html(context, data_dict): - '''Return the dashboard activity stream of the given user as HTML. + '''Return the authorized user's dashboard activity stream as HTML. The activity stream is rendered as a snippet of HTML meant to be included in an HTML page, i.e. it doesn't have any HTML header or footer. - :param id: The id or name of the user. - :type id: string - :rtype: string ''' @@ -2245,6 +2216,38 @@ def dashboard_activity_list_html(context, data_dict): return activity_streams.activity_list_to_html(context, activity_stream) +def dashboard_new_activities_count(context, data_dict): + '''Return the number of new activities in the user's dashboard. + + Return the number of new activities in the authorized user's dashboard + activity stream. + + Activities from the user herself are not counted by this function even + though they appear in the dashboard (users don't want to be notified about + things they did themselves). + + :rtype: int + + ''' + _check_access('dashboard_new_activities_count', context, data_dict) + activities = logic.get_action('dashboard_activity_list')( + context, data_dict) + return len([activity for activity in activities if activity['is_new']]) + + +def dashboard_mark_all_new_activities_as_old(context, data_dict): + '''Mark all the authorized user's new dashboard activities as old. + + This will reset dashboard_new_activities_count to 0. + + ''' + _check_access('dashboard_mark_all_new_activities_as_old', context, + data_dict) + model = context['model'] + user_id = model.User.get(context['user']).id + model.Dashboard.update_activity_stream_last_viewed(user_id) + + def _unpick_search(sort, allowed_fields=None, total=None): ''' This is a helper function that takes a sort string eg 'name asc, last_modified desc' and returns a list of diff --git a/ckan/logic/auth/get.py b/ckan/logic/auth/get.py index 395bace3b15..7db9082f89f 100644 --- a/ckan/logic/auth/get.py +++ b/ckan/logic/auth/get.py @@ -189,3 +189,27 @@ def get_site_user(context, data_dict): return {'success': False, 'msg': 'Only internal services allowed to use this action'} else: return {'success': True} + + +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: + return {'success': False, + 'msg': _("You must be logged in to access your dashboard.")} + + +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) diff --git a/ckan/logic/auth/publisher/get.py b/ckan/logic/auth/publisher/get.py index 0e936c95105..41383de6a41 100644 --- a/ckan/logic/auth/publisher/get.py +++ b/ckan/logic/auth/publisher/get.py @@ -5,6 +5,11 @@ from ckan.logic.auth.publisher import _groups_intersect from ckan.authz import Authorizer from ckan.logic.auth import get_package_object, get_group_object, get_resource_object +from ckan.logic.auth.get import ( + dashboard_new_activities_count, + dashboard_activity_list, + dashboard_mark_all_new_activities_as_old, + ) def site_read(context, data_dict): """\ diff --git a/ckan/migration/versions/062_add_dashboard_table.py b/ckan/migration/versions/062_add_dashboard_table.py new file mode 100644 index 00000000000..f5f23c82138 --- /dev/null +++ b/ckan/migration/versions/062_add_dashboard_table.py @@ -0,0 +1,16 @@ +from sqlalchemy import * +from migrate import * + +def upgrade(migrate_engine): + metadata = MetaData() + metadata.bind = migrate_engine + migrate_engine.execute(''' +CREATE TABLE dashboard ( + user_id text NOT NULL, + activity_stream_last_viewed timestamp without time zone NOT NULL +); +ALTER TABLE dashboard + ADD CONSTRAINT dashboard_pkey PRIMARY KEY (user_id); +ALTER TABLE dashboard + ADD CONSTRAINT dashboard_user_id_fkey FOREIGN KEY (user_id) REFERENCES "user"(id) ON UPDATE CASCADE ON DELETE CASCADE; + ''') diff --git a/ckan/model/__init__.py b/ckan/model/__init__.py index 5601f79bf5c..9ed834e0e6b 100644 --- a/ckan/model/__init__.py +++ b/ckan/model/__init__.py @@ -148,6 +148,9 @@ DomainObjectOperation, DomainObject, ) +from dashboard import ( + Dashboard, +) import ckan.migration diff --git a/ckan/model/activity.py b/ckan/model/activity.py index ff72337f04d..cddf9e04e01 100644 --- a/ckan/model/activity.py +++ b/ckan/model/activity.py @@ -1,6 +1,6 @@ import datetime -from sqlalchemy import orm, types, Column, Table, ForeignKey +from sqlalchemy import orm, types, Column, Table, ForeignKey, desc, or_ import meta import types as _types @@ -48,6 +48,7 @@ def __init__(self, user_id, object_id, revision_id, activity_type, meta.mapper(Activity, activity_table) + class ActivityDetail(domain_object.DomainObject): def __init__(self, activity_id, object_id, object_type, activity_type, @@ -61,6 +62,225 @@ def __init__(self, activity_id, object_id, object_type, activity_type, else: self.data = data + meta.mapper(ActivityDetail, activity_detail_table, properties = { 'activity':orm.relation ( Activity, backref=orm.backref('activity_detail')) }) + + +def _most_recent_activities(q, limit): + '''Return the 'limit' most recent activites from activity query 'q'.''' + import ckan.model as model + q = q.order_by(desc(model.Activity.timestamp)) + if limit: + q = q.limit(limit) + return q.all() + + +def _activities_from_user_query(user_id): + '''Return an SQLAlchemy query for all activities from user_id.''' + import ckan.model as model + q = model.Session.query(model.Activity) + q = q.filter(model.Activity.user_id == user_id) + return q + + +def _activities_about_user_query(user_id): + '''Return an SQLAlchemy query for all activities about user_id.''' + import ckan.model as model + q = model.Session.query(model.Activity) + q = q.filter(model.Activity.object_id == user_id) + return q + + +def _user_activity_query(user_id): + '''Return an SQLAlchemy query for all activities from or about user_id.''' + q = _activities_from_user_query(user_id) + q = q.union(_activities_about_user_query(user_id)) + return q + + +def user_activity_list(user_id, limit=15): + '''Return user_id's public activity stream. + + Return a list of all activities from or about the given user, i.e. where + the given user is the subject or object of the activity, e.g.: + + "{USER} created the dataset {DATASET}" + "{OTHER_USER} started following {USER}" + etc. + + ''' + q = _user_activity_query(user_id) + return _most_recent_activities(q, limit) + + +def _package_activity_query(package_id): + '''Return an SQLAlchemy query for all activities about package_id. + + ''' + import ckan.model as model + q = model.Session.query(model.Activity) + q = q.filter_by(object_id=package_id) + return q + + +def package_activity_list(package_id, limit=15): + '''Return the given dataset (package)'s public activity stream. + + 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}" + "{USER} updated the dataset {DATASET}" + etc. + + ''' + q = _package_activity_query(package_id) + return _most_recent_activities(q, limit) + + +def _group_activity_query(group_id, limit=15): + '''Return an SQLAlchemy query for all activities about group_id. + + Returns a query for all activities whose object is either the group itself + or one of the group's datasets. + + ''' + import ckan.model as model + + group = model.Group.get(group_id) + if not group: + # Return a query with no results. + return model.Session.query(model.Activity).filter("0=1") + + dataset_ids = [dataset.id for dataset in group.packages()] + + q = model.Session.query(model.Activity) + if dataset_ids: + q = q.filter(or_(model.Activity.object_id == group_id, + model.Activity.object_id.in_(dataset_ids))) + else: + q = q.filter(model.Activity.object_id == group_id) + return q + + +def group_activity_list(group_id, limit=15): + '''Return the given group's public activity stream. + + Returns all activities where the given group or one of its datasets is the + object of the activity, e.g.: + + "{USER} updated the group {GROUP}" + "{USER} updated the dataset {DATASET}" + etc. + + ''' + q = _group_activity_query(group_id) + return _most_recent_activities(q, limit) + + +def _activites_from_users_followed_by_user_query(user_id): + '''Return a query for all activities from users that user_id follows.''' + import ckan.model as model + q = model.Session.query(model.Activity) + q = q.join(model.UserFollowingUser, + model.UserFollowingUser.object_id == model.Activity.user_id) + q = q.filter(model.UserFollowingUser.follower_id == user_id) + return q + + +def _activities_from_datasets_followed_by_user_query(user_id): + '''Return a query for all activities from datasets that user_id follows.''' + import ckan.model as model + q = model.Session.query(model.Activity) + q = q.join(model.UserFollowingDataset, + model.UserFollowingDataset.object_id == model.Activity.object_id) + q = q.filter(model.UserFollowingDataset.follower_id == user_id) + return q + + +def _activities_from_groups_followed_by_user_query(user_id): + '''Return a query for all activities about groups the given user follows. + + Return a query for all activities about the groups the given user follows, + or about any of the group's datasets. This is the union of + _group_activity_query(group_id) for each of the groups the user follows. + + ''' + import ckan.model as model + + # Get a list of the group's that the user is following. + follower_objects = model.UserFollowingGroup.followee_list(user_id) + if not follower_objects: + # Return a query with no results. + return model.Session.query(model.Activity).filter("0=1") + + q = _group_activity_query(follower_objects[0].object_id) + q = q.union_all(*[_group_activity_query(follower.object_id) + for follower in follower_objects[1:]]) + return q + + +def _activities_from_everything_followed_by_user_query(user_id): + '''Return a query for all activities from everything user_id follows.''' + q = _activites_from_users_followed_by_user_query(user_id) + q = q.union(_activities_from_datasets_followed_by_user_query(user_id)) + q = q.union(_activities_from_groups_followed_by_user_query(user_id)) + return q + + +def activities_from_everything_followed_by_user(user_id, limit=15): + '''Return activities from everything that the given user is following. + + Returns all activities where the object of the activity is anything + (user, dataset, group...) that the given user is following. + + ''' + q = _activities_from_everything_followed_by_user_query(user_id) + return _most_recent_activities(q, limit) + + +def _dashboard_activity_query(user_id): + '''Return an SQLAlchemy query for user_id's dashboard activity stream.''' + q = _user_activity_query(user_id) + q = q.union(_activities_from_everything_followed_by_user_query(user_id)) + return q + + +def dashboard_activity_list(user_id, limit=15): + '''Return the given user's dashboard activity stream. + + Returns activities from the user's public activity stream, plus + activities from everything that the user is following. + + This is the union of user_activity_list(user_id) and + activities_from_everything_followed_by_user(user_id). + + ''' + q = _dashboard_activity_query(user_id) + return _most_recent_activities(q, limit) + + +def _changed_packages_activity_query(): + '''Return an SQLAlchemyu query for all changed package activities. + + Return a query for all activities with activity_type '*package', e.g. + 'new_package', 'changed_package', 'deleted_package'. + + ''' + import ckan.model as model + q = model.Session.query(model.Activity) + q = q.filter(model.Activity.activity_type.endswith('package')) + return q + + +def recently_changed_packages_activity_list(limit=15): + '''Return the site-wide stream of recently changed package activities. + + This activity stream includes recent 'new package', 'changed package' and + 'deleted package' activities for the whole site. + + ''' + q = _changed_packages_activity_query() + return _most_recent_activities(q, limit) diff --git a/ckan/model/dashboard.py b/ckan/model/dashboard.py new file mode 100644 index 00000000000..438fc865eca --- /dev/null +++ b/ckan/model/dashboard.py @@ -0,0 +1,47 @@ +import datetime +import sqlalchemy +import meta + +dashboard_table = sqlalchemy.Table('dashboard', meta.metadata, + sqlalchemy.Column('user_id', sqlalchemy.types.UnicodeText, + sqlalchemy.ForeignKey('user.id', onupdate='CASCADE', + ondelete='CASCADE'), + primary_key=True, nullable=False), + sqlalchemy.Column('activity_stream_last_viewed', sqlalchemy.types.DateTime, + nullable=False) +) + + +class Dashboard(object): + '''Saved data used for the user's dashboard.''' + + def __init__(self, user_id): + self.user_id = user_id + self.activity_stream_last_viewed = datetime.datetime.now() + + @classmethod + def get_activity_stream_last_viewed(cls, user_id): + query = meta.Session.query(Dashboard) + query = query.filter(Dashboard.user_id == user_id) + try: + row = query.one() + return row.activity_stream_last_viewed + except sqlalchemy.orm.exc.NoResultFound: + # No dashboard row has been created for this user so they have no + # activity_stream_last_viewed date. Return the oldest date we can + # (i.e. all activities are new to this user). + return datetime.datetime.min + + @classmethod + def update_activity_stream_last_viewed(cls, user_id): + query = meta.Session.query(Dashboard) + query = query.filter(Dashboard.user_id == user_id) + try: + row = query.one() + row.activity_stream_last_viewed = datetime.datetime.now() + except sqlalchemy.orm.exc.NoResultFound: + row = Dashboard(user_id) + meta.Session.add(row) + meta.Session.commit() + +meta.mapper(Dashboard, dashboard_table) diff --git a/ckan/model/group.py b/ckan/model/group.py index 62767671538..35dfb73b461 100644 --- a/ckan/model/group.py +++ b/ckan/model/group.py @@ -143,35 +143,6 @@ def set_approval_status(self, status): if status == "denied": pass - def members_of_type(self, object_type, capacity=None): - from ckan import model - object_type_string = object_type.__name__.lower() - query = meta.Session.query(object_type).\ - filter(model.Group.id == self.id).\ - filter(model.Member.state == 'active').\ - filter(model.Member.table_name == object_type_string) - - if hasattr(object_type, 'state'): - query = query.filter(object_type.state == 'active') - - if capacity: - query = query.filter(model.Member.capacity == capacity) - - query = query.join(model.Member, member_table.c.table_id == - getattr(object_type, 'id')).\ - join(model.Group, group_table.c.id == member_table.c.group_id) - - return query - - def add_child(self, object_instance): - object_type_string = object_instance.__class__.__name__.lower() - if not object_instance in self.members_of_type( - object_instance.__class__).all(): - member = Member(group=self, - table_id=getattr(object_instance, 'id'), - table_name=object_type_string) - meta.Session.add(member) - def get_children_groups(self, type='group'): # Returns a list of dicts where each dict contains "id", "name", # and "title" When querying with a CTE specifying a model in the @@ -184,20 +155,48 @@ def get_children_groups(self, type='group'): return [{"id":idf, "name": name, "title": title} for idf, name, title in results] - def active_packages(self, load_eager=True, with_private=False): - query = meta.Session.query(_package.Package).\ - filter_by(state=vdm.sqlalchemy.State.ACTIVE).\ - filter(group_table.c.id == self.id).\ - filter(member_table.c.state == 'active') + def packages(self, with_private=False, limit=None, + return_query=False): + '''Return this group's active and pending packages. + + Returns all packages in this group with VDM revision state ACTIVE or + PENDING. + + :param with_private: if True, include the group's private packages + :type with_private: boolean + + :param limit: the maximum number of packages to return + :type limit: int + + :param return_query: if True, return the SQLAlchemy query object + instead of the list of Packages resulting from the query + :type return_query: boolean + + :returns: a list of this group's packages + :rtype: list of ckan.model.package.Package objects + + ''' + query = meta.Session.query(_package.Package) + query = query.filter( + or_(_package.Package.state == vdm.sqlalchemy.State.ACTIVE, + _package.Package.state == vdm.sqlalchemy.State.PENDING)) + query = query.filter(group_table.c.id == self.id) if not with_private: query = query.filter(member_table.c.capacity == 'public') - query = query.join(member_table, member_table.c.table_id == - _package.Package.id).\ - join(group_table, group_table.c.id == member_table.c.group_id) + query = query.join(member_table, + member_table.c.table_id == _package.Package.id) + query = query.join(group_table, + group_table.c.id == member_table.c.group_id) + + if limit is not None: + query = query.limit(limit) - return query + if return_query: + return query + else: + return query.all() @classmethod def search_by_name_or_title(cls, text_query, group_type=None): @@ -209,23 +208,12 @@ def search_by_name_or_title(cls, text_query, group_type=None): q = q.filter(cls.type == group_type) return q.order_by(cls.title) - def as_dict(self, ref_package_by='name'): - _dict = domain_object.DomainObject.as_dict(self) - _dict['packages'] = [getattr(package, ref_package_by) - for package in self.packages] - _dict['extras'] = dict([(key, value) for key, value - in self.extras.items()]) - if (self.type == 'organization'): - _dict['users'] = [getattr(user, "name") - for user in self.members_of_type(_user.User)] - return _dict - def add_package_by_name(self, package_name): if not package_name: return package = _package.Package.by_name(package_name) assert package - if not package in self.members_of_type(package.__class__).all(): + if not package in self.packages(): member = Member(group=self, table_id=package.id, table_name='package') meta.Session.add(member) diff --git a/ckan/public/base/css/main.css b/ckan/public/base/css/main.css index c5abc691441..10703174a6a 100644 --- a/ckan/public/base/css/main.css +++ b/ckan/public/base/css/main.css @@ -6750,7 +6750,11 @@ li .icon-large:before { .hero .tags .tag { margin-right: 15px; } -.masthead { +header.masthead { + *zoom: 1; + color: #ffffff; + padding: 5px 10px 3px; + height: 55px; background-color: #005974; background-image: -moz-linear-gradient(top, #005d7a, #00536b); background-image: -ms-linear-gradient(top, #005d7a, #00536b); @@ -6760,53 +6764,32 @@ li .icon-large:before { background-image: linear-gradient(top, #005d7a, #00536b); background-repeat: repeat-x; filter: progid:DXImageTransform.Microsoft.gradient(startColorstr='#005d7a', endColorstr='#00536b', GradientType=0); - *zoom: 1; - color: #ffffff; - padding: 5px 10px 3px; } -.masthead:before, -.masthead:after { +header.masthead:before, +header.masthead:after { display: table; content: ""; } -.masthead:after { +header.masthead:after { clear: both; } -.masthead .container { +header.masthead .container { position: relative; } -.masthead a { +header.masthead a { color: #ffffff; } -.masthead hgroup, -.masthead nav { - *zoom: 1; - min-height: 52px; - position: relative; - display: inline-block; -} -.masthead hgroup:before, -.masthead nav:before, -.masthead hgroup:after, -.masthead nav:after { - display: table; - content: ""; -} -.masthead hgroup:after, -.masthead nav:after { - clear: both; -} -.masthead hgroup h1, -.masthead hgroup h2 { +header.masthead hgroup h1, +header.masthead hgroup h2 { + float: left; font-size: 34px; line-height: 1.5; - float: left; } -.masthead hgroup h1 { - font-family: "Arial Black", "Arial Bold", Gadget, sans-serif; - letter-spacing: -2px; +header.masthead hgroup h1 { + font-weight: 900; + letter-spacing: -1px; } -.masthead hgroup h2 { +header.masthead hgroup h2 { position: absolute; bottom: -3px; left: 0; @@ -6816,219 +6799,188 @@ li .icon-large:before { line-height: 1.2; white-space: nowrap; } -.masthead .tagline-right h2 { - left: auto; +header.masthead .content { + position: absolute; + top: 10px; right: 0; } -.masthead hgroup a { - text-decoration: none; -} -.masthead .logo { - margin-right: 10px; +header.masthead .section { float: left; } -.masthead .logo img { - display: block; +header.masthead .navigation { + margin-right: 20px; } -.masthead .content { - position: absolute; - top: 8px; - right: 0; - bottom: 0; -} -.masthead .content > * { - float: right; +header.masthead .navigation ul.unstyled { + *zoom: 1; + margin: 5px 0; } -.masthead nav { - display: inline-block; - line-height: 40px; +header.masthead .navigation ul.unstyled:before, +header.masthead .navigation ul.unstyled:after { + display: table; + content: ""; } -.masthead nav ul { - margin: 0; +header.masthead .navigation ul.unstyled:after { + clear: both; } -.masthead nav li { - display: inline-block; +header.masthead .navigation ul.unstyled li { + display: block; + float: left; } -.masthead nav li a { +header.masthead .navigation ul.unstyled li a { display: block; font-size: 12px; font-weight: bold; padding: 4px 10px; -} -.masthead nav li a.active { -webkit-border-radius: 3px; -moz-border-radius: 3px; border-radius: 3px; - -webkit-box-shadow: inset 0 1px 0 rgba(0, 0, 0, 0.3); - -moz-box-shadow: inset 0 1px 0 rgba(0, 0, 0, 0.3); - box-shadow: inset 0 1px 0 rgba(0, 0, 0, 0.3); - box-shadow: inset 0 1px 0 rgba(0, 0, 0, 0.3), 0 1px 0 rgba(255, 255, 255, 0.3); - background-color: rgba(255, 255, 255, 0.08); } -.masthead .account { - position: relative; - padding-top: 7px; +header.masthead .navigation ul.unstyled li a.active { + background-color: #0d6581; + box-shadow: 0 -1px 0 #084152, 0 1px 0 #26758e; } -.masthead .account.avatar { - padding-left: 55px; -} -.masthead .section { - margin-left: 20px; - padding-left: 20px; - min-height: 35px; -} -.masthead .section:before, -.masthead .section:after { - content: ""; - display: block; - position: absolute; - top: NaN; - left: 0; - bottom: NaN; - width: 1px; - background-color: #007094; - background-image: -webkit-gradient(radial, center center, 0, center center, 460, from(rgba(255, 255, 255, 0.1)), to(rgba(255, 255, 255, 0))); - background-image: -webkit-radial-gradient(circle, rgba(255, 255, 255, 0.1), rgba(255, 255, 255, 0)); - background-image: -moz-radial-gradient(circle, rgba(255, 255, 255, 0.1), rgba(255, 255, 255, 0)); - background-image: -ms-radial-gradient(circle, rgba(255, 255, 255, 0.1), rgba(255, 255, 255, 0)); - background-image: -o-radial-gradient(circle, rgba(255, 255, 255, 0.1), rgba(255, 255, 255, 0)); - background-color: rgba(255, 255, 255, 0.08000000000000002); - background-image: -webkit-gradient(linear, 0 0, 0 100%, from(rgba(255, 255, 255, 0)), color-stop(0.5, rgba(255, 255, 255, 0.1)), to(rgba(255, 255, 255, 0))); - background-image: -webkit-linear-gradient(rgba(255, 255, 255, 0), rgba(255, 255, 255, 0.1) 0.5, rgba(255, 255, 255, 0)); - background-image: -moz-linear-gradient(top, rgba(255, 255, 255, 0), rgba(255, 255, 255, 0.1) 0.5, rgba(255, 255, 255, 0)); - background-image: -ms-linear-gradient(rgba(255, 255, 255, 0), rgba(255, 255, 255, 0.1) 0.5, rgba(255, 255, 255, 0)); - background-image: -o-linear-gradient(rgba(255, 255, 255, 0), rgba(255, 255, 255, 0.1) 0.5, rgba(255, 255, 255, 0)); - background-image: linear-gradient(rgba(255, 255, 255, 0), rgba(255, 255, 255, 0.1) 0.5, rgba(255, 255, 255, 0)); - background-repeat: no-repeat; - background-color: rgba(255, 255, 255, 0); -} -.masthead .section:after { - left: -1px; - background-color: #004a61; - background-image: -webkit-gradient(radial, center center, 0, center center, 460, from(rgba(0, 0, 0, 0.2)), to(rgba(0, 0, 0, 0))); - background-image: -webkit-radial-gradient(circle, rgba(0, 0, 0, 0.2), rgba(0, 0, 0, 0)); - background-image: -moz-radial-gradient(circle, rgba(0, 0, 0, 0.2), rgba(0, 0, 0, 0)); - background-image: -ms-radial-gradient(circle, rgba(0, 0, 0, 0.2), rgba(0, 0, 0, 0)); - background-image: -o-radial-gradient(circle, rgba(0, 0, 0, 0.2), rgba(0, 0, 0, 0)); - background-color: rgba(0, 0, 0, 0.16000000000000003); - background-image: -webkit-gradient(linear, 0 0, 0 100%, from(rgba(0, 0, 0, 0)), color-stop(0.5, rgba(0, 0, 0, 0.2)), to(rgba(0, 0, 0, 0))); - background-image: -webkit-linear-gradient(rgba(0, 0, 0, 0), rgba(0, 0, 0, 0.2) 0.5, rgba(0, 0, 0, 0)); - background-image: -moz-linear-gradient(top, rgba(0, 0, 0, 0), rgba(0, 0, 0, 0.2) 0.5, rgba(0, 0, 0, 0)); - background-image: -ms-linear-gradient(rgba(0, 0, 0, 0), rgba(0, 0, 0, 0.2) 0.5, rgba(0, 0, 0, 0)); - background-image: -o-linear-gradient(rgba(0, 0, 0, 0), rgba(0, 0, 0, 0.2) 0.5, rgba(0, 0, 0, 0)); - background-image: linear-gradient(rgba(0, 0, 0, 0), rgba(0, 0, 0, 0.2) 0.5, rgba(0, 0, 0, 0)); - background-repeat: no-repeat; - background-color: rgba(0, 0, 0, 0); -} -.masthead .section.first:after, -.masthead .section.first:before { - content: none; +header.masthead .site-search { + margin: 3px 0; } -.masthead .navigation { - margin-right: -10px; +header.masthead .site-search input { + width: 190px; + font-size: 11px; + padding: 4px; } -.masthead .account .image { - position: absolute; - left: 10px; - top: 6px; - padding-right: 10px; - opacity: 0.8; +header.masthead .account { + background: #003f52; + padding: 3px 5px; + margin: 2px 0 2px 30px; + -webkit-border-radius: 4px; + -moz-border-radius: 4px; + border-radius: 4px; + -webkit-box-shadow: inset 0 2px 4px #002b38; + -moz-box-shadow: inset 0 2px 4px #002b38; + box-shadow: inset 0 2px 4px #002b38; } -.masthead .account .image img { - -webkit-border-radius: 100px; - -moz-border-radius: 100px; - border-radius: 100px; - -webkit-box-shadow: 0 -2px 4px rgba(0, 0, 0, 0.3); - -moz-box-shadow: 0 -2px 4px rgba(0, 0, 0, 0.3); - box-shadow: 0 -2px 4px rgba(0, 0, 0, 0.3); +header.masthead .account ul.unstyled { + *zoom: 1; } -.masthead .account .image i { - position: absolute; - top: 0; - left: 30px; - line-height: 25px; +header.masthead .account ul.unstyled:before, +header.masthead .account ul.unstyled:after { + display: table; + content: ""; } -.masthead .account .image:hover { - opacity: 1; +header.masthead .account ul.unstyled:after { + clear: both; } -.masthead .account .image:hover i { - font-weight: normal; - text-decoration: none; +header.masthead .account ul.unstyled li { + display: block; + float: left; } -.masthead .account.open .image { - opacity: 1; +header.masthead .account ul.unstyled li a { + display: block; + font-size: 12px; + font-weight: bold; + padding: 4px 10px; } -.masthead .account.open .image img { - -webkit-box-shadow: 0 -1px 2px rgba(0, 0, 0, 0.5); - -moz-box-shadow: 0 -1px 2px rgba(0, 0, 0, 0.5); - box-shadow: 0 -1px 2px rgba(0, 0, 0, 0.5); +header.masthead .account ul.unstyled li a.sub { + font-weight: 300; + border-left: 1px solid #002b38; } -.masthead .account .image img { - display: block; +header.masthead .account .dropdown { + float: left; } -.masthead .links { +header.masthead .account .button { display: block; - position: relative; - top: -2px; + text-decoration: none; + background-color: #005d7a; + color: #80aebd; + text-shadow: 0 1px 1px #003647; + -webkit-border-radius: 4px; + -moz-border-radius: 4px; + border-radius: 4px; + -webkit-box-shadow: inset 0 1px 0 #007094; + -moz-box-shadow: inset 0 1px 0 #007094; + box-shadow: inset 0 1px 0 #007094; } -.masthead .links a { - line-height: 13px; - float: left; +header.masthead .account .image { + margin: 2px 0; + padding: 0 4px 0 0; + overflow: hidden; + font-size: 10px; + -webkit-border-radius: 4px; + -moz-border-radius: 4px; + border-radius: 4px; } -.masthead .links .sub { - font-size: 11px; - clear: left; - color: rgba(255, 255, 255, 0.6); +header.masthead .account .image i { + vertical-align: -1px; } -.masthead .dropdown-menu { - min-width: 130px; - margin-top: -5px; +header.masthead .account .image img { + opacity: 0.7; + border-right: 1px solid #00536b; + -webkit-border-radius: 4px 0 0 4px; + -moz-border-radius: 4px 0 0 4px; + border-radius: 4px 0 0 4px; } -.masthead .dropdown-menu li a { - color: #187794; +header.masthead .account .notifications { + padding: 4px 8px 3px 8px; + margin: 2px 5px 2px 0; } -.masthead .dropdown-menu li a i { - margin-right: 5px; +header.masthead .account .notifications.notifications-important { + color: #ffffff; + background-color: #c9403a; + text-shadow: 0 1px 1px #a3322d; + -webkit-box-shadow: inset 0 1px 0 #ce534e; + -moz-box-shadow: inset 0 1px 0 #ce534e; + box-shadow: inset 0 1px 0 #ce534e; +} +header.masthead .account .dropdown.open .image img, +header.masthead .account .dropdown .image:hover img { + opacity: 1; + border-right-color: #007b9e; } -.masthead .dropdown-menu li a:hover { +header.masthead .account .dropdown.open .button, +header.masthead .account .dropdown .button:hover { color: #ffffff; + background-color: #669eaf; + -webkit-box-shadow: inset 0 1px 0 #80aebd; + -moz-box-shadow: inset 0 1px 0 #80aebd; + box-shadow: inset 0 1px 0 #80aebd; + text-decoration: none; } -.masthead .site-search { - position: relative; - margin: 0; - margin-left: 20px; - padding: 0 0 0 20px; +header.masthead .account .dropdown.open .notifications-important, +header.masthead .account .dropdown .notifications-important:hover { + background-color: #d46762; + text-shadow: 0 1px 1px #c9403a; + -webkit-box-shadow: inset 0 1px 0 #d97a76; + -moz-box-shadow: inset 0 1px 0 #d97a76; + box-shadow: inset 0 1px 0 #d97a76; } -.masthead .site-search .field { - padding: 0; - margin-top: 5px; +header.masthead .account.authed { + margin: 0 0 0 30px; } -.masthead .site-search input { - width: 190px; - font-size: 11px; - padding: 4px; +header.masthead .account.not-authed { + padding-top: 2px; + padding-bottom: 2px; } -.masthead .header-image { - font: inherit; - text-indent: -900em; - min-height: 50px; +header.masthead .dropdown-menu { + margin-top: -1px; } -.masthead .header-image .logo { - position: absolute; - top: 0; - left: 0; - display: block; - width: 240px; - height: 50px; - text-indent: 0; +header.masthead .user-dropdown-menu a { + color: #005d7a; +} +header.masthead .user-dropdown-menu a:hover { + color: #ffffff; } -.masthead .debug { +header.masthead .debug { position: absolute; - top: 10px; + bottom: 10px; left: 10px; + font-size: 11px; color: rgba(255, 255, 255, 0.5); + line-height: 1.2; } .site-footer { + *zoom: 1; + color: #ffffff; + padding: 5px 10px 3px; + height: 55px; background-color: #005974; background-image: -moz-linear-gradient(top, #005d7a, #00536b); background-image: -ms-linear-gradient(top, #005d7a, #00536b); @@ -7038,9 +6990,6 @@ li .icon-large:before { background-image: linear-gradient(top, #005d7a, #00536b); background-repeat: repeat-x; filter: progid:DXImageTransform.Microsoft.gradient(startColorstr='#005d7a', endColorstr='#00536b', GradientType=0); - *zoom: 1; - color: #ffffff; - padding: 5px 10px 3px; font-size: 12px; padding: 20px 0; } @@ -7264,6 +7213,7 @@ li .icon-large:before { background: transparent url('../../../base/images/dotted.png') 14px 0 repeat-y; } .activity .item { + position: relative; margin: 0 0 15px 0; padding: 0; *zoom: 1; @@ -7316,6 +7266,24 @@ li .icon-large:before { white-space: nowrap; margin-left: 5px; } +.activity .item .new { + display: block; + position: absolute; + overflow: hidden; + top: -3px; + left: -3px; + width: 10px; + height: 10px; + background-color: #A35647; + border: 1px solid #FFF; + text-indent: -1000px; + -webkit-border-radius: 100px; + -moz-border-radius: 100px; + border-radius: 100px; + -webkit-box-shadow: 0 1px 2px rgba(0, 0, 0, 0.2); + -moz-box-shadow: 0 1px 2px rgba(0, 0, 0, 0.2); + box-shadow: 0 1px 2px rgba(0, 0, 0, 0.2); +} .popover .about { margin-bottom: 10px; } @@ -7383,6 +7351,9 @@ li .icon-large:before { .activity .item.new-related-item i { background-color: #95a669; } +.activity .item.follow-group i { + background-color: #8ba669; +} .popover-context-loading .popover-title { display: none; } @@ -7405,6 +7376,9 @@ li .icon-large:before { -moz-border-radius: 100px; border-radius: 100px; } +.module-my-datasets .empty { + padding: 10px; +} body { background-color: #00536b; } @@ -7537,6 +7511,20 @@ iframe { -moz-box-shadow: 0 0 5px rgba(0, 0, 0, 0.3); box-shadow: 0 0 5px rgba(0, 0, 0, 0.3); } +.ie7 .masthead nav ul li a.active, +.ie8 .masthead nav ul li a.active { + position: relative; + top: -1px; + background-color: #006584; + border-top: 1px solid #00516b; + border-bottom: 1px solid #007094; +} +.ie8 .masthead .account a.image { + display: block; + width: 25px; + padding-right: 10px; + white-space: nowrap; +} .ie7 .alert { position: relative; } @@ -7620,6 +7608,10 @@ iframe { .ie7 .stages li .highlight { width: auto; } +.ie7 .masthead { + position: relative; + z-index: 1; +} .ie7 .masthead .logo img, .ie7 .masthead nav { *display: inline; @@ -7643,6 +7635,9 @@ iframe { .ie7 .masthead .header-image { display: block; } +.ie7 .masthead .account .dropdown-menu { + z-index: 10000; +} .ie7 .footer-links { *zoom: 1; } @@ -7657,10 +7652,6 @@ iframe { .ie7 .footer-links li { float: left; } -.ie7 .nav-item.active > a { - background-image: url(../images/background-tag-ie7.png); - background-position: 0 0; -} .ie7 .module-narrow .nav-item.image { *zoom: 1; } diff --git a/ckan/public/base/javascript/modules/dashboard.js b/ckan/public/base/javascript/modules/dashboard.js new file mode 100644 index 00000000000..7c30c655c6c --- /dev/null +++ b/ckan/public/base/javascript/modules/dashboard.js @@ -0,0 +1,11 @@ +this.ckan.module('dashboard', function ($, _) { + return { + initialize: function () { + if ($('.new', this.el)) { + setTimeout(function() { + $('.masthead .notifications').removeClass('notifications-important').html('0'); + }, 1000); + } + } + }; +}); diff --git a/ckan/public/base/javascript/resource.config b/ckan/public/base/javascript/resource.config index 8db348394b7..93ab18052e0 100644 --- a/ckan/public/base/javascript/resource.config +++ b/ckan/public/base/javascript/resource.config @@ -33,6 +33,7 @@ ckan = modules/resource-upload-field.js modules/follow.js modules/popover-context.js + modules/dashboard.js main = apply_html_class diff --git a/ckan/public/base/less/activity.less b/ckan/public/base/less/activity.less index dd082969542..1b6fbf4f32e 100644 --- a/ckan/public/base/less/activity.less +++ b/ckan/public/base/less/activity.less @@ -4,6 +4,7 @@ list-style-type: none; background: transparent url('@{imagePath}/dotted.png') 14px 0 repeat-y; .item { + position: relative; margin: 0 0 15px 0; padding: 0; .clearfix; @@ -41,6 +42,20 @@ white-space: nowrap; margin-left: 5px; } + .new { + display: block; + position: absolute; + overflow: hidden; + top: -3px; + left: -3px; + width: 10px; + height: 10px; + background-color: #A35647; + border: 1px solid #FFF; + text-indent: -1000px; + .border-radius(100px); + .box-shadow(0 1px 2px rgba(0, 0, 0, 0.2)); + } } } diff --git a/ckan/public/base/less/ckan.less b/ckan/public/base/less/ckan.less index 21a8d6ab07e..028c7de93a5 100644 --- a/ckan/public/base/less/ckan.less +++ b/ckan/public/base/less/ckan.less @@ -17,6 +17,7 @@ @import "activity.less"; @import "popover-context.less"; @import "follower-list.less"; +@import "dashboard.less"; body { // Using the masthead/footer gradient prevents the color from changing diff --git a/ckan/public/base/less/dashboard.less b/ckan/public/base/less/dashboard.less new file mode 100644 index 00000000000..63223bbcd7b --- /dev/null +++ b/ckan/public/base/less/dashboard.less @@ -0,0 +1,5 @@ +.module-my-datasets { + .empty { + padding: 10px; + } +} diff --git a/ckan/public/base/less/footer.less b/ckan/public/base/less/footer.less index 226ecc4249f..8783e68a85a 100644 --- a/ckan/public/base/less/footer.less +++ b/ckan/public/base/less/footer.less @@ -1,5 +1,5 @@ .site-footer { - .masthead; + .masthead(); font-size: 12px; padding: 20px 0; } diff --git a/ckan/public/base/less/iehacks.less b/ckan/public/base/less/iehacks.less index ece1e36bd2d..3d21b56ead5 100644 --- a/ckan/public/base/less/iehacks.less +++ b/ckan/public/base/less/iehacks.less @@ -68,6 +68,8 @@ .masthead .account a.image { display: block; width: 25px; + padding-right: 10px; + white-space: nowrap; } } @@ -192,10 +194,6 @@ } // Navs - .nav-item.active > a { - background-image: url(../images/background-tag-ie7.png); - background-position: 0 0; - } .module-narrow .nav-item.image { .clearfix; } diff --git a/ckan/public/base/less/masthead.less b/ckan/public/base/less/masthead.less index 59718a5f5e4..50eb9fdbd11 100644 --- a/ckan/public/base/less/masthead.less +++ b/ckan/public/base/less/masthead.less @@ -1,269 +1,202 @@ @mastheadPadding: 5px 10px 3px; +@notificationsBg: #C9403A; + +@menuActiveBg: mix(@mastheadBackgroundColorStart, @mastheadLinkColor, 95%); +@menuActiveHi: mix(@mastheadBackgroundColorStart, @mastheadLinkColor, 85%); +@menuActiveLo: darken(@menuActiveBg, 10%); + +.masthead() { + .clearfix(); + color: @mastheadTextColor; + padding: @mastheadPadding; + height: 55px; + #gradient > .vertical(@mastheadBackgroundColorStart, @mastheadBackgroundColorEnd); +} + +header.masthead { + + .masthead(); + + .container { + position: relative; + } + + a { + color: @mastheadLinkColor; + } + + hgroup { + h1, + h2 { + float: left; + font-size: 34px; + line-height: 1.5; + } + h1 { + font-weight: 900; + letter-spacing: -1px; + } + h2 { + position: absolute; + bottom: -3px; + left: 0; + margin: 0; + font-size: 15px; + font-weight: normal; + line-height: 1.2; + white-space: nowrap; + } + } + + .content { + position: absolute; + top: 10px; + right: 0; + } + + .section { + float: left; + } + + .navigation { + margin-right: 20px; + ul.unstyled { + .clearfix(); + margin: 5px 0; + li { + display: block; + float: left; + a { + display: block; + font-size: 12px; + font-weight: bold; + padding: 4px 10px; + .border-radius(3px); + &.active { + background-color: @menuActiveBg; + box-shadow: 0 -1px 0 @menuActiveLo, 0 1px 0 @menuActiveHi; + } + } + } + } + } + + .site-search { + margin: 3px 0; + input { + width: 190px; + font-size: 11px; + padding: 4px; + } + } + + .account { + background: darken(@mastheadBackgroundColorEnd, 5); + padding: 3px 5px; + margin: 2px 0 2px 30px; + .border-radius(4px); + .box-shadow(inset 0 2px 4px darken(@mastheadBackgroundColorEnd, 10)); + ul.unstyled { + .clearfix(); + li { + display: block; + float: left; + a { + display: block; + font-size: 12px; + font-weight: bold; + padding: 4px 10px; + &.sub { + font-weight: 300; + border-left: 1px solid darken(@mastheadBackgroundColorEnd, 10); + } + } + } + } + .dropdown { + float: left; + } + .button { + display: block; + text-decoration: none; + background-color: @mastheadBackgroundColorStart; + color: mix(@mastheadBackgroundColorStart, @mastheadLinkColor, 50%); + text-shadow: 0 1px 1px darken(@mastheadBackgroundColorStart, 10); + .border-radius(4px); + .box-shadow(inset 0 1px 0 lighten(@mastheadBackgroundColorStart, 5)); + } + .image { + margin: 2px 0; + padding: 0 4px 0 0; + overflow: hidden; + font-size: 10px; + .border-radius(4px); + i { + vertical-align: -1px; + } + img { + opacity: 0.7; + border-right: 1px solid @mastheadBackgroundColorEnd; + .border-radius(4px 0 0 4px); + } + } + .notifications { + padding: 4px 8px 3px 8px; + margin: 2px 5px 2px 0; + &.notifications-important { + color: @mastheadLinkColor; + background-color: @notificationsBg; + text-shadow: 0 1px 1px darken(@notificationsBg, 10); + .box-shadow(inset 0 1px 0 lighten(@notificationsBg, 5)); + } + } + .dropdown { + &.open .image img, + .image:hover img { + opacity: 1; + border-right-color: lighten(@mastheadBackgroundColorEnd, 10); + } + &.open .button, + .button:hover { + color: @mastheadLinkColor; + background-color: mix(@mastheadBackgroundColorStart, @mastheadLinkColor, 60%); + .box-shadow(inset 0 1px 0 mix(@mastheadBackgroundColorStart, @mastheadLinkColor, 50%)); + text-decoration: none; + } + &.open .notifications-important, + .notifications-important:hover { + background-color: lighten(@notificationsBg, 10); + text-shadow: 0 1px 1px @notificationsBg; + .box-shadow(inset 0 1px 0 lighten(@notificationsBg, 15)); + } + } + &.authed { + margin: 0 0 0 30px; + } + &.not-authed { + padding-top: 2px; + padding-bottom: 2px; + } + } + + .dropdown-menu { + margin-top: -1px; + } + + .user-dropdown-menu { + a { + color: @mastheadBackgroundColorStart; + &:hover { + color: @mastheadLinkColor; + } + } + } + + .debug { + position: absolute; + top: 10px; + left: 10px; + color: rgba(255, 255, 255, 0.5); + } -.masthead { - #gradient > .vertical(@mastheadBackgroundColorStart, @mastheadBackgroundColorEnd); - .clearfix(); - color: @mastheadTextColor; - padding: @mastheadPadding; -} - -.masthead .container { - position: relative; -} - -.masthead a { - color: @mastheadLinkColor; -} - -.masthead hgroup, -.masthead nav { - .clearfix; - min-height: 52px; - position: relative; - display: inline-block; -} - -.masthead hgroup h1, -.masthead hgroup h2 { - font-size: 34px; - line-height: 1.5; - float: left; -} - -.masthead hgroup h1 { - font-family: "Arial Black", "Arial Bold", Gadget, sans-serif; - letter-spacing: -2px; // Sorry world of typography. -} - -.masthead hgroup h2 { - position: absolute; - bottom: -3px; - left: 0; - margin: 0; - font-size: 15px; - font-weight: normal; - line-height: 1.2; - white-space: nowrap; -} - -.masthead .tagline-right h2 { - left: auto; - right: 0; -} - -.masthead hgroup a { - text-decoration: none; -} - -.masthead .logo { - margin-right: 10px; - float: left; -} - -.masthead .logo img { - display: block; -} - -.masthead .content { - position: absolute; - top: 8px; - right: 0; - bottom: 0; -} - -.masthead .content > * { - float: right; -} - -.masthead nav { - display: inline-block; - line-height: 40px; -} - -.masthead nav ul { - margin: 0; -} - -.masthead nav li { - display: inline-block; -} - -.masthead nav li a { - display: block; - font-size: 12px; - font-weight: bold; - padding: 4px 10px; -} - -.masthead nav li a.active { - @insetBoxShadow: inset 0 1px 0 rgba(0, 0, 0, 0.3); - .border-radius(3px); - .box-shadow(@insetBoxShadow); - box-shadow: @insetBoxShadow, 0 1px 0 rgba(255, 255, 255, 0.3); - background-color: rgba(255, 255, 255, 0.08); -} - -.masthead .account { - position: relative; - padding-top: 7px; -} - -.masthead .account.avatar { - padding-left: 55px; -} - -.masthead .section { - margin-left: 20px; - padding-left: 20px; - min-height: 35px; -} - -.masthead .section:before, -.masthead .section:after { - content: ""; - display: block; - position: absolute; - top: -@mastheadPadding - 10; - left: 0; - bottom: -@mastheadPadding - 10; - width: 1px; - background-color: lighten(@mastheadBackgroundColorStart, 5); - #gradient > .radial(rgba(255, 255, 255, 0.1), rgba(255, 255, 255, 0)); - .rgba-vertical-gradient-three-colors(rgba(255, 255, 255, 0), rgba(255, 255, 255, 0.1), 0.5, rgba(255, 255, 255, 0)); - background-color: rgba(255, 255, 255, 0); -} - -.masthead .section:before { -} - -.masthead .section:after { - left: -1px; - background-color: darken(@mastheadBackgroundColorStart, 5); - #gradient > .radial(rgba(0, 0, 0, 0.2), rgba(0, 0, 0, 0)); - .rgba-vertical-gradient-three-colors(rgba(0, 0, 0, 0), rgba(0, 0, 0, 0.2), 0.5, rgba(0, 0, 0, 0)); - background-color: rgba(0, 0, 0, 0); -} - -.masthead .section.first:after, -.masthead .section.first:before { - content: none; -} - -.masthead .navigation { - margin-right: -10px; // Pull nav in to the right. -} - -.masthead .account { - .image { - position: absolute; - left: 10px; - top: 6px; - padding-right: 10px; - opacity: 0.8; - img { - .border-radius(100px); - .box-shadow(0 -2px 4px rgba(0, 0, 0, 0.3)) - } - i { - position: absolute; - top: 0; - left: 30px; - line-height: 25px; - } - &:hover { - opacity: 1; - i { - font-weight: normal; - text-decoration: none; - } - } - } - &.open .image { - opacity: 1; - img { - .box-shadow(0 -1px 2px rgba(0, 0, 0, 0.5)) - } - } -} - -.masthead .account .image img { - display: block; -} - -.masthead .links { - display: block; - position: relative; - top: -2px; -} - -.masthead .links a { - line-height: 13px; - float: left; -} - -.masthead .links .sub { - font-size: 11px; - clear: left; - color: rgba(255, 255, 255, 0.6); -} - -.masthead .dropdown-menu { - min-width: 130px; - margin-top: -5px; - li a { - color: @layoutLinkColor; - i { - margin-right: 5px; - } - &:hover { - color: @mastheadLinkColor; - } - } -} - -.masthead .site-search { - position: relative; - margin: 0; - margin-left: 20px; - padding: 0 0 0 20px; -} - -.masthead .site-search .field { - padding: 0; - margin-top: 5px; -} - -.masthead .site-search input { - width: 190px; - font-size: 11px; - padding: 4px; -} - -.masthead .site-search button { - -} - -// Logo Image Replacement - -.masthead .header-image { - font: inherit; // Override Bootstrap .hide-text - text-indent: -900em; - min-height: 50px; -} - -.masthead .header-image .logo { - position: absolute; - top: 0; - left: 0; - display: block; - width: 240px; - height: 50px; - text-indent: 0; -} - -.masthead .debug { - position: absolute; - top: 10px; - left: 10px; - color: rgba(255, 255, 255, 0.5); } diff --git a/ckan/templates/header.html b/ckan/templates/header.html index 9fd3d6a3727..6f03bbd63de 100644 --- a/ckan/templates/header.html +++ b/ckan/templates/header.html @@ -1,5 +1,5 @@
- {% if config.debug %} + {% if config.debug and not g.debug_supress_header %}
Controller : {{ c.controller }}
Action : {{ c.action }}
{% endif %}
@@ -15,46 +15,11 @@

{% endif %}
- {% if c.userobj %} - - {% else %} - - {% endif %} + - + {% if c.userobj %} + + {% else %} + + {% endif %}

diff --git a/ckan/templates/package/activity.html b/ckan/templates/package/activity.html new file mode 100644 index 00000000000..ee0e2dd7fc0 --- /dev/null +++ b/ckan/templates/package/activity.html @@ -0,0 +1,10 @@ +{% extends "package/read.html" %} + +{% block subtitle %}{{ _('Activity Stream') }} - {{ c.pkg_dict.title or c.pkg_dict.name }}{% endblock %} + +{% block package_content %} +
+

{{ _('Activity Stream') }}

+ {{ c.package_activity_stream | safe }} +
+{% endblock %} diff --git a/ckan/templates/package/read.html b/ckan/templates/package/read.html index 886f63ef348..35fc36b5125 100644 --- a/ckan/templates/package/read.html +++ b/ckan/templates/package/read.html @@ -45,6 +45,9 @@ {% link_for _('Dataset'), controller='package', action='read', id=pkg.name, icon='sitemap' %} + + {% link_for _('Activity Stream'), controller='package', action='activity', id=pkg.name, icon='time' %} + {% link_for _('Followers'), controller='package', action='followers', id=pkg.name, icon='group' %} diff --git a/ckan/templates/snippets/activity_item.html b/ckan/templates/snippets/activity_item.html index b7c3640da45..1cf57772027 100644 --- a/ckan/templates/snippets/activity_item.html +++ b/ckan/templates/snippets/activity_item.html @@ -1,4 +1,7 @@
  • + {% if activity.is_new %} + {{ _('New activity item') }} + {% endif %}

    {{ h.literal(activity.msg.format(**activity.data)) }} diff --git a/ckan/templates/user/dashboard.html b/ckan/templates/user/dashboard.html index c63ad430497..2beaaaddea9 100644 --- a/ckan/templates/user/dashboard.html +++ b/ckan/templates/user/dashboard.html @@ -5,7 +5,6 @@ {% block subtitle %}{{ _('Dashboard') }}{% endblock %} {% block breadcrumb_content %} -

  • {% link_for _('Users'), controller='user', action='index' %}
  • {{ _('Dashboard') }}
  • {% endblock %} @@ -17,9 +16,9 @@ {% block primary_content %}
    -
    +

    {{ _('News feed') }} {{ _('Activity from users and datasets that you follow') }}

    - {{ h.dashboard_activity_stream(c.user_dict['id']) }} + {{ c.dashboard_activity_stream }}
    {% endblock %} diff --git a/ckan/tests/__init__.py b/ckan/tests/__init__.py index 721c55f6f49..0d4be8b4e49 100644 --- a/ckan/tests/__init__.py +++ b/ckan/tests/__init__.py @@ -400,3 +400,61 @@ class StatusCodes: STATUS_404_NOT_FOUND = 404 STATUS_409_CONFLICT = 409 + +def call_action_api(app, action, apikey=None, status=200, **kwargs): + '''POST an HTTP request to the CKAN API and return the result. + + Any additional keyword arguments that you pass to this function as **kwargs + are posted as params to the API. + + Usage: + + package_dict = post(app, 'package_create', apikey=apikey, + name='my_package') + assert package_dict['name'] == 'my_package' + + num_followers = post(app, 'user_follower_count', id='annafan') + + If you are expecting an error from the API and want to check the contents + of the error dict, you have to use the status param otherwise an exception + will be raised: + + error_dict = post(app, 'group_activity_list', status=403, + id='invalid_id') + assert error_dict['message'] == 'Access Denied' + + :param app: the test app to post to + :type app: paste.fixture.TestApp + + :param action: the action to post to, e.g. 'package_create' + :type action: string + + :param apikey: the API key to put in the Authorization header of the post + (optional, default: None) + :type apikey: string + + :param status: the HTTP status code expected in the response from the CKAN + API, e.g. 403, if a different status code is received an exception will + be raised (optional, default: 200) + :type status: int + + :param **kwargs: any other keyword arguments passed to this function will + be posted to the API as params + + :raises paste.fixture.AppError: if the HTTP status code of the response + from the CKAN API is different from the status param passed to this + function + + :returns: the 'result' or 'error' dictionary from the CKAN API response + :rtype: dictionary + + ''' + params = json.dumps(kwargs) + response = 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'] diff --git a/ckan/tests/functional/api/__init__.py b/ckan/tests/functional/api/__init__.py index 1c6e2442feb..08c9762d91d 100644 --- a/ckan/tests/functional/api/__init__.py +++ b/ckan/tests/functional/api/__init__.py @@ -1,10 +1,16 @@ from nose.tools import assert_equal import copy + def change_lists_to_sets(iterable): - '''recursive method to drill down into iterables to - convert any list or tuples into sets. Does not work - though for dictionaries in lists.''' + '''Convert any lists or tuples in `iterable` into sets. + + Recursively drill down into iterable and convert any list or tuples to + sets. + + Does not work for dictionaries in lists. + + ''' if isinstance(iterable, dict): for key in iterable: if isinstance(iterable[key], (list, tuple)): @@ -25,11 +31,14 @@ def change_lists_to_sets(iterable): else: raise NotImplementedError + def assert_dicts_equal_ignoring_ordering(dict1, dict2): - '''Asserts dicts are equal, assuming that the ordering of - any lists is unimportant.''' + '''Assert that dict1 and dict2 are equal. + + Assumes that the ordering of any lists in the dicts is unimportant. + + ''' dicts = [copy.deepcopy(dict1), copy.deepcopy(dict2)] for d in dicts: d = change_lists_to_sets(d) - #from nose.tools import set_trace; set_trace() assert_equal(dicts[0], dicts[1]) diff --git a/ckan/tests/functional/api/test_activity.py b/ckan/tests/functional/api/test_activity.py index 1126efd0484..dfc14f9f6c4 100644 --- a/ckan/tests/functional/api/test_activity.py +++ b/ckan/tests/functional/api/test_activity.py @@ -1,3 +1,13 @@ +'''Functional tests for the public activity streams API. + +This module tests the contents of the various public activity streams: +use activity streams, dataset activity streams, group activity streams, etc. + +This module _does not_ test the private user dashboard activity stream (which +is different because the contents depend on what the user is following), that +is tested in test_dashboard.py. + +''' import datetime import logging logger = logging.getLogger(__name__) @@ -163,12 +173,6 @@ def setup_class(self): 'apikey': normal_user.apikey, 'name': normal_user.name, } - follower = model.User.get('tester') - self.follower = { - 'id': follower.id, - 'apikey': follower.apikey, - 'name': follower.name, - } warandpeace = model.Package.get('warandpeace') self.warandpeace = { 'id': warandpeace.id, @@ -177,50 +181,14 @@ def setup_class(self): self.annakarenina = { 'id': annakarenina.id, } - self.users = [self.sysadmin_user, self.normal_user, self.follower] + self.users = [self.sysadmin_user, self.normal_user] self.app = paste.fixture.TestApp(pylons.test.pylonsapp) - # Make follower follow everything else. - params = {'id': 'testsysadmin'} - extra_environ = {'Authorization': str(self.follower['apikey'])} - response = self.app.post('/api/action/follow_user', - params=json.dumps(params), extra_environ=extra_environ).json - assert response['success'] is True - params = {'id': 'annafan'} - extra_environ = {'Authorization': str(self.follower['apikey'])} - response = self.app.post('/api/action/follow_user', - params=json.dumps(params), extra_environ=extra_environ).json - assert response['success'] is True - params = {'id': 'warandpeace'} - extra_environ = {'Authorization': str(self.follower['apikey'])} - response = self.app.post('/api/action/follow_dataset', - params=json.dumps(params), extra_environ=extra_environ).json - assert response['success'] is True - params = {'id': 'annakarenina'} - extra_environ = {'Authorization': str(self.follower['apikey'])} - response = self.app.post('/api/action/follow_dataset', - params=json.dumps(params), extra_environ=extra_environ).json - assert response['success'] is True - - self.followees = \ - [ - self.sysadmin_user['id'], - self.normal_user['id'], - self.follower['id'], - self.warandpeace['id'], - self.annakarenina['id'] - ] - @classmethod def teardown_class(self): import ckan.model as model model.repo.rebuild_db() - def dashboard_activity_stream(self, user_id): - response = self.app.get( - "/api/2/rest/user/{0}/dashboard_activity".format(user_id)) - return json.loads(response.body) - def user_activity_stream(self, user_id, apikey=None): if apikey: extra_environ = {'Authorization': str(apikey)} @@ -286,35 +254,24 @@ def record_details(self, user_id, package_id=None, group_ids=None, details['recently changed datasets stream'] = \ self.recently_changed_datasets_stream(apikey) - details['follower dashboard activity stream'] = ( - self.dashboard_activity_stream(self.follower['id'])) - details['time'] = datetime.datetime.now() return details - def check_dashboard( - self, - before, after, wanted_difference, - potential_followees): - difference = find_new_activities( - before['follower dashboard activity stream'], - after['follower dashboard activity stream']) - if any(potential_followee in self.followees - for potential_followee in potential_followees): - assert difference == wanted_difference - else: - assert len(difference) == 0 - def _create_package(self, user, name=None): if user: user_id = user['id'] + apikey = user['apikey'] else: user_id = 'not logged in' + apikey = None + + before = self.record_details(user_id, apikey=apikey) # Create a new package. request_data = make_package(name) before = self.record_details(user_id=user_id, - group_ids=[group['name'] for group in request_data['groups']]) + group_ids=[group['name'] for group in request_data['groups']], + apikey=apikey) extra_environ = {'Authorization': str(user['apikey'])} response = self.app.post('/api/action/package_create', json.dumps(request_data), extra_environ=extra_environ) @@ -324,7 +281,8 @@ def _create_package(self, user, name=None): after = self.record_details(user_id=user_id, package_id=package_created['id'], - group_ids=[group['name'] for group in package_created['groups']]) + group_ids=[group['name'] for group in package_created['groups']], + apikey=apikey) # Find the new activity in the user's activity stream. user_new_activities = (find_new_activities( @@ -345,15 +303,14 @@ def _create_package(self, user, name=None): after['recently changed datasets stream']) assert new_rcd_activities == [activity] - self.check_dashboard(before, after, user_new_activities, [user_id]) - # The same new activity should appear in the activity streams of the # package's groups. for group_dict in package_created['groups']: grp_new_activities = find_new_activities( before['group activity streams'][group_dict['name']], after['group activity streams'][group_dict['name']]) - assert grp_new_activities == [activity] + assert [activity['id'] for activity in grp_new_activities] == [ + activity['id']] # Check that the new activity has the right attributes. assert activity['object_id'] == package_created['id'], \ @@ -409,11 +366,13 @@ def _create_package(self, user, name=None): def _add_resource(self, package, user): if user: user_id = user['id'] + apikey = user['apikey'] else: user_id = 'not logged in' + apikey = None before = self.record_details(user_id, package['id'], - [group['id'] for group in package['groups']]) + [group['name'] for group in package['groups']], apikey=apikey) resource_ids_before = [resource['id'] for resource in package['resources']] @@ -424,7 +383,7 @@ def _add_resource(self, package, user): updated_package = package_update(self.app, package, user['apikey']) after = self.record_details(user_id, package['id'], - [group['id'] for group in package['groups']]) + [group['name'] for group in package['groups']], apikey=apikey) resource_ids_after = [resource['id'] for resource in updated_package['resources']] assert len(resource_ids_after) == len(resource_ids_before) + 1 @@ -458,9 +417,6 @@ def _add_resource(self, package, user): after['group activity streams'][group_dict['name']]) assert grp_new_activities == [activity] - self.check_dashboard(before, after, user_new_activities, - [user_id, package['id']]) - # Check that the new activity has the right attributes. assert activity['object_id'] == updated_package['id'], \ str(activity['object_id']) @@ -497,10 +453,14 @@ def _add_resource(self, package, user): def _delete_extra(self, package_dict, user): if user: user_id = user['id'] + apikey = user['apikey'] else: user_id = 'not logged in' + apikey = None - before = self.record_details(user_id, package_dict['id']) + before = self.record_details(user_id, package_dict['id'], + [group['name'] for group in package_dict['groups']], + apikey=apikey) extras_before = list(package_dict['extras']) assert len(extras_before) > 0, ( @@ -511,7 +471,9 @@ def _delete_extra(self, package_dict, user): updated_package = package_update(self.app, package_dict, user['apikey']) - after = self.record_details(user_id, package_dict['id']) + after = self.record_details(user_id, package_dict['id'], + [group['name'] for group in package_dict['groups']], + apikey=apikey) extras_after = updated_package['extras'] assert len(extras_after) == len(extras_before) - 1, ( "%s != %s" % (len(extras_after), len(extras_before) - 1)) @@ -545,9 +507,6 @@ def _delete_extra(self, package_dict, user): after['group activity streams'][group_dict['name']]) assert grp_new_activities == [activity] - self.check_dashboard(before, after, user_new_activities, - [user_id, package_dict['id']]) - # Check that the new activity has the right attributes. assert activity['object_id'] == updated_package['id'], \ str(activity['object_id']) @@ -585,11 +544,14 @@ def _delete_extra(self, package_dict, user): def _update_extra(self, package_dict, user): if user: user_id = user['id'] + apikey = user['apikey'] else: user_id = 'not logged in' + apikey=None before = self.record_details(user_id, package_dict['id'], - [group['name'] for group in package_dict['groups']]) + [group['name'] for group in package_dict['groups']], + apikey=apikey) extras_before = package_dict['extras'] assert len(extras_before) > 0, ( @@ -606,7 +568,8 @@ def _update_extra(self, package_dict, user): user['apikey']) after = self.record_details(user_id, package_dict['id'], - [group['name'] for group in package_dict['groups']]) + [group['name'] for group in package_dict['groups']], + apikey=apikey) extras_after = updated_package['extras'] assert len(extras_after) == len(extras_before), ( "%s != %s" % (len(extras_after), len(extras_before))) @@ -632,9 +595,6 @@ def _update_extra(self, package_dict, user): after['recently changed datasets stream']) \ == user_new_activities - self.check_dashboard(before, after, user_new_activities, - [user_id, package_dict['id']]) - # If the package has any groups, the same new activity should appear # in the activity stream of each group. for group_dict in package_dict['groups']: @@ -682,10 +642,14 @@ def _add_extra(self, package_dict, user, key=None): key = 'quality' if user: user_id = user['id'] + apikey = user['apikey'] else: user_id = 'not logged in' + apikey = None - before = self.record_details(user_id, package_dict['id']) + before = self.record_details(user_id, package_dict['id'], + [group['name'] for group in package_dict['groups']], + apikey=apikey) # Make a copy of the package's extras before we add a new extra, # so we can compare the extras before and after updating the package. @@ -697,7 +661,9 @@ def _add_extra(self, package_dict, user, key=None): updated_package = package_update(self.app, package_dict, user['apikey']) - after = self.record_details(user_id, package_dict['id']) + after = self.record_details(user_id, package_dict['id'], + [group['name'] for group in package_dict['groups']], + apikey=apikey) extras_after = updated_package['extras'] assert len(extras_after) == len(extras_before) + 1, ( "%s != %s" % (len(extras_after), len(extras_before) + 1)) @@ -731,9 +697,6 @@ def _add_extra(self, package_dict, user, key=None): after['group activity streams'][group_dict['name']]) assert grp_new_activities == [activity] - self.check_dashboard(before, after, user_new_activities, - [user_id, package_dict['id']]) - # Check that the new activity has the right attributes. assert activity['object_id'] == updated_package['id'], \ str(activity['object_id']) @@ -769,14 +732,16 @@ def _add_extra(self, package_dict, user, key=None): str(detail['activity_type'])) def _create_activity(self, user, package, params): - before = self.record_details(user['id'], package['id']) + before = self.record_details(user['id'], package['id'], + apikey=user['apikey']) response = self.app.post('/api/action/activity_create', params=json.dumps(params), extra_environ={'Authorization': str(self.sysadmin_user['apikey'])}) assert response.json['success'] is True - after = self.record_details(user['id'], package['id']) + after = self.record_details(user['id'], package['id'], + apikey=user['apikey']) # Find the new activity in the user's activity stream. user_new_activities = (find_new_activities( @@ -792,9 +757,6 @@ def _create_activity(self, user, package, params): after['package activity stream'])) assert pkg_new_activities == user_new_activities - self.check_dashboard(before, after, user_new_activities, - [user['id'], package['id']]) - # Check that the new activity has the right attributes. assert activity['object_id'] == params['object_id'], ( str(activity['object_id'])) @@ -840,8 +802,6 @@ def _delete_group(self, group, user): new_activities, ("The same activity should also " "appear in the group's activity stream.") - self.check_dashboard(before, after, new_activities, [user['id']]) - # Check that the new activity has the right attributes. assert activity['object_id'] == group['id'], str(activity['object_id']) assert activity['user_id'] == user['id'], str(activity['user_id']) @@ -862,13 +822,15 @@ def _update_group(self, group, user): item and detail are emitted. """ - before = self.record_details(user['id'], group_ids=[group['id']]) + before = self.record_details(user['id'], group_ids=[group['id']], + apikey=user['apikey']) # Update the group. group_dict = {'id': group['id'], 'title': 'edited'} group_update(self.app, group_dict, user['apikey']) - after = self.record_details(user['id'], group_ids=[group['id']]) + after = self.record_details(user['id'], group_ids=[group['id']], + apikey=user['apikey']) # Find the new activity. new_activities = find_new_activities(before['user activity stream'], @@ -883,8 +845,6 @@ def _update_group(self, group, user): new_activities, ("The same activity should also " "appear in the group's activity stream.") - self.check_dashboard(before, after, new_activities, [user['id']]) - # Check that the new activity has the right attributes. assert activity['object_id'] == group['id'], str(activity['object_id']) assert activity['user_id'] == user['id'], str(activity['user_id']) @@ -912,7 +872,8 @@ def _update_user(self, user): assert response_dict['success'] is True user_dict = response_dict['result'] - before = self.record_details(user_dict['id']) + before = self.record_details(user_dict['id'], + apikey=user_dict['apikey']) # Update the user. user_dict['about'] = 'edited' @@ -921,7 +882,8 @@ def _update_user(self, user): self.app.post('/api/action/user_update', json.dumps(user_dict), extra_environ={'Authorization': str(user['apikey'])}) - after = self.record_details(user_dict['id']) + after = self.record_details(user_dict['id'], + apikey=user_dict['apikey']) # Find the new activity. new_activities = find_new_activities(before['user activity stream'], @@ -930,8 +892,6 @@ def _update_user(self, user): "the user's activity stream, but found %i" % len(new_activities)) activity = new_activities[0] - self.check_dashboard(before, after, new_activities, [user_dict['id']]) - # Check that the new activity has the right attributes. assert activity['object_id'] == user_dict['id'], ( str(activity['object_id'])) @@ -954,7 +914,8 @@ def _delete_resources(self, package): """ before = self.record_details(self.normal_user['id'], package['id'], - [group['name'] for group in package['groups']]) + [group['name'] for group in package['groups']], + apikey=self.normal_user['apikey']) num_resources = len(package['resources']) assert num_resources > 0, \ @@ -965,7 +926,8 @@ def _delete_resources(self, package): package_update(self.app, package, self.normal_user['apikey']) after = self.record_details(self.normal_user['id'], package['id'], - [group['name'] for group in package['groups']]) + [group['name'] for group in package['groups']], + apikey=self.normal_user['apikey']) # Find the new activity in the user's activity stream. user_new_activities = (find_new_activities( @@ -996,9 +958,6 @@ def _delete_resources(self, package): after['group activity streams'][group_dict['name']]) assert grp_new_activities == [activity] - self.check_dashboard(before, after, user_new_activities, - [package['id']]) - # Check that the new activity has the right attributes. assert activity['object_id'] == package['id'], ( str(activity['object_id'])) @@ -1037,10 +996,12 @@ def _update_package(self, package, user): """ if user: user_id = user['id'] + apikey = user['apikey'] else: user_id = 'not logged in' + apikey = None - before = self.record_details(user_id, package['id']) + before = self.record_details(user_id, package['id'], apikey=apikey) # Update the package. if package['title'] != 'edited': @@ -1050,7 +1011,7 @@ def _update_package(self, package, user): package['title'] = 'edited again' package_update(self.app, package, user['apikey']) - after = self.record_details(user_id, package['id']) + after = self.record_details(user_id, package['id'], apikey=apikey) # Find the new activity in the user's activity stream. user_new_activities = (find_new_activities( @@ -1073,9 +1034,6 @@ def _update_package(self, package, user): after['recently changed datasets stream']) \ == user_new_activities - self.check_dashboard(before, after, user_new_activities, - [user_id, package['id']]) - # If the package has any groups, the same new activity should appear # in the activity stream of each group. for group_dict in package['groups']: @@ -1119,16 +1077,18 @@ def _update_resource(self, package, resource, user): """ if user: user_id = user['id'] + apikey = user['apikey'] else: user_id = 'not logged in' + apikey = None - before = self.record_details(user_id, package['id']) + before = self.record_details(user_id, package['id'], apikey=apikey) # Update the resource. resource['name'] = 'edited' package_update(self.app, package) - after = self.record_details(user_id, package['id']) + after = self.record_details(user_id, package['id'], apikey=apikey) # Find the new activity in the user's activity stream. user_new_activities = (find_new_activities( @@ -1151,9 +1111,6 @@ def _update_resource(self, package, resource, user): after['recently changed datasets stream']) \ == user_new_activities - self.check_dashboard(before, after, user_new_activities, - [user_id, package['id']]) - # If the package has any groups, the same new activity should appear # in the activity stream of each group. for group_dict in package['groups']: @@ -1230,9 +1187,6 @@ def _delete_package(self, package): after['recently changed datasets stream']) \ == user_new_activities - self.check_dashboard(before, after, user_new_activities, - [self.sysadmin_user['id'], package['id']]) - # If the package has any groups, the same new activity should appear # in the activity stream of each group. for group_dict in package['groups']: @@ -1316,14 +1270,16 @@ def test_01_remove_tag(self): assert len(pkg_dict['tags']) >= 1, ("The package has to have at least" " one tag to test removing a tag.") before = self.record_details(user['id'], pkg_dict['id'], - [group['name'] for group in pkg_dict['groups']]) + [group['name'] for group in pkg_dict['groups']], + apikey=user['apikey']) data_dict = { 'id': pkg_dict['id'], 'tags': pkg_dict['tags'][0:-1], } package_update(self.app, data_dict, user['apikey']) after = self.record_details(user['id'], pkg_dict['id'], - [group['name'] for group in pkg_dict['groups']]) + [group['name'] for group in pkg_dict['groups']], + apikey=user['apikey']) # Find the new activity in the user's activity stream. user_new_activities = (find_new_activities( @@ -1346,9 +1302,6 @@ def test_01_remove_tag(self): after['recently changed datasets stream']) \ == user_new_activities - self.check_dashboard(before, after, user_new_activities, - [user['id'], pkg_dict['id']]) - # If the package has any groups, the same new activity should appear # in the activity stream of each group. for group_dict in pkg_dict['groups']: @@ -1491,7 +1444,8 @@ def test_create_user(self): assert response_dict['success'] is True user_created = response_dict['result'] - after = self.record_details(user_created['id']) + after = self.record_details(user_created['id'], + apikey=user_created['apikey']) user_activities = after['user activity stream'] assert len(user_activities) == 1, ("There should be 1 activity in " @@ -1533,7 +1487,7 @@ def test_create_group(self): user = self.normal_user - before = self.record_details(user['id']) + before = self.record_details(user['id'], apikey=user['apikey']) # Create a new group. request_data = {'name': 'a-new-group', 'title': 'A New Group'} @@ -1545,7 +1499,7 @@ def test_create_group(self): group_created = response_dict['result'] after = self.record_details(user['id'], - group_ids=[group_created['id']]) + group_ids=[group_created['id']], apikey=user['apikey']) # Find the new activity. new_activities = find_new_activities(before['user activity stream'], @@ -1558,8 +1512,6 @@ def test_create_group(self): new_activities, ("The same activity should also appear in " "the group's activity stream.") - self.check_dashboard(before, after, new_activities, [user['id']]) - # Check that the new activity has the right attributes. assert activity['object_id'] == group_created['id'], \ str(activity['object_id']) @@ -1601,13 +1553,15 @@ def test_add_tag(self): pkg_dict = package_show(self.app, {'id': pkg_name}) # Add one new tag to the package. - before = self.record_details(user['id'], pkg_dict['id']) + before = self.record_details(user['id'], pkg_dict['id'], + apikey=user['apikey']) new_tag_name = 'test tag' assert new_tag_name not in [tag['name'] for tag in pkg_dict['tags']] pkg_dict['tags'].append({'name': new_tag_name}) package_update(self.app, pkg_dict, user['apikey']) - after = self.record_details(user['id'], pkg_dict['id']) + after = self.record_details(user['id'], pkg_dict['id'], + apikey=user['apikey']) # Find the new activity in the user's activity stream. user_new_activities = (find_new_activities( @@ -1630,9 +1584,6 @@ def test_add_tag(self): after['recently changed datasets stream']) \ == user_new_activities - self.check_dashboard(before, after, user_new_activities, - [user['id'], pkg_dict['id']]) - # If the package has any groups, the same new activity should appear # in the activity stream of each group. for group_dict in pkg_dict['groups']: @@ -2033,7 +1984,8 @@ def test_delete_extras(self): def test_follow_dataset(self): user = self.normal_user - before = self.record_details(user['id']) + before = self.record_details(user['id'], self.warandpeace['id'], + apikey=user['apikey']) data = {'id': self.warandpeace['id']} extra_environ = {'Authorization': str(user['apikey'])} response = self.app.post('/api/action/follow_dataset', @@ -2041,7 +1993,8 @@ def test_follow_dataset(self): response_dict = json.loads(response.body) assert response_dict['success'] is True - after = self.record_details(user['id'], self.warandpeace['id']) + after = self.record_details(user['id'], self.warandpeace['id'], + apikey=user['apikey']) # Find the new activity in the user's activity stream. user_new_activities = (find_new_activities( @@ -2056,8 +2009,6 @@ def test_follow_dataset(self): for activity in user_new_activities: assert activity in pkg_new_activities - self.check_dashboard(before, after, user_new_activities, [user['id']]) - # Check that the new activity has the right attributes. assert activity['object_id'] == self.warandpeace['id'], \ str(activity['object_id']) @@ -2077,8 +2028,9 @@ def test_follow_dataset(self): def test_follow_user(self): user = self.normal_user - before = self.record_details(user['id']) - followee_before = self.record_details(self.sysadmin_user['id']) + before = self.record_details(user['id'], apikey=user['apikey']) + followee_before = self.record_details(self.sysadmin_user['id'], + apikey=self.sysadmin_user['apikey']) data = {'id': self.sysadmin_user['id']} extra_environ = {'Authorization': str(user['apikey'])} response = self.app.post('/api/action/follow_user', @@ -2086,8 +2038,9 @@ def test_follow_user(self): response_dict = json.loads(response.body) assert response_dict['success'] is True - after = self.record_details(user['id']) - followee_after = self.record_details(self.sysadmin_user['id']) + after = self.record_details(user['id'], apikey=user['apikey']) + followee_after = self.record_details(self.sysadmin_user['id'], + apikey=self.sysadmin_user['apikey']) # Find the new activity in the user's activity stream. user_new_activities = (find_new_activities( @@ -2097,26 +2050,6 @@ def test_follow_user(self): len(user_new_activities)) activity = user_new_activities[0] - # Check that the new activity appears in the user's private activity - # stream. - user_new_activities = (find_new_activities( - before['follower dashboard activity stream'], - after['follower dashboard activity stream'])) - assert len(user_new_activities) == 1, ("There should be 1 new " - " activity in the user's activity stream, but found %i" % - len(user_new_activities)) - assert user_new_activities[0] == activity - - # Check that the new activity appears in the followee's private - # activity stream. - followee_new_activities = (find_new_activities( - followee_before['follower dashboard activity stream'], - followee_after['follower dashboard activity stream'])) - assert len(followee_new_activities) == 1, ("There should be 1 new " - " activity in the user's activity stream, but found %i" % - len(followee_new_activities)) - assert followee_new_activities[0] == activity - # Check that the new activity has the right attributes. assert activity['object_id'] == self.sysadmin_user['id'], \ str(activity['object_id']) diff --git a/ckan/tests/functional/api/test_dashboard.py b/ckan/tests/functional/api/test_dashboard.py new file mode 100644 index 00000000000..1b285a222c3 --- /dev/null +++ b/ckan/tests/functional/api/test_dashboard.py @@ -0,0 +1,333 @@ +'''Test for the dashboard API. + +This module tests the various functions of the user dashboard, such as the +contents of the dashboard activity stream and reporting the number of new +activities. + +''' +import ckan +from ckan.lib.helpers import json +import paste +import pylons.test + + +class TestDashboard(object): + '''Tests for the logic action functions related to the user's dashboard.''' + + @classmethod + def user_create(cls): + '''Create a new user.''' + params = json.dumps({ + 'name': 'mr_new_user', + 'email': 'mr@newuser.com', + 'password': 'iammrnew', + }) + response = cls.app.post('/api/action/user_create', params=params, + extra_environ={'Authorization': str(cls.joeadmin['apikey'])}) + assert response.json['success'] is True + new_user = response.json['result'] + return new_user + + @classmethod + def setup_class(cls): + ckan.tests.CreateTestData.create() + cls.app = paste.fixture.TestApp(pylons.test.pylonsapp) + joeadmin = ckan.model.User.get('joeadmin') + cls.joeadmin = { + 'id': joeadmin.id, + 'apikey': joeadmin.apikey + } + annafan = ckan.model.User.get('annafan') + cls.annafan = { + 'id': annafan.id, + 'apikey': annafan.apikey + } + testsysadmin = ckan.model.User.get('testsysadmin') + cls.testsysadmin = { + 'id': testsysadmin.id, + 'apikey': testsysadmin.apikey + } + cls.new_user = cls.user_create() + + @classmethod + 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.''' + 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. + + ''' + 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 + that are currently marked as new.''' + activity_list = self.dashboard_activity_list(user) + return [activity for activity in activity_list if activity['is_new']] + + def dashboard_mark_all_new_activities_as_old(self, user): + 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_dashboard_activity_list_for_new_user(self): + '''Test the contents of a new user's dashboard activity stream.''' + activities = self.dashboard_activity_list(self.new_user) + # We expect to find a single 'new user' activity. + assert len(activities) == 1 + activity = activities[0] + assert activity['activity_type'] == 'new user' + assert activity['user_id'] == activity['object_id'] + assert activity['user_id'] == self.new_user['id'] + + def test_01_new_activities_count_for_new_user(self): + '''Test that a newly registered user's new activities count is 0.''' + assert self.dashboard_new_activities_count(self.new_user) == 0 + + def test_01_new_activities_for_new_user(self): + '''Test that a newly registered user has no activities marked as new + in their dashboard activity stream.''' + assert len(self.dashboard_new_activities(self.new_user)) == 0 + + def test_02_own_activities_do_not_count_as_new(self): + '''Make a user do some activities and check that her own activities + don't increase her new activities count.''' + + # The user has to view her dashboard activity stream first to mark any + # existing activities as read. For example when she follows a dataset + # below, past activities from the dataset (e.g. when someone created + # the dataset, etc.) will appear in her dashboard, and if she has never + # viewed her dashboard then those activities will be considered + # "unseen". + # We would have to do this if, when you follow something, you only get + # the activities from that object since you started following it, and + # not all its past activities as well. + self.dashboard_mark_all_new_activities_as_old(self.new_user) + + # Create a new dataset. + params = json.dumps({ + 'name': 'my_new_package', + }) + response = self.app.post('/api/action/package_create', params=params, + extra_environ={'Authorization': str(self.new_user['apikey'])}) + assert response.json['success'] is True + + # Follow a dataset. + params = json.dumps({'id': 'warandpeace'}) + response = self.app.post('/api/action/follow_dataset', params=params, + extra_environ={'Authorization': str(self.new_user['apikey'])}) + assert response.json['success'] is True + + # Follow a user. + params = json.dumps({'id': 'annafan'}) + response = self.app.post('/api/action/follow_user', params=params, + extra_environ={'Authorization': str(self.new_user['apikey'])}) + assert response.json['success'] is True + + # Follow a group. + params = json.dumps({'id': 'roger'}) + response = self.app.post('/api/action/follow_group', params=params, + extra_environ={'Authorization': str(self.new_user['apikey'])}) + assert response.json['success'] is True + + # Update the dataset that we're following. + params = json.dumps({'name': 'warandpeace', 'notes': 'updated'}) + response = self.app.post('/api/action/package_update', params=params, + extra_environ={'Authorization': str(self.new_user['apikey'])}) + assert response.json['success'] is True + + # User's own actions should not increase her activity count. + assert self.dashboard_new_activities_count(self.new_user) == 0 + + def test_03_dashboard_activity_list_own_activities(self): + '''Test that a user's own activities appear in her dashboard.''' + activities = self.dashboard_activity_list(self.new_user) + + # FIXME: There should actually be 6 activities here, but when you + # follow something it's old activities (from before you followed it) + # appear in your activity stream. So here we get more activities than + # expected. + assert len(activities) == 8 + + assert activities[0]['activity_type'] == 'changed package' + assert activities[1]['activity_type'] == 'follow group' + assert activities[2]['activity_type'] == 'follow user' + assert activities[3]['activity_type'] == 'follow dataset' + assert activities[4]['activity_type'] == 'new package' + assert activities[5]['activity_type'] == 'new user' + + # FIXME: Shouldn't need the [:6] here, it's because when you follow + # something its old activities (from before you started following it) + # appear in your dashboard. + for activity in activities[:6]: + assert activity['user_id'] == self.new_user['id'] + + def test_03_own_activities_not_marked_as_new(self): + '''Make a user do some activities and check that her own activities + aren't marked as new in her dashboard activity stream.''' + assert len(self.dashboard_new_activities(self.new_user)) == 0 + + def test_04_activities_from_followed_datasets(self): + '''Activities from followed datasets should show in dashboard.''' + + activities_before = self.dashboard_activity_list(self.new_user) + + # Make someone else who new_user is not following update a dataset that + # new_user is following. + params = json.dumps({'name': 'warandpeace', 'notes': 'updated again'}) + response = self.app.post('/api/action/package_update', params=params, + extra_environ={'Authorization': str(self.joeadmin['apikey'])}) + assert response.json['success'] is True + + # Check the new activity in new_user's dashboard. + activities = self.dashboard_activity_list(self.new_user) + new_activities = [activity for activity in activities + if activity not in activities_before] + assert len(new_activities) == 1 + activity = new_activities[0] + assert activity['activity_type'] == 'changed package' + assert activity['user_id'] == self.joeadmin['id'] + assert activity['data']['package']['name'] == 'warandpeace' + + def test_04_activities_from_followed_users(self): + '''Activities from followed users should show in the dashboard.''' + + activities_before = self.dashboard_activity_list(self.new_user) + + # Make someone that the user is following create a new dataset. + params = json.dumps({'name': 'annas_new_dataset'}) + response = self.app.post('/api/action/package_create', params=params, + extra_environ={'Authorization': str(self.annafan['apikey'])}) + assert response.json['success'] is True + + # Check the new activity in new_user's dashboard. + activities = self.dashboard_activity_list(self.new_user) + new_activities = [activity for activity in activities + if activity not in activities_before] + assert len(new_activities) == 1 + activity = new_activities[0] + assert activity['activity_type'] == 'new package' + assert activity['user_id'] == self.annafan['id'] + assert activity['data']['package']['name'] == 'annas_new_dataset' + + def test_04_activities_from_followed_groups(self): + '''Activities from followed groups should show in the dashboard.''' + + activities_before = self.dashboard_activity_list(self.new_user) + + # Make someone that the user is not following update a group that the + # user is following. + params = json.dumps({'id': 'roger', 'description': 'updated'}) + response = self.app.post('/api/action/group_update', params=params, + extra_environ={'Authorization': str(self.testsysadmin['apikey'])}) + assert response.json['success'] is True + + # Check the new activity in new_user's dashboard. + activities = self.dashboard_activity_list(self.new_user) + new_activities = [activity for activity in activities + if activity not in activities_before] + assert len(new_activities) == 1 + activity = new_activities[0] + assert activity['activity_type'] == 'changed group' + assert activity['user_id'] == self.testsysadmin['id'] + assert activity['data']['group']['name'] == 'roger' + + def test_04_activities_from_datasets_of_followed_groups(self): + '''Activities from datasets of followed groups should show in the + dashboard. + + ''' + activities_before = self.dashboard_activity_list(self.new_user) + + # Make someone that the user is not following update a dataset that the + # user is not following either, but that belongs to a group that the + # user is following. + params = json.dumps({'name': 'annakarenina', 'notes': 'updated'}) + response = self.app.post('/api/action/package_update', params=params, + extra_environ={'Authorization': str(self.testsysadmin['apikey'])}) + assert response.json['success'] is True + + # Check the new activity in new_user's dashboard. + activities = self.dashboard_activity_list(self.new_user) + new_activities = [activity for activity in activities + if activity not in activities_before] + assert len(new_activities) == 1 + activity = new_activities[0] + assert activity['activity_type'] == 'changed package' + assert activity['user_id'] == self.testsysadmin['id'] + assert activity['data']['package']['name'] == 'annakarenina' + + def test_05_new_activities_count(self): + '''Test that new activities from objects that a user follows increase + her new activities count.''' + assert self.dashboard_new_activities_count(self.new_user) == 4 + + def test_06_activities_marked_as_new(self): + '''Test that new activities from objects that a user follows are + marked as new in her dashboard activity stream.''' + assert len(self.dashboard_new_activities(self.new_user)) == 4 + + def test_07_mark_new_activities_as_read(self): + '''Test that a user's new activities are marked as old when she views + her dashboard activity stream.''' + assert self.dashboard_new_activities_count(self.new_user) > 0 + assert len(self.dashboard_new_activities(self.new_user)) > 0 + self.dashboard_mark_all_new_activities_as_old(self.new_user) + assert self.dashboard_new_activities_count(self.new_user) == 0 + assert len(self.dashboard_new_activities(self.new_user)) == 0 + + def test_08_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): + notes = "Updated {n} times".format(n=n) + params = json.dumps({'name': 'warandpeace', 'notes': notes}) + 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 + + def test_09_activities_that_should_not_show(self): + '''Test that other activities do not appear on the user's dashboard.''' + + before = self.dashboard_activity_list(self.new_user) + + # Make someone else who new_user is not following create a new dataset. + params = json.dumps({'name': 'irrelevant_dataset'}) + response = self.app.post('/api/action/package_create', params=params, + extra_environ={'Authorization': str(self.testsysadmin['apikey'])}) + assert response.json['success'] is True + + after = self.dashboard_activity_list(self.new_user) + + assert before == after diff --git a/ckan/tests/functional/api/test_follow.py b/ckan/tests/functional/api/test_follow.py index ab0df832f48..90bc4a23101 100644 --- a/ckan/tests/functional/api/test_follow.py +++ b/ckan/tests/functional/api/test_follow.py @@ -1,9 +1,21 @@ +'''Test for the follower API. + +This module tests following, unfollowing, getting a list of what you're +following or the number of things you're following, getting a list of who's +following you or the number of followers you have, testing whether or not +you're following something, etc. + +This module _does not_ test the user dashboard activity stream (which shows +activities from everything you're following), that is tested in +test_dashboard.py. + +''' import datetime import paste import pylons.test import ckan -from ckan.lib.helpers import json from ckan.tests import are_foreign_keys_supported, SkipTest +import ckan.tests def datetime_from_string(s): '''Return a standard datetime.datetime object initialised from a string in @@ -24,83 +36,54 @@ def follow_user(app, follower_id, apikey, object_id, object_arg): ''' # Record the object's followers count before. - params = json.dumps({'id': object_id}) - response = app.post('/api/action/user_follower_count', - params=params).json - assert response['success'] is True - follower_count_before = response['result'] + follower_count_before = ckan.tests.call_action_api(app, + 'user_follower_count', id=object_id) # Record the follower's followees count before. - params = json.dumps({'id': follower_id}) - response = app.post('/api/action/user_followee_count', - params=params).json - assert response['success'] is True - followee_count_before = response['result'] + followee_count_before = ckan.tests.call_action_api(app, + 'user_followee_count', id=follower_id) # Check that the user is not already following the object. - params = json.dumps({'id': object_id}) - extra_environ = {'Authorization': str(apikey)} - response = app.post('/api/action/am_following_user', - params=params, extra_environ=extra_environ).json - assert response['success'] is True - assert response['result'] is False + result = ckan.tests.call_action_api(app, 'am_following_user', + id=object_id, apikey=apikey) + assert result is False # Make the user start following the object. before = datetime.datetime.now() - params = {'id': object_arg} - extra_environ = {'Authorization': str(apikey)} - response = app.post('/api/action/follow_user', - params=json.dumps(params), extra_environ=extra_environ).json + follower = ckan.tests.call_action_api(app, 'follow_user', id=object_arg, + apikey=apikey) after = datetime.datetime.now() - assert response['success'] is True - assert response['result'] - follower = response['result'] assert follower['follower_id'] == follower_id assert follower['object_id'] == object_id timestamp = datetime_from_string(follower['datetime']) assert (timestamp >= before and timestamp <= after), str(timestamp) # Check that am_following_user now returns True. - params = json.dumps({'id': object_id}) - extra_environ = {'Authorization': str(apikey)} - response = app.post('/api/action/am_following_user', - params=params, extra_environ=extra_environ).json - assert response['success'] is True - assert response['result'] is True + result = ckan.tests.call_action_api(app, 'am_following_user', + id=object_id, apikey=apikey) + assert result is True # Check that the follower appears in the object's list of followers. - params = json.dumps({'id': object_id}) - response = app.post('/api/action/user_follower_list', - params=params).json - assert response['success'] is True - assert response['result'] - followers = response['result'] + followers = ckan.tests.call_action_api(app, 'user_follower_list', + id=object_id) assert len(followers) == follower_count_before + 1 assert len([follower for follower in followers if follower['id'] == follower_id]) == 1 # Check that the object appears in the follower's list of followees. - params = json.dumps({'id': follower_id}) - response = app.post('/api/action/user_followee_list', - params=params).json - assert response['success'] is True - assert response['result'] - followees = response['result'] + followees = ckan.tests.call_action_api(app, 'user_followee_list', + id=follower_id) assert len(followees) == followee_count_before + 1 assert len([followee for followee in followees if followee['id'] == object_id]) == 1 # Check that the object's follower count has increased by 1. - params = json.dumps({'id': object_id}) - response = app.post('/api/action/user_follower_count', - params=params).json - assert response['success'] is True - assert response['result'] == follower_count_before + 1 + follower_count_after = ckan.tests.call_action_api(app, + 'user_follower_count', id=object_id) + assert follower_count_after == follower_count_before + 1 # Check that the follower's followee count has increased by 1. - params = json.dumps({'id': follower_id}) - response = app.post('/api/action/user_followee_count', - params=params).json - assert response['success'] is True - assert response['result'] == followee_count_before + 1 + followee_count_after = ckan.tests.call_action_api(app, + 'user_followee_count', id=follower_id) + assert followee_count_after == followee_count_before + 1 def follow_dataset(app, follower_id, apikey, dataset_id, dataset_arg): '''Test a user starting to follow a dataset via the API. @@ -113,83 +96,54 @@ def follow_dataset(app, follower_id, apikey, dataset_id, dataset_arg): ''' # Record the dataset's followers count before. - params = json.dumps({'id': dataset_id}) - response = app.post('/api/action/dataset_follower_count', - params=params).json - assert response['success'] is True - follower_count_before = response['result'] + follower_count_before = ckan.tests.call_action_api(app, + 'dataset_follower_count', id=dataset_id) # Record the follower's followees count before. - params = json.dumps({'id': follower_id}) - response = app.post('/api/action/dataset_followee_count', - params=params).json - assert response['success'] is True - followee_count_before = response['result'] + followee_count_before = ckan.tests.call_action_api(app, + 'dataset_followee_count', id=follower_id) # Check that the user is not already following the dataset. - params = json.dumps({'id': dataset_id}) - extra_environ = {'Authorization': str(apikey)} - response = app.post('/api/action/am_following_dataset', - params=params, extra_environ=extra_environ).json - assert response['success'] is True - assert response['result'] is False + result = ckan.tests.call_action_api(app, 'am_following_dataset', + id=dataset_id, apikey=apikey) + assert result is False # Make the user start following the dataset. before = datetime.datetime.now() - params = {'id': dataset_arg} - extra_environ = {'Authorization': str(apikey)} - response = app.post('/api/action/follow_dataset', - params=json.dumps(params), extra_environ=extra_environ).json + follower = ckan.tests.call_action_api(app, 'follow_dataset', + id=dataset_arg, apikey=apikey) after = datetime.datetime.now() - assert response['success'] is True - assert response['result'] - follower = response['result'] assert follower['follower_id'] == follower_id assert follower['object_id'] == dataset_id timestamp = datetime_from_string(follower['datetime']) assert (timestamp >= before and timestamp <= after), str(timestamp) # Check that am_following_dataset now returns True. - params = json.dumps({'id': dataset_id}) - extra_environ = {'Authorization': str(apikey)} - response = app.post('/api/action/am_following_dataset', - params=params, extra_environ=extra_environ).json - assert response['success'] is True - assert response['result'] is True + result = ckan.tests.call_action_api(app, 'am_following_dataset', + id=dataset_id, apikey=apikey) + assert result is True # Check that the follower appears in the dataset's list of followers. - params = json.dumps({'id': dataset_id}) - response = app.post('/api/action/dataset_follower_list', - params=params).json - assert response['success'] is True - assert response['result'] - followers = response['result'] + followers = ckan.tests.call_action_api(app, 'dataset_follower_list', + id=dataset_id) assert len(followers) == follower_count_before + 1 assert len([follower for follower in followers if follower['id'] == follower_id]) == 1 # Check that the dataset appears in the follower's list of followees. - params = json.dumps({'id': follower_id}) - response = app.post('/api/action/dataset_followee_list', - params=params).json - assert response['success'] is True - assert response['result'] - followees = response['result'] + followees = ckan.tests.call_action_api(app, 'dataset_followee_list', + id=follower_id) assert len(followees) == followee_count_before + 1 assert len([followee for followee in followees if followee['id'] == dataset_id]) == 1 # Check that the dataset's follower count has increased by 1. - params = json.dumps({'id': dataset_id}) - response = app.post('/api/action/dataset_follower_count', - params=params).json - assert response['success'] is True - assert response['result'] == follower_count_before + 1 + follower_count_after = ckan.tests.call_action_api(app, + 'dataset_follower_count', id=dataset_id) + assert follower_count_after == follower_count_before + 1 # Check that the follower's followee count has increased by 1. - params = json.dumps({'id': follower_id}) - response = app.post('/api/action/dataset_followee_count', - params=params).json - assert response['success'] is True - assert response['result'] == followee_count_before + 1 + followee_count_after = ckan.tests.call_action_api(app, + 'dataset_followee_count', id=follower_id) + assert followee_count_after == followee_count_before + 1 def follow_group(app, user_id, apikey, group_id, group_arg): '''Test a user starting to follow a group via the API. @@ -202,85 +156,56 @@ def follow_group(app, user_id, apikey, group_id, group_arg): ''' # Record the group's followers count before. - params = json.dumps({'id': group_id}) - response = app.post('/api/action/group_follower_count', - params=params).json - assert response['success'] is True - follower_count_before = response['result'] + follower_count_before = ckan.tests.call_action_api(app, + 'group_follower_count', id=group_id) # Record the user's followees count before. - params = json.dumps({'id': user_id}) - response = app.post('/api/action/group_followee_count', - params=params).json - assert response['success'] is True - followee_count_before = response['result'] + followee_count_before = ckan.tests.call_action_api(app, + 'group_followee_count', id=user_id) # Check that the user is not already following the group. - params = json.dumps({'id': group_id}) - extra_environ = {'Authorization': str(apikey)} - response = app.post('/api/action/am_following_group', - params=params, extra_environ=extra_environ).json - assert response['success'] is True - assert response['result'] is False + result = ckan.tests.call_action_api(app, 'am_following_group', + id=group_id, apikey=apikey) + assert result is False # Make the user start following the group. before = datetime.datetime.now() - params = {'id': group_id} - extra_environ = {'Authorization': str(apikey)} - response = app.post('/api/action/follow_group', - params=json.dumps(params), extra_environ=extra_environ).json + follower = ckan.tests.call_action_api(app, 'follow_group', id=group_id, + apikey=apikey) after = datetime.datetime.now() - assert response['success'] is True - assert response['result'] - follower = response['result'] assert follower['follower_id'] == user_id assert follower['object_id'] == group_id timestamp = datetime_from_string(follower['datetime']) assert (timestamp >= before and timestamp <= after), str(timestamp) # Check that am_following_group now returns True. - params = json.dumps({'id': group_id}) - extra_environ = {'Authorization': str(apikey)} - response = app.post('/api/action/am_following_group', - params=params, extra_environ=extra_environ).json - assert response['success'] is True - assert response['result'] is True + result = ckan.tests.call_action_api(app, 'am_following_group', + id=group_id, apikey=apikey) + assert result is True # Check that the user appears in the group's list of followers. - params = json.dumps({'id': group_id}) - response = app.post('/api/action/group_follower_list', - params=params).json - assert response['success'] is True - assert response['result'] - followers = response['result'] + followers = ckan.tests.call_action_api(app, 'group_follower_list', + id=group_id) assert len(followers) == follower_count_before + 1 assert len([follower for follower in followers if follower['id'] == user_id]) == 1 # Check that the group appears in the user's list of followees. - params = json.dumps({'id': user_id}) - response = app.post('/api/action/group_followee_list', - params=params).json - assert response['success'] is True - assert response['result'] - followees = response['result'] + followees = ckan.tests.call_action_api(app, 'group_followee_list', + id=user_id) assert len(followees) == followee_count_before + 1 assert len([followee for followee in followees if followee['id'] == group_id]) == 1 # Check that the group's follower count has increased by 1. - params = json.dumps({'id': group_id}) - response = app.post('/api/action/group_follower_count', - params=params).json - assert response['success'] is True - assert response['result'] == follower_count_before + 1 + follower_count_after = ckan.tests.call_action_api(app, + 'group_follower_count', id=group_id) + assert follower_count_after == follower_count_before + 1 # Check that the user's followee count has increased by 1. - params = json.dumps({'id': user_id}) - response = app.post('/api/action/group_followee_count', - params=params).json - assert response['success'] is True - assert response['result'] == followee_count_before + 1 + followee_count_after = ckan.tests.call_action_api(app, + 'group_followee_count', id=user_id) + assert followee_count_after == followee_count_before + 1 class TestFollow(object): @@ -333,82 +258,61 @@ def teardown_class(self): def test_01_user_follow_user_bad_apikey(self): for apikey in ('bad api key', '', ' ', 'None', '3', '35.7', 'xxx'): - params = json.dumps({'id': self.russianfan['id']}) - extra_environ = {'Authorization': apikey} - response = self.app.post('/api/action/follow_user', - params=params, extra_environ=extra_environ, status=403).json - assert response['success'] is False - assert response['error']['message'] == 'Access denied' + error = ckan.tests.call_action_api(self.app, 'follow_user', + id=self.russianfan['id'], apikey=apikey, + status=403) + assert error['message'] == 'Access denied' def test_01_user_follow_dataset_bad_apikey(self): for apikey in ('bad api key', '', ' ', 'None', '3', '35.7', 'xxx'): - params = json.dumps({'id': self.warandpeace['id']}) - extra_environ = {'Authorization': apikey} - response = self.app.post('/api/action/follow_dataset', - params=params, extra_environ=extra_environ, status=403).json - assert response['success'] is False - assert response['error']['message'] == 'Access denied' + error = ckan.tests.call_action_api(self.app, 'follow_dataset', + id=self.warandpeace['id'], apikey=apikey, + status=403) + assert error['message'] == 'Access denied' def test_01_user_follow_group_bad_apikey(self): for apikey in ('bad api key', '', ' ', 'None', '3', '35.7', 'xxx'): - params = json.dumps({'id': self.rogers_group['id']}) - extra_environ = {'Authorization': apikey} - response = self.app.post('/api/action/follow_group', - params=params, extra_environ=extra_environ, status=403).json - assert response['success'] is False - assert response['error']['message'] == 'Access denied' + error = ckan.tests.call_action_api(self.app, 'follow_group', + id=self.rogers_group['id'], apikey=apikey, + status=403) + assert error['message'] == 'Access denied' def test_01_user_follow_user_missing_apikey(self): - params = json.dumps({'id': self.russianfan['id']}) - response = self.app.post('/api/action/follow_user', - params=params, status=403).json - assert response['success'] is False - assert response['error']['message'] == 'Access denied' + error = ckan.tests.call_action_api(self.app, 'follow_user', + id=self.russianfan['id'], status=403) + assert error['message'] == 'Access denied' def test_01_user_follow_dataset_missing_apikey(self): - params = json.dumps({'id': self.warandpeace['id']}) - response = self.app.post('/api/action/follow_dataset', - params=params, status=403).json - assert response['success'] is False - assert response['error']['message'] == 'Access denied' + error = ckan.tests.call_action_api(self.app, 'follow_dataset', + id=self.warandpeace['id'], status=403) + assert error['message'] == 'Access denied' def test_01_user_follow_group_missing_apikey(self): - params = json.dumps({'id': self.rogers_group['id']}) - response = self.app.post('/api/action/follow_group', - params=params, status=403).json - assert response['success'] is False - assert response['error']['message'] == 'Access denied' + error = ckan.tests.call_action_api(self.app, 'follow_group', + id=self.rogers_group['id'], status=403) + assert error['message'] == 'Access denied' def test_01_follow_bad_object_id(self): for action in ('follow_user', 'follow_dataset', 'follow_group'): for object_id in ('bad id', ' ', 3, 35.7, 'xxx'): - params = json.dumps({'id': object_id}) - extra_environ = {'Authorization': str(self.annafan['apikey'])} - response = self.app.post('/api/action/{0}'.format(action), - params=params, extra_environ=extra_environ, - status=409).json - assert response['success'] is False - assert response['error']['id'][0].startswith('Not found') + error = ckan.tests.call_action_api(self.app, action, + id=object_id, + apikey=self.annafan['apikey'], status=409) + assert error['id'][0].startswith('Not found') def test_01_follow_empty_object_id(self): for action in ('follow_user', 'follow_dataset', 'follow_group'): for object_id in ('', None): - params = json.dumps({'id': object_id}) - extra_environ = {'Authorization': str(self.annafan['apikey'])} - response = self.app.post('/api/action/{0}'.format(action), - params=params, extra_environ=extra_environ, - status=409).json - assert response['success'] is False - assert response['error']['id'] == ['Missing value'] + error = ckan.tests.call_action_api(self.app, action, + id=object_id, + apikey=self.annafan['apikey'], status=409) + assert error['id'] == ['Missing value'] def test_01_follow_missing_object_id(self): for action in ('follow_user', 'follow_dataset', 'follow_group'): - params = json.dumps({}) - extra_environ = {'Authorization': str(self.annafan['apikey'])} - response = self.app.post('/api/action/{0}'.format(action), - params=params, extra_environ=extra_environ, status=409).json - assert response['success'] is False - assert response['error']['id'] == ['Missing value'] + error = ckan.tests.call_action_api(self.app, action, + apikey=self.annafan['apikey'], status=409) + assert error['id'] == ['Missing value'] def test_02_user_follow_user_by_id(self): follow_user(self.app, self.annafan['id'], self.annafan['apikey'], @@ -437,204 +341,142 @@ def test_02_user_follow_group_by_name(self): def test_03_user_follow_user_already_following(self): for object_id in (self.russianfan['id'], self.russianfan['name'], self.testsysadmin['id'], self.testsysadmin['name']): - params = json.dumps({'id': object_id}) - extra_environ = { - 'Authorization': str(self.annafan['apikey']), - } - response = self.app.post('/api/action/follow_user', - params=params, extra_environ=extra_environ, status=409).json - assert response['success'] == False - assert response['error']['message'].startswith( - 'You are already following ') + error = ckan.tests.call_action_api(self.app, 'follow_user', + id=object_id, apikey=self.annafan['apikey'], + status=409) + assert error['message'].startswith('You are already following ') def test_03_user_follow_dataset_already_following(self): for object_id in (self.warandpeace['id'], self.warandpeace['name']): - params = json.dumps({'id': object_id}) - extra_environ = { - 'Authorization': str(self.annafan['apikey']), - } - response = self.app.post('/api/action/follow_dataset', - params=params, extra_environ=extra_environ, status=409).json - assert response['success'] == False - assert response['error']['message'].startswith( - 'You are already following ') + error = ckan.tests.call_action_api(self.app, 'follow_dataset', + id=object_id, apikey=self.annafan['apikey'], + status=409) + assert error['message'].startswith('You are already following ') def test_03_user_follow_group_already_following(self): for group_id in (self.rogers_group['id'], self.rogers_group['name']): - params = json.dumps({'id': group_id}) - extra_environ = { - 'Authorization': str(self.annafan['apikey']), - } - response = self.app.post('/api/action/follow_group', - params=params, extra_environ=extra_environ, status=409).json - assert response['success'] is False - assert response['error']['message'].startswith( - 'You are already following ') + error = ckan.tests.call_action_api(self.app, 'follow_group', + id=group_id, apikey=self.annafan['apikey'], + status=409) + assert error['message'].startswith('You are already following ') def test_03_user_cannot_follow_herself(self): - params = json.dumps({'id': self.annafan['id']}) - extra_environ = { - 'Authorization': str(self.annafan['apikey']), - } - response = self.app.post('/api/action/follow_user', - params=params, extra_environ=extra_environ, status=409).json - assert response['success'] is False - assert response['error']['message'] == 'You cannot follow yourself' + error = ckan.tests.call_action_api(self.app, 'follow_user', + apikey=self.annafan['apikey'], status=409, + id=self.annafan['id']) + assert error['message'] == 'You cannot follow yourself' def test_04_follower_count_bad_id(self): for action in ('user_follower_count', 'dataset_follower_count', 'group_follower_count'): for object_id in ('bad id', ' ', 3, 35.7, 'xxx', ''): - params = json.dumps({'id': object_id}) - response = self.app.post('/api/action/{0}'.format(action), - params=params, status=409).json - assert response['success'] is False - assert 'id' in response['error'] + error = ckan.tests.call_action_api(self.app, action, + status=409, id=object_id) + assert 'id' in error def test_04_follower_count_missing_id(self): for action in ('user_follower_count', 'dataset_follower_count', 'group_follower_count'): - params = json.dumps({}) - response = self.app.post('/api/action/{0}'.format(action), - params=params, status=409).json - assert response['success'] is False - assert response['error']['id'] == ['Missing value'] + error = ckan.tests.call_action_api(self.app, action, status=409) + assert error['id'] == ['Missing value'] def test_04_user_follower_count_no_followers(self): - params = json.dumps({'id': self.annafan['id']}) - response = self.app.post('/api/action/user_follower_count', - params=params).json - assert response['success'] is True - assert response['result'] == 0 + follower_count = ckan.tests.call_action_api(self.app, + 'user_follower_count', id=self.annafan['id']) + assert follower_count == 0 def test_04_dataset_follower_count_no_followers(self): - params = json.dumps({'id': self.annakarenina['id']}) - response = self.app.post('/api/action/dataset_follower_count', - params=params).json - assert response['success'] is True - assert response['result'] == 0 + follower_count = ckan.tests.call_action_api(self.app, + 'dataset_follower_count', id=self.annakarenina['id']) + assert follower_count == 0 def test_04_group_follower_count_no_followers(self): - params = json.dumps({'id': self.davids_group['id']}) - response = self.app.post('/api/action/group_follower_count', - params=params).json - assert response['success'] is True - assert response['result'] == 0 + follower_count = ckan.tests.call_action_api(self.app, + 'group_follower_count', id=self.davids_group['id']) + assert follower_count == 0 def test_04_follower_list_bad_id(self): for action in ('user_follower_list', 'dataset_follower_list', 'group_follower_list'): for object_id in ('bad id', ' ', 3, 35.7, 'xxx', ''): - params = json.dumps({'id': object_id}) - response = self.app.post('/api/action/{0}'.format(action), - params=params, status=409).json - assert response['success'] is False - assert response['error']['id'] + error = ckan.tests.call_action_api(self.app, action, + status=409, id=object_id) + assert error['id'] def test_04_follower_list_missing_id(self): for action in ('user_follower_list', 'dataset_follower_list', 'group_follower_list'): - params = json.dumps({}) - response = self.app.post('/api/action/{0}'.format(action), - params=params, status=409).json - assert response['success'] is False - assert response['error']['id'] == ['Missing value'] + error = ckan.tests.call_action_api(self.app, action, status=409) + assert error['id'] == ['Missing value'] def test_04_user_follower_list_no_followers(self): - params = json.dumps({'id': self.annafan['id']}) - response = self.app.post('/api/action/user_follower_list', - params=params).json - assert response['success'] is True - assert response['result'] == [] + followers = ckan.tests.call_action_api(self.app, 'user_follower_list', + id=self.annafan['id']) + assert followers == [] def test_04_dataset_follower_list_no_followers(self): - params = json.dumps({'id': self.annakarenina['id']}) - response = self.app.post('/api/action/dataset_follower_list', - params=params).json - assert response['success'] is True - assert response['result'] == [] + followers = ckan.tests.call_action_api(self.app, + 'dataset_follower_list', id=self.annakarenina['id']) + assert followers == [] def test_04_group_follower_list_no_followers(self): - params = json.dumps({'id': self.davids_group['id']}) - response = self.app.post('/api/action/group_follower_list', - params=params).json - assert response['success'] is True - assert response['result'] == [] + followers = ckan.tests.call_action_api(self.app, 'group_follower_list', + id=self.davids_group['id']) + assert followers == [] def test_04_am_following_bad_id(self): for action in ('am_following_dataset', 'am_following_user', 'am_following_group'): for object_id in ('bad id', ' ', 3, 35.7, 'xxx'): - params = json.dumps({'id': object_id}) - extra_environ = {'Authorization': str(self.annafan['apikey'])} - response = self.app.post('/api/action/{0}'.format(action), - params=params, extra_environ=extra_environ, - status=409).json - assert response['success'] is False - assert response['error']['id'][0].startswith('Not found: ') + error = ckan.tests.call_action_api(self.app, action, + apikey=self.annafan['apikey'], status=409, id=object_id) + assert error['id'][0].startswith('Not found: ') def test_04_am_following_missing_id(self): for action in ('am_following_dataset', 'am_following_user', 'am_following_group'): for id in ('missing', None, ''): if id == 'missing': - params = json.dumps({}) + error = ckan.tests.call_action_api(self.app, action, + apikey=self.annafan['apikey'], status=409) else: - params = json.dumps({'id':id}) - extra_environ = {'Authorization': str(self.annafan['apikey'])} - response = self.app.post('/api/action/{0}'.format(action), - params=params, extra_environ=extra_environ, - status=409).json - assert response['success'] is False - assert response['error']['id'] == [u'Missing value'] + error = ckan.tests.call_action_api(self.app, action, + apikey=self.annafan['apikey'], status=409, id=id) + assert error['id'] == [u'Missing value'] def test_04_am_following_dataset_bad_apikey(self): for apikey in ('bad api key', '', ' ', 'None', '3', '35.7', 'xxx'): - params = json.dumps({'id': self.warandpeace['id']}) - extra_environ = {'Authorization': apikey} - response = self.app.post('/api/action/am_following_dataset', - params=params, extra_environ=extra_environ, status=403).json - assert response['success'] == False - assert response['error']['message'] == 'Access denied' + error = ckan.tests.call_action_api(self.app, + 'am_following_dataset', apikey=apikey, status=403, + id=self.warandpeace['id']) + assert error['message'] == 'Access denied' def test_04_am_following_dataset_missing_apikey(self): - params = json.dumps({'id': self.warandpeace['id']}) - response = self.app.post('/api/action/am_following_dataset', - params=params, status=403).json - assert response['success'] == False - assert response['error']['message'] == 'Access denied' + error = ckan.tests.call_action_api(self.app, 'am_following_dataset', + status=403, id=self.warandpeace['id']) + assert error['message'] == 'Access denied' def test_04_am_following_user_bad_apikey(self): for apikey in ('bad api key', '', ' ', 'None', '3', '35.7', 'xxx'): - params = json.dumps({'id': self.annafan['id']}) - extra_environ = {'Authorization': apikey} - response = self.app.post('/api/action/am_following_user', - params=params, extra_environ=extra_environ, status=403).json - assert response['success'] == False - assert response['error']['message'] == 'Access denied' + error = ckan.tests.call_action_api(self.app, 'am_following_user', + apikey=apikey, status=403, id=self.annafan['id']) + assert error['message'] == 'Access denied' def test_04_am_following_user_missing_apikey(self): - params = json.dumps({'id': self.annafan['id']}) - response = self.app.post('/api/action/am_following_user', - params=params, status=403).json - assert response['success'] == False - assert response['error']['message'] == 'Access denied' + error = ckan.tests.call_action_api(self.app, 'am_following_user', + status=403, id=self.annafan['id']) + assert error['message'] == 'Access denied' def test_04_am_following_group_bad_apikey(self): for apikey in ('bad api key', '', ' ', 'None', '3', '35.7', 'xxx'): - params = json.dumps({'id': self.rogers_group['id']}) - extra_environ = {'Authorization': apikey} - response = self.app.post('/api/action/am_following_group', - params=params, extra_environ=extra_environ, status=403).json - assert response['success'] == False - assert response['error']['message'] == 'Access denied' + error = ckan.tests.call_action_api(self.app, 'am_following_group', + apikey=apikey, status=403, id=self.rogers_group['id']) + assert error['message'] == 'Access denied' def test_04_am_following_group_missing_apikey(self): - params = json.dumps({'id': self.rogers_group['id']}) - response = self.app.post('/api/action/am_following_group', - params=params, status=403).json - assert response['success'] == False - assert response['error']['message'] == 'Access denied' + error = ckan.tests.call_action_api(self.app, 'am_following_group', + status=403, id=self.rogers_group['id']) + assert error['message'] == 'Access denied' class TestFollowerDelete(object): @@ -717,45 +559,30 @@ def test_01_unfollow_user_not_exists(self): she is not following. ''' - params = json.dumps({'id': self.russianfan['id']}) - extra_environ = { - 'Authorization': str(self.annafan['apikey']), - } - response = self.app.post('/api/action/unfollow_user', - params=params, extra_environ=extra_environ, status=404).json - assert response['success'] == False - assert response['error']['message'].startswith( - 'Not found: You are not following ') + error = ckan.tests.call_action_api(self.app, 'unfollow_user', + apikey=self.annafan['apikey'], status=404, + id=self.russianfan['id']) + assert error['message'].startswith('Not found: You are not following ') def test_01_unfollow_dataset_not_exists(self): '''Test the error response when a user tries to unfollow a dataset that she is not following. ''' - params = json.dumps({'id': self.annakarenina['id']}) - extra_environ = { - 'Authorization': str(self.annafan['apikey']), - } - response = self.app.post('/api/action/unfollow_dataset', - params=params, extra_environ=extra_environ, status=404).json - assert response['success'] == False - assert response['error']['message'].startswith( - 'Not found: You are not following ') + error = ckan.tests.call_action_api(self.app, 'unfollow_dataset', + apikey=self.annafan['apikey'], status=404, + id=self.annakarenina['id']) + assert error['message'].startswith('Not found: You are not following') def test_01_unfollow_group_not_exists(self): '''Test the error response when a user tries to unfollow a group that she is not following. ''' - params = json.dumps({'id': self.rogers_group['id']}) - extra_environ = { - 'Authorization': str(self.annafan['apikey']), - } - response = self.app.post('/api/action/unfollow_group', - params=params, extra_environ=extra_environ, status=404).json - assert response['success'] is False - assert response['error']['message'].startswith( - 'Not found: You are not following ') + error = ckan.tests.call_action_api(self.app, 'unfollow_group', + apikey=self.annafan['apikey'], status=404, + id=self.rogers_group['id']) + assert error['message'].startswith('Not found: You are not following') def test_01_unfollow_bad_apikey(self): '''Test the error response when a user tries to unfollow something @@ -765,58 +592,36 @@ def test_01_unfollow_bad_apikey(self): for action in ('unfollow_user', 'unfollow_dataset', 'unfollow_group'): for apikey in ('bad api key', '', ' ', 'None', '3', '35.7', 'xxx'): - params = json.dumps({ - 'id': self.joeadmin['id'], - }) - extra_environ = { - 'Authorization': apikey, - } - response = self.app.post('/api/action/{0}'.format(action), - params=params, extra_environ=extra_environ, - status=(403,409)).json - assert response['success'] is False - assert response['error']['message'] == 'Access denied' + error = ckan.tests.call_action_api(self.app, action, + apikey=apikey, status=403, id=self.joeadmin['id']) + assert error['message'] == 'Access denied' def test_01_unfollow_missing_apikey(self): '''Test error response when calling unfollow_* without api key.''' for action in ('unfollow_user', 'unfollow_dataset', 'unfollow_group'): - params = json.dumps({ - 'id': self.joeadmin['id'], - }) - response = self.app.post('/api/action/{0}'.format(action), - params=params, status=403).json - assert response['success'] is False - assert response['error']['message'] == 'Access denied' + error = ckan.tests.call_action_api(self.app, action, status=403, + id=self.joeadmin['id']) + assert error['message'] == 'Access denied' def test_01_unfollow_bad_object_id(self): '''Test error response when calling unfollow_* with bad object id.''' for action in ('unfollow_user', 'unfollow_dataset', 'unfollow_group'): for object_id in ('bad id', ' ', 3, 35.7, 'xxx'): - params = json.dumps({ - 'id': object_id, - }) - extra_environ = { - 'Authorization': str(self.annafan['apikey']), - } - response = self.app.post('/api/action/{0}'.format(action), - params=params, extra_environ=extra_environ, - status=409).json - assert response['success'] is False - assert response['error']['id'][0].startswith('Not found') + error = ckan.tests.call_action_api(self.app, action, + apikey=self.annafan['apikey'], status=409, + id=object_id) + assert error['id'][0].startswith('Not found') def test_01_unfollow_missing_object_id(self): for action in ('unfollow_user', 'unfollow_dataset', 'unfollow_group'): for id in ('missing', None, ''): if id == 'missing': - params = json.dumps({}) + error = ckan.tests.call_action_api(self.app, action, + apikey=self.annafan['apikey'], status=409) else: - params = json.dumps({'id': id}) - extra_environ = {'Authorization': str(self.annafan['apikey'])} - response = self.app.post('/api/action/{0}'.format(action), - params=params, extra_environ=extra_environ, - status=409).json - assert response['success'] is False - assert response['error']['id'] == [u'Missing value'] + error = ckan.tests.call_action_api(self.app, action, + apikey=self.annafan['apikey'], status=409, id=id) + assert error['id'] == [u'Missing value'] def _unfollow_user(self, follower_id, apikey, object_id, object_arg): '''Test a user unfollowing a user via the API. @@ -829,53 +634,33 @@ def _unfollow_user(self, follower_id, apikey, object_id, object_arg): ''' # Record the user's number of followers before. - params = json.dumps({'id': object_id}) - response = self.app.post('/api/action/user_follower_count', - params=params).json - assert response['success'] is True - count_before = response['result'] + count_before = ckan.tests.call_action_api(self.app, + 'user_follower_count', id=object_id) # Check that the user is following the object. - params = json.dumps({'id': object_id}) - extra_environ = {'Authorization': str(apikey)} - response = self.app.post('/api/action/am_following_user', - params=params, extra_environ=extra_environ).json - assert response['success'] is True - assert response['result'] is True + am_following = ckan.tests.call_action_api(self.app, + 'am_following_user', apikey=apikey, id=object_id) + assert am_following is True # Make the user unfollow the object. - params = { - 'id': object_arg, - } - extra_environ = {'Authorization': str(apikey)} - response = self.app.post('/api/action/unfollow_user', - params=json.dumps(params), extra_environ=extra_environ).json - assert response['success'] is True + ckan.tests.call_action_api(self.app, 'unfollow_user', apikey=apikey, + id=object_arg) # Check that am_following_user now returns False. - params = json.dumps({'id': object_id}) - extra_environ = {'Authorization': str(apikey)} - response = self.app.post('/api/action/am_following_user', - params=params, extra_environ=extra_environ).json - assert response['success'] is True - assert response['result'] is False + am_following = ckan.tests.call_action_api(self.app, + 'am_following_user', apikey=apikey, id=object_id) + assert am_following is False # Check that the user doesn't appear in the object's list of followers. - params = json.dumps({'id': object_id}) - response = self.app.post('/api/action/user_follower_list', - params=params).json - assert response['success'] is True - assert response['result'] - followers = response['result'] + followers = ckan.tests.call_action_api(self.app, 'user_follower_list', + id=object_id) assert len([follower for follower in followers if follower['id'] == follower_id]) == 0 # Check that the object's follower count has decreased by 1. - params = json.dumps({'id': object_id}) - response = self.app.post('/api/action/user_follower_count', - params=params).json - assert response['success'] is True - assert response['result'] == count_before - 1 + count_after = ckan.tests.call_action_api(self.app, + 'user_follower_count', id=object_id) + assert count_after == count_before - 1 def _unfollow_dataset(self, user_id, apikey, dataset_id, dataset_arg): '''Test a user unfollowing a dataset via the API. @@ -888,54 +673,34 @@ def _unfollow_dataset(self, user_id, apikey, dataset_id, dataset_arg): ''' # Record the dataset's number of followers before. - params = json.dumps({'id': dataset_id}) - response = self.app.post('/api/action/dataset_follower_count', - params=params).json - assert response['success'] is True - count_before = response['result'] + count_before = ckan.tests.call_action_api(self.app, + 'dataset_follower_count', id=dataset_id) # Check that the user is following the dataset. - params = json.dumps({'id': dataset_id}) - extra_environ = {'Authorization': str(apikey)} - response = self.app.post('/api/action/am_following_dataset', - params=params, extra_environ=extra_environ).json - assert response['success'] is True - assert response['result'] is True + am_following = ckan.tests.call_action_api(self.app, + 'am_following_dataset', apikey=apikey, id=dataset_id) + assert am_following is True # Make the user unfollow the dataset. - params = { - 'id': dataset_arg, - } - extra_environ = {'Authorization': str(apikey)} - response = self.app.post('/api/action/unfollow_dataset', - params=json.dumps(params), extra_environ=extra_environ).json - assert response['success'] is True + ckan.tests.call_action_api(self.app, 'unfollow_dataset', apikey=apikey, + id=dataset_arg) # Check that am_following_dataset now returns False. - params = json.dumps({'id': dataset_id}) - extra_environ = {'Authorization': str(apikey)} - response = self.app.post('/api/action/am_following_dataset', - params=params, extra_environ=extra_environ).json - assert response['success'] is True - assert response['result'] is False + am_following = ckan.tests.call_action_api(self.app, + 'am_following_dataset', apikey=apikey, id=dataset_id) + assert am_following is False # Check that the user doesn't appear in the dataset's list of # followers. - params = json.dumps({'id': dataset_id}) - response = self.app.post('/api/action/dataset_follower_list', - params=params).json - assert response['success'] is True - assert response['result'] - followers = response['result'] + followers = ckan.tests.call_action_api(self.app, + 'dataset_follower_list', id=dataset_id) assert len([follower for follower in followers if follower['id'] == user_id]) == 0 # Check that the dataset's follower count has decreased by 1. - params = json.dumps({'id': dataset_id}) - response = self.app.post('/api/action/dataset_follower_count', - params=params).json - assert response['success'] is True - assert response['result'] == count_before - 1 + count_after = ckan.tests.call_action_api(self.app, + 'dataset_follower_count', id=dataset_id) + assert count_after == count_before - 1 def _unfollow_group(self, user_id, apikey, group_id, group_arg): '''Test a user unfollowing a group via the API. @@ -948,54 +713,34 @@ def _unfollow_group(self, user_id, apikey, group_id, group_arg): ''' # Record the group's number of followers before. - params = json.dumps({'id': group_id}) - response = self.app.post('/api/action/group_follower_count', - params=params).json - assert response['success'] is True - count_before = response['result'] + count_before = ckan.tests.call_action_api(self.app, + 'group_follower_count', id=group_id) # Check that the user is following the group. - params = json.dumps({'id': group_id}) - extra_environ = {'Authorization': str(apikey)} - response = self.app.post('/api/action/am_following_group', - params=params, extra_environ=extra_environ).json - assert response['success'] is True - assert response['result'] is True + am_following = ckan.tests.call_action_api(self.app, + 'am_following_group', apikey=apikey, id=group_id) + assert am_following is True # Make the user unfollow the group. - params = { - 'id': group_arg, - } - extra_environ = {'Authorization': str(apikey)} - response = self.app.post('/api/action/unfollow_group', - params=json.dumps(params), extra_environ=extra_environ).json - assert response['success'] is True + ckan.tests.call_action_api(self.app, 'unfollow_group', apikey=apikey, + id=group_arg) # Check that am_following_group now returns False. - params = json.dumps({'id': group_id}) - extra_environ = {'Authorization': str(apikey)} - response = self.app.post('/api/action/am_following_group', - params=params, extra_environ=extra_environ).json - assert response['success'] is True - assert response['result'] is False + am_following = ckan.tests.call_action_api(self.app, + 'am_following_group', apikey=apikey, id=group_id) + assert am_following is False # Check that the user doesn't appear in the group's list of # followers. - params = json.dumps({'id': group_id}) - response = self.app.post('/api/action/group_follower_list', - params=params).json - assert response['success'] is True - assert 'result' in response - followers = response['result'] + followers = ckan.tests.call_action_api(self.app, 'group_follower_list', + id=group_id) assert len([follower for follower in followers if follower['id'] == user_id]) == 0 # Check that the group's follower count has decreased by 1. - params = json.dumps({'id': group_id}) - response = self.app.post('/api/action/group_follower_count', - params=params).json - assert response['success'] is True - assert response['result'] == count_before - 1 + count_after = ckan.tests.call_action_api(self.app, + 'group_follower_count', id=group_id) + assert count_after == count_before - 1 def test_02_follower_delete_by_id(self): self._unfollow_user(self.annafan['id'], self.annafan['apikey'], @@ -1102,136 +847,93 @@ def test_01_on_delete_cascade_api(self): ''' # It should no longer be possible to get joeadmin's follower list. - params = json.dumps({'id': 'joeadmin'}) - response = self.app.post('/api/action/user_follower_list', - params=params, status=409).json - assert response['success'] is False - assert response['error'].has_key('id') + error = ckan.tests.call_action_api(self.app, 'user_follower_list', + status=409, id='joeadmin') + assert 'id' in error # It should no longer be possible to get warandpeace's follower list. - params = json.dumps({'id': 'warandpeace'}) - response = self.app.post('/api/action/dataset_follower_list', - params=params, status=409).json - assert response['success'] is False - assert response['error'].has_key('id') + error = ckan.tests.call_action_api(self.app, 'dataset_follower_list', + status=409, id='warandpeace') + assert 'id' in error # It should no longer be possible to get david's follower list. - params = json.dumps({'id': 'david'}) - response = self.app.post('/api/action/group_follower_list', - params=params, status=409).json - assert response['success'] is False - assert response['error'].has_key('id') + error = ckan.tests.call_action_api(self.app, 'group_follower_list', + status=409, id='david') + assert 'id' in error # It should no longer be possible to get joeadmin's follower count. - params = json.dumps({'id': 'joeadmin'}) - response = self.app.post('/api/action/user_follower_count', - params=params, status=409).json - assert response['success'] is False - assert response['error'].has_key('id') + error = ckan.tests.call_action_api(self.app, 'user_follower_count', + status=409, id='joeadmin') + assert 'id' in error # It should no longer be possible to get warandpeace's follower count. - params = json.dumps({'id': 'warandpeace'}) - response = self.app.post('/api/action/dataset_follower_count', - params=params, status=409).json - assert response['success'] is False - assert response['error'].has_key('id') + error = ckan.tests.call_action_api(self.app, 'dataset_follower_count', + status=409, id='warandpeace') + assert 'id' in error # It should no longer be possible to get david's follower count. - params = json.dumps({'id': 'david'}) - response = self.app.post('/api/action/group_follower_count', - params=params, status=409).json - assert response['success'] is False - assert response['error'].has_key('id') + error = ckan.tests.call_action_api(self.app, 'group_follower_count', + status=409, id='david') + assert 'id' in error # It should no longer be possible to get am_following for joeadmin. - params = json.dumps({'id': 'joeadmin'}) - extra_environ = {'Authorization': str(self.testsysadmin['apikey'])} - response = self.app.post('/api/action/am_following_user', - params=params, extra_environ=extra_environ, status=409).json - assert response['success'] is False - assert response['error'].has_key('id') + error = ckan.tests.call_action_api(self.app, 'am_following_user', + apikey=self.testsysadmin['apikey'], status=409, id='joeadmin') + assert 'id' in error # It should no longer be possible to get am_following for warandpeace. - params = json.dumps({'id': 'warandpeace'}) - extra_environ = {'Authorization': str(self.testsysadmin['apikey'])} - response = self.app.post('/api/action/am_following_dataset', - params=params, extra_environ=extra_environ, status=409).json - assert response['success'] is False - assert response['error'].has_key('id') + error = ckan.tests.call_action_api(self.app, 'am_following_dataset', + apikey=self.testsysadmin['apikey'], status=409, + id='warandpeace') + assert 'id' in error # It should no longer be possible to get am_following for david. - params = json.dumps({'id': 'david'}) - extra_environ = {'Authorization': str(self.testsysadmin['apikey'])} - response = self.app.post('/api/action/am_following_group', - params=params, extra_environ=extra_environ, status=409).json - assert response['success'] is False - assert response['error'].has_key('id') + error = ckan.tests.call_action_api(self.app, 'am_following_group', + apikey=self.testsysadmin['apikey'], status=409, id='david') + assert 'id' in error # It should no longer be possible to unfollow joeadmin. - params = json.dumps({'id': 'joeadmin'}) - extra_environ = {'Authorization': str(self.tester['apikey'])} - response = self.app.post('/api/action/unfollow_user', - params=params, extra_environ=extra_environ, status=409).json - assert response['success'] is False - assert response['error']['id'] == ['Not found: User'] + error = ckan.tests.call_action_api(self.app, 'unfollow_user', + apikey=self.tester['apikey'], status=409, id='joeadmin') + assert error['id'] == ['Not found: User'] # It should no longer be possible to unfollow warandpeace. - params = json.dumps({'id': 'warandpeace'}) - extra_environ = {'Authorization': str(self.testsysadmin['apikey'])} - response = self.app.post('/api/action/unfollow_dataset', - params=params, extra_environ=extra_environ, status=409).json - assert response['success'] is False - assert response['error']['id'] == ['Not found: Dataset'] + error = ckan.tests.call_action_api(self.app, 'unfollow_dataset', + apikey=self.testsysadmin['apikey'], status=409, + id='warandpeace') + assert error['id'] == ['Not found: Dataset'] # It should no longer be possible to unfollow david. - params = json.dumps({'id': 'david'}) - extra_environ = {'Authorization': str(self.testsysadmin['apikey'])} - response = self.app.post('/api/action/unfollow_group', - params=params, extra_environ=extra_environ, status=409).json - assert response['success'] is False - assert response['error']['id'] == ['Not found: Group'] + error = ckan.tests.call_action_api(self.app, 'unfollow_group', + apikey=self.testsysadmin['apikey'], status=409, id='david') + assert error['id'] == ['Not found: Group'] # It should no longer be possible to follow joeadmin. - params = json.dumps({'id': 'joeadmin'}) - extra_environ = {'Authorization': str(self.annafan['apikey'])} - response = self.app.post('/api/action/follow_user', - params=params, extra_environ=extra_environ, status=409).json - assert response['success'] is False - assert response['error'].has_key('id') + error = ckan.tests.call_action_api(self.app, 'follow_user', + apikey=self.annafan['apikey'], status=409, id='joeadmin') + assert 'id' in error # It should no longer be possible to follow warandpeace. - params = json.dumps({'id': 'warandpeace'}) - extra_environ = {'Authorization': str(self.annafan['apikey'])} - response = self.app.post('/api/action/follow_dataset', - params=params, extra_environ=extra_environ, status=409).json - assert response['success'] is False - assert response['error'].has_key('id') + error = ckan.tests.call_action_api(self.app, 'follow_dataset', + apikey=self.annafan['apikey'], status=409, id='warandpeace') + assert 'id' in error # It should no longer be possible to follow david. - params = json.dumps({'id': 'david'}) - extra_environ = {'Authorization': str(self.annafan['apikey'])} - response = self.app.post('/api/action/follow_group', - params=params, extra_environ=extra_environ, status=409).json - assert response['success'] is False - assert response['error'].has_key('id') + error = ckan.tests.call_action_api(self.app, 'follow_group', + apikey=self.annafan['apikey'], status=409, id='david') + assert 'id' in error # Users who joeadmin was following should no longer have him in their # follower list. - params = json.dumps({'id': self.testsysadmin['id']}) - response = self.app.post('/api/action/user_follower_list', - params=params).json - assert response['success'] is True - followers = [follower['name'] for follower in response['result']] - assert 'joeadmin' not in followers + followers = ckan.tests.call_action_api(self.app, 'user_follower_list', + id=self.testsysadmin['id']) + assert 'joeadmin' not in [follower['name'] for follower in followers] # Datasets who joeadmin was following should no longer have him in # their follower list. - params = json.dumps({'id': self.annakarenina['id']}) - response = self.app.post('/api/action/dataset_follower_list', - params=params).json - assert response['success'] is True - followers = [follower['name'] for follower in response['result']] - assert 'joeadmin' not in followers + followers = ckan.tests.call_action_api(self.app, + 'dataset_follower_list', id=self.annakarenina['id']) + assert 'joeadmin' not in [follower['name'] for follower in followers] def test_02_on_delete_cascade_db(self): if not are_foreign_keys_supported(): diff --git a/ckan/tests/functional/test_group.py b/ckan/tests/functional/test_group.py index 7bfc04aaadd..57e2c62574c 100644 --- a/ckan/tests/functional/test_group.py +++ b/ckan/tests/functional/test_group.py @@ -187,7 +187,7 @@ def test_index(self): groupname = 'david' group = model.Group.by_name(unicode(groupname)) group_title = group.title - group_packages_count = len(group.active_packages().all()) + group_packages_count = len(group.packages()) group_description = group.description self.check_named_element(res, 'tr', group_title, group_packages_count, @@ -321,7 +321,7 @@ def test_2_edit(self): assert group.description == newdesc, group # now look at datasets - assert len(group.active_packages().all()) == 3 + assert len(group.packages()) == 3 def test_3_edit_form_has_new_package(self): # check for dataset in autocomplete @@ -369,7 +369,7 @@ def test_4_new_duplicate_package(self): # check package only added to the group once group = model.Group.by_name(group_name) - pkg_names = [pkg.name for pkg in group.active_packages().all()] + pkg_names = [pkg.name for pkg in group.packages()] assert_equal(pkg_names, [self.packagename]) def test_edit_plugin_hook(self): @@ -435,7 +435,7 @@ def update_group(res, name, with_pkg=True): # We have the datasets in the DB, but we should also see that many # on the group read page. - assert len(group.active_packages().all()) == 3 + assert len(group.packages()) == 3 offset = url_for(controller='group', action='read', id='newname') res = self.app.get(offset, status=200, @@ -543,9 +543,9 @@ def test_2_new(self): group = model.Group.by_name(group_name) assert group.title == group_title, group assert group.description == group_description, group - assert len(group.active_packages().all()) == 1 + assert len(group.packages()) == 1 pkg = model.Package.by_name(self.packagename) - assert group.active_packages().all() == [pkg] + assert group.packages() == [pkg] def test_3_new_duplicate_group(self): prefix = '' @@ -702,7 +702,7 @@ def test_index(self): groupname = 'david' group = model.Group.by_name(unicode(groupname)) group_title = group.title - group_packages_count = len(group.active_packages().all()) + group_packages_count = len(group.packages()) group_description = group.description self.check_named_element(res, 'tr', group_title, group_packages_count, @@ -820,7 +820,7 @@ def test_2_edit(self): assert group.description == newdesc, group # now look at datasets - assert len(group.active_packages().all()) == 3 + assert len(group.packages()) == 3 def test_3_edit_form_has_new_package(self): # check for dataset in autocomplete @@ -868,7 +868,7 @@ def test_4_new_duplicate_package(self): # check package only added to the group once group = model.Group.by_name(group_name) - pkg_names = [pkg.name for pkg in group.active_packages().all()] + pkg_names = [pkg.name for pkg in group.packages()] assert_equal(pkg_names, [self.packagename]) def test_edit_plugin_hook(self): diff --git a/ckan/tests/functional/test_user.py b/ckan/tests/functional/test_user.py index ee8db1350b0..1c4a7614012 100644 --- a/ckan/tests/functional/test_user.py +++ b/ckan/tests/functional/test_user.py @@ -187,8 +187,8 @@ def test_login(self): # then get redirected to user's dashboard res = res.follow() assert_equal(res.status, 302) - assert res.header('Location').startswith('http://localhost/user/dashboard') or \ - res.header('Location').startswith('/user/dashboard') + assert res.header('Location').startswith('http://localhost/dashboard') or \ + res.header('Location').startswith('/dashboard') res = res.follow() assert_equal(res.status, 200) assert 'testlogin is now logged in' in res.body diff --git a/ckan/tests/lib/test_dictization_schema.py b/ckan/tests/lib/test_dictization_schema.py index ad9fa290cf7..ef1391b98b8 100644 --- a/ckan/tests/lib/test_dictization_schema.py +++ b/ckan/tests/lib/test_dictization_schema.py @@ -148,7 +148,7 @@ def test_2_group_schema(self): del data['extras'] converted_data, errors = validate(data, default_group_schema(), self.context) - group_pack = sorted(group.active_packages().all(), key=lambda x:x.id) + group_pack = sorted(group.packages(), key=lambda x:x.id) converted_data["packages"] = sorted(converted_data["packages"], key=lambda x:x["id"]) diff --git a/ckan/tests/mock_publisher_auth.py b/ckan/tests/mock_publisher_auth.py index 749588a4fd9..475666a21eb 100644 --- a/ckan/tests/mock_publisher_auth.py +++ b/ckan/tests/mock_publisher_auth.py @@ -1,21 +1,24 @@ -from ckan.new_authz import is_authorized from ckan.logic import NotAuthorized +import logging + +log = logging.getLogger("mock_publisher_auth") + class MockPublisherAuth(object): """ MockPublisherAuth """ - + def __init__(self): self.functions = {} self._load() def _load(self): - for auth_module_name in ['get', 'create', 'update','delete']: + for auth_module_name in ['get', 'create', 'update', 'delete']: module_path = 'ckan.logic.auth.publisher.%s' % (auth_module_name,) try: module = __import__(module_path) - except ImportError,e: + except ImportError: log.debug('No auth module for action "%s"' % auth_module_name) continue @@ -25,10 +28,9 @@ def _load(self): for key, v in module.__dict__.items(): if not key.startswith('_'): self.functions[key] = v - - - def check_access(self,action, context, data_dict): + + def check_access(self, action, context, data_dict): logic_authorization = self.functions[action](context, data_dict) if not logic_authorization['success']: - msg = logic_authorization.get('msg','') + msg = logic_authorization.get('msg', '') raise NotAuthorized(msg) diff --git a/ckan/tests/models/test_group.py b/ckan/tests/models/test_group.py index 703b63778f8..424e8f3c198 100644 --- a/ckan/tests/models/test_group.py +++ b/ckan/tests/models/test_group.py @@ -25,7 +25,7 @@ def test_1_basic(self): grp = model.Group.by_name(u'group1') assert grp.title == u'Test Group' assert grp.description == u'This is a test group' - assert grp.active_packages().all() == [] + assert grp.packages() == [] def test_2_add_packages(self): model.repo.new_revision() @@ -50,7 +50,7 @@ def test_2_add_packages(self): assert grp.title == u'Russian Group' anna = model.Package.by_name(u'annakarenina') war = model.Package.by_name(u'warandpeace') - assert set(grp.active_packages().all()) == set((anna, war)), grp.active_packages().all() + assert set(grp.packages()) == set((anna, war)), grp.packages() assert grp in anna.get_groups() def test_3_search(self): diff --git a/ckan_deb/usr/lib/ckan/common.sh b/ckan_deb/usr/lib/ckan/common.sh index 0c4a33306eb..acd9abf863b 100644 --- a/ckan_deb/usr/lib/ckan/common.sh +++ b/ckan_deb/usr/lib/ckan/common.sh @@ -155,7 +155,7 @@ ckan_ensure_db_exists () { COMMAND_OUTPUT=`sudo -u postgres psql -c "select datname from pg_database where datname='$INSTANCE'"` if ! [[ "$COMMAND_OUTPUT" =~ ${INSTANCE} ]] ; then echo "Creating the database ..." - sudo -u postgres createdb -O ${INSTANCE} ${INSTANCE} + sudo -u postgres createdb -O ${INSTANCE} ${INSTANCE} -E utf-8 paster --plugin=ckan db init --config=/etc/ckan/${INSTANCE}/${INSTANCE}.ini fi fi diff --git a/doc/architecture.rst b/doc/architecture.rst new file mode 100644 index 00000000000..13098bddb5b --- /dev/null +++ b/doc/architecture.rst @@ -0,0 +1,206 @@ +====================== +CKAN Code Architecture +====================== + +This section tries to give some guidelines for writing code that is consistent +with the intended, overall design and architecture of CKAN. + + +Encapsulate SQLAlchemy in ``ckan.model`` +```````````````````````````````````````` + +Ideally SQLAlchemy should only be used within ``ckan.model`` and not from other +packages such as ``ckan.logic``. For example instead of using an SQLAlchemy +query from the logic package to retrieve a particular user from the database, +we add a ``get()`` method to ``ckan.model.user.User``:: + + @classmethod + def get(cls, user_id): + query = ... + . + . + . + return query.first() + +Now we can call this method from the logic package. + +Database Migrations +``````````````````` + +When changes are made to the model classes in ``ckan.model`` that alter CKAN's +database schema, a migration script has to be added to migrate old CKAN +databases to the new database schema when they upgrade their copies of CKAN. +See :doc:`migration`. + +Always go through the Action Functions +`````````````````````````````````````` + +Whenever some code, for example in ``ckan.lib`` or ``ckan.controllers``, wants +to get, create, update or delete an object from CKAN's model it should do so by +calling a function from the ``ckan.logic.action`` package, and *not* by +accessing ``ckan.model`` directly. + + +Action Functions are Exposed in the API +``````````````````````````````````````` + +The functions in ``ckan.logic.action`` are exposed to the world as the +:doc:`apiv3`. The API URL for an action function is automatically generated +from the function name, for example +``ckan.logic.action.create.package_create()`` is exposed at +``/api/action/package_create``. See `Steve Yegge's Google platforms rant +`_ for some +interesting discussion about APIs. + +**All** publicly visible functions in the +``ckan.logic.action.{create,delete,get,update}`` namespaces will be exposed +through the :doc:`apiv3`. **This includes functions imported** by those +modules, **as well as any helper functions** defined within those modules. To +prevent inadvertent exposure of non-action functions through the action api, +care should be taken to: + +1. Import modules correctly (see `Imports`_). For example: :: + + import ckan.lib.search as search + + search.query_for(...) + +2. Hide any locally defined helper functions: :: + + def _a_useful_helper_function(x, y, z): + '''This function is not exposed because it is marked as private``` + return x+y+z + +3. Bring imported convenience functions into the module namespace as private + members: :: + + _get_or_bust = logic.get_or_bust + + +Use ``get_action()`` +```````````````` + +Don't call ``logic.action`` functions directly, instead use ``get_action()``. +This allows plugins to override action functions using the ``IActions`` plugin +interface. For example:: + + ckan.logic.get_action('group_activity_list_html')(...) + +Instead of :: + + ckan.logic.action.get.group_activity_list_html(...) + + +Auth Functions and ``check_access()`` +`````````````` + +Each action function defined in ``ckan.logic.action`` should use its own +corresponding auth function defined in ``ckan.logic.auth``. Instead of calling +its auth function directly, an action function should go through +``ckan.logic.check_access`` (which is aliased ``_check_access`` in the action +modules) because this allows plugins to override auth functions using the +``IAuthFunctions`` plugin interface. For example:: + + def package_show(context, data_dict): + _check_access('package_show', context, data_dict) + +``check_access`` will raise an exception if the user is not authorized, which +the action function should not catch. When this happens the user will be shown +an authorization error in their browser (or will receive one in their response +from the API). + + +``logic.get_or_bust()`` +````````````` + +The ``data_dict`` parameter of logic action functions may be user provided, so +required files may be invalid or absent. Naive Code like:: + + id = data_dict['id'] + +may raise a ``KeyError`` and cause CKAN to crash with a 500 Server Error +and no message to explain what went wrong. Instead do:: + + id = _get_or_bust(data_dict, "id") + +which will raise ``ValidationError`` if ``"id"`` is not in ``data_dict``. The +``ValidationError`` will be caught and the user will get a 400 Bad Request +response and an error message explaining the problem. + + +Validation and ``ckan.logic.schema`` +```````````````````````````````````` + +Logic action functions can use schema defined in ``ckan.logic.schema`` to +validate the contents of the ``data_dict`` parameters that users pass to them. + +An action function should first check for a custom schema provided in the +context, and failing that should retrieve its default schema directly, and +then call ``_validate()`` to validate and convert the data. For example, here +is the validation code from the ``user_create()`` action function:: + + schema = context.get('schema') or ckan.logic.schema.default_user_schema() + session = context['session'] + validated_data_dict, errors = _validate(data_dict, schema, context) + if errors: + session.rollback() + raise ValidationError(errors) + + +Controller & Template Helper Functions +-------------------------------------- + +``ckan.lib.helpers`` contains helper functions that can be used from +``ckan.controllers`` or from templates. When developing for ckan core, only use +the helper functions found in ``ckan.lib.helpers.__allowed_functions__``. + + +.. _Testing: + +Testing +------- + +- Functional tests which test the behaviour of the web user interface, and the + APIs should be placed within ``ckan/tests/functional``. These tests can be a + lot slower to run that unit tests which don't access the database or solr. So + try to bear that in mind, and attempt to cover just what is neccessary, leaving + what can be tested via unit-testing in unit-tests. + +- ``nose.tools.assert_in`` and ``nose.tools.assert_not_in`` are only available + in Python>=2.7. So import them from ``ckan.tests``, which will provide + alternatives if they're not available. + +- the `mock`_ library can be used to create and interrogate mock objects. + +See :doc:`test` for further information on testing in CKAN. + +.. _mock: http://pypi.python.org/pypi/mock + +Writing Extensions +------------------ + +Please see :doc:`writing-extensions` for information about writing ckan +extensions, including details on the API available to extensions. + +Deprecation +----------- + +- Anything that may be used by extensions (see :doc:`writing-extensions`) needs + to maintain backward compatibility at call-site. ie - template helper + functions and functions defined in the plugins toolkit. + +- The length of time of deprecation is evaluated on a function-by-function + basis. At minimum, a function should be marked as deprecated during a point + release. + +- To mark a helper function, use the ``deprecated`` decorator found in + ``ckan.lib.maintain`` eg: :: + + + @deprecated() + def facet_items(*args, **kwargs): + """ + DEPRECATED: Use the new facet data structure, and `unselected_facet_items()` + """ + # rest of function definition. + diff --git a/doc/buildbot.rst b/doc/buildbot.rst deleted file mode 100644 index 484518b555e..00000000000 --- a/doc/buildbot.rst +++ /dev/null @@ -1,169 +0,0 @@ -================ -Install Buildbot -================ - -This section provides information for CKAN core developers setting up buildbot on an Ubuntu Lucid machine. - -If you simply want to check the status of the latest CKAN builds, visit http://buildbot.okfn.org/. - -Apt Installs -============ - -Install CKAN core dependencies from Lucid distribution:: - - sudo apt-get install build-essential libxml2-dev libxslt-dev - sudo apt-get install wget mercurial postgresql libpq-dev git-core - sudo apt-get install python-dev python-psycopg2 python-virtualenv - sudo apt-get install subversion - -Maybe need this too:: - - sudo apt-get install python-include - -Buildbot software:: - - sudo apt-get install buildbot - -Deb building software:: - - sudo apt-get install -y dh-make devscripts fakeroot cdbs - -Fabric:: - - sudo apt-get install -y fabric - -If you get errors with postgres and locales you might need to do these:: - - sudo apt-get install language-pack-en-base - sudo dpkg-reconfigure locales - - -Postgres Setup -============== - -If installation before failed to create a cluster, do this after fixing errors:: - - sudo pg_createcluster 8.4 main --start - -Create users and databases:: - - sudo -u postgres createuser -S -D -R -P buildslave - # set this password (matches buildbot scripts): biomaik15 - sudo -u postgres createdb -O buildslave ckan1 - sudo -u postgres createdb -O buildslave ckanext - - -Buildslave Setup -================ - -Rough commands:: - - sudo useradd -m -s /bin/bash buildslave - sudo chown buildslave:buildslave /home/buildslave - sudo su buildslave - cd ~ - git clone https://github.com/okfn/buildbot-scripts.git - ssh-keygen -t rsa - cp /home/buildslave/.ssh/id_rsa.pub ~/.ssh/authorized_keys - mkdir -p ckan/build - cd ckan/build - python ~/ckan-default.py - buildbot create-slave ~ localhost:9989 okfn - vim ~/info/admin - vim ~/info/host - mkdir /home/buildslave/pip_cache - virtualenv pyenv-tools - pip -E pyenv-tools install buildkit - - -Buildmaster Setup -================= - -Rough commands:: - - mkdir ~/buildmaster - buildbot create-master ~/buildmaster - ln -s /home/buildslave/master/master.cfg ~/buildmaster/master.cfg - cd ~/buildmaster - buildbot checkconfig - - -Startup -======= - -Setup the daemons for master and slave:: - - sudo vim /etc/default/buildbot - -This file should be edited to be like this:: - - BB_NUMBER[0]=0 # index for the other values; negative disables the bot - BB_NAME[0]="okfn" # short name printed on startup / stop - BB_USER[0]="okfn" # user to run as - BB_BASEDIR[0]="/home/okfn/buildmaster" # basedir argument to buildbot (absolute path) - BB_OPTIONS[0]="" # buildbot options - BB_PREFIXCMD[0]="" # prefix command, i.e. nice, linux32, dchroot - - BB_NUMBER[1]=1 # index for the other values; negative disables the bot - BB_NAME[1]="okfn" # short name printed on startup / stop - BB_USER[1]="buildslave" # user to run as - BB_BASEDIR[1]="/home/buildslave" # basedir argument to buildbot (absolute path) - BB_OPTIONS[1]="" # buildbot options - BB_PREFIXCMD[1]="" # prefix command, i.e. nice, linux32, dchroot - -Start master and slave (according to /etc/default/buildbot):: - - sudo /etc/init.d/buildbot start - -Now check you can view buildbot at http://localhost:8010/ - - -Connect Ports -============= - -It's preferable to view the buildbot site at port 80 rather than 8010. - -If there is no other web service on this machine, you might connect up the addresses using ``iptables``:: - - sudo iptables -t nat -A PREROUTING -p tcp --dport 80 -j REDIRECT --to-port 8010 - -Otherwise it is best to set up a reverse proxy. Using Apache, edit this file:: - - sudo vim /etc/apache2/sites-available/buildbot.okfn.org - -to look like this:: - - - ServerName buildbot.okfn.org - - ProxyPassReverse ts Off - - Order deny,allow - Allow from all - - ProxyPass / http://127.0.0.1:8010/ - ProxyPassReverse / http://127.0.0.1:8010/ - ProxyPreserveHost On - - -or the old one had:: - - - ServerAdmin sysadmin@okfn.org - ServerName buildbot.okfn.org - DocumentRoot /var/www/ - - Order allow,deny - allow from all - - RewriteEngine On - RewriteRule /(.*) http://localhost:8010/$1 [P,L] - - -Then:: - - sudo apt-get install libapache2-mod-proxy-html - sudo a2enmod proxy_http - sudo a2ensite buildbot.okfn.org - sudo /etc/init.d/apache2 reload - diff --git a/doc/coding-standards.rst b/doc/coding-standards.rst deleted file mode 100644 index 731c12cb760..00000000000 --- a/doc/coding-standards.rst +++ /dev/null @@ -1,1346 +0,0 @@ -===================== -CKAN Coding Standards -===================== - -Commit Guidelines -================= - -Generally, follow the `commit guidelines from the Pro Git book`_: - -- Try to make each commit a logically separate, digestible changeset. - -- The first line of the commit message should concisely summarise the - changeset. - -- Optionally, follow with a blank line and then a more detailed explanation of - the changeset. - -- Use the imperative present tense as if you were giving commands to the - codebase to change its behaviour, e.g. *Add tests for...*, *make xyzzy do - frotz...*, this helps to make the commit message easy to read. - -- Try to write the commit message so that a new CKAN developer could understand - it, i.e. using plain English as far as possible, and not referring to too - much assumed knowledge or to external resources such as mailing list - discussions (summarize the relevant points in the commit message instead). - -.. _commit guidelines from the Pro Git book: http://git-scm.com/book/en/Distributed-Git-Contributing-to-a-Project#Commit-Guidelines - -In CKAN we also refer to `trac.ckan.org`_ ticket numbers in commit messages -wherever relevant. This makes the release manager's job much easier! Of -course, you don't have to reference a ticket from your commit message if there -isn't a ticket for it, e.g. if you find a typo in a docstring and quickly fix -it you wouldn't bother to create a ticket for this. - -Put the ticket number in square brackets (e.g. ``[#123]``) at the start of the -first line of the commit message. You can also reference other Trac tickets -elsewhere in your commit message by just using the ticket number on its own -(e.g. ``see #456``). For example: - -:: - - [#2505] Update source install instructions - - Following feedback from markw (see #2406). - -.. _trac.ckan.org: http://trac.ckan.org/ - -Longer example CKAN commit message: - -:: - - [#2304] Refactor user controller a little - - Move initialisation of a few more template variables into - _setup_template_variables(), and change read(), edit(), and followers() to use - it. This removes some code duplication and fixes issues with the followers - count and follow button not being initialisd on all user controller pages. - - Change new() to _not_ use _setup_template_variables() as it only needs - c.is_sysadmin and not the rest. - - Also fix templates/user/layout.html so that the Followers tab appears on both - your own user page (when logged in) and on other user's pages. - -Feature Branches ----------------- - -All ticketed work should be developed on a corresponding feature branch forked -from master. The name of the branch should inlude the ticket's number, the -ticket type, and a brief one-line synopsis of the purpose of the ticket. eg: -``2298-feature-add-sort-by-controls-to-search-page``. This allows the ticket -number to be esaily searchable through github's web interface. - -Once work on the branch has been completed and it is ready to be merged into -master, make a pull request on github. Another member of the CKAN team will -review the changes; and provide feedback through the github pull request page. -If the piece of work touches on an area of code `owned` by another team member, -then notify them of the changes by email. - -Submitting Code Patches ------------------------ - -See the wiki for instructions on `how to submit a patch`_ via GitHub or email. - -.. _how to submit a patch: http://wiki.ckan.org/Submitting_a_code_patch - -Releases --------- - -See :doc:`release-cycle` for details on the release process. - -Merging -------- - -When merging a feature or bug branch into master: - -- Use the ``--no-ff`` option in the ``git merge`` command -- Add an entry to the ``CHANGELOG`` file - -The full postgresql test suite must pass before merging into master. :: - - nosetests --ckan --with-pylons=test-core.ini ckan - -See :doc:`test` for more information on running tests, including running the -core extension tests. - -Python Coding Standards -======================= - -For python code, we follow `PEP 8`_, plus a few of our own rules. The -important bits are laid out below, but if in doubt, refer to `PEP 8`_ and -common sense. - -Layout and formatting ---------------------- - -- Don't use tabs. Use 4 spaces. - -- Maximum line length is 79 characters. - -- Continuation lines should align vertically within the parentheses, or with - a hanging indent. See `PEP 8's Indent Section`_ for more details. - -- Avoid extraneous whitespace. See `PEP 8's Whitespace Section`_ for more details. - -- Clean up formatting issues in master, not on a feature branch. Unless of - course you're changing that piece of code anyway. This will help avoid - spurious merge conflicts, and aid in reading pull requests. - -- Use the single-quote character, ``'``, rather than the double-quote - character, ``"``, for string literals. - -.. _PEP 8: http://www.python.org/dev/peps/pep-0008/ -.. _PEP 8's Indent Section: http://www.python.org/dev/peps/pep-0008/#indentation -.. _PEP 8's Whitespace Section: http://www.python.org/dev/peps/pep-0008/#whitespace-in-expressions-and-statements - -Imports -------- - -- Import whole modules, rather than using ``from foo import bar``. It's ok - to alias imported modules to make things more concise, ie this *is* - acceptable: :: - - import foo.bar.baz as f - -- Make all imports at the start of the file, after the module docstring. - Imports should be grouped in the following order: - - 1. Standard library imports - 2. Third-party imports - 3. CKAN imports - -Logging -------- - -- Keep messages short. - -- Don't include object representations in the log message. It **is** useful - to include an domain model identifier where appropriate. - -- Choose an appropriate log-level: - - +----------+--------------------------------------------------------------+ - | Level | Description | - +==========+==============================================================+ - | DEBUG | Detailed information, of no interest when everything is | - | | working well but invaluable when diagnosing problems. | - +----------+--------------------------------------------------------------+ - | INFO | Affirmations that things are working as expected, e.g. | - | | "service has started" or "indexing run complete". Often | - | | ignored. | - +----------+--------------------------------------------------------------+ - | WARNING | There may be a problem in the near future, and this gives | - | | advance warning of it. But the application is able to proceed| - | | normally. | - +----------+--------------------------------------------------------------+ - | ERROR | The application has been unable to proceed as expected, due | - | | to the problem being logged. | - +----------+--------------------------------------------------------------+ - | CRITICAL | This is a serious error, and some kind of application | - | | meltdown might be imminent. | - +----------+--------------------------------------------------------------+ - - (`Source - `_) - -i18n ----- - -To construct an internationalised string, use `str.format`_, giving -meaningful names to each replacement field. For example: :: - - _(' ... {foo} ... {bar} ...').format(foo='foo-value', bar='bar-value') - -.. _str.format: http://docs.python.org/library/stdtypes.html#str.format - -Docstring Standards -------------------- - -We want CKAN's docstrings to be clear and easy to read for programmers who are -smart and competent but who may not know a lot of CKAN technical jargon and -whose first language may not be English. We also want it to be easy to maintain -the docstrings and keep them up to date with the actual behaviour of the code -as it changes over time. So: - -- Keep docstrings short, describe only what's necessary and no more -- Keep docstrings simple, use plain English, try not to use a long word - where a short one will do, and try to cut out words where possible -- Try to avoid repetition - -PEP 257 -``````` - -Generally, follow `PEP 257`_. We'll only describe the ways that CKAN differs -from or extends PEP 257 below. - -.. _PEP 257: http://www.python.org/dev/peps/pep-0257/ - -CKAN docstrings deviate from PEP 257 in a couple of ways: - -- We use ``'''triple single quotes'''`` around docstrings, not ``"""triple - double quotes"""`` (put triple single quotes around one-line docstrings as - well as multi-line ones, it makes them easier to expand later) -- We use Sphinx directives for documenting parameters, exceptions and return - values (see below) - -Sphinx -`````` -Use `Sphinx directives`_ for documenting the parameters, exceptions and returns -of functions: - -- Use ``:param`` and ``:type`` to describe each parameter -- Use ``:returns`` and ``:rtype`` to describe each return -- Use ``:raises`` to describe each exception raised - -Example of a short docstring: - -:: - - @property - def packages(self): - '''Return a list of all packages that have this tag, sorted by name. - - :rtype: list of ckan.model.package.Package objects - - ''' - -Example of a longer docstring: - -:: - - @classmethod - def search_by_name(cls, search_term, vocab_id_or_name=None): - '''Return all tags whose names contain a given string. - - By default only free tags (tags which do not belong to any vocabulary) - are returned. If the optional argument ``vocab_id_or_name`` is given - then only tags from that vocabulary are returned. - - :param search_term: the string to search for in the tag names - :type search_term: string - :param vocab_id_or_name: the id or name of the vocabulary to look in - (optional, default: None) - :type vocab_id_or_name: string - - :returns: a list of tags that match the search term - :rtype: list of ckan.model.tag.Tag objects - - ''' - - -The phrases that follow ``:param foo:``, ``:type foo:``, or ``:returns:`` -should not start with capital letters or end with full stops. These should be -short phrases and not full sentences. If more detail is required put it in the -function description instead. - -Indicate optional arguments by ending their descriptions with (optional) in -brackets. Where relevant also indicate the default value: (optional, default: -5). It's also helpful to list all required parameters before optional ones. - -.. _Sphinx directives: http://sphinx.pocoo.org/markup/desc.html#info-field-lists - -You can also use a little inline `reStructuredText markup`_ in docstrings, e.g. -``*stars for emphasis*`` or ````double-backticks for literal text```` - -.. _reStructuredText markup: http://docutils.sourceforge.net/docs/user/rst/quickref.html#inline-markup - -CKAN Action API Docstrings -`````````````````````````` - -Docstrings from CKAN's action API are processed with `autodoc`_ and -included in the API chapter of CKAN's documentation. The intended audience of -these docstrings is users of the CKAN API and not (just) CKAN core developers. - -In the Python source each API function has the same two arguments (``context`` -and ``data_dict``), but the docstrings should document the keys that the -functions read from ``data_dict`` and not ``context`` and ``data_dict`` -themselves, as this is what the user has to POST in the JSON dict when calling -the API. - -Where practical, it's helpful to give examples of param and return values in -API docstrings. - -CKAN datasets used to be called packages and the old name still appears in the -source, e.g. in function names like package_list(). When documenting functions -like this write dataset not package, but the first time you do this put package -after it in brackets to avoid any confusion, e.g. - -:: - - def package_show(context, data_dict): - '''Return the metadata of a dataset (package) and its resources. - -Example of a ckan.logic.action API docstring: - -:: - - def vocabulary_create(context, data_dict): - '''Create a new tag vocabulary. - - You must be a sysadmin to create vocabularies. - - :param name: the name of the new vocabulary, e.g. ``'Genre'`` - :type name: string - :param tags: the new tags to add to the new vocabulary, for the format of - tag dictionaries see ``tag_create()`` - :type tags: list of tag dictionaries - - :returns: the newly-created vocabulary - :rtype: dictionary - - ''' - -.. _Autodoc: http://sphinx.pocoo.org/ext/autodoc.html - -Tools ------ - -Running the `PEP 8 style guide checker`_ is good for checking adherence to `PEP -8`_ formatting. As mentioned above, only perform style clean-ups on master to -help avoid spurious merge conflicts. - -`PyLint`_ is a useful tool for analysing python source code for errors and signs of poor quality. - -`pyflakes`_ is another useful tool for passive analysis of python source code. -There's also a `pyflakes vim plugin`_ which will highlight unused variables, -undeclared variables, syntax errors and unused imports. - -.. _PEP 8 style guide checker: http://pypi.python.org/pypi/pep8 -.. _PyLint: http://www.logilab.org/857 -.. _pyflakes: http://pypi.python.org/pypi/pyflakes -.. _pyflakes vim plugin: http://www.vim.org/scripts/script.php?script_id=2441 - -CKAN Code Areas -=============== - -This section describes some guidelines for making changes in particular areas -of the codebase, as well as general concepts particular to CKAN. - -General -------- - -Some rules to adhere to when making changes to the codebase in general. - -.. todo:: Is there anything to include in this 'General' section? - -Domain Models -------------- - -This section describes things to bear in mind when making changes to the domain -models. For more information about CKAN's domain models, see -:doc:`domain-model`. - -The structure of the CKAN data is described in the 'model'. This is in the code -at `ckan/model`. - -Many of the domain objects are Revisioned and some are Stateful. These are -concepts introduced by `vdm`_. - -.. _vdm: http://okfn.org/projects/vdm/ -.. _sqlalchemy migrate: http://code.google.com/p/sqlalchemy-migrate SQLAlchemy Migrate - -Migration -````````` -When edits are made to the model code, then before the code can be used on a -CKAN instance with existing data, the existing data has to be migrated. This is -achieved with a migration script. - -CKAN currently uses to manage these scripts. When you deploy new code to a -CKAN instance, as part of the process you run any required migration scripts -with: :: - - paster --plugin=ckan db upgrade --config={.ini file} - -The scripts give their model version numbers in their filenames and are stored -in ``ckan/migration/versions/``. - -The current version the database is migrated to is also stored in the database. -When you run the upgrade, as each migration script is run it prints to the -console something like ``11->12``. If no upgrade is required because it is up -to date, then nothing is printed. - -Creating a new migration script -``````````````````````````````` -A migration script should be checked into CKAN at the same time as the model -changes it is related to. Before pushing the changes, ensure the tests pass -when running against the migrated model, which requires the -``--ckan-migration`` setting. - -To create a new migration script, create a python file in -``ckan/migration/versions/`` and name it with a prefix numbered one higher than -the previous one and some words describing the change. - -You need to use the special engine provided by the SqlAlchemy Migrate. Here is -the standard header for your migrate script: :: - - from sqlalchemy import * - from migrate import * - -The migration operations go in the upgrade function: :: - - def upgrade(migrate_engine): - metadata = MetaData() - metadata.bind = migrate_engine - -The following process should be followed when doing a migration. This process -is here to make the process easier and to validate if any mistakes have been -made: - -1. Get a dump of the database schema before you add your new migrate scripts. :: - - paster --plugin=ckan db clean --config={.ini file} - paster --plugin=ckan db upgrade --config={.ini file} - pg_dump -h host -s -f old.sql dbname - -2. Get a dump of the database as you have specified it in the model. :: - - paster --plugin=ckan db clean --config={.ini file} - - #this makes the database as defined in the model - paster --plugin=ckan db create-from-model -config={.ini file} - pg_dump -h host -s -f new.sql dbname - -3. Get agpdiff (apt-get it). It produces sql it thinks that you need to run on - the database in order to get it to the updated schema. :: - - apgdiff old.sql new.sql > upgrade.diff - -(or if you don't want to install java use http://apgdiff.startnet.biz/diff_online.php) - -4. The upgrade.diff file created will have all the changes needed in sql. - Delete the drop index lines as they are not created in the model. - -5. Put the resulting sql in your migrate script, e.g. :: - - migrate_engine.execute('''update table .........; update table ....''') - -6. Do a dump again, then a diff again to see if the the only thing left are drop index statements. - -7. run nosetests with ``--ckan-migration`` flag. - -It's that simple. Well almost. - -* If you are doing any table/field renaming adding that to your new migrate - script first and use this as a base for your diff (i.e add a migrate script - with these renaming before 1). This way the resulting sql won't try to drop and - recreate the field/table! - -* It sometimes drops the foreign key constraints in the wrong order causing an - error so you may need to rearrange the order in the resulting upgrade.diff. - -* If you need to do any data transfer in the migrations then do it between the - dropping of the constraints and adding of new ones. - -* May need to add some tests if you are doing data migrations. - -An example of a script doing it this way is ``034_resource_group_table.py``. -This script copies the definitions of the original tables in order to do the -renaming the tables/fields. - -In order to do some basic data migration testing extra assertions should be -added to the migration script. Examples of this can also be found in -``034_resource_group_table.py`` for example. - -This statement is run at the top of the migration script to get the count of -rows: :: - - package_count = migrate_engine.execute('''select count(*) from package''').first()[0] - -And the following is run after to make sure that row count is the same: :: - - resource_group_after = migrate_engine.execute('''select count(*) from resource_group''').first()[0] - assert resource_group_after == package_count - -The Action Layer ----------------- - -When making changes to the action layer, found in the four modules -``ckan/logic/action/{create,delete,get,update}`` there are a few things to bear -in mind. - -Server Errors -````````````` - -When writing action layer code, bear in mind that the input provided in the -``data_dict`` may be user-provided. This means that required fields should be -checked for existence and validity prior to use. For example, code such as :: - - id = data_dict['id'] - -will raise a ``KeyError`` if the user hasn't provided an ``id`` field in their -data dict. This results in a 500 error, and no message to explain what went -wrong. The correct response by the action function would be to raise a -``ValidationError`` instead, as this will be caught and will provide the user -with a `bad request` response, alongside an error message explaining the issue. - -To this end, there's a helper function, ``logic.get_or_bust()`` which can be -used to safely retrieve a value from a dict: :: - - id = _get_or_bust(data_dict, "id") - -Function visibility -``````````````````` - -**All** publicly visible functions in the -``ckan.logic.action.{create,delete,get,update}`` namespaces will be exposed -through the :doc:`apiv3`. **This includes functions imported** by those -modules, **as well as any helper functions** defined within those modules. To -prevent inadvertent exposure of non-action functions through the action api, -care should be taken to: - -1. Import modules correctly (see `Imports`_). For example: :: - - import ckan.lib.search as search - - search.query_for(...) - -2. Hide any locally defined helper functions: :: - - def _a_useful_helper_function(x, y, z): - '''This function is not exposed because it is marked as private``` - return x+y+z - -3. Bring imported convenience functions into the module namespace as private - members: :: - - _get_or_bust = logic.get_or_bust - -Documentation -````````````` - -Please refer to `CKAN Action API Docstrings`_ for information about writing -docstrings for the action functions. It is **very** important that action -functions are documented as they are not only consumed by CKAN developers but -by CKAN users. - -Controllers ------------ - -Guidelines when writing controller actions: - -- Use ``get_action``, rather than calling the action directly; and rather than - calling the action directly, as this allows extensions to overide the action's - behaviour. ie use :: - - ckan.logic.get_action('group_activity_list_html')(...) - - Instead of :: - - ckan.logic.action.get.group_activity_list_html(...) - -- Controllers have access to helper functions in ``ckan.lib.helpers``. - When developing for ckan core, only use the helper functions found in - ``ckan.lib.helpers.__allowed_functions__``. - -.. todo:: Anything else for controllers? - -Templating ----------- - -Helper Functions -```````````````` - -Templates have access to a set of helper functions in ``ckan.lib.helpers``. -When developing for ckan core, only use the helper functions found in -``ckan.lib.helpers.__allowed_functions__``. - -.. todo:: Jinja2 templates - -Testing -------- - -- Functional tests which test the behaviour of the web user interface, and the - APIs should be placed within ``ckan/tests/functional``. These tests can be a - lot slower to run that unit tests which don't access the database or solr. So - try to bear that in mind, and attempt to cover just what is neccessary, leaving - what can be tested via unit-testing in unit-tests. - -- ``nose.tools.assert_in`` and ``nose.tools.assert_not_in`` are only available - in Python>=2.7. So import them from ``ckan.tests``, which will provide - alternatives if they're not available. - -- the `mock`_ library can be used to create and interrogate mock objects. - -See :doc:`test` for further information on testing in CKAN. - -.. _mock: http://pypi.python.org/pypi/mock - -Writing Extensions ------------------- - -Please see :doc:`writing-extensions` for information about writing ckan -extensions, including details on the API available to extensions. - -Deprecation ------------ - -- Anything that may be used by extensions (see :doc:`writing-extensions`) needs - to maintain backward compatibility at call-site. ie - template helper - functions and functions defined in the plugins toolkit. - -- The length of time of deprecation is evaluated on a function-by-function - basis. At minimum, a function should be marked as deprecated during a point - release. - -- To mark a helper function, use the ``deprecated`` decorator found in - ``ckan.lib.maintain`` eg: :: - - - @deprecated() - def facet_items(*args, **kwargs): - """ - DEPRECATED: Use the new facet data structure, and `unselected_facet_items()` - """ - # rest of function definition. - -Javascript Coding Standards -=========================== - -Formatting ----------- - -.. _OKFN Coding Standards: http://wiki.okfn.org/Coding_Standards#Javascript -.. _idiomatic.js: https://github.com/rwldrn/idiomatic.js/ -.. _Douglas Crockford's: http://javascript.crockford.com/code.html - -All JavaScript documents must use **two spaces** for indentation and files -should have no trailing whitespace. This is contrary to the `OKFN Coding -Standards`_ but matches what's in use in the current code base. - -Coding style must follow the `idiomatic.js`_ style but with the following -exceptions. - -.. note:: Idiomatic is heavily based upon `Douglas Crockford's`_ style - guide which is recommended by the `OKFN Coding Standards`_. - -White Space -``````````` - -Two spaces must be used for indentation at all times. Unlike in idiomatic -whitespace must not be used _inside_ parentheses between the parentheses -and their Contents. :: - - // BAD: Too much whitespace. - function getUrl( full ) { - var url = '/styleguide/javascript/'; - if ( full ) { - url = 'http://okfn.github.com/ckan' + url; - } - return url; - } - - // GOOD: - function getUrl(full) { - var url = '/styleguide/javascript/'; - if (full) { - url = 'http://okfn.github.com/ckan' + url; - } - return url; - } - -.. note:: See section 2.D.1.1 of idiomatic for more examples of this syntax. - -Quotes -`````` - -Single quotes should be used everywhere unless writing JSON or the string -contains them. This makes it easier to create strings containing HTML. :: - - jQuery('
    ').appendTo('body'); - -Object properties need not be quoted unless required by the interpreter. :: - - var object = { - name: 'bill', - 'class': 'user-name' - }; - -Variable declarations -````````````````````` - -One ``var`` statement must be used per variable assignment. These must be -declared at the top of the function in which they are being used. :: - - // GOOD: - var good = "string"; - var alsoGood = "another; - - // GOOD: - var good = "string"; - var okay = [ - "hmm", "a bit", "better" - ]; - - // BAD: - var good = "string", - iffy = [ - "hmm", "not", "great" - ]; - -Declare variables at the top of the function in which they are first used. This -avoids issues with variable hoisting. If a variable is not assigned a value -until later in the function then it it okay to define more than one per -statement. :: - - // BAD: contrived example. - function lowercaseNames(names) { - var names = []; - - for (var index = 0, length = names.length; index < length; index += 1) { - var name = names[index]; - names.push(name.toLowerCase()); - } - - var sorted = names.sort(); - return sorted; - } - - // GOOD: - function lowercaseNames(names) { - var names = []; - var index, sorted, name; - - for (index = 0, length = names.length; index < length; index += 1) { - name = names[index]; - names.push(names[index].toLowerCase()); - } - - sorted = names.sort(); - return sorted; - } - -Naming ------- - -All properties, functions and methods must use lowercase camelCase: :: - - var myUsername = 'bill'; - var methods = { - getSomething: function () {} - }; - -Constructor functions must use uppercase CamelCase: :: - - function DatasetSearchView() { - } - -Constants must be uppercase with spaces delimited by underscores: :: - - var env = { - PRODUCTION: 'production', - DEVELOPMENT: 'development', - TESTING: 'testing' - }; - -Event handlers and callback functions should be prefixed with "on": :: - - function onDownloadClick(event) {} - - jQuery('.download').click(onDownloadClick); - -Boolean variables or methods returning boolean functions should prefix -the variable name with "is": :: - - function isAdmin() {} - - var canEdit = isUser() && isAdmin(); - - -.. note:: Alternatives are "has", "can" and "should" if they make more sense - -Private methods should be prefixed with an underscore: :: - - View.extend({ - "click": "_onClick", - _onClick: function (event) { - } - }); - -Functions should be declared as named functions rather than assigning an -anonymous function to a variable. :: - - // GOOD: - function getName() { - } - - // BAD: - var getName = function () { - }; - -Named functions are generally easier to debug as they appear named in the -debugger. - -Comments --------- - -Comments should be used to explain anything that may be unclear when you return -to it in six months time. Single line comments should be used for all inline -comments that do not form part of the documentation. :: - - // Export the function to either the exports or global object depending - // on the current environment. This can be either an AMD module, CommonJS - // module or a browser. - if (typeof module.define === 'function' && module.define.amd) { - module.define('broadcast', function () { - return Broadcast; - }); - } else if (module.exports) { - module.exports = Broadcast; - } else { - module.Broadcast = Broadcast; - } - -JSHint ------- - -All JavaScript should pass `JSHint`_ before being committed. This can -be installed using ``npm`` (which is bundled with `node`_) by running: :: - - $ npm -g install jshint - -Each project should include a jshint.json file with appropriate configuration -options for the tool. Most text editors can also be configured to read from -this file. - -.. _node: http://nodejs.org -.. _jshint: http://www.jshint.com - -Documentation -------------- - -For documentation we use a simple markup format to document all methods. The -documentation should provide enough information to show the reader what the -method does, arguments it accepts and a general example of usage. Also -for API's and third party libraries, providing links to external documentation -is encouraged. - -The formatting is as follows:: - - /* My method description. Should describe what the method does and where - * it should be used. - * - * param1 - The method params, one per line (default: null) - * param2 - A default can be provided in brackets at the end. - * - * Example - * - * // Indented two spaces. Should give a common example of use. - * client.getTemplate('index.html', {limit: 1}, function (html) { - * module.el.html(html); - * }); - * - * Returns describes what the object returns. - */ - -For example:: - - /* Loads an HTML template from the CKAN snippet API endpoint. Template - * variables can be passed through the API using the params object. - * - * Optional success and error callbacks can be provided or these can - * be attached using the returns jQuery promise object. - * - * filename - The filename of the template to load. - * params - An optional object containing key/value arguments to be - * passed into the template. - * success - An optional success callback to be called on load. This will - * recieve the HTML string as the first argument. - * error - An optional error callback to be called if the request fails. - * - * Example - * - * client.getTemplate('index.html', {limit: 1}, function (html) { - * module.el.html(html); - * }); - * - * Returns a jqXHR promise object that can be used to attach callbacks. - */ - -Testing -------- - -For unit testing we use the following libraries. - -- `Mocha`_: As a BDD unit testing framework. -- `Sinon`_: Provides spies, stubs and mocks for methods and functions. -- `Chai`_: Provides common assertions. - -.. _Mocha: http://visionmedia.github.com/mocha/ -.. _Sinon: http://chaijs.com/ -.. _Chai: http://sinonjs.org/docs/ - -Tests are run from the test/index.html directory. We use the BDD interface -(``describe()``, ``it()`` etc.) provided by mocha and the assert interface -provided by chai. - -Generally we try and have the core functionality of all libraries and modules -unit tested. - -Best Practices --------------- - -Forms -````` - -All forms should work without JavaScript enabled. This means that they must -submit ``application/x-www-form-urlencoded`` data to the server and receive an appropriate -response. The server should check for the ``X-Requested-With: XMLHTTPRequest`` -header to determine if the request is an ajax one. If so it can return an -appropriate format, otherwise it should issue a 303 redirect. - -The one exception to this rule is if a form or button is injected with -JavaScript after the page has loaded. It's then not part of the HTML document -and can submit any data format it pleases. - -Ajax -```````` - -Ajax requests can be used to improve the experience of submitting forms and -other actions that require server interactions. Nearly all requests will -go through the following states. - -1. User clicks button. -2. JavaScript intercepts the click and disables the button (add ``disabled`` - attr). -3. A loading indicator is displayed (add class ``.loading`` to button). -4. The request is made to the server. -5. a) On success the interface is updated. - b) On error a message is displayed to the user if there is no other way to - resolve the issue. -6. The loading indicator is removed. -7. The button is re-enabled. - -Here's a possible example for submitting a search form using jQuery. :: - - jQuery('#search-form').submit(function (event) { - var form = $(this); - var button = form.find('[type=submit]'); - - // Prevent the browser submitting the form. - event.preventDefault(); - - button.prop('disabled', true).addClass('loading'); - - jQuery.ajax({ - type: this.method, - data: form.serialize(), - success: function (results) { - updatePageWithResults(results); - }, - error: function () { - showSearchError('Sorry we were unable to complete this search'); - }, - complete: function () { - button.prop('disabled', false).removeClass('loading'); - } - }); - }); - -This covers possible issues that might arise from submitting the form as well -as providing the user with adequate feedback that the page is doing something. -Disabling the button prevents the form being submitted twice and the error -feedback should hopefully offer a solution for the error that occurred. - -Event Handlers -`````````````` - -When using event handlers to listen for browser events it's a common -requirement to want to cancel the default browser action. This should be -done by calling the ``event.preventDefault()`` method: :: - - jQuery('button').click(function (event) { - event.preventDefault(); - }); - -It is also possible to return ``false`` from the callback function. Avoid doing -this as it also calls the ``event.stopPropagation()`` method which prevents the -event from bubbling up the DOM tree. This prevents other handlers listening -for the same event. For example an analytics click handler attached to the -```` element. - -Also jQuery (1.7+) now provides the `.on()`_ and `.off()`_ methods as -alternatives to ``.bind()``, ``.unbind()``, ``.delegate()`` and -``.undelegate()`` and they should be preferred for all tasks. - -.. _.on(): http://api.jquery.com/on/ -.. _.off(): http://api.jquery.com/off/ - -Templating -`````````` - -Small templates that will not require customisation by the instance can be -placed inline. If you need to create multi-line templates use an array rather -than escaping newlines within a string:: - - var template = [ - '
  • ', - '', - '
  • ' - ].join(''); - -Always localise text strings within your templates. If you are including them -inline this can always be done with jQuery:: - - jQuery(template).find('span').text(_('This is my text string')); - -Larger templates can be loaded in using the CKAN snippet API. Modules get -access to this functionality via the ``sandbox.client`` object:: - - initialize: function () { - var el = this.el; - this.sandbox.client.getTemplate('dataset.html', function (html) { - el.html(html); - }); - } - -The primary benefits of this is that the localisation can be done by the server -and it keeps the JavaScript modules free from large strings. - -HTML Coding Standards -===================== - -Formatting ----------- - -All HTML documents must use **two spaces** for indentation and there should be -no trailing whitespace. XHTML syntax must be used (this is more a Genshi -requirement) and all attributes must use double quotes around attributes. :: - - - - -HTML5 elements should be used where appropriate reserving ``
    `` and ```` -elements for situations where there is no semantic value (such as wrapping -elements to provide styling hooks). - -Doctype and layout ------------------- - -All documents must be using the HTML5 doctype and the ```` element should -have a ``"lang"`` attribute. The ```` should also at a minimum include -``"viewport"`` and ``"charset"`` meta tags. :: - - - - - - - Example Site - - - - -Forms ------ - -Form fields must always include a ``