From bbe459527f280e5f611bd34228a069025cfeb829 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Adri=C3=A0=20Mercader?= <amercadero@gmail.com>
Date: Fri, 13 May 2011 14:17:58 +0100
Subject: [PATCH] [forms] Major refactoring of the harvest forms. Forms no
 longer use the DGU form API, and are handled similarly to the new ones on
 CKAN core (logic, schema, validators...). The UI is also more consistent with
 the CKAN one.

---
 ckanext/harvest/controllers/view.py           | 215 ++++++++----------
 ckanext/harvest/lib/__init__.py               | 112 +++++----
 ckanext/harvest/logic/__init__.py             |   7 +
 ckanext/harvest/logic/schema.py               |  33 +++
 ckanext/harvest/logic/validators.py           |  74 ++++++
 ckanext/harvest/model/__init__.py             |   6 +-
 ckanext/harvest/plugin.py                     |  34 +--
 .../templates/ckanext/harvest/create.html     |  28 ---
 .../{ckanext/harvest => }/index.html          |  10 +-
 ckanext/harvest/templates/source/edit.html    |  23 ++
 ckanext/harvest/templates/source/new.html     |  22 ++
 .../templates/source/new_source_form.html     |  47 ++++
 .../harvest/show.html => source/read.html}    |   2 +-
 13 files changed, 395 insertions(+), 218 deletions(-)
 create mode 100644 ckanext/harvest/logic/__init__.py
 create mode 100644 ckanext/harvest/logic/schema.py
 create mode 100644 ckanext/harvest/logic/validators.py
 delete mode 100644 ckanext/harvest/templates/ckanext/harvest/create.html
 rename ckanext/harvest/templates/{ckanext/harvest => }/index.html (82%)
 create mode 100644 ckanext/harvest/templates/source/edit.html
 create mode 100644 ckanext/harvest/templates/source/new.html
 create mode 100644 ckanext/harvest/templates/source/new_source_form.html
 rename ckanext/harvest/templates/{ckanext/harvest/show.html => source/read.html} (98%)

diff --git a/ckanext/harvest/controllers/view.py b/ckanext/harvest/controllers/view.py
index 69b6319..a2b0d6c 100644
--- a/ckanext/harvest/controllers/view.py
+++ b/ckanext/harvest/controllers/view.py
@@ -1,21 +1,21 @@
-import urllib2
-
 from pylons.i18n import _
 
 import ckan.lib.helpers as h, json
 from ckan.lib.base import BaseController, c, g, request, \
                           response, session, render, config, abort, redirect
 
-from ckan.model import Package
-
-from ckanext.harvest.lib import *
+from ckan.lib.navl.dictization_functions import DataError
+from ckan.logic import NotFound, ValidationError
+from ckanext.harvest.logic.schema import harvest_source_form_schema
+from ckanext.harvest.lib import create_harvest_source, edit_harvest_source, \
+                                get_harvest_source, get_harvest_sources, \
+                                create_harvest_job, get_registered_harvesters_types
+    
+import logging
+log = logging.getLogger(__name__)
 
 class ViewController(BaseController):
 
-    api_url = config.get('ckan.api_url', 'http://localhost:5000').rstrip('/')+'/api/2/rest'
-    form_api_url = config.get('ckan.api_url', 'http://localhost:5000').rstrip('/')+'/api/2/form'
-    api_key = config.get('ckan.harvest.api_key')
-
     def __before__(self, action, **env):
         super(ViewController, self).__before__(action, **env)
         # All calls to this controller must be with a sysadmin key
@@ -24,138 +24,123 @@ class ViewController(BaseController):
             status = 401
             abort(status, response_msg)
 
-    def _do_request(self,url,data = None):
-
-        http_request = urllib2.Request(
-            url = url,
-            headers = {'Authorization' : self.api_key}
-        )
-
-        if data:
-            http_request.add_data(data)
-
-        try:
-            return urllib2.urlopen(http_request)
-        except urllib2.HTTPError as e:
-            raise
-
     def index(self):
         # Request all harvest sources
         c.sources = get_harvest_sources()
 
-        return render('ckanext/harvest/index.html')
+        return render('index.html')
 
