[#4316] Redirect /dataset/[id] -> /dataset/[name] etc

ckan/controllers/group.py

@@ -225,6 +225,11 @@ def read(self, id, limit=20):
         except (NotFound, NotAuthorized):
             abort(404, _('Group not found'))
 
+        # if the user specified a group id, redirect to the group name
+        if data_dict['id'] == c.group_dict['id']:
+            h.redirect_to(controller=group_type, action='read',
+                          id=c.group_dict['name'])
+
         self._read(id, limit, group_type)
         return render(self._read_template(c.group_dict['type']),
                       extra_vars={'group_type': group_type})

ckan/controllers/package.py

@@ -388,6 +388,11 @@ def read(self, id):
         except (NotFound, NotAuthorized):
             abort(404, _('Dataset not found'))
 
+        # if the user specified a package id, redirect to the package name
+        if data_dict['id'] == c.pkg_dict['id']:
+            h.redirect_to(controller='package', action='read',
+                          id=c.pkg_dict['name'])
+
         # used by disqus plugin
         c.current_package_id = c.pkg.id
 

ckan/tests/controllers/test_group.py

@@ -189,21 +189,28 @@ def test_all_fields_saved(self):
 
 
 class TestGroupRead(helpers.FunctionalTestBase):
-    def setup(self):
-        super(TestGroupRead, self).setup()
-        self.app = helpers._get_test_app()
-        self.user = factories.User()
-        self.user_env = {'REMOTE_USER': self.user['name'].encode('ascii')}
-        self.group = factories.Group(user=self.user)
 
     def test_group_read(self):
-        response = self.app.get(url=url_for(controller='group',
-                                            action='read',
-                                            id=self.group['id']),
-                                status=200,
-                                extra_environ=self.user_env)
-        assert_in(self.group['title'], response)
-        assert_in(self.group['description'], response)
+        group = factories.Group()
+        app = helpers._get_test_app()
+        response = app.get(url=url_for(controller='group',
+                                       action='read',
+                                       id=group['name']))
+        assert_in(group['title'], response)
+        assert_in(group['description'], response)
+
+    def test_read_redirect_when_given_id(self):
+        group = factories.Group()
+        app = helpers._get_test_app()
+        response = app.get(url_for(controller='group', action='read',
+                                   id=group['id']),
+                           status=302)
+        # redirect replaces the ID with the name in the URL
+        redirected_response = response.follow()
+        expected_url = url_for(controller='group', action='read',
+                               id=group['name'])
+        assert_equal(redirected_response.request.path, expected_url)
+
 
 
 class TestGroupDelete(helpers.FunctionalTestBase):

ckan/tests/controllers/test_organization.py

@@ -82,21 +82,35 @@ def test_error_message_shown_when_no_organization_list_permission(self, mock_che
 
 
 class TestOrganizationRead(helpers.FunctionalTestBase):
-    def setup(self):
-        super(TestOrganizationRead, self).setup()
-        self.app = helpers._get_test_app()
-        self.user = factories.User()
-        self.user_env = {'REMOTE_USER': self.user['name'].encode('ascii')}
-        self.organization = factories.Organization(user=self.user)
+    def test_group_read(self):
+        org = factories.Organization()
+        app = helpers._get_test_app()
+        response = app.get(url=url_for(controller='organization',
+                                       action='read',
+                                       id=org['name']))
+        assert_in(org['title'], response)
+        assert_in(org['description'], response)
 
-    def test_organization_read(self):
-        response = self.app.get(url=url_for(controller='organization',
-                                            action='read',
-                                            id=self.organization['id']),
-                                status=200,
-                                extra_environ=self.user_env)
-        assert_in(self.organization['title'], response)
-        assert_in(self.organization['description'], response)
+    def test_read_redirect_when_given_id(self):
+        org = factories.Organization()
+        app = helpers._get_test_app()
+        response = app.get(url_for(controller='organization', action='read',
+                                   id=org['id']),
+                           status=302)
+        # redirect replaces the ID with the name in the URL
+        redirected_response = response.follow()
+        expected_url = url_for(controller='organization', action='read',
+                               id=org['name'])
+        assert_equal(redirected_response.request.path, expected_url)
+
+    # def test_organization_read(self):
+    #     response = self.app.get(url=url_for(controller='organization',
+    #                                         action='read',
+    #                                         id=self.organization['id']),
+    #                             status=200,
+    #                             extra_environ=self.user_env)
+    #     assert_in(self.organization['title'], response)
+    #     assert_in(self.organization['description'], response)
 
 
 class TestOrganizationEdit(helpers.FunctionalTestBase):

ckan/tests/controllers/test_package.py

@@ -9,12 +9,10 @@
     assert_in
 )
 
-from mock import patch, MagicMock
 from ckan.lib.helpers import url_for
 
 import ckan.model as model
 import ckan.plugins as p
-from ckan.lib import search
 
 import ckan.tests.helpers as helpers
 import ckan.tests.factories as factories
@@ -565,6 +563,18 @@ def test_read(self):
         response.mustcontain('Test Dataset')
         response.mustcontain('Just another test dataset')
 
+    def test_read_redirect_when_given_id(self):
+        dataset = factories.Dataset()
+        app = helpers._get_test_app()
+        response = app.get(url_for(controller='package', action='read',
+                                   id=dataset['id']),
+                           status=302)
+        # redirect replaces the ID with the name in the URL
+        redirected_response = response.follow()
+        expected_url = url_for(controller='package', action='read',
+                               id=dataset['name'])
+        assert_equal(redirected_response.request.path, expected_url)
+
     def test_organization_members_can_read_private_datasets(self):
         members = {
             'member': factories.User(),