Skip to content

Commit

Permalink
More revision removals and fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
David Read committed Feb 22, 2019
1 parent e6401d1 commit aacd9e2
Show file tree
Hide file tree
Showing 9 changed files with 24 additions and 342 deletions.
16 changes: 8 additions & 8 deletions ckan/lib/activity_streams_session_extension.py
Expand Up @@ -8,10 +8,10 @@
log = logging.getLogger(__name__)


def activity_stream_item(obj, activity_type, revision, user_id):
def activity_stream_item(obj, activity_type, user_id):
method = getattr(obj, "activity_stream_item", None)
if callable(method):
return method(activity_type, revision, user_id)
return method(activity_type, user_id)
else:
# Object did not have a suitable activity_stream_item() method; it must
# not be a package
Expand Down Expand Up @@ -41,13 +41,13 @@ def before_commit(self, session):

try:
object_cache = session._object_cache
revision = session.revision
except AttributeError:
# session had no _object_cache or no revision; skipping this commit
# session had no _object_cache; skipping this commit
return

if revision.user:
user_id = revision.user.id
user = 'DUNNO' # TODO!!
if user:
user_id = user
else:
# If the user is not logged in then revision.user is None and
# revision.author is their IP address. Just log them as 'not logged
Expand All @@ -63,7 +63,7 @@ def before_commit(self, session):
# logged as changed packages.
# Looking for new packages...
for obj in object_cache['new']:
activity = activity_stream_item(obj, 'new', revision, user_id)
activity = activity_stream_item(obj, 'new', user_id)
if activity is None:
continue
# The object returns an activity stream item, so we know that the
Expand Down Expand Up @@ -113,7 +113,7 @@ def before_commit(self, session):
continue

activity = activity_stream_item(
package, "changed", revision, user_id)
package, "changed", user_id)
if activity is None:
continue
activities[package.id] = activity
Expand Down
6 changes: 4 additions & 2 deletions ckan/logic/schema.py
Expand Up @@ -676,8 +676,9 @@ def default_create_resource_view_schema(resource_view):