-    def create(self):
+    def new(self,data = None,errors = None, error_summary = None):
 
-        # This is the DGU form API, so we don't use self.api_url
-        form_url = self.form_api_url + '/harvestsource/create'
-        if request.method == 'GET':
+        if ('save' in request.params) and not data:
+            return self._save_new()
+
+        data = data or {}
+        errors = errors or {}
+        error_summary = error_summary or {}
+        #TODO: Use new description interface to build the types select and descriptions
+        vars = {'data': data, 'errors': errors, 'error_summary': error_summary, 'types': get_registered_harvesters_types()}
+        
+        c.form = render('source/new_source_form.html', extra_vars=vars)
+        return render('source/new.html')
+
+    def _save_new(self):
+        try:
+            data_dict = dict(request.params)
+            self._check_data_dict(data_dict)
+
+            source = create_harvest_source(data_dict)
+
+            # Create a harvest job for the new source
+            create_harvest_job(source['id'])
+
+            h.flash_success(_('New harvest source added successfully.'
+                    'A new harvest job for the source has also been created.'))
+            redirect(h.url_for('harvest'))
+        except DataError,e:
+            abort(400, 'Integrity Error')
+        except ValidationError,e:
+            errors = e.error_dict
+            error_summary = e.error_summary if 'error_summary' in e else None
+            return self.new(data_dict, errors, error_summary)
+
+    def edit(self, id, data = None,errors = None, error_summary = None):
+
+        if ('save' in request.params) and not data:
+            return self._save_edit(id)
+
+        if not data:
             try:
-                # Request the fields
-                c.form = self._do_request(form_url).read()
-                c.mode = 'create'
-            except urllib2.HTTPError as e:
-                msg = 'An error occurred: [%s %s]' % (str(e.getcode()),e.msg)
-                h.flash_error(msg)
-            return render('ckanext/harvest/create.html')
-        if request.method == 'POST':
-            # Build an object like the one expected by the DGU form API
-            data = {
-                'form_data':
-                    {'HarvestSource--url': request.POST['HarvestSource--url'],
-                     'HarvestSource--description': request.POST['HarvestSource--description'],
-                     'HarvestSource--type': request.POST['HarvestSource--type'],
-                    },
-                'user_id':'',
-                'publisher_id':''
-            }
-            data = json.dumps(data)
-            try:
-                rq = self._do_request(form_url,data)
+                old_data = get_harvest_source(id)
+            except NotFound:
+                abort(404, _('Harvest Source not found'))
 
-                h.flash_success('Harvesting source added successfully')
-                redirect(h.url_for('harvest'))
+        data = data or old_data
+        errors = errors or {}
+        error_summary = error_summary or {}
+        #TODO: Use new description interface to build the types select and descriptions
+        vars = {'data': data, 'errors': errors, 'error_summary': error_summary, 'types': get_registered_harvesters_types()}
+        
+        c.form = render('source/new_source_form.html', extra_vars=vars)
+        return render('source/edit.html')
 
-            except urllib2.HTTPError as e:
-                msg = 'An error occurred: [%s %s]' % (str(e.getcode()),e.msg)
-                # The form API returns just a 500, so we are not exactly sure of what
-                # happened, but most probably it was a duplicate entry
-                if e.getcode() == 500:
-                    msg = msg + ' Does the source already exist?'
-                elif e.getcode() == 400:
-                    err_msg = e.read()
-                    if '<form' in c.form:
-                        c.form = err_msg
-                        c.mode = 'create'
-                        return render('ckanext/harvest/create.html')
-                    else:
-                        msg = err_msg
+    def _save_edit(self,id):
+        try:
+            data_dict = dict(request.params)
+            self._check_data_dict(data_dict)
 
-                h.flash_error(msg)
-                redirect(h.url_for('harvest'))
+            source = edit_harvest_source(id,data_dict)
 
