okfn / ckanext-lacounts

CKAN extension for the LA Counts project
GNU Affero General Public License v3.0
8 stars 5 forks source link

Manual tagging with topics #175

Closed roll closed 5 years ago

roll commented 5 years ago
roll commented 5 years ago

@amercader Please review and test.

It works for me (still testing ckan source because my harvester doesn't want to finish reharvesting).

roll commented 5 years ago

Upd. For ckan it doesn't work on harvesting because the ckan harvester doesn't update not changed datasets. But it works if we change some topic (via jobs)

roll commented 5 years ago

@amercader Could you please re-take a look?

I tested on manually created dataset because still having this harvesting problem.

amercader commented 5 years ago

@roll there were some hardcore low-level CKAN things that prevented the harvesting from working. The patch below fixed the errors for me:

  1. At this stage fields can be present but with the Missing placeholder, so we need to check for that
  2. Harvesters assign ids to newly created datasets for ... reasons. We can not rely on the presence of id only to know if a dataset is supposed to exist
  3. For lists of objects data should be "flattened" at this stage, eg not this:
data[('groups', )] = [{'name': 'census'}, {'name': 'food'}]

but this:

data[('groups', 0, 'name')] = 'census'
data[('groups', 1, 'name')] = 'food'
diff --git a/ckanext/lacounts/validators.py b/ckanext/lacounts/validators.py
index 0734688..16b7cea 100644
--- a/ckanext/lacounts/validators.py
+++ b/ckanext/lacounts/validators.py
@@ -70,10 +70,14 @@ def convert_groups_override(key, data, errors, context):
     package = None
     group_ids = set()
     group_override_ids = set(value if isinstance(value, list) else value.strip('{}').split(','))
-    if data.get(('id',)):
+    if data.get(('id',)) and data[('id', )] is not toolkit.missing:
+
         context = {'model': model}
-        package = toolkit.get_action('package_show')(context, {'id': data[('id',)]})
-        group_ids = set(map(lambda group: group['id'], package['groups']))
+        try:
+            package = toolkit.get_action('package_show')(context, {'id': data[('id',)]})
+            group_ids = set(map(lambda group: group['id'], package['groups']))
+        except toolkit.ObjectNotFound:
+            pass

     # Form groups_override value
     groups_add = group_override_ids.difference(group_ids)
@@ -95,4 +99,5 @@ def convert_groups_override(key, data, errors, context):

     # Save groups_override/groups
     data[('groups_override',)] = json.dumps(groups_override)
-    data[('groups',)] = groups
+    for index, group in enumerate(groups):
+        data[('groups', index, 'id')] = group['id']
amercader commented 5 years ago

Functionality works fine, nice work