Merge branch 'master' of github.com:okfn/ckan into 1228-dictize-white…

…space

CONTRIBUTING.rst

@@ -60,12 +60,14 @@ When writing code for CKAN, try to respect our coding standards:
    html-coding-standards
    css-coding-standards
    javascript-coding-standards
-
-* `CKAN Coding Standards <http://docs.ckan.org/en/latest/ckan-coding-standards.html>`_
-* `Python Coding Standards <http://docs.ckan.org/en/latest/python-coding-standards.html>`_
-* `HTML Coding Standards <http://docs.ckan.org/en/latest/html-coding-standards.html>`_
-* `CSS Coding Standards <http://docs.ckan.org/en/latest/css-coding-standards.html>`_
-* `JavaScript Coding Standards <http://docs.ckan.org/en/latest/javascript-coding-standards.html>`_
+   testing-coding-standards
+
+* `CKAN coding standards <http://docs.ckan.org/en/latest/ckan-coding-standards.html>`_
+* `Python coding standards <http://docs.ckan.org/en/latest/python-coding-standards.html>`_
+* `HTML coding standards <http://docs.ckan.org/en/latest/html-coding-standards.html>`_
+* `CSS coding standards <http://docs.ckan.org/en/latest/css-coding-standards.html>`_
+* `JavaScript coding standards <http://docs.ckan.org/en/latest/javascript-coding-standards.html>`_
+* `Testing coding standards <http://docs.ckan.org/en/latest/testing-coding-standards.html>`_
 
 
 ---------------

ckan/ckan_nose_plugin.py

@@ -17,6 +17,10 @@ def startContext(self, ctx):
         # import needs to be here or setup happens too early
         import ckan.model as model
 
+        if 'new_tests' in repr(ctx):
+            # We don't want to do the stuff below for new-style tests.
+            return
+
         if isclass(ctx):
             if hasattr(ctx, "no_db") and ctx.no_db:
                 return
@@ -38,7 +42,7 @@ def startContext(self, ctx):
             from ckan.plugins.interfaces import IConfigurable
             for plugin in PluginImplementations(IConfigurable):
                 plugin.configure(config)
-            
+
             model.repo.init_db()
 
     def options(self, parser, env):

ckan/controllers/admin.py

@@ -34,6 +34,11 @@ def _get_config_form_items(self):
                   {'text': 'Green', 'value': '/base/css/green.css'},
                   {'text': 'Maroon', 'value': '/base/css/maroon.css'},
                   {'text': 'Fuchsia', 'value': '/base/css/fuchsia.css'}]
+
+        homepages = [{'value': '1', 'text': 'Introductory area, search, featured group and featured organization'},
+                     {'value': '2', 'text': 'Search, stats, introductory area, featured organization and featured group'},
+                     {'value': '3', 'text': 'Search, introductory area and stats'}]
+
         items = [
             {'name': 'ckan.site_title', 'control': 'input', 'label': _('Site Title'), 'placeholder': ''},
             {'name': 'ckan.main_css', 'control': 'select', 'options': styles, 'label': _('Style'), 'placeholder': ''},
@@ -42,6 +47,7 @@ def _get_config_form_items(self):
             {'name': 'ckan.site_about', 'control': 'markdown', 'label': _('About'), 'placeholder': _('About page text')},
             {'name': 'ckan.site_intro_text', 'control': 'markdown', 'label': _('Intro Text'), 'placeholder': _('Text on home page')},
             {'name': 'ckan.site_custom_css', 'control': 'textarea', 'label': _('Custom CSS'), 'placeholder': _('Customisable css inserted into the page header')},
+            {'name': 'ckan.homepage_style', 'control': 'select', 'options': homepages, 'label': _('Homepage'), 'placeholder': ''},
         ]
         return items
 

ckan/lib/app_globals.py

@@ -29,6 +29,7 @@
     'ckan.site_about',
     'ckan.site_intro_text',
     'ckan.site_custom_css',