@validator_args
def default_create_resource_view_schema_unfiltered(
not_empty, unicode_safe, ignore_missing, empty):
not_empty, resource_id_exists, unicode_safe, ignore_missing, empty):
return {
'resource_id': [not_empty, resource_id_exists],
'title': [not_empty, unicode_safe],
'description': [ignore_missing, unicode_safe],
'view_type': [not_empty, unicode_safe],
Expand Down Expand Up @@ -708,10 +709,11 @@ def default_update_resource_view_schema(resource_view):

@validator_args
def default_update_resource_view_schema_changes(
not_missing, not_empty, unicode_safe, ignore,
not_missing, not_empty, unicode_safe, resource_id_exists, ignore,
ignore_missing):
return {
'id': [not_missing, not_empty, unicode_safe],
'resource_id': [ignore_missing, resource_id_exists],
'title': [ignore_missing, unicode_safe],
'view_type': [ignore], # cannot change after create
'package_id': [ignore]
Expand Down
11 changes: 5 additions & 6 deletions ckan/model/group_extra.py
Expand Up @@ -44,9 +44,8 @@ class GroupExtra(
def _create_extra(key, value):
return GroupExtra(key=text_type(key), value=value)

# Don't think we need this
# _extras_active = vdm.sqlalchemy.stateful.DeferredProperty('_extras',
# vdm.sqlalchemy.stateful.StatefulDict, base_modifier=lambda x: x.get_as_of())
# setattr(group.Group, 'extras_active', _extras_active)
# group.Group.extras = vdm.sqlalchemy.stateful.OurAssociationProxy('extras_active', 'value',
# creator=_create_extra)
_extras_active = vdm.sqlalchemy.stateful.DeferredProperty('_extras',
vdm.sqlalchemy.stateful.StatefulDict)
setattr(group.Group, 'extras_active', _extras_active)
group.Group.extras = vdm.sqlalchemy.stateful.OurAssociationProxy('extras_active', 'value',
creator=_create_extra)
1 change: 0 additions & 1 deletion ckan/model/package.py
Expand Up @@ -466,7 +466,6 @@ def activity_stream_item(self, activity_type, user_id):
return activity.Activity(
user_id,
self.id,
'', # revisions are no more
"%s package" % activity_type,
{
'package': dictized_package,
Expand Down
11 changes: 5 additions & 6 deletions ckan/model/package_extra.py
Expand Up @@ -55,9 +55,8 @@ def related_packages(self):
def _create_extra(key, value):
return PackageExtra(key=text_type(key), value=value)

# # don't think we need this?
# _extras_active = vdm.sqlalchemy.stateful.DeferredProperty('_extras',
# vdm.sqlalchemy.stateful.StatefulDict, base_modifier=lambda x: x.get_as_of())
# setattr(_package.Package, 'extras_active', _extras_active)
# _package.Package.extras = vdm.sqlalchemy.stateful.OurAssociationProxy('extras_active', 'value',
# creator=_create_extra)
_extras_active = vdm.sqlalchemy.stateful.DeferredProperty('_extras',
vdm.sqlalchemy.stateful.StatefulDict)
setattr(_package.Package, 'extras_active', _extras_active)
_package.Package.extras = vdm.sqlalchemy.stateful.OurAssociationProxy('extras_active', 'value',
creator=_create_extra)
4 changes: 2 additions & 2 deletions ckan/model/system_info.py
Expand Up @@ -74,6 +74,6 @@ def set_system_info(key, value):
else:
obj.value = text_type(value)

from ckan.model import repo

meta.Session.add(obj)
meta.Session.commit()
return True
163 changes: 0 additions & 163 deletions ckan/tests/controllers/test_admin.py
Expand Up @@ -253,166 +253,3 @@ def test_homepage_style(self):
reset_index_response = app.get('/')
assert_true('<!-- Snippet home/layout1.html start -->'
in reset_index_response)


class TestTrashView(helpers.FunctionalTestBase):
'''View tests for permanently deleting datasets with Admin Trash.'''

@helpers.change_config('debug', True)
def test_trash_view_anon_user(self):
'''An anon user shouldn't be able to access trash view.'''
app = self._get_test_app()

trash_url = url_for(controller='admin', action='trash')
trash_response = app.get(trash_url, status=403)

def test_trash_view_normal_user(self):
'''A normal logged in user shouldn't be able to access trash view.'''
user = factories.User()
app = self._get_test_app()

env = {'REMOTE_USER': user['name'].encode('ascii')}
trash_url = url_for(controller='admin', action='trash')
trash_response = app.get(trash_url, extra_environ=env, status=403)
assert_true('Need to be system administrator to administer'
in trash_response)

def test_trash_view_sysadmin(self):
'''A sysadmin should be able to access trash view.'''
user = factories.Sysadmin()
app = self._get_test_app()

env = {'REMOTE_USER': user['name'].encode('ascii')}
trash_url = url_for(controller='admin', action='trash')
trash_response = app.get(trash_url, extra_environ=env, status=200)
# On the purge page
assert_true('form-purge-packages' in trash_response)

def test_trash_no_datasets(self):
'''Getting the trash view with no 'deleted' datasets should list no
datasets.'''
factories.Dataset()
user = factories.Sysadmin()
app = self._get_test_app()

env = {'REMOTE_USER': user['name'].encode('ascii')}
trash_url = url_for(controller='admin', action='trash')
trash_response = app.get(trash_url, extra_environ=env, status=200)

trash_response_html = BeautifulSoup(trash_response.body)
# it's called a 'user list' for some reason
trash_pkg_list = trash_response_html.select('ul.user-list li')
# no packages available to purge
assert_equal(len(trash_pkg_list), 0)

def test_trash_with_deleted_datasets(self):
'''Getting the trash view with 'deleted' datasets should list the
datasets.'''
user = factories.Sysadmin()
factories.Dataset(state='deleted')
factories.Dataset(state='deleted')
factories.Dataset()
app = self._get_test_app()

env = {'REMOTE_USER': user['name'].encode('ascii')}
trash_url = url_for(controller='admin', action='trash')
trash_response = app.get(trash_url, extra_environ=env, status=200)

trash_response_html = BeautifulSoup(trash_response.body)
# it's called a 'user list' for some reason
trash_pkg_list = trash_response_html.select('ul.user-list li')
# Two packages in the list to purge
assert_equal(len(trash_pkg_list), 2)

def test_trash_purge_deleted_datasets(self):
'''Posting the trash view with 'deleted' datasets, purges the
datasets.'''
user = factories.Sysadmin()
factories.Dataset(state='deleted')
factories.Dataset(state='deleted')
factories.Dataset()
app = self._get_test_app()

# how many datasets before purge
pkgs_before_purge = model.Session.query(model.Package).count()
assert_equal(pkgs_before_purge, 3)

env = {'REMOTE_USER': user['name'].encode('ascii')}
trash_url = url_for(controller='admin', action='trash')
trash_response = app.get(trash_url, extra_environ=env, status=200)

# submit the purge form
purge_form = trash_response.forms['form-purge-packages']
purge_response = webtest_submit(purge_form, 'purge-packages',
status=302, extra_environ=env)
purge_response = purge_response.follow(extra_environ=env)
# redirected back to trash page
assert_true('Purge complete' in purge_response)

# how many datasets after purge
pkgs_before_purge = model.Session.query(model.Package).count()
assert_equal(pkgs_before_purge, 1)


class TestAdminConfigUpdate(helpers.FunctionalTestBase):

def teardown(self):
'''Reset the database and clear the search indexes.'''
helpers.reset_db()

def _update_config_option(self):
sysadmin = factories.Sysadmin()
env = {'REMOTE_USER': sysadmin['name'].encode('ascii')}
app = self._get_test_app()
url = url_for(controller='admin', action='config')

response = app.get(url=url, extra_environ=env)
form = response.forms[1]
form['ckan.site_title'] = 'My Updated Site Title'

webtest_submit(form, 'save', status=302, extra_environ=env)

def test_admin_config_update(self):
'''Changing a config option using the admin interface appropriately
updates value returned by config_option_show,
system_info.get_system_info and in the title tag in templates.'''

# test value before update
# config_option_show returns default value
before_update = helpers.call_action('config_option_show',
key='ckan.site_title')
assert_equal(before_update, 'CKAN')

# system_info.get_system_info returns None, or default
# test value before update
before_update = get_system_info('ckan.site_title')
assert_equal(before_update, None)
# test value before update with default
before_update_default = get_system_info('ckan.site_title',
config['ckan.site_title'])
assert_equal(before_update_default, 'CKAN')

# title tag contains default value
app = self._get_test_app()
home_page_before = app.get('/', status=200)
assert_true('Welcome - CKAN' in home_page_before)

# update the option
self._update_config_option()

# test config_option_show returns new value after update
after_update = helpers.call_action('config_option_show',
key='ckan.site_title')
assert_equal(after_update, 'My Updated Site Title')

# system_info.get_system_info returns new value
after_update = get_system_info('ckan.site_title')
assert_equal(after_update, 'My Updated Site Title')
# test value after update with default
after_update_default = get_system_info('ckan.site_title',
config['ckan.site_title'])
assert_equal(after_update_default, 'My Updated Site Title')

# title tag contains new value
home_page_after = app.get('/', status=200)
assert_true('Welcome - My Updated Site Title' in home_page_after)
50 changes: 0 additions & 50 deletions ckan/tests/legacy/models/test_group.py
Expand Up @@ -186,53 +186,3 @@ def test_groups_allowed_to_be_its_parent(self):
assert_in('cabinet-office', names)
assert_not_in('natonal-health-service', names)
assert_not_in('nhs-wirral-ccg', names)

class TestGroupRevisions:
@classmethod
def setup_class(self):
model.Session.remove()
CreateTestData.create()
self.name = u'revisiontest'

# create pkg
self.descriptions = [u'Written by Puccini', u'Written by Rossini', u'Not written at all', u'Written again', u'Written off']
self.grp = model.Group(name=self.name)
model.Session.add(self.grp)
self.grp.description = self.descriptions[0]
self.grp.extras['mykey'] = self.descriptions[0]
model.repo.commit_and_remove()

# edit pkg
for i in range(5)[1:]:
grp = model.Group.by_name(self.name)
grp.description = self.descriptions[i]
grp.extras['mykey'] = self.descriptions[i]
model.repo.commit_and_remove()

self.grp = model.Group.by_name(self.name)

@classmethod
def teardown_class(self):
#grp = model.Group.by_name(self.name)
#grp.purge()
#model.repo.commit_and_remove()
model.repo.rebuild_db()

def test_1_all_revisions(self):
all_rev = self.grp.all_revisions
num_descs = len(self.descriptions)
assert len(all_rev) == num_descs, len(all_rev)
for i, rev in enumerate(all_rev):
assert rev.description == self.descriptions[num_descs - i - 1], \
'%s != %s' % (rev.description, self.descriptions[i])

def test_2_extras(self):
all_rev = self.grp.all_revisions
num_descs = len(self.descriptions)
assert len(all_rev) == num_descs, len(all_rev)
for i, rev in enumerate(all_rev):
#print "REVISION", dir(rev)
#assert rev.extras['mykey'] == self.descriptions[num_descs - i - 1], \
# '%s != %s' % (rev.extras['mykey'], self.descriptions[i])
pass

0 comments on commit aacd9e2

Please sign in to comment.