-    def show(self,id):
+            h.flash_success(_('Harvest source edited successfully.'))
+            redirect(h.url_for('harvest'))
+        except DataError,e:
+            abort(400, _('Integrity Error'))
+        except NotFound, e:
+            abort(404, _('Harvest Source not found'))
+        except ValidationError,e:
+            errors = e.error_dict
+            error_summary = e.error_summary if 'error_summary' in e else None
+            return self.edit(id,data_dict, errors, error_summary)
+
+    def _check_data_dict(self, data_dict):
+        '''Check if the return data is correct'''
+        surplus_keys_schema = ['id','publisher_id','user_id','active','save']
+
+        schema_keys = harvest_source_form_schema().keys()
+        keys_in_schema = set(schema_keys) - set(surplus_keys_schema)
+
+        if keys_in_schema - set(data_dict.keys()):
+            log.info(_('Incorrect form fields posted'))
+            raise DataError(data_dict)
+
+    def read(self,id):
         try:
             c.source = get_harvest_source(id)
 
-            return render('ckanext/harvest/show.html')
-        except:
-            abort(404,'Harvest source not found')
+            return render('source/read.html')
+        except NotFound:
+            abort(404,_('Harvest source not found'))
 
 
     def delete(self,id):
         try:
             delete_harvest_source(id)
-            h.flash_success('Harvesting source deleted successfully')
-        except Exception as e:
-            msg = 'An error occurred: [%s]' % e.message
-            h.flash_error(msg)
 
-        redirect(h.url_for('harvest'))
+            h.flash_success(_('Harvesting source deleted successfully'))
+            redirect(h.url_for('harvest'))
+        except NotFound:
+            abort(404,_('Harvest source not found'))
 
-    def edit(self,id):
-
-        form_url = self.form_api_url + '/harvestsource/edit/%s' % id
-        if request.method == 'GET':
-            # Request the fields
-            c.form = self._do_request(form_url).read()
-            c.mode = 'edit'
-
-            return render('ckanext/harvest/create.html')
-        if request.method == 'POST':
-            # Build an object like the one expected by the DGU form API
-            data = {
-                'form_data':
-                    {'HarvestSource-%s-url' % id: request.POST['HarvestSource-%s-url' % id] ,
-                     'HarvestSource-%s-type' % id: request.POST['HarvestSource-%s-type' % id],
-                     'HarvestSource-%s-description' % id: request.POST['HarvestSource-%s-description' % id]},
-                'user_id':'',
-                'publisher_id':''
-            }
-            data = json.dumps(data)
-            try:
-                r = self._do_request(form_url,data)
-
-                h.flash_success('Harvesting source edited successfully')
-
-                redirect(h.url_for('harvest'))
-            except urllib2.HTTPError as e:
-                if e.getcode() == 400:
-                    c.form = e.read()
-                    c.mode = 'edit'
-                    return render('ckanext/harvest/create.html')
-                else:
-                    msg = 'An error occurred: [%s %s]' % (str(e.getcode()),e.msg)
-                    h.flash_error(msg)
-                    redirect(h.url_for('harvest'))
 
     def create_harvesting_job(self,id):
         try:
             create_harvest_job(id)
-            h.flash_success('Refresh requested, harvesting will take place within 15 minutes.')
+            h.flash_success(_('Refresh requested, harvesting will take place within 15 minutes.'))
+            redirect(h.url_for('harvest'))
+        except NotFound:
+            abort(404,_('Harvest source not found'))
         except Exception as e:
             msg = 'An error occurred: [%s]' % e.message
             h.flash_error(msg)
-
-        redirect(h.url_for('harvest'))
+            redirect(h.url_for('harvest'))
 
diff --git a/ckanext/harvest/lib/__init__.py b/ckanext/harvest/lib/__init__.py
index 5a627ee..b8c9f75 100644
--- a/ckanext/harvest/lib/__init__.py
+++ b/ckanext/harvest/lib/__init__.py
@@ -1,14 +1,22 @@
 import urlparse
+import re
+
 from sqlalchemy import distinct,func
 from ckan.model import Session, repo
 from ckan.model import Package
+from ckan.lib.navl.dictization_functions import validate 
+from ckan.logic import NotFound, ValidationError
+
+from ckanext.harvest.logic.schema import harvest_source_form_schema
+
 from ckan.plugins import PluginImplementations
 from ckanext.harvest.model import HarvestSource, HarvestJob, HarvestObject, \
                                   HarvestGatherError, HarvestObjectError
 from ckanext.harvest.queue import get_gather_publisher
 from ckanext.harvest.interfaces import IHarvester
 