+    'ckan.homepage_style',
 ]
 
 config_details = {
@@ -46,6 +47,7 @@
     'ckan.dumps_format': {},
     'ckan.api_url': {},
     'ofs.impl': {'name': 'ofs_impl'},
+    'ckan.homepage_style': {'default': '1'},
 
     # split string
     'search.facets': {'default': 'organization groups tags res_format license_id',

ckan/lib/dictization/model_dictize.py

@@ -446,6 +446,7 @@ def user_dictize(user, context):
         result_dict = d.table_dictize(user, context)
 
     del result_dict['password']
+    del result_dict['reset_key']
 
     result_dict['display_name'] = user.display_name
     result_dict['email_hash'] = user.email_hash

ckan/lib/helpers.py

@@ -1664,6 +1664,80 @@ def new_activities():
     return action({}, {})
 
 
+def get_featured_organizations(count=1):
+    '''Returns a list of favourite organization in the form
+    of organization_list action function
+    '''
+    config_orgs = config.get('ckan.featured_orgs', '').split()
+    orgs = featured_group_org(get_action='organization_show',
+                              list_action='organization_list',
+                              count=count,
+                              items=config_orgs)
+    return orgs
+
+
+def get_featured_groups(count=1):
+    '''Returns a list of favourite group the form
+    of organization_list action function
+    '''
+    config_groups = config.get('ckan.featured_groups', '').split()
+    groups = featured_group_org(get_action='group_show',
+                                list_action='group_list',
+                                count=count,
+                                items=config_groups)
+    return groups
+
+
+def featured_group_org(items, get_action, list_action, count):
+    def get_group(id):
+        context = {'ignore_auth': True,
+                   'limits': {'packages': 2},
+                   'for_view': True}
+        data_dict = {'id': id}
+
+        try:
+            out = logic.get_action(get_action)(context, data_dict)
+        except logic.ObjectNotFound:
+            return None
+        return out
+
+    groups_data = []
+
+    extras = logic.get_action(list_action)({}, {})
+
+    # list of found ids to prevent duplicates
+    found = []
+    for group_name in items + extras:
+        group = get_group(group_name)
+        if not group:
+            continue
+        # check if duplicate
+        if group['id'] in found:
+            continue
+        found.append(group['id'])
+        groups_data.append(group)
+        if len(groups_data) == count:
+            break
+
+    return groups_data
+
+
+def get_site_statistics():
+    stats = {}
+    stats['dataset_count'] = logic.get_action('package_search')(
+        {}, {"rows": 1})['count']
+    stats['group_count'] = len(logic.get_action('group_list')({}, {}))
+    stats['organization_count'] = len(
+        logic.get_action('organization_list')({}, {}))
+    result = model.Session.execute(
+        '''select count(*) from related r
+           left join related_dataset rd on r.id = rd.related_id
+           where rd.status = 'active' or rd.id is null''').first()[0]
+    stats['related_count'] = result
+
+    return stats
+
+
 # these are the functions that will end up in `h` template helpers
 __allowed_functions__ = [
     # functions defined in ckan.lib.helpers
@@ -1761,4 +1835,7 @@ def new_activities():
     'radio',
     'submit',
     'asbool',
+    'get_featured_organizations',
+    'get_featured_groups',
+    'get_site_statistics',
 ]

ckan/lib/navl/validators.py

@@ -76,7 +76,20 @@ def callable(key, data, errors, context):
     return callable
 
 def ignore_missing(key, data, errors, context):
+    '''If the key is missing from the data, ignore the rest of the key's
+    schema.
 
+    By putting ignore_missing at the start of the schema list for a key,
+    you can allow users to post a dict without the key and the dict will pass
+    validation. But if they post a dict that does contain the key, then any
+    validators after ignore_missing in the key's schema list will be applied.
+
+    :raises ckan.lib.navl.dictization_functions.StopOnError: if ``data[key]``
+        is :py:data:`ckan.lib.navl.dictization_functions.missing` or ``None``
+
+    :returns: ``None``
+
+    '''
     value = data.get(key)
 
     if value is missing or value is None:

ckan/logic/validators.py

@@ -281,20 +281,39 @@ def extras_unicode_convert(extras, context):
     return extras
 
 name_match = re.compile('[a-z0-9_\-]*$')
-def name_validator(val, context):
+def name_validator(value, context):
+    '''Return the given value if it's a valid name, otherwise raise Invalid.
+
+    If it's a valid name, the given value will be returned unmodified.
+
+    This function applies general validation rules for names of packages,
+    groups, users, etc.
+
+    Most schemas also have their own custom name validator function to apply
+    custom validation rules after this function, for example a
+    ``package_name_validator()`` to check that no package with the given name
+    already exists.
+
+    :raises ckan.lib.navl.dictization_functions.Invalid: if ``value`` is not
+        a valid name
+
+    '''
+    if not isinstance(value, basestring):
+        raise Invalid(_('Names must be strings'))
+
     # check basic textual rules
-    if val in ['new', 'edit', 'search']:
+    if value in ['new', 'edit', 'search']:
         raise Invalid(_('That name cannot be used'))
 
-    if len(val) < 2:
+    if len(value) < 2:
         raise Invalid(_('Name must be at least %s characters long') % 2)
-    if len(val) > PACKAGE_NAME_MAX_LENGTH:
+    if len(value) > PACKAGE_NAME_MAX_LENGTH:
         raise Invalid(_('Name must be a maximum of %i characters long') % \
                       PACKAGE_NAME_MAX_LENGTH)
-    if not name_match.match(val):
+    if not name_match.match(value):
         raise Invalid(_('Url must be purely lowercase alphanumeric '
                         '(ascii) characters and these symbols: -_'))
-    return val
+    return value
 
 def package_name_validator(key, data, errors, context):
     model = context["model"]
@@ -480,20 +499,37 @@ def ignore_not_group_admin(key, data, errors, context):
     data.pop(key)
 
 def user_name_validator(key, data, errors, context):
-    model = context["model"]
-    session = context["session"]
-    user = context.get("user_obj")
+    '''Validate a new user name.
 
-    query = session.query(model.User.name).filter_by(name=data[key])
-    if user:
-        user_id = user.id
-    else:
-        user_id = data.get(key[:-1] + ("id",))
-    if user_id and user_id is not missing:
-        query = query.filter(model.User.id <> user_id)
-    result = query.first()
-    if result:
-        errors[key].append(_('That login name is not available.'))
+    Append an error message to ``errors[key]`` if a user named ``data[key]``
+    already exists. Otherwise, do nothing.
+
+    :raises ckan.lib.navl.dictization_functions.Invalid: if ``data[key]`` is
+        not a string
+    :rtype: None
+
+    '''
+    model = context['model']
+    new_user_name = data[key]
+
+    if not isinstance(new_user_name, basestring):
+        raise Invalid(_('User names must be strings'))
+
+    user = model.User.get(new_user_name)
+    if user is not None:
+        # A user with new_user_name already exists in the database.
+
+        user_obj_from_context = context.get('user_obj')
+        if user_obj_from_context and user_obj_from_context.id == user.id:
+            # If there's a user_obj in context with the same id as the user
+            # found in the db, then we must be doing a user_update and not
+            # updating the user name, so don't return an error.
+            return
+        else:
+            # Otherwise return an error: there's already another user with that
+            # name, so you can create a new user with that name or update an
+            # existing user's name to that name.
+            errors[key].append(_('That login name is not available.'))
 
 def user_both_passwords_entered(key, data, errors, context):
 
@@ -507,7 +543,13 @@ def user_both_passwords_entered(key, data, errors, context):
 def user_password_validator(key, data, errors, context):
     value = data[key]
 
-    if not value == '' and not isinstance(value, Missing) and not len(value) >= 4:
+    if isinstance(value, Missing):
+        pass
+    elif not isinstance(value, basestring):
+        errors[('password',)].append(_('Passwords must be strings'))
+    elif value == '':
+        pass
+    elif len(value) < 4:
         errors[('password',)].append(_('Your password must be 4 characters or longer'))
 
 def user_passwords_match(key, data, errors, context):

ckan/new_tests/controllers/__init__.py

@@ -0,0 +1,53 @@
+'''
+Controller tests probably shouldn't use mocking.
+
+.. todo::
+
+   Write the tests for one controller, figuring out the best way to write
+   controller tests. Then fill in this guidelines section, using the first set
+   of controller tests as an example.
+
+   Some things have been decided already:
+
+   * All controller methods should have tests
+
+   * Controller tests should be high-level tests that work by posting simulated
+     HTTP requests to CKAN URLs and testing the response. So the controller
+     tests are also testing CKAN's templates and rendering - these are CKAN's
+     front-end tests.
+
+     For example, maybe we use a webtests testapp and then use beautiful soup
+     to parse the HTML?
+
+   * In general the tests for a controller shouldn't need to be too detailed,
+     because there shouldn't be a lot of complicated logic and code in
+     controller classes. The logic should be handled in other places such as
+     :mod:`ckan.logic` and :mod:`ckan.lib`, where it can be tested easily and
+     also shared with other code.
+
+   * The tests for a controller should:
+
+     * Make sure that the template renders without crashing.
+
+     * Test that the page contents seem basically correct, or test certain
+       important elements in the page contents (but don't do too much HTML
+       parsing).
+
+     * Test that submitting any forms on the page works without crashing and
+       has the expected side-effects.
+
+     * When asserting side-effects after submitting a form, controller tests
+       should user the :func:`ckan.new_tests.helpers.call_action` function. For
+       example after creating a new user by submitting the new user form, a
+       test could call the :func:`~ckan.logic.action.get.user_show` action
+       function to verify that the user was created with the correct values.
+
+.. warning::
+
+   Some CKAN controllers *do* contain a lot of complicated logic code.  These
+   controllers should be refactored to move the logic into :mod:`ckan.logic` or
+   :mod:`ckan.lib` where it can be tested easily.  Unfortunately in cases like
+   this it may be necessary to write a lot of controller tests to get this
+   code's behavior into a test harness before it can be safely refactored.
+
+'''