[ckan-changes] commit/ckan: rgrp: [bugfix, dictixation][s]: fixed bug (#1239) whereby deletion and re-adding or a tag to package was not working (tag would not be re-added).

Sat Jul 23 17:16:53 UTC 2011

1 new changeset in ckan:

http://bitbucket.org/okfn/ckan/changeset/adf254b7c507/
changeset:   adf254b7c507
branch:      release-v1.4.2
user:        rgrp
date:        2011-07-23 19:16:15
summary:     [bugfix,dictixation][s]: fixed bug (#1239) whereby deletion and re-adding or a tag to package was not working (tag would not be re-added).

* Bug details: when saving tags nothing was happening where the 'new' tag already had a corresponding PackageTag but it was deleted state. Comments in code and code changes (see diff) are more eloquent than i can be here.
* Added failing test to api/model/test_package.py (would have preferred to add to tests/lib/test_dictization, however the test methods there seem very interpendent (and large) and it was hard to add changes without breaking other things ...)
* NB: looking at rest of dictize code suspicious that this bug exists for other stateful relations (e.g. Package to Group)
affected #:  2 files (1.7 KB)

--- a/ckan/lib/dictization/model_save.py	Tue Jul 19 19:45:57 2011 +0100
+++ b/ckan/lib/dictization/model_save.py	Sat Jul 23 18:16:15 2011 +0100
@@ -139,12 +139,19 @@
     tag_package_tag = dict((package_tag.tag, package_tag) 
                             for package_tag in
                             package.package_tag_all)
+    
+    tag_package_tag_inactive = dict(
+        [ (tag,pt) for tag,pt in tag_package_tag.items() if
+            pt.state in ['deleted', 'pending-deleted'] ]
+        )
 
     tags = set()
     for tag_dict in tag_dicts:
         obj = table_dict_save(tag_dict, model.Tag, context)
         tags.add(obj)
 
+    # 3 cases
+    # case 1: currently active but not in new list
     for tag in set(tag_package_tag.keys()) - tags:
         package_tag = tag_package_tag[tag]
         if pending and package_tag.state <> 'deleted':
@@ -152,12 +159,19 @@
         else:
             package_tag.state = 'deleted'
 
+    # in new list but never used before
     for tag in tags - set(tag_package_tag.keys()):
         state = 'pending' if pending else 'active'
         package_tag_obj = model.PackageTag(package, tag, state)
         session.add(package_tag_obj)
         tag_package_tag[tag] = package_tag_obj
 
+    # in new list and already used but in deleted state
+    for tag in tags.intersection(set(tag_package_tag_inactive.keys())):
+        state = 'pending' if pending else 'active'
+        package_tag = tag_package_tag[tag]
+        package_tag.state = state
+
     package.package_tag_all[:] = tag_package_tag.values()
 
 def package_group_list_save(group_dicts, package, context):


--- a/ckan/tests/functional/api/model/test_package.py	Tue Jul 19 19:45:57 2011 +0100
+++ b/ckan/tests/functional/api/model/test_package.py	Sat Jul 23 18:16:15 2011 +0100
@@ -418,6 +418,37 @@
         # - title
         assert len(package.extras) == 1, package.extras
 
+    def test_entity_update_readd_tag(self):
+        name = self.package_fixture_data['name']
+        old_fixture_data = {
+            'name': name,
+            'tags': ['tag1', 'tag2']
+        }
+        new_fixture_data = {
+            'name': name,
+            'tags': ['tag1']
+        }
+        self.create_package_roles_revision(old_fixture_data)
+        offset = self.package_offset(name)
+        params = '%s=1' % self.dumps(new_fixture_data)
+        res = self.app.post(offset, params=params, status=self.STATUS_200_OK,
+                            extra_environ=self.extra_environ)
+
+        # Check the returned package is as expected
+        pkg = self.loads(res.body)
+        assert_equal(pkg['name'], new_fixture_data['name'])
+        assert_equal(pkg['tags'], ['tag1'])
+
+        package = self.get_package_by_name(new_fixture_data['name'])
+        assert len(package.tags) == 1, package.tags
+
+        # now reinstate the tag
+        params = '%s=1' % self.dumps(old_fixture_data)
+        res = self.app.post(offset, params=params, status=self.STATUS_200_OK,
+                            extra_environ=self.extra_environ)
+        pkg = self.loads(res.body)
+        assert_equal(pkg['tags'], ['tag1', 'tag2'])
+
     def test_entity_update_conflict(self):
         package1_name = self.package_fixture_data['name']
         package1_data = {'name': package1_name}

Repository URL: https://bitbucket.org/okfn/ckan/

--

This is a commit notification from bitbucket.org. You are receiving
this because you have the service enabled, addressing the recipient of
this email.