-log = __import__("logging").getLogger(__name__)
+import logging
+log = logging.getLogger(__name__)
 
 
 def _get_source_status(source):
@@ -183,49 +191,68 @@ def _normalize_url(url):
 
     return check_url
 
-def get_harvest_source(id,default=Exception,attr=None):
-    source = HarvestSource.get(id,default=default,attr=attr)
-    if source:
-        return _source_as_dict(source)
-    else:
-        return default
+def _prettify(field_name):
+    field_name = re.sub('(?<!\w)[Uu]rl(?!\w)', 'URL', field_name.replace('_', ' ').capitalize())
+    return field_name.replace('_', ' ')
+
+def _error_summary(error_dict):
+
+    error_summary = {}
+    for key, error in error_dict.iteritems():
+        error_summary[_prettify(key)] = error[0]
+    return error_summary
+
+def get_harvest_source(id,attr=None):
+    source = HarvestSource.get(id,attr=attr)
+
+    if not source:
+        raise NotFound
+
+    return _source_as_dict(source)
 
 def get_harvest_sources(**kwds):
     sources = HarvestSource.filter(**kwds).all()
     return [_source_as_dict(source) for source in sources]
 
-def create_harvest_source(source_dict):
-    if not 'url' in source_dict or not source_dict['url'] or \
-        not 'type' in source_dict or not source_dict['type']:
-        raise Exception('Missing mandatory properties: url, type')
+def create_harvest_source(data_dict):
 
-    # Check if source already exists
-    existing_source = _url_exists(source_dict['url'])
-    if existing_source:
-        raise Exception('There already is an active Harvest Source for this URL: %s' % source_dict['url'])
+    schema = harvest_source_form_schema()
+    data, errors = validate(data_dict, schema)
+
+    if errors:
+        Session.rollback()
+        raise ValidationError(errors,_error_summary(errors))
 
     source = HarvestSource()
-    source.url = source_dict['url']
-    source.type = source_dict['type']
+    source.url = data['url']
+    source.type = data['type']
+
     opt = ['active','description','user_id','publisher_id']
     for o in opt:
-        if o in source_dict and source_dict[o] is not None:
-            source.__setattr__(o,source_dict[o])
+        if o in data and data[o] is not None:
+            source.__setattr__(o,data[o])
 
     source.save()
 
-
     return _source_as_dict(source)
 
-def edit_harvest_source(source_id,source_dict):
-    try:
-        source = HarvestSource.get(source_id)
-    except:
-        raise Exception('Source %s does not exist' % source_id)
+def edit_harvest_source(source_id,data_dict):
+    schema = harvest_source_form_schema()
+
+    source = HarvestSource.get(source_id)
+    
+    # Add source id to the dict, as some validators will need it
+    data_dict["id"] = source.id
+
+    data, errors = validate(data_dict, schema)
+    if errors:
+        Session.rollback()
+        raise ValidationError(errors,_error_summary(errors))
+
     fields = ['url','type','active','description','user_id','publisher_id']
     for f in fields:
-        if f in source_dict and source_dict[f] is not None and source_dict[f] != '':
-            source.__setattr__(f,source_dict[f])
+        if f in data_dict and data_dict[f] is not None and data_dict[f] != '':
+            source.__setattr__(f,data_dict[f])
 
     source.save()
 
@@ -251,12 +278,12 @@ def remove_harvest_source(source_id):
 
     return True
 
-def get_harvest_job(id,default=Exception,attr=None):
-    job = HarvestJob.get(id,default=default,attr=attr)
-    if job:
-        return _job_as_dict(job)
-    else:
-        return default
+def get_harvest_job(id,attr=None):
+    job = HarvestJob.get(id,attr=attr)
+    if not job:
+        raise NotFound
+
+    return _job_as_dict(job)
 
 def get_harvest_jobs(**kwds):
     jobs = HarvestJob.filter(**kwds).all()
@@ -304,13 +331,13 @@ def run_harvest_jobs():
     publisher.close()
     return sent_jobs
 
-def get_harvest_object(id,default=Exception,attr=None):
-    obj = HarvestObject.get(id,default=default,attr=attr)
-    if obj:
-        return _object_as_dict(obj)
-    else:
-        return default
+def get_harvest_object(id,attr=None):
+    obj = HarvestObject.get(id,attr=attr)
+    if not obj:
+        raise NotFound
 
+    return _object_as_dict(obj)
+    
 def get_harvest_objects(**kwds):
     objects = HarvestObject.filter(**kwds).all()
     return [_object_as_dict(obj) for obj in objects]
@@ -351,3 +378,10 @@ def import_last_objects(source_id=None):
         last_obj_guid = obj.guid
 
     return imported_objects
+
+def get_registered_harvesters_types():
+    # TODO: Use new description interface when implemented
+    available_types = []
+    for harvester in PluginImplementations(IHarvester):
+        available_types.append(harvester.get_type())
+    return available_types
diff --git a/ckanext/harvest/logic/__init__.py b/ckanext/harvest/logic/__init__.py
new file mode 100644
index 0000000..d0ed2fc
--- /dev/null
+++ b/ckanext/harvest/logic/__init__.py
@@ -0,0 +1,7 @@
+try:
+    import pkg_resources
+    pkg_resources.declare_namespace(__name__)
+except ImportError:
+    import pkgutil
+    __path__ = pkgutil.extend_path(__path__, __name__)
+
diff --git a/ckanext/harvest/logic/schema.py b/ckanext/harvest/logic/schema.py
new file mode 100644
index 0000000..8b76403
--- /dev/null
+++ b/ckanext/harvest/logic/schema.py
@@ -0,0 +1,33 @@
+from ckan.lib.navl.validators import (ignore_missing,
+                                      not_empty,
+                                      empty,
+                                      ignore,
+                                      not_missing
+                                     )
+
+from ckanext.harvest.logic.validators import harvest_source_id_exists, \
+                                            harvest_source_url_validator, \
+                                            harvest_source_type_exists
+
+def default_harvest_source_schema():
+
+    schema = {
+        'id': [ignore_missing, unicode, harvest_source_id_exists],
+        'url': [not_empty, unicode, harvest_source_url_validator],
+        'type': [not_empty, unicode, harvest_source_type_exists],
+        'description': [ignore_missing],
+        'active': [ignore_missing],
+        'user_id': [ignore_missing],
+        'publisher_id': [ignore_missing],
+        #'config'
+    }
+
+    return schema
+
+
+def harvest_source_form_schema():
+
+    schema = default_harvest_source_schema()
+    schema['save'] = [ignore]
+
+    return schema
diff --git a/ckanext/harvest/logic/validators.py b/ckanext/harvest/logic/validators.py
new file mode 100644
index 0000000..85c1454
--- /dev/null
+++ b/ckanext/harvest/logic/validators.py
@@ -0,0 +1,74 @@
+import urlparse
+
+from ckan.lib.navl.dictization_functions import Invalid, missing
+from ckan.model import Session
+from ckan.plugins import PluginImplementations
+
+from ckanext.harvest.model import HarvestSource
+from ckanext.harvest.interfaces import IHarvester
+ 
+
+#TODO: use context?
+
+def harvest_source_id_exists(value, context):
+    
+    result = HarvestSource.get(value,None)
+
+    if not result:
+        raise Invalid('Harvest Source with id %r does not exist.' % str(value))
+    return value
+
+def _normalize_url(url):
+    o = urlparse.urlparse(url)
+
+    # Normalize port
+    if ':' in o.netloc:
+        parts = o.netloc.split(':')
+        if (o.scheme == 'http' and parts[1] == '80') or \
+           (o.scheme == 'https' and parts[1] == '443'):
+            netloc = parts[0]
+        else:
+            netloc = ':'.join(parts)
+    else:
+        netloc = o.netloc
+    
+    # Remove trailing slash
+    path = o.path.rstrip('/')
+
+    check_url = urlparse.urlunparse((
+            o.scheme,
+            netloc,
+            path,
+            None,None,None))
+
+    return check_url
+
+def harvest_source_url_validator(key,data,errors,context):
+    new_url = _normalize_url(data[key])
+    source_id = data.get(('id',),'')
+    if source_id:
+        # When editing a source we need to avoid its own URL
+        existing_sources = Session.query(HarvestSource.url,HarvestSource.active) \
+                       .filter(HarvestSource.id!=source_id).all()
+    else:
+        existing_sources = Session.query(HarvestSource.url,HarvestSource.active).all()
+
+    for url,active in existing_sources:
+        url = _normalize_url(url)
+        if url == new_url and active == True:
+            raise Invalid('There already is an active Harvest Source for this URL: %s' % data[key])
+
+    return data[key] 
+
+def harvest_source_type_exists(value,context):
+    #TODO: use new description interface
+
+    # Get all the registered harvester types
+    available_types = []
+    for harvester in PluginImplementations(IHarvester):
+        available_types.append(harvester.get_type())
+
+    if not value in available_types:
+        raise Invalid('Unknown harvester type: %s. Have you registered a harvester for this type?' % value)
+    
+    return value
diff --git a/ckanext/harvest/model/__init__.py b/ckanext/harvest/model/__init__.py
index 6dff685..0812b9d 100644
--- a/ckanext/harvest/model/__init__.py
+++ b/ckanext/harvest/model/__init__.py
@@ -31,7 +31,7 @@ class HarvestDomainObject(DomainObject):
     key_attr = 'id'
 
     @classmethod
-    def get(self, key, default=Exception, attr=None):
+    def get(self, key, default=None, attr=None):
         '''Finds a single entity in the register.'''
         if attr == None:
             attr = self.key_attr
@@ -39,10 +39,8 @@ class HarvestDomainObject(DomainObject):
         o = self.filter(**kwds).first()
         if o:
             return o
-        if default != Exception:
-            return default
         else:
-            raise Exception('%s not found: %s' % (self.__name__, key))
+            return default
 
     @classmethod
     def filter(self, **kwds):
diff --git a/ckanext/harvest/plugin.py b/ckanext/harvest/plugin.py
index 86de235..e909fd1 100644
--- a/ckanext/harvest/plugin.py
+++ b/ckanext/harvest/plugin.py
@@ -22,35 +22,17 @@ class Harvest(SingletonPlugin):
         pass
 
     def before_map(self, map):
-        map.connect('harvest', '/harvest',
-            controller='ckanext.harvest.controllers.view:ViewController',
-            action='index')
-            
-        map.connect('harvest_create_form', '/harvest/create',
-            controller='ckanext.harvest.controllers.view:ViewController',
-            conditions=dict(method=['GET']),
-            action='create')
 
-        map.connect('harvest_create', '/harvest/create',
-            controller='ckanext.harvest.controllers.view:ViewController',
-            conditions=dict(method=['POST']),
-            action='create')
+        controller = 'ckanext.harvest.controllers.view:ViewController'
+        map.connect('harvest', '/harvest',controller=controller,action='index')
 
-        map.connect('harvest_show', '/harvest/:id',
-            controller='ckanext.harvest.controllers.view:ViewController',
-            action='show')
+        map.connect('/harvest/new', controller=controller, action='new')
+        map.connect('/harvest/edit/:id', controller=controller, action='edit') 
+        map.connect('/harvest/delete/:id',controller=controller, action='delete')
+        map.connect('/harvest/:id', controller=controller, action='read')
 
-        map.connect('harvest_edit', '/harvest/:id/edit',
-            controller='ckanext.harvest.controllers.view:ViewController',
-            action='edit')
-
-        map.connect('harvest_delete', '/harvest/:id/delete',
-            controller='ckanext.harvest.controllers.view:ViewController',
-            action='delete')
-
-        map.connect('harvesting_job_create', '/harvest/:id/refresh',
-            controller='ckanext.harvest.controllers.view:ViewController',
-            action='create_harvesting_job')
+        map.connect('harvesting_job_create', '/harvest/refresh/:id',controller=controller, 
+                action='create_harvesting_job')
       
         return map
 
diff --git a/ckanext/harvest/templates/ckanext/harvest/create.html b/ckanext/harvest/templates/ckanext/harvest/create.html
deleted file mode 100644
index 374be13..0000000
--- a/ckanext/harvest/templates/ckanext/harvest/create.html
+++ /dev/null
@@ -1,28 +0,0 @@
-<?python
-    if c.mode == 'create':
-        title = 'Add harvesting source'
-    else:
-        title = 'Edit harvesting source'
-?>
-<html xmlns:py="http://genshi.edgewall.org/"
-  xmlns:i18n="http://genshi.edgewall.org/i18n"
-  xmlns:xi="http://www.w3.org/2001/XInclude"
-  py:strip="">
-
-  <py:def function="page_title">${title}</py:def>
-
-  <py:def function="optional_head">
-    <link type="text/css" rel="stylesheet" media="all" href="/ckanext/harvest/style.css" />
-  </py:def>
-  
-<div py:match="content">
-    <div class="harvest-content">
-        <h1>${title}</h1>
-        <form action="${c.mode}" method="POST">
-        ${Markup(c.form)}
-            <input id="save" name="save" value="Save" type="submit" />
-        </form>
-    </div>
-</div>
-<xi:include href="../../layout.html" />
-</html>
diff --git a/ckanext/harvest/templates/ckanext/harvest/index.html b/ckanext/harvest/templates/index.html
similarity index 82%
rename from ckanext/harvest/templates/ckanext/harvest/index.html
rename to ckanext/harvest/templates/index.html
index 52ffb5a..e59ee50 100644
--- a/ckanext/harvest/templates/ckanext/harvest/index.html
+++ b/ckanext/harvest/templates/index.html
@@ -12,7 +12,7 @@
 <div py:match="content">
   <div class="harvest-content">
   <h1>Harvesting Sources</h1>
-    <a id="new-harvest-source" href="harvest/create">Add a harvesting source</a>
+    <a id="new-harvest-source" href="harvest/new">Add a harvesting source</a>
     <py:choose>
         <py:when test="c.sources">
 
@@ -31,9 +31,9 @@
         </tr>
 
         <tr py:for="source in c.sources">
-            <td>${h.link_to('view', 'harvest/' + source.id)}</td>
-            <td>${h.link_to('edit', 'harvest/' + source.id + '/edit')}</td>
-            <td>${h.link_to('refresh', 'harvest/' + source.id + '/refresh')}</td>
+            <td>${h.link_to('view', 'harvest/%s' % source.id)}</td>
+            <td>${h.link_to('edit', 'harvest/edit/%s' % source.id)}</td>
+            <td>${h.link_to('refresh', 'harvest/refresh/%s' % source.id)}</td>
             <td>${source.url}</td>
             <td>${source.type}</td>
             <td>${source.active}</td>
@@ -59,5 +59,5 @@
  
   </div>
 </div>
-<xi:include href="../../layout.html" />
+<xi:include href="layout.html" />
 </html>
diff --git a/ckanext/harvest/templates/source/edit.html b/ckanext/harvest/templates/source/edit.html
new file mode 100644
index 0000000..96465b5
--- /dev/null
+++ b/ckanext/harvest/templates/source/edit.html
@@ -0,0 +1,23 @@
+<html xmlns:py="http://genshi.edgewall.org/"
+  xmlns:i18n="http://genshi.edgewall.org/i18n"
+  xmlns:xi="http://www.w3.org/2001/XInclude"
+  py:strip="">
+
+    <py:def function="page_title">Edit - Harvest Source</py:def>
+
+    <py:def function="body_class">hide-sidebar</py:def>
+    <py:def function="optional_head">
+        <link rel="stylesheet" href="${g.site_url}/css/forms.css" type="text/css" media="screen, print" />
+    </py:def>
+
+    <div py:match="content">
+        <div class="harvest-content">
+            <h2>Edit harvest source </h2>
+
+
+            ${h.literal(c.form)}
+
+        </div>
+    </div>
+    <xi:include href="../layout.html" />
+    </html>
diff --git a/ckanext/harvest/templates/source/new.html b/ckanext/harvest/templates/source/new.html
new file mode 100644
index 0000000..16315fa
--- /dev/null
+++ b/ckanext/harvest/templates/source/new.html
@@ -0,0 +1,22 @@
+<html xmlns:py="http://genshi.edgewall.org/"
+  xmlns:i18n="http://genshi.edgewall.org/i18n"
+  xmlns:xi="http://www.w3.org/2001/XInclude"
+  py:strip="">
+
+    <py:def function="page_title">New - Harvest Source</py:def>
+
+    <py:def function="body_class">hide-sidebar</py:def>
+    <py:def function="optional_head">
+        <link rel="stylesheet" href="${g.site_url}/css/forms.css" type="text/css" media="screen, print" />
+    </py:def>
+
+    <div py:match="content">
+        <div class="harvest-content">
+            <h2>New harvest source </h2>
+
+            ${h.literal(c.form)}
+
+        </div>
+    </div>
+    <xi:include href="../layout.html" />
+    </html>
diff --git a/ckanext/harvest/templates/source/new_source_form.html b/ckanext/harvest/templates/source/new_source_form.html
new file mode 100644
index 0000000..f404dde
--- /dev/null
+++ b/ckanext/harvest/templates/source/new_source_form.html
@@ -0,0 +1,47 @@
+<form id="source-new" class="ckan" method="post"
+  py:attrs="{'class':'has-errors'} if errors else {}"
+  xmlns:i18n="http://genshi.edgewall.org/i18n"
+  xmlns:py="http://genshi.edgewall.org/"
+  xmlns:xi="http://www.w3.org/2001/XInclude">
+
+  <div class="error-explanation" py:if="error_summary">
+<h2>Errors in form</h2>
+<p>The form contains invalid entries:</p>
+<ul>
+  <li py:for="key, error in error_summary.items()">${"%s: %s" % (key, error)}</li>
+</ul>
+</div>
+
+    <fieldset> 
+    <legend>Details</legend> 
+        <dl> 
+            <dt><label class="field_req" for="url">URL for source of metadata *</label></dt> 
+            <dd><input id="url" name="url" size="80" type="text" value="${data.get('url', '')}" /></dd>
+            <dd class="field_error" py:if="errors.get('url', '')">${errors.get('url', '')}</dd>
+            <dd class="instructions basic">This should include the <tt>http://</tt> part of the URL</dd> 
+            <dt><label class="field_req" for="type">Source Type *</label></dt> 
+            <dd> 
+                <select id="type" name="type">
+                    <py:for each="type in types">
+                     <option value="${type}" py:attrs="{'selected': 'selected' if data.get('type', '') == type else None}" >${type}</option>
+                    </py:for>
+                    <option value="FAKW">FAKW</option>
+                </select>
+            </dd>
+            <dd class="field_error" py:if="errors.get('type', '')">${errors.get('type', '')}</dd>
+            <dd class="instructions basic">Which type of source does the URL above represent?
+                TODO: get these from the harvesters
+                <ul> 
+                    <li>A server's CSW interface</li> 
+                    <li>A Web Accessible Folder (WAF) displaying a list of GEMINI 2.1 documents</li> 
+                    <li>A single GEMINI 2.1 document</li> 
+                </ul>         
+            </dd> 
+            <dt><label class="field_opt" for="description">Description</label></dt> 
+            <dd><textarea id="description" name="description" cols="30" rows="2" style="height:75px">${data.get('description', '')}</textarea></dd> 
+            <dd class="instructions basic">You can add your own notes here about what the URL above represents to remind you later.</dd> 
+        </dl> 
+    </fieldset> 
+    <input id="save" name="save" value="Save" type="submit" /> 
+
+</form>
diff --git a/ckanext/harvest/templates/ckanext/harvest/show.html b/ckanext/harvest/templates/source/read.html
similarity index 98%
rename from ckanext/harvest/templates/ckanext/harvest/show.html
rename to ckanext/harvest/templates/source/read.html
index 76aefcd..665fff7 100644
--- a/ckanext/harvest/templates/ckanext/harvest/show.html
+++ b/ckanext/harvest/templates/source/read.html
@@ -89,5 +89,5 @@
     </py:if>
   </div>
 </div>
-<xi:include href="../../layout.html" />
+<xi:include href="../layout.html" />
 </html>