From 89b41052c596d60eb733d7202ad5d6dd7bb15c78 Mon Sep 17 00:00:00 2001 From: hayley-leblanc Date: Wed, 24 Jul 2019 13:08:08 +0200 Subject: [PATCH 01/47] Add more detailed change summary for metadata fields Adds a more readable summary of changes between two versions of a dataset (only for dataset metadata fields - title, description, license, etc. and only for those that have actually changed) to the Changes page. The comprehensive but less readable metadata diff is still there but is opened with a button and not shown by default. Does not yet show a summary for custom fields or fields that come with extensions - only shows changes for the default fields. --- ckan/lib/helpers.py | 166 ++++++++++++++++++++++++++++ ckan/templates/package/changes.html | 54 ++++++++- 2 files changed, 218 insertions(+), 2 deletions(-) diff --git a/ckan/lib/helpers.py b/ckan/lib/helpers.py index 577d62232ac..5b0f2db1e18 100644 --- a/ckan/lib/helpers.py +++ b/ckan/lib/helpers.py @@ -2688,3 +2688,169 @@ def sanitize_id(id_): ValueError. ''' return str(uuid.UUID(id_)) + +@core_helper +def compare_pkg_dicts(original, new): + # TODO: clean this up or make it shorter somehow + + change_list = [] + + for key in original: + log.info("original[" + str(key) + "]: " + str(original[key])) + if key in new: + log.info("new[" + str(key) + "]: " + str(new[key])) + + + s = "" + seq1 = ("", + new['title'], "") + + # if the title has changed + if original['title'] != new['title']: + seq2 = ("", + new['title'], "") + change_list.append(["Changed title to", s.join(seq2)]) + + # if the owner organization changed + if original['owner_org'] != new['owner_org']: + seq2 = ("", + original['organization']['title'], "") + seq3 = ("", + new['organization']['title'], "") + change_list.append(["Moved", s.join(seq1), + "from organization", + s.join(seq2), + "to organization", + s.join(seq3)]) + + # if the maintainer of the dataset changed + if original['maintainer'] != new['maintainer']: + # if the original dataset had a maintainer + if original['maintainer']: + change_list.append(["Set maintainer of", s.join(seq1), "to", new['maintainer'], "(previously", original['maintainer'] + ")"]) + else: + change_list.append(["Set maintainer of", s.join(seq1), "to", new['maintainer']]) + + + # if the maintainer email of the dataset changed + if original['maintainer_email'] != new['maintainer_email']: + seq2 = ("", new['maintainer_email'], "") + # if the original dataset had a maintainer email + if original['maintainer_email']: + seq3 = ("", original['maintainer_email'], "") + change_list.append(["Set maintainer e-mail of", s.join(seq1), "to", s.join(seq2), "(previously", s.join(seq3) + ")"]) + else: + change_list.append(["Set maintainer e-mail of", s.join(seq1), "to", s.join(seq2)]) + + # if the author of the dataset changed + if original['author'] != new['author']: + # if the original dataset had an author + if original['author']: + change_list.append(["Set author of", s.join(seq1), "to", new['author'], "(previously", original['author'] + ")"]) + else: + change_list.append(["Set author of", s.join(seq1), "to", new['author']]) + + # if the author email of the dataset changed + if original['author_email'] != new['author_email']: + seq2 = ("", new['author_email'], "") + # if the original dataset had a author email + if original['author_email']: + seq3 = ("", original['author_email'], "") + change_list.append(["Set author e-mail of", s.join(seq1), "to", s.join(seq2), "(previously", s.join(seq3) + ")"]) + else: + change_list.append(["Set author e-mail of", s.join(seq1), "to", s.join(seq2)]) + + # if the visibility of the dataset changed + if original['private'] != new['private']: + change_list.append(["Set visibility of", s.join(seq1), "to", "Private" if new['private'] else "Public"]) + + # if the description of the dataset changed + if original['notes'] != new['notes']: + # displays the two descriptions (like how they are displayed on resource views) + # TODO: figure out the best way to format this stuff + + # if the original dataset had a description + if original['notes']: + change_list.append(["Updated description of", s.join(seq1), + "from
", "
" + original['notes'] + "
", + "to
", "
" + new['notes'] + "
"]) + else: + change_list.append(["Updated description of", s.join(seq1), + "to
", "
" + new['notes'] + "
"]) + + # make sets out of the tags for each dataset + original_tags = set([tag['name'] for tag in original['tags']]) + new_tags = set([tag['name'] for tag in new['tags']]) + # if the tags have changed + if original_tags != new_tags: + deleted_tags = original_tags - new_tags + deleted_tags_list = list(deleted_tags) + if len(deleted_tags) == 1: + seq2 = ("", + deleted_tags_list[0], "") + change_list.append(["Removed tag", s.join(seq2), "from", s.join(seq1)]) + elif len(deleted_tags) > 1: + seq2 = ["
  • " + deleted_tags_list[i] + "
    • " + for i in range(0, len(deleted_tags))] + change_list.append(["Removed the following tags from", s.join(seq1), ""]) + + added_tags = new_tags - original_tags + added_tags_list = list(added_tags) + if len(added_tags) == 1: + seq2 = ("", + added_tags_list[0], "") + change_list.append(["Added tag", s.join(seq2), "to", s.join(seq1)]) + elif len(added_tags) > 1: + seq2 = ["
  • " + added_tags_list[i] + "
    • " + for i in range(0, len(added_tags))] + change_list.append(["Added the following tags to", s.join(seq1), ""]) + + # if the license has changed + if original['license_title'] != new['license_title']: + seq2 = () + seq3 = () + # if the license has a URL, use it + if 'license_url' in original and original['license_url']: + seq2 = ("", original['license_title'], "") + else: + seq2 = (original['license_title']) + if 'license_url' in new and new['license_url']: + seq3 = ("", new['license_title'], "") + else: + seq3 = (new['license_title']) + change_list.append(["Changed the license of", s.join(seq1), "to", s.join(seq3), "(previously", s.join(seq2) + ")"]) + + # if the name of the dataset has changed + # this is only visible to the user via the dataset's URL, so display the change using that + if original['name'] != new['name']: + old_url = url_for(qualified=True, controller="dataset", + action="read", id=original['name']) + new_url = url_for(qualified=True, controller="dataset", + action="read", id=new['name']) + seq2 = ("", old_url, "") + seq3 = ("", new_url, "") + change_list.append(["Moved the dataset from", s.join(seq2), "to", s.join(seq3)]) + + # if the source URL (metadata value, not the actual URL of the dataset) has changed + if original['url'] != new['url']: + seq2 = ("", original['url'], "") + seq3 = ("", new['url'], "") + if original['url']: + change_list.append(["Changed the source URL of", s.join(seq1), "from", s.join(seq2), "to", s.join(seq3)]) + else: + change_list.append(["Changed the source URL of", s.join(seq1), "to", s.join(seq3)]) + + # if the user-provided version has changed + if original['version'] != new['version']: + if original['version']: + change_list.append(["Changed the version of", s.join(seq1), "from", original['version'], "to", new['version']]) + else: + change_list.append(["Changed the version of", s.join(seq1), "to", new['version']]) + + + + return change_list diff --git a/ckan/templates/package/changes.html b/ckan/templates/package/changes.html index eb1fd899063..45e8caf8e83 100644 --- a/ckan/templates/package/changes.html +++ b/ckan/templates/package/changes.html @@ -15,20 +15,70 @@
    {% block package_changes_header %}

    {{ _('Changes') }}

    - Dataset: {% link_for pkg_dict.title, controller='package', action='read', id=pkg_dict.name %} + {##} {% endblock %} + {# iterate through the activities that are NOT the original one - + iterate, rather than just look at the second one, so that we could + potentially add a way to look at the diff between larger sets of + datasets in the future #} + + {% for i in range(1, activity_diff.activities|length) %} + On {{ h.render_datetime(activity_diff.activities[1].timestamp, with_hours=True, with_seconds=True) }}, {{ h.linked_user(activity_diff.activities[1].user_id) }}: +
    + + {% set changes = h.compare_pkg_dicts(activity_diff.activities[0].data.package, activity_diff.activities[1].data.package) %} + +
    + + {% endfor %} + + {# button to show JSON metadata diff - not shown by default #} +
    + + + + + +
    {% endblock %} From 680ede035eab5bca1f1779f8bcb838e6a8d119f5 Mon Sep 17 00:00:00 2001 From: hayley-leblanc Date: Fri, 19 Jul 2019 10:58:54 +0200 Subject: [PATCH 02/47] Save activity data for private datasets --- ckan/lib/activity_streams_session_extension.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ckan/lib/activity_streams_session_extension.py b/ckan/lib/activity_streams_session_extension.py index 3dec8a95173..decdeeb6cde 100644 --- a/ckan/lib/activity_streams_session_extension.py +++ b/ckan/lib/activity_streams_session_extension.py @@ -70,8 +70,8 @@ def before_commit(self, session): # object is a package. # Don't create activities for private datasets. - if obj.private: - continue + #if obj.private: + # continue activities[obj.id] = activity @@ -106,8 +106,8 @@ def before_commit(self, session): continue # Don't create activities for private datasets. - if package.private: - continue + #if package.private: + # continue if package.id in activities: continue From 3fc42cedd079f469eb053451de3125c1c37cdc0c Mon Sep 17 00:00:00 2001 From: hayley-leblanc Date: Wed, 24 Jul 2019 15:19:09 +0200 Subject: [PATCH 03/47] Display changes to metadata fields added by extensions --- ckan/lib/helpers.py | 34 +++++++++++++++++++++++++++++ ckan/templates/package/changes.html | 3 +-- 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/ckan/lib/helpers.py b/ckan/lib/helpers.py index 5b0f2db1e18..5af4e986a0e 100644 --- a/ckan/lib/helpers.py +++ b/ckan/lib/helpers.py @@ -2692,6 +2692,7 @@ def sanitize_id(id_): @core_helper def compare_pkg_dicts(original, new): # TODO: clean this up or make it shorter somehow + # TODO: what happens if someone REMOVES a value from a field that previously had a value? change_list = [] @@ -2851,6 +2852,39 @@ def compare_pkg_dicts(original, new): else: change_list.append(["Changed the version of", s.join(seq1), "to", new['version']]) + # list of the default metadata fields for a dataset + # any fields that are not part of this list are custom fields added by a user or extension + fields = ['owner_org', 'maintainer', 'maintainer_email', 'relationships_as_object', 'private', 'num_tags', + 'id', 'metadata_created', 'metadata_modified', 'author', 'author_email', 'state', 'version', + 'license_id', 'type', 'resources', 'num_resources', 'tags', 'title', 'groups', 'creator_user_id', + 'relationships_as_subject', 'name', 'isopen', 'url', 'notes', 'license_title', 'extras', + 'license_url', 'organization', 'revision_id'] + fields_set = set(fields) + + # if there are any fields from extensions that are in the new dataset and + # have been updated, print a generic message stating that + original_set = set(original.keys()) + new_set = set(new.keys()) + + addl_fields_new = new_set - fields_set # set of additional fields in the new dictionary + addl_fields_original = original_set - fields_set # set of additional fields in the original dictionary + addl_fields = addl_fields_new.intersection(addl_fields_original) # set of additional fields in both + + # do NOT display a change if any additional fields have been added or deleted, + # since that is not a change made by the user from the web interface + + # if additional fields have been changed + addl_fields_list = list(addl_fields) + for field in addl_fields_list: + if original[field] != new[field]: + if original[field]: + change_list.append(["Changed field", field.capitalize(), "to", new[field], "(previously", original[field] + ")", "in", s.join(seq1)]) + + # check the extras field to see if anything has been added, deleted, or changed + # that is where custom fields added via the web interface go - they do not become + # actual fields in a package dict + # TODO: add this ^^ + return change_list diff --git a/ckan/templates/package/changes.html b/ckan/templates/package/changes.html index 45e8caf8e83..996494955ef 100644 --- a/ckan/templates/package/changes.html +++ b/ckan/templates/package/changes.html @@ -31,10 +31,9 @@

    {{ _('Changes') }}

    {% for i in range(1, activity_diff.activities|length) %} On {{ h.render_datetime(activity_diff.activities[1].timestamp, with_hours=True, with_seconds=True) }}, {{ h.linked_user(activity_diff.activities[1].user_id) }}: -
    {% set changes = h.compare_pkg_dicts(activity_diff.activities[0].data.package, activity_diff.activities[1].data.package) %} -
      +
        {% for change in changes %}
      • {% for element in change %} From 9732e969ff160b7be2eb3b6ce210834f9b2e4e59 Mon Sep 17 00:00:00 2001 From: hayley-leblanc Date: Thu, 25 Jul 2019 10:36:15 +0200 Subject: [PATCH 04/47] Check changes to custom fields and add function for each check --- ckan/lib/helpers.py | 355 ++++++++++++++++++++++++++++++-------------- 1 file changed, 241 insertions(+), 114 deletions(-) diff --git a/ckan/lib/helpers.py b/ckan/lib/helpers.py index 5af4e986a0e..8ee2057671b 100644 --- a/ckan/lib/helpers.py +++ b/ckan/lib/helpers.py @@ -2691,167 +2691,221 @@ def sanitize_id(id_): @core_helper def compare_pkg_dicts(original, new): - # TODO: clean this up or make it shorter somehow # TODO: what happens if someone REMOVES a value from a field that previously had a value? change_list = [] - for key in original: - log.info("original[" + str(key) + "]: " + str(original[key])) - if key in new: - log.info("new[" + str(key) + "]: " + str(new[key])) - - s = "" seq1 = ("", new['title'], "") + new_pkg = s.join(seq1) # if the title has changed if original['title'] != new['title']: - seq2 = ("", - new['title'], "") - change_list.append(["Changed title to", s.join(seq2)]) + _title_change(change_list, original, new) # if the owner organization changed if original['owner_org'] != new['owner_org']: - seq2 = ("", - original['organization']['title'], "") - seq3 = ("", - new['organization']['title'], "") - change_list.append(["Moved", s.join(seq1), - "from organization", - s.join(seq2), - "to organization", - s.join(seq3)]) + _org_change(change_list, original, new, new_pkg) # if the maintainer of the dataset changed if original['maintainer'] != new['maintainer']: - # if the original dataset had a maintainer - if original['maintainer']: - change_list.append(["Set maintainer of", s.join(seq1), "to", new['maintainer'], "(previously", original['maintainer'] + ")"]) - else: - change_list.append(["Set maintainer of", s.join(seq1), "to", new['maintainer']]) - + _maintainer_change(change_list, original, new, new_pkg) # if the maintainer email of the dataset changed if original['maintainer_email'] != new['maintainer_email']: - seq2 = ("", new['maintainer_email'], "") - # if the original dataset had a maintainer email - if original['maintainer_email']: - seq3 = ("", original['maintainer_email'], "") - change_list.append(["Set maintainer e-mail of", s.join(seq1), "to", s.join(seq2), "(previously", s.join(seq3) + ")"]) - else: - change_list.append(["Set maintainer e-mail of", s.join(seq1), "to", s.join(seq2)]) + _maintainer_email_change(change_list, original, new, new_pkg) # if the author of the dataset changed if original['author'] != new['author']: - # if the original dataset had an author - if original['author']: - change_list.append(["Set author of", s.join(seq1), "to", new['author'], "(previously", original['author'] + ")"]) - else: - change_list.append(["Set author of", s.join(seq1), "to", new['author']]) + _author_change(change_list, original, new, new_pkg) # if the author email of the dataset changed if original['author_email'] != new['author_email']: - seq2 = ("", new['author_email'], "") - # if the original dataset had a author email - if original['author_email']: - seq3 = ("", original['author_email'], "") - change_list.append(["Set author e-mail of", s.join(seq1), "to", s.join(seq2), "(previously", s.join(seq3) + ")"]) - else: - change_list.append(["Set author e-mail of", s.join(seq1), "to", s.join(seq2)]) + _author_email_change(change_list, original, new, new_pkg) # if the visibility of the dataset changed if original['private'] != new['private']: - change_list.append(["Set visibility of", s.join(seq1), "to", "Private" if new['private'] else "Public"]) + change_list.append(["Set visibility of", new_pkg, "to", "Private" if new['private'] else "Public"]) # if the description of the dataset changed if original['notes'] != new['notes']: - # displays the two descriptions (like how they are displayed on resource views) - # TODO: figure out the best way to format this stuff - - # if the original dataset had a description - if original['notes']: - change_list.append(["Updated description of", s.join(seq1), - "from
        ", "
        " + original['notes'] + "
        ", - "to
        ", "
        " + new['notes'] + "
        "]) - else: - change_list.append(["Updated description of", s.join(seq1), - "to
        ", "
        " + new['notes'] + "
        "]) + _description_change(change_list, original, new, new_pkg) # make sets out of the tags for each dataset original_tags = set([tag['name'] for tag in original['tags']]) new_tags = set([tag['name'] for tag in new['tags']]) # if the tags have changed if original_tags != new_tags: - deleted_tags = original_tags - new_tags - deleted_tags_list = list(deleted_tags) - if len(deleted_tags) == 1: - seq2 = ("", - deleted_tags_list[0], "") - change_list.append(["Removed tag", s.join(seq2), "from", s.join(seq1)]) - elif len(deleted_tags) > 1: - seq2 = ["
      • " + deleted_tags_list[i] + "
        • " - for i in range(0, len(deleted_tags))] - change_list.append(["Removed the following tags from", s.join(seq1), "
          ", s.join(seq2), "
        "]) - - added_tags = new_tags - original_tags - added_tags_list = list(added_tags) - if len(added_tags) == 1: - seq2 = ("", - added_tags_list[0], "") - change_list.append(["Added tag", s.join(seq2), "to", s.join(seq1)]) - elif len(added_tags) > 1: - seq2 = ["
      • " + added_tags_list[i] + "
        • " - for i in range(0, len(added_tags))] - change_list.append(["Added the following tags to", s.join(seq1), "
          ", s.join(seq2), "
        "]) + _tag_change(change_list, new_tags, original_tags, new_pkg) # if the license has changed if original['license_title'] != new['license_title']: - seq2 = () - seq3 = () - # if the license has a URL, use it - if 'license_url' in original and original['license_url']: - seq2 = ("", original['license_title'], "") - else: - seq2 = (original['license_title']) - if 'license_url' in new and new['license_url']: - seq3 = ("", new['license_title'], "") - else: - seq3 = (new['license_title']) - change_list.append(["Changed the license of", s.join(seq1), "to", s.join(seq3), "(previously", s.join(seq2) + ")"]) + _license_change(change_list, original, new, new_pkg) # if the name of the dataset has changed # this is only visible to the user via the dataset's URL, so display the change using that if original['name'] != new['name']: - old_url = url_for(qualified=True, controller="dataset", - action="read", id=original['name']) - new_url = url_for(qualified=True, controller="dataset", - action="read", id=new['name']) - seq2 = ("", old_url, "") - seq3 = ("", new_url, "") - change_list.append(["Moved the dataset from", s.join(seq2), "to", s.join(seq3)]) + _name_change(change_list, original, new) # if the source URL (metadata value, not the actual URL of the dataset) has changed if original['url'] != new['url']: - seq2 = ("", original['url'], "") - seq3 = ("", new['url'], "") - if original['url']: - change_list.append(["Changed the source URL of", s.join(seq1), "from", s.join(seq2), "to", s.join(seq3)]) - else: - change_list.append(["Changed the source URL of", s.join(seq1), "to", s.join(seq3)]) + _source_url_change(change_list, original, new, new_pkg) # if the user-provided version has changed if original['version'] != new['version']: - if original['version']: - change_list.append(["Changed the version of", s.join(seq1), "from", original['version'], "to", new['version']]) - else: - change_list.append(["Changed the version of", s.join(seq1), "to", new['version']]) + _version_change(change_list, original, new, new_pkg) + + # check whether fields added by extensions or custom fields (in the "extras" field) + # have been changed + + _extension_fields(change_list, original, new, new_pkg) + _extra_fields(change_list, original, new, new_pkg) + + return change_list + +def _extras_to_dict(extras_list): + ret_dict = {} + # the extras_list is a list of dictionaries + for dict in extras_list: + ret_dict[dict['key']] = dict['value'] + + return ret_dict + +def _title_change(change_list, original, new): + s = "" + seq2 = ("", + new['title'], "") + change_list.append(["Changed title to", s.join(seq2), "(previously", original['title'] + ")"]) + +def _org_change(change_list, original, new, new_pkg): + s = "" + seq2 = ("", + original['organization']['title'], "") + seq3 = ("", + new['organization']['title'], "") + change_list.append(["Moved", new_pkg, + "from organization", + s.join(seq2), + "to organization", + s.join(seq3)]) + +def _maintainer_change(change_list, original, new, new_pkg): + # if the original dataset had a maintainer + if original['maintainer']: + change_list.append(["Set maintainer of", new_pkg, "to", new['maintainer'], "(previously", original['maintainer'] + ")"]) + else: + change_list.append(["Set maintainer of", new_pkg, "to", new['maintainer']]) + +def _maintainer_email_change(change_list, original, new, new_pkg): + s = "" + seq2 = ("", new['maintainer_email'], "") + # if the original dataset had a maintainer email + if original['maintainer_email']: + seq3 = ("", original['maintainer_email'], "") + change_list.append(["Set maintainer e-mail of", new_pkg, "to", s.join(seq2), "(previously", s.join(seq3) + ")"]) + else: + change_list.append(["Set maintainer e-mail of", new_pkg, "to", s.join(seq2)]) + +def _author_change(change_list, original, new, new_pkg): + # if the original dataset had an author + if original['author']: + change_list.append(["Set author of", new_pkg, "to", new['author'], "(previously", original['author'] + ")"]) + else: + change_list.append(["Set author of", new_pkg, "to", new['author']]) + +def _author_email_change(change_list, original, new, new_pkg): + s = "" + seq2 = ("", new['author_email'], "") + # if the original dataset had a author email + if original['author_email']: + seq3 = ("", original['author_email'], "") + change_list.append(["Set author e-mail of", new_pkg, "to", s.join(seq2), "(previously", s.join(seq3) + ")"]) + else: + change_list.append(["Set author e-mail of", new_pkg, "to", s.join(seq2)]) + +def _description_change(change_list, original, new, new_pkg): + # displays the two descriptions (like how they are displayed on resource views) + # TODO: figure out the best way to format this stuff + + # if the original dataset had a description + if original['notes']: + change_list.append(["Updated description of", new_pkg, + "from
        ", "
        " + original['notes'] + "
        ", + "to
        ", "
        " + new['notes'] + "
        "]) + else: + change_list.append(["Updated description of", new_pkg, + "to
        ", "
        " + new['notes'] + "
        "]) + +def _tag_change(change_list, new_tags, original_tags, new_pkg): + s = "" + deleted_tags = original_tags - new_tags + deleted_tags_list = list(deleted_tags) + if len(deleted_tags) == 1: + seq2 = ("", + deleted_tags_list[0], "") + change_list.append(["Removed tag", s.join(seq2), "from", new_pkg]) + elif len(deleted_tags) > 1: + seq2 = ["
      • " + deleted_tags_list[i] + "
        • " + for i in range(0, len(deleted_tags))] + change_list.append(["Removed the following tags from", new_pkg, "
          ", s.join(seq2), "
        "]) + + added_tags = new_tags - original_tags + added_tags_list = list(added_tags) + if len(added_tags) == 1: + seq2 = ("", + added_tags_list[0], "") + change_list.append(["Added tag", s.join(seq2), "to", new_pkg]) + elif len(added_tags) > 1: + seq2 = ["
      • " + added_tags_list[i] + "
        • " + for i in range(0, len(added_tags))] + change_list.append(["Added the following tags to", new_pkg, "
          ", s.join(seq2), "
        "]) + +def _license_change(change_list, original, new, new_pkg): + s = "" + seq2 = () + seq3 = () + # if the license has a URL, use it + if 'license_url' in original and original['license_url']: + seq2 = ("", original['license_title'], "") + else: + seq2 = (original['license_title']) + if 'license_url' in new and new['license_url']: + seq3 = ("", new['license_title'], "") + else: + seq3 = (new['license_title']) + change_list.append(["Changed the license of", new_pkg, "to", s.join(seq3), "(previously", s.join(seq2) + ")"]) + +def _name_change(change_list, original, new): + s = "" + old_url = url_for(qualified=True, controller="dataset", + action="read", id=original['name']) + new_url = url_for(qualified=True, controller="dataset", + action="read", id=new['name']) + seq2 = ("", old_url, "") + seq3 = ("", new_url, "") + change_list.append(["Moved the dataset from", s.join(seq2), "to", s.join(seq3)]) + +def _source_url_change(change_list, original, new, new_pkg): + s = "" + seq2 = ("", original['url'], "") + seq3 = ("", new['url'], "") + if original['url']: + change_list.append(["Changed the source URL of", new_pkg, "from", s.join(seq2), "to", s.join(seq3)]) + else: + change_list.append(["Changed the source URL of", new_pkg, "to", s.join(seq3)]) + +def _version_change(change_list, original, new, new_pkg): + if original['version']: + change_list.append(["Changed the version of", new_pkg, "from", original['version'], "to", new['version']]) + else: + change_list.append(["Changed the version of", new_pkg, "to", new['version']]) +def _extension_fields(change_list, original, new, new_pkg): # list of the default metadata fields for a dataset # any fields that are not part of this list are custom fields added by a user or extension fields = ['owner_org', 'maintainer', 'maintainer_email', 'relationships_as_object', 'private', 'num_tags', @@ -2878,13 +2932,86 @@ def compare_pkg_dicts(original, new): for field in addl_fields_list: if original[field] != new[field]: if original[field]: - change_list.append(["Changed field", field.capitalize(), "to", new[field], "(previously", original[field] + ")", "in", s.join(seq1)]) + change_list.append(["Changed value of field", field.capitalize(), "to", new[field], "(previously", original[field] + ")", "in", new_pkg]) + else: + change_list.append(["Changed value of field", field.capitalize(), "to", new[field], "in", new_pkg]) +def _extra_fields(change_list, original, new, new_pkg): # check the extras field to see if anything has been added, deleted, or changed # that is where custom fields added via the web interface go - they do not become # actual fields in a package dict - # TODO: add this ^^ - - - return change_list + # what if they added the field but didn't add a value? + s = "" + if 'extras' in new: + extra_fields_new = _extras_to_dict(new['extras']) + extra_new_set = set(extra_fields_new.keys()) + + # if the original version has an extra fields, we need to compare the new version's + # extras to the original ones + if 'extras' in original: + extra_fields_original = _extras_to_dict(original['extras']) + extra_original_set = set(extra_fields_original.keys()) + + # if some fields were added + new_fields = list(extra_new_set - extra_original_set) + if len(new_fields) == 1: + if extra_fields_new[new_fields[0]]: + change_list.append(["Added field", s.join(("", new_fields[0], "")), + "with value", s.join(("", extra_fields_new[new_fields[0]], "")), + "to", new_pkg]) + else: + change_list.append(["Added field", s.join(("", new_fields[0], "")), + "to", new_pkg]) + elif len(new_fields) > 1: + seq2 = ["
      • " + new_fields[i] + " with value " + extra_fields_new[new_fields[i]] + "
        • " if extra_fields_new[new_fields[i]] + else "
      • " + new_fields[i] + "
        • " + for i in range(0, len(new_fields))] + change_list.append(["Added the following fields to", new_pkg, "
          ", s.join(seq2), "
        "]) + + # if some fields were deleted + deleted_fields = list(extra_original_set - extra_new_set) + if len(deleted_fields) == 1: + change_list.append(["Removed field", s.join(("", deleted_fields[0], "")), "from", new_pkg]) + elif len(deleted_fields) > 1: + seq2 = ["
      • " + deleted_fields[i] + "
        • " for i in range(0, len(deleted_fields))] + change_list.append(["Removed the following fields from", new_pkg, "
          ", s.join(seq2), "
        "]) + + # if some existing fields were changed + extra_fields = list(extra_new_set.intersection(extra_original_set)) # list of extra fields in both the original and new versions + for field in extra_fields: + if extra_fields_original[field] != extra_fields_new[field]: + if extra_fields_original[field]: + change_list.append(["Changed value of field", s.join(("", field, "")), + "to", s.join(("", extra_fields_new[field], "")), + "(previously", s.join(("", extra_fields_original[field], "")) + ")", + "in", new_pkg]) + else: + change_list.append(["Changed value of field", s.join(("", field, "")), + "to", s.join(("", extra_fields_new[field], "")), + "in", new_pkg]) + + # if the original version didn't have an extras field, the user could only have added a field (not changed or deleted) + else: + new_fields = list(extra_new_set) + if len(new_fields) == 1: + if extra_fields_new[new_fields[0]]: + change_list.append(["Added field", s.join(("", new_fields[0], "")), + "with value", s.join(("", extra_fields_new[new_fields[0]], "")), + "to", new_pkg]) + else: + change_list.append(["Added field", s.join(("", new_fields[0], "")), + "to", new_pkg]) + elif len(new_fields) > 1: + seq2 = ["
      • " + new_fields[i] + " with value " + extra_fields_new[new_fields[i]] + "
        • " if extra_fields_new[new_fields[i]] + else "
      • " + new_fields[i] + "
        • " + for i in range(0, len(new_fields))] + change_list.append(["Added the following fields to", new_pkg, "
          ", s.join(seq2), "
        "]) + + elif 'extras' in original: + deleted_fields = _extras_to_dict(original['extras']).keys() + if len(deleted_fields) == 1: + change_list.append(["Removed field", s.join(("", deleted_fields[0], "")), "from", new_pkg]) + elif len(deleted_fields) > 1: + seq2 = ["
      • " + deleted_fields[i] + "
        • " for i in range(0, len(deleted_fields))] + change_list.append(["Removed the following fields from", new_pkg, "
          ", s.join(seq2), "
        "]) From 3cdffc9e100375b1abb845281684880ddae01290 Mon Sep 17 00:00:00 2001 From: hayley-leblanc Date: Thu, 25 Jul 2019 11:02:52 +0200 Subject: [PATCH 05/47] Handle cases where the value in a metadata field is removed --- ckan/lib/helpers.py | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/ckan/lib/helpers.py b/ckan/lib/helpers.py index 8ee2057671b..182da154160 100644 --- a/ckan/lib/helpers.py +++ b/ckan/lib/helpers.py @@ -2691,8 +2691,6 @@ def sanitize_id(id_): @core_helper def compare_pkg_dicts(original, new): - # TODO: what happens if someone REMOVES a value from a field that previously had a value? - change_list = [] s = "" @@ -2796,8 +2794,10 @@ def _org_change(change_list, original, new, new_pkg): def _maintainer_change(change_list, original, new, new_pkg): # if the original dataset had a maintainer - if original['maintainer']: + if original['maintainer'] and new['maintainer']: change_list.append(["Set maintainer of", new_pkg, "to", new['maintainer'], "(previously", original['maintainer'] + ")"]) + elif not new['maintainer']: + change_list.append(["Removed maintainer from", new_pkg]) else: change_list.append(["Set maintainer of", new_pkg, "to", new['maintainer']]) @@ -2805,16 +2805,20 @@ def _maintainer_email_change(change_list, original, new, new_pkg): s = "" seq2 = ("", new['maintainer_email'], "") # if the original dataset had a maintainer email - if original['maintainer_email']: + if original['maintainer_email'] and new['maintainer_email']: seq3 = ("", original['maintainer_email'], "") change_list.append(["Set maintainer e-mail of", new_pkg, "to", s.join(seq2), "(previously", s.join(seq3) + ")"]) + elif not new['maintainer_email']: + change_list.append(["Removed maintainer e-mail from", new_pkg]) else: change_list.append(["Set maintainer e-mail of", new_pkg, "to", s.join(seq2)]) def _author_change(change_list, original, new, new_pkg): # if the original dataset had an author - if original['author']: + if original['author'] and new['author']: change_list.append(["Set author of", new_pkg, "to", new['author'], "(previously", original['author'] + ")"]) + elif not new['author']: + change_list.append(["Removed author from", new_pkg]) else: change_list.append(["Set author of", new_pkg, "to", new['author']]) @@ -2822,9 +2826,11 @@ def _author_email_change(change_list, original, new, new_pkg): s = "" seq2 = ("", new['author_email'], "") # if the original dataset had a author email - if original['author_email']: + if original['author_email'] and new['author_email']: seq3 = ("", original['author_email'], "") change_list.append(["Set author e-mail of", new_pkg, "to", s.join(seq2), "(previously", s.join(seq3) + ")"]) + elif not new['author_email']: + change_list.append(["Removed author e-mail from", new_pkg]) else: change_list.append(["Set author e-mail of", new_pkg, "to", s.join(seq2)]) @@ -2833,10 +2839,12 @@ def _description_change(change_list, original, new, new_pkg): # TODO: figure out the best way to format this stuff # if the original dataset had a description - if original['notes']: + if original['notes'] and new['notes']: change_list.append(["Updated description of", new_pkg, "from
        ", "
        " + original['notes'] + "
        ", "to
        ", "
        " + new['notes'] + "
        "]) + elif not new['notes']: + change_list.append(["Removed description from", new_pkg]) else: change_list.append(["Updated description of", new_pkg, "to
        ", "
        " + new['notes'] + "
        "]) @@ -2894,14 +2902,18 @@ def _source_url_change(change_list, original, new, new_pkg): s = "" seq2 = ("", original['url'], "") seq3 = ("", new['url'], "") - if original['url']: + if original['url'] and new['url']: change_list.append(["Changed the source URL of", new_pkg, "from", s.join(seq2), "to", s.join(seq3)]) + elif not new['url']: + change_list.append(["Removed source URL from", new_pkg]) else: change_list.append(["Changed the source URL of", new_pkg, "to", s.join(seq3)]) def _version_change(change_list, original, new, new_pkg): - if original['version']: + if original['version'] and new['url']: change_list.append(["Changed the version of", new_pkg, "from", original['version'], "to", new['version']]) + elif not new['url']: + change_list.append(["Removed version number from", new_pkg]) else: change_list.append(["Changed the version of", new_pkg, "to", new['version']]) From fb0c55dd7e0e77813951386e2918430cde6a43e7 Mon Sep 17 00:00:00 2001 From: hayley-leblanc Date: Thu, 25 Jul 2019 11:26:40 +0200 Subject: [PATCH 06/47] Add function documentation --- ckan/lib/helpers.py | 97 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 95 insertions(+), 2 deletions(-) diff --git a/ckan/lib/helpers.py b/ckan/lib/helpers.py index 182da154160..db59bd9ac2b 100644 --- a/ckan/lib/helpers.py +++ b/ckan/lib/helpers.py @@ -2691,6 +2691,15 @@ def sanitize_id(id_): @core_helper def compare_pkg_dicts(original, new): + ''' + Takes two package dictionaries that represent consecutive versions of + the same dataset and returns a list of detailed & formatted summaries of + the changes between the two versions. original and new are the two package + dictionaries. The function assumes that both dictionaries will have + all of the default package dictionary keys, and also checks for additional + keys added by extensions and custom fields added by the user in the web + interface. + ''' change_list = [] s = "" @@ -2764,6 +2773,28 @@ def compare_pkg_dicts(original, new): return change_list def _extras_to_dict(extras_list): + ''' + Takes a list of dictionaries with the following format: + [ + { + "key": , + "value": + }, + ..., + { + "key": , + "value": + } + ] + and converts it into a single dictionary with the following + format: + { + key_0: value_0, + ..., + key_n: value_n + + } + ''' ret_dict = {} # the extras_list is a list of dictionaries for dict in extras_list: @@ -2772,6 +2803,10 @@ def _extras_to_dict(extras_list): return ret_dict def _title_change(change_list, original, new): + ''' + Appends a summary of a change to a dataset's title between two versions + (original and new) to change_list. + ''' s = "" seq2 = ("", @@ -2779,6 +2814,10 @@ def _title_change(change_list, original, new): change_list.append(["Changed title to", s.join(seq2), "(previously", original['title'] + ")"]) def _org_change(change_list, original, new, new_pkg): + ''' + Appends a summary of a change to a dataset's organization between two versions + (original and new) to change_list. + ''' s = "" seq2 = ("", @@ -2793,6 +2832,10 @@ def _org_change(change_list, original, new, new_pkg): s.join(seq3)]) def _maintainer_change(change_list, original, new, new_pkg): + ''' + Appends a summary of a change to a dataset's maintainer field between two + versions (original and new) to change_list. + ''' # if the original dataset had a maintainer if original['maintainer'] and new['maintainer']: change_list.append(["Set maintainer of", new_pkg, "to", new['maintainer'], "(previously", original['maintainer'] + ")"]) @@ -2802,6 +2845,10 @@ def _maintainer_change(change_list, original, new, new_pkg): change_list.append(["Set maintainer of", new_pkg, "to", new['maintainer']]) def _maintainer_email_change(change_list, original, new, new_pkg): + ''' + Appends a summary of a change to a dataset's maintainer e-mail address field + between two versions (original and new) to change_list. + ''' s = "" seq2 = ("", new['maintainer_email'], "") # if the original dataset had a maintainer email @@ -2814,6 +2861,10 @@ def _maintainer_email_change(change_list, original, new, new_pkg): change_list.append(["Set maintainer e-mail of", new_pkg, "to", s.join(seq2)]) def _author_change(change_list, original, new, new_pkg): + ''' + Appends a summary of a change to a dataset's author field between two versions + (original and new) to change_list. + ''' # if the original dataset had an author if original['author'] and new['author']: change_list.append(["Set author of", new_pkg, "to", new['author'], "(previously", original['author'] + ")"]) @@ -2823,6 +2874,10 @@ def _author_change(change_list, original, new, new_pkg): change_list.append(["Set author of", new_pkg, "to", new['author']]) def _author_email_change(change_list, original, new, new_pkg): + ''' + Appends a summary of a change to a dataset's author e-mail address field + between two versions (original and new) to change_list. + ''' s = "" seq2 = ("", new['author_email'], "") # if the original dataset had a author email @@ -2835,8 +2890,12 @@ def _author_email_change(change_list, original, new, new_pkg): change_list.append(["Set author e-mail of", new_pkg, "to", s.join(seq2)]) def _description_change(change_list, original, new, new_pkg): - # displays the two descriptions (like how they are displayed on resource views) - # TODO: figure out the best way to format this stuff + ''' + Appends a summary of a change to a dataset's description between two versions + (original and new) to change_list. + ''' + + # TODO: find a better way to format the descriptions along with the change summary # if the original dataset had a description if original['notes'] and new['notes']: @@ -2850,6 +2909,10 @@ def _description_change(change_list, original, new, new_pkg): "to
        ", "
        " + new['notes'] + "
        "]) def _tag_change(change_list, new_tags, original_tags, new_pkg): + ''' + Appends a summary of a change to a dataset's tag list between two versions + (original and new) to change_list. + ''' s = "" deleted_tags = original_tags - new_tags deleted_tags_list = list(deleted_tags) @@ -2874,6 +2937,10 @@ def _tag_change(change_list, new_tags, original_tags, new_pkg): change_list.append(["Added the following tags to", new_pkg, "
          ", s.join(seq2), "
        "]) def _license_change(change_list, original, new, new_pkg): + ''' + Appends a summary of a change to a dataset's license between two versions + (original and new) to change_list. + ''' s = "" seq2 = () seq3 = () @@ -2889,6 +2956,10 @@ def _license_change(change_list, original, new, new_pkg): change_list.append(["Changed the license of", new_pkg, "to", s.join(seq3), "(previously", s.join(seq2) + ")"]) def _name_change(change_list, original, new): + ''' + Appends a summary of a change to a dataset's name (and thus the URL it can + be accessed at) between two versions (original and new) to change_list. + ''' s = "" old_url = url_for(qualified=True, controller="dataset", action="read", id=original['name']) @@ -2899,6 +2970,11 @@ def _name_change(change_list, original, new): change_list.append(["Moved the dataset from", s.join(seq2), "to", s.join(seq3)]) def _source_url_change(change_list, original, new, new_pkg): + ''' + Appends a summary of a change to a dataset's source URL (metadata field, not + its actual URL in the datahub) between two versions (original and new) to + change_list. + ''' s = "" seq2 = ("", original['url'], "") seq3 = ("", new['url'], "") @@ -2910,6 +2986,10 @@ def _source_url_change(change_list, original, new, new_pkg): change_list.append(["Changed the source URL of", new_pkg, "to", s.join(seq3)]) def _version_change(change_list, original, new, new_pkg): + ''' + Appends a summary of a change to a dataset's version field (inputted by the user, + not from version control) between two versions (original and new) to change_list. + ''' if original['version'] and new['url']: change_list.append(["Changed the version of", new_pkg, "from", original['version'], "to", new['version']]) elif not new['url']: @@ -2918,6 +2998,14 @@ def _version_change(change_list, original, new, new_pkg): change_list.append(["Changed the version of", new_pkg, "to", new['version']]) def _extension_fields(change_list, original, new, new_pkg): + ''' + Checks whether any fields that have been added to the package dictionaries + by CKAN extensions have been changed between versions. If there have been + any changes between the two versions (original and new), a general summary + of the change is appended to change_list. This function does not produce + summaries for fields added or deleted by extensions, since these changes are + not triggered by the user in the web interface or API. + ''' # list of the default metadata fields for a dataset # any fields that are not part of this list are custom fields added by a user or extension fields = ['owner_org', 'maintainer', 'maintainer_email', 'relationships_as_object', 'private', 'num_tags', @@ -2952,6 +3040,11 @@ def _extra_fields(change_list, original, new, new_pkg): # check the extras field to see if anything has been added, deleted, or changed # that is where custom fields added via the web interface go - they do not become # actual fields in a package dict + ''' + Checks whether a user has added, removed, or changed any custom fields from + the web interface (or API?) and appends a summary of each change to + change_list. + ''' # what if they added the field but didn't add a value? s = "" From 6921941df981ce8af5bf37669cdbaece2f5011a1 Mon Sep 17 00:00:00 2001 From: hayley-leblanc Date: Thu, 25 Jul 2019 14:24:25 +0200 Subject: [PATCH 07/47] Add activity summaries for changes to resources --- ckan/lib/helpers.py | 200 +++++++++++++++++++++++----- ckan/templates/package/changes.html | 2 +- 2 files changed, 168 insertions(+), 34 deletions(-) diff --git a/ckan/lib/helpers.py b/ckan/lib/helpers.py index db59bd9ac2b..48abe98c618 100644 --- a/ckan/lib/helpers.py +++ b/ckan/lib/helpers.py @@ -2690,7 +2690,7 @@ def sanitize_id(id_): return str(uuid.UUID(id_)) @core_helper -def compare_pkg_dicts(original, new): +def compare_pkg_dicts(original, new, old_activity_id): ''' Takes two package dictionaries that represent consecutive versions of the same dataset and returns a list of detailed & formatted summaries of @@ -2708,6 +2708,172 @@ def compare_pkg_dicts(original, new): new['title'], "") new_pkg = s.join(seq1) + _check_metadata_changes(change_list, original, new, new_pkg) + + _check_resource_changes(change_list, original, new, new_pkg, old_activity_id) + + return change_list + +def _extras_to_dict(extras_list): + ''' + Takes a list of dictionaries with the following format: + [ + { + "key": , + "value": + }, + ..., + { + "key": , + "value": + } + ] + and converts it into a single dictionary with the following + format: + { + key_0: value_0, + ..., + key_n: value_n + + } + ''' + ret_dict = {} + # the extras_list is a list of dictionaries + for dict in extras_list: + ret_dict[dict['key']] = dict['value'] + + return ret_dict + +def _check_resource_changes(change_list, original, new, new_pkg, old_activity_id): + ''' + Checks whether a dataset's resources have changed - whether new ones have been uploaded, + existing ones have been deleted, or existing ones have been edited. For existing + resources, checks whether their names, formats, and/or descriptions have changed, as well + as whether a new file has been uploaded for the resource. + ''' + + # TODO: clean this up + + # make a set of the resource IDs present in original and new + original_resource_set = set() + original_resource_dict = {} + new_resource_set = set() + new_resource_dict = {} + s = "" + + for resource in original['resources']: + original_resource_set.add(resource['id']) + original_resource_dict[resource['id']] = {'name': resource['name'], + 'url': resource['url'], + 'description': resource['description'], + 'format': resource['format']} + + + for resource in new['resources']: + new_resource_set.add(resource['id']) + new_resource_dict[resource['id']] = {'name': resource['name'], + 'url': resource['url'], + 'description': resource['description'], + 'format': resource['format']} + + # get the IDs of the resources that have been added between the versions + new_resources = list(new_resource_set - original_resource_set) + for resource_id in new_resources: + seq2 = ("", + new_resource_dict[resource_id]['name'], "") + change_list.append(["Added resource", s.join(seq2), "to", new_pkg]) + + # get the IDs of resources that have been deleted between versions + deleted_resources = list(original_resource_set - new_resource_set) + for resource_id in deleted_resources: + seq2 = ("", + original_resource_dict[resource_id]['name'], "") + change_list.append(["Deleted resource", s.join(seq2), "from", new_pkg]) + + # now check the resources that are in both and see if any have been changed + + # TODO: only one resource can be edited at a time like this right? + # so we could stop once we find the one that is edited + resources = new_resource_set.intersection(original_resource_set) + for resource_id in resources: + original_metadata = original_resource_dict[resource_id] + new_metadata = new_resource_dict[resource_id] + + if original_metadata['name'] != new_metadata['name']: + seq2 = ("", + original_resource_dict[resource_id]['name'], "") + seq3 = ("", + new_resource_dict[resource_id]['name'], "") + change_list.append(["Renamed resource", s.join(seq2), "to", s.join(seq3), "in", new_pkg]) + + # you can't remove a format, but if a resource's format isn't recognized, it won't have one set + # if a format was not originally set and the user set one + if not original_metadata['format'] and new_metadata['format']: + seq2 = ("", + new_resource_dict[resource_id]['name'], "") + seq3 = ("", + new_metadata['format'], "") + change_list.append(["Set format of resource", s.join(seq2), "to", s.join(seq3), "in", new_pkg]) + # if both versions have a format but the format changed + elif original_metadata['format'] != new_metadata['format']: + seq2 = ("", + original_resource_dict[resource_id]['name'], "") + seq3 = ("", + new_metadata['format'], "") + seq4 = ("", + original_metadata['format'], "") + change_list.append(["Set format of resource", s.join(seq2), "to", s.join(seq3), "(previously", s.join(seq4) + ")", "in", new_pkg]) + + # if the description changed + if not original_metadata['description'] and new_metadata['description']: + seq2 = ("", + new_resource_dict[resource_id]['name'], "") + change_list.append(["Updated description of resource", s.join(seq2), "in", + new_pkg, "to
        ", + "
        " + new_metadata['description'] + "
        "]) + + # if there was a description but the user removed it + elif original_metadata['description'] and not new_metadata['description']: + seq2 = ("", + new_resource_dict[resource_id]['name'], "") + change_list.append(["Removed description from resource", s.join(seq2), "in", new_pkg]) + + # if both have descriptions but they are different + elif original_metadata['description'] != new_metadata['description']: + seq2 = ("", + new_resource_dict[resource_id]['name'], "") + change_list.append(["Updated description of resource", s.join(seq2), "in", + new_pkg, "from
        ", + "
        " + original_metadata['description'] + "
        ", + "to
        ", + "
        " + new_metadata['description'] + "
        "]) + + # check if the user uploaded a new file + # TODO: use regular expressions to determine the actual name of the new and old files + if original_metadata['url'] != new_metadata['url']: + seq2 = ("", + new_resource_dict[resource_id]['name'], "") + change_list.append(["Uploaded a new file to resource", s.join(seq2), "in", new_pkg]) + +def _check_metadata_changes(change_list, original, new, new_pkg): + ''' + Checks whether a dataset's metadata fields (fields in its package dictionary + not including resources) have changed between two consecutive versions and + puts a list of formatted summaries of these changes in change_list. + ''' # if the title has changed if original['title'] != new['title']: _title_change(change_list, original, new) @@ -2770,38 +2936,6 @@ def compare_pkg_dicts(original, new): _extension_fields(change_list, original, new, new_pkg) _extra_fields(change_list, original, new, new_pkg) - return change_list - -def _extras_to_dict(extras_list): - ''' - Takes a list of dictionaries with the following format: - [ - { - "key": , - "value": - }, - ..., - { - "key": , - "value": - } - ] - and converts it into a single dictionary with the following - format: - { - key_0: value_0, - ..., - key_n: value_n - - } - ''' - ret_dict = {} - # the extras_list is a list of dictionaries - for dict in extras_list: - ret_dict[dict['key']] = dict['value'] - - return ret_dict - def _title_change(change_list, original, new): ''' Appends a summary of a change to a dataset's title between two versions diff --git a/ckan/templates/package/changes.html b/ckan/templates/package/changes.html index 996494955ef..27a84f417fe 100644 --- a/ckan/templates/package/changes.html +++ b/ckan/templates/package/changes.html @@ -32,7 +32,7 @@

        {{ _('Changes') }}

        {% for i in range(1, activity_diff.activities|length) %} On {{ h.render_datetime(activity_diff.activities[1].timestamp, with_hours=True, with_seconds=True) }}, {{ h.linked_user(activity_diff.activities[1].user_id) }}: - {% set changes = h.compare_pkg_dicts(activity_diff.activities[0].data.package, activity_diff.activities[1].data.package) %} + {% set changes = h.compare_pkg_dicts(activity_diff.activities[0].data.package, activity_diff.activities[1].data.package, activity_diff.activities[0].id) %}
          {% for change in changes %}
        • From 43ffc3e99a48523347af612a06f2247a0b8b6579 Mon Sep 17 00:00:00 2001 From: hayley-leblanc Date: Fri, 26 Jul 2019 11:04:27 +0200 Subject: [PATCH 08/47] Add ability to view multiple consecutive change summaries --- ckan/lib/helpers.py | 20 +++++ ckan/templates/package/changes.html | 73 ++++++------------- .../package/snippets/change_item.html | 42 +++++++++++ .../snippets/activities/changed_package.html | 2 + ckan/views/dataset.py | 52 ++++++++++++- 5 files changed, 138 insertions(+), 51 deletions(-) create mode 100644 ckan/templates/package/snippets/change_item.html diff --git a/ckan/lib/helpers.py b/ckan/lib/helpers.py index 48abe98c618..8b92e6fdf76 100644 --- a/ckan/lib/helpers.py +++ b/ckan/lib/helpers.py @@ -2712,6 +2712,11 @@ def compare_pkg_dicts(original, new, old_activity_id): _check_resource_changes(change_list, original, new, new_pkg, old_activity_id) + # if the dataset was updated but none of the fields we check were changed, + # display a message stating that + if len(change_list) == 0: + change_list.append(["No fields were updated. See metadata diff for more details."]) + return change_list def _extras_to_dict(extras_list): @@ -3254,3 +3259,18 @@ def _extra_fields(change_list, original, new, new_pkg): elif len(deleted_fields) > 1: seq2 = ["
        • " + deleted_fields[i] + "
          • " for i in range(0, len(deleted_fields))] change_list.append(["Removed the following fields from", new_pkg, "
            ", s.join(seq2), "
          "]) + + +@core_helper +def activity_list_select(pkg_activity_list, activity_id): + select_list = [] + for activity in pkg_activity_list: + #entry = render_datetime(activity['timestamp']) + " #" + activity['id'][0:8] + #entry = activity['id'] + entry = render_datetime(activity['timestamp'], with_hours=True) + if activity['id'] == activity_id: + select_list.append("") + else: + select_list.append("") + + return select_list diff --git a/ckan/templates/package/changes.html b/ckan/templates/package/changes.html index 27a84f417fe..506bfec8a3a 100644 --- a/ckan/templates/package/changes.html +++ b/ckan/templates/package/changes.html @@ -7,12 +7,14 @@ {% block breadcrumb_content %} {{ super() }}
        • {% link_for _('Changes'), named_route='dataset.activity', id=pkg_dict.name %}
          • -
        • {% link_for activity_diff.activities[1].id|truncate(30), named_route='dataset.changes', id=activity_diff.activities[1].id %}
          • +
        • {% link_for activity_diffs[0].activities[1].id|truncate(30), named_route='dataset.changes', id=activity_diffs[0].activities[1].id %}
          • {% endblock %} {% block primary %}
          +
          +
          {% block package_changes_header %}

          {{ _('Changes') }}

          {##} {% endblock %} - {# iterate through the activities that are NOT the original one - - iterate, rather than just look at the second one, so that we could - potentially add a way to look at the diff between larger sets of - datasets in the future #} - - {% for i in range(1, activity_diff.activities|length) %} - On {{ h.render_datetime(activity_diff.activities[1].timestamp, with_hours=True, with_seconds=True) }}, {{ h.linked_user(activity_diff.activities[1].user_id) }}: + {% set select_list1 = h.activity_list_select(pkg_activity_list, activity_diffs[0].activities[0].id) %} + {% set select_list2 = h.activity_list_select(pkg_activity_list, activity_diffs[0].activities[1].id) %} +
          + View changes from + to + + +
          - {% set changes = h.compare_pkg_dicts(activity_diff.activities[0].data.package, activity_diff.activities[1].data.package, activity_diff.activities[0].id) %} -
            - {% for change in changes %} -
          • - {% for element in change %} - {{ element|safe }} - {% endfor %} -
            • -
            - {% endfor %} -

          + {% for i in range(activity_diffs|length) %} + {% snippet "package/snippets/change_item.html", activity_diff=activity_diffs[i], pkg_dict=pkg_dict %} +
          +
          {% endfor %} - {# button to show JSON metadata diff - not shown by default #} -
          - - - - - -
          {% endblock %} diff --git a/ckan/templates/package/snippets/change_item.html b/ckan/templates/package/snippets/change_item.html new file mode 100644 index 00000000000..521ca019456 --- /dev/null +++ b/ckan/templates/package/snippets/change_item.html @@ -0,0 +1,42 @@ +On {{ h.render_datetime(activity_diff.activities[1].timestamp, with_hours=True, with_seconds=True) }}, {{ h.linked_user(activity_diff.activities[1].user_id) }}: + +{% set changes = h.compare_pkg_dicts(activity_diff.activities[0].data.package, activity_diff.activities[1].data.package, activity_diff.activities[0].id) %} +
            +{% for change in changes %} +
          • + {% for element in change %} + {{ element|safe }} + {% endfor %} +
            • +
            +{% endfor %} +
          + +{# button to show JSON metadata diff - not shown by default #} + + + + diff --git a/ckan/templates/snippets/activities/changed_package.html b/ckan/templates/snippets/activities/changed_package.html index 4bef2b6d5fa..a1dd2155a41 100644 --- a/ckan/templates/snippets/activities/changed_package.html +++ b/ckan/templates/snippets/activities/changed_package.html @@ -17,6 +17,8 @@ {{ _('Changes') }} +
          + Activity ID: {{ activity.id }} {% endif %}

          diff --git a/ckan/views/dataset.py b/ckan/views/dataset.py index 40e8dad4992..a6a8421bc61 100644 --- a/ckan/views/dataset.py +++ b/ckan/views/dataset.py @@ -1103,14 +1103,62 @@ def changes(id, package_type=None): # changed, and we need a link to it which works pkg_id = activity_diff[u'activities'][1][u'data'][u'package'][u'id'] current_pkg_dict = get_action(u'package_show')(context, {u'id': pkg_id}) + pkg_activity_list = get_action(u'package_activity_list')(context, {u'id': pkg_id, u'limit': 100}) return base.render( u'package/changes.html', { - u'activity_diff': activity_diff, + u'activity_diffs': [activity_diff], u'pkg_dict': current_pkg_dict, + u'pkg_activity_list': pkg_activity_list, } ) +def change_range(package_type=None): + + newest_id = h.get_request_param('newest_id') + oldest_id = h.get_request_param('oldest_id') + + context = { + u'model': model, u'session': model.Session, + u'user': g.user, u'auth_user_obj': g.userobj + } + + done = False + current_id = newest_id + diff_list = [] + + while done == False: + try: + activity_diff = get_action(u'activity_diff')( + context, {u'id': current_id, u'object_type': u'package', + u'diff_type': u'html'}) + except NotFound as e: + log.info(u'Activity not found: {} - {}'.format(str(e), activity_id)) + return base.abort(404, _(u'Activity not found')) + except NotAuthorized: + return base.abort(403, _(u'Unauthorized to view activity data')) + + diff_list.append(activity_diff) + + if activity_diff['activities'][0]['id'] == oldest_id: + done = True + else: + current_id = activity_diff['activities'][0]['id'] + + pkg_id = diff_list[0][u'activities'][1][u'data'][u'package'][u'id'] + current_pkg_dict = get_action(u'package_show')(context, {u'id': pkg_id}) + pkg_activity_list = get_action(u'package_activity_list')(context, {u'id': pkg_id, u'limit': 100}) + + return base.render( + u'package/changes.html', { + u'activity_diffs': diff_list, + u'pkg_dict': current_pkg_dict, + u'pkg_activity_list': pkg_activity_list, + } + ) + + + # deprecated def history(package_type, id): @@ -1142,6 +1190,8 @@ def register_dataset_plugin_rules(blueprint): blueprint.add_url_rule(u'/changes/', view_func=changes) blueprint.add_url_rule(u'//history', view_func=history) + blueprint.add_url_rule(u'/change_range', view_func=change_range) + # Duplicate resource create and edit for backward compatibility. Note, # we cannot use resource.CreateView directly here, because of # circular imports From a5a187d09f6d8a465b9600421df89bcbe12d1b50 Mon Sep 17 00:00:00 2001 From: hayley-leblanc Date: Fri, 26 Jul 2019 12:49:33 +0200 Subject: [PATCH 09/47] Fix out-of-order change summary viewing and set drop down menus to current versions --- ckan/lib/helpers.py | 4 ++-- ckan/templates/package/changes.html | 12 +++--------- ckan/views/dataset.py | 23 ++++++++++++++++++++--- 3 files changed, 25 insertions(+), 14 deletions(-) diff --git a/ckan/lib/helpers.py b/ckan/lib/helpers.py index 8b92e6fdf76..70306090c47 100644 --- a/ckan/lib/helpers.py +++ b/ckan/lib/helpers.py @@ -3262,13 +3262,13 @@ def _extra_fields(change_list, original, new, new_pkg): @core_helper -def activity_list_select(pkg_activity_list, activity_id): +def activity_list_select(pkg_activity_list, current_activity_id): select_list = [] for activity in pkg_activity_list: #entry = render_datetime(activity['timestamp']) + " #" + activity['id'][0:8] #entry = activity['id'] entry = render_datetime(activity['timestamp'], with_hours=True) - if activity['id'] == activity_id: + if activity['id'] == current_activity_id: select_list.append("") else: select_list.append("") diff --git a/ckan/templates/package/changes.html b/ckan/templates/package/changes.html index 506bfec8a3a..454eceb4ce2 100644 --- a/ckan/templates/package/changes.html +++ b/ckan/templates/package/changes.html @@ -17,18 +17,12 @@ {% block package_changes_header %}

          {{ _('Changes') }}

          - {##} {% endblock %} - - {% set select_list1 = h.activity_list_select(pkg_activity_list, activity_diffs[0].activities[0].id) %} + {% set select_list1 = h.activity_list_select(pkg_activity_list, activity_diffs[-1].activities[0].id) %} {% set select_list2 = h.activity_list_select(pkg_activity_list, activity_diffs[0].activities[1].id) %}
          + + View changes from View changes from - 
                       {{ select_list1[1:]|safe }}
          to - 
                       {{ select_list2|safe }}
          @@ -39,10 +40,12 @@

          {{ _('Changes') }}


          + {# iterate through the list of activity diffs #} +
          {% for i in range(activity_diffs|length) %} {% snippet "package/snippets/change_item.html", activity_diff=activity_diffs[i], pkg_dict=pkg_dict %}
          -
          +
          {% endfor %} diff --git a/ckan/templates/package/snippets/change_item.html b/ckan/templates/package/snippets/change_item.html index 521ca019456..6512e9b34c6 100644 --- a/ckan/templates/package/snippets/change_item.html +++ b/ckan/templates/package/snippets/change_item.html @@ -1,4 +1,4 @@ -On {{ h.render_datetime(activity_diff.activities[1].timestamp, with_hours=True, with_seconds=True) }}, {{ h.linked_user(activity_diff.activities[1].user_id) }}: +On {{ h.render_datetime(activity_diff.activities[1].timestamp, with_hours=True, with_seconds=True) }}, {{ h.linked_user(activity_diff.activities[1].user_id) }}: {% set changes = h.compare_pkg_dicts(activity_diff.activities[0].data.package, activity_diff.activities[1].data.package, activity_diff.activities[0].id) %}
            diff --git a/ckan/views/dataset.py b/ckan/views/dataset.py index 05de2e2fb33..bb997501969 100644 --- a/ckan/views/dataset.py +++ b/ckan/views/dataset.py @@ -1115,6 +1115,11 @@ def changes(id, package_type=None): ) def change_range(package_type=None): + ''' + Called when a user specifies a range of versions they want to look at changes between. + Verifies that the range is valid and finds the set of activity diffs for the changes + in the given version range, then re-renders changes.html with the list. + ''' newest_id = h.get_request_param('newest_id') oldest_id = h.get_request_param('oldest_id') From 4e2d0cb37de61201991e224f0d786a43953d723e Mon Sep 17 00:00:00 2001 From: hayley-leblanc Date: Mon, 29 Jul 2019 10:41:10 +0200 Subject: [PATCH 11/47] Replace activity_id with current_id in change_range --- ckan/views/dataset.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ckan/views/dataset.py b/ckan/views/dataset.py index bb997501969..0c5712e0cfc 100644 --- a/ckan/views/dataset.py +++ b/ckan/views/dataset.py @@ -1158,7 +1158,7 @@ def change_range(package_type=None): context, {u'id': current_id, u'object_type': u'package', u'diff_type': u'html'}) except NotFound as e: - log.info(u'Activity not found: {} - {}'.format(str(e), activity_id)) + log.info(u'Activity not found: {} - {}'.format(str(e), current_id)) return base.abort(404, _(u'Activity not found')) except NotAuthorized: return base.abort(403, _(u'Unauthorized to view activity data')) From dd0041cd3b7ff3ab76e7279e4b9321e84680479f Mon Sep 17 00:00:00 2001 From: hayley-leblanc Date: Mon, 29 Jul 2019 11:22:12 +0200 Subject: [PATCH 12/47] Formatting --- .../lib/activity_streams_session_extension.py | 4 +- ckan/lib/changes.py | 516 ++++++++++++------ ckan/lib/helpers.py | 30 +- ckan/views/dataset.py | 40 +- 4 files changed, 400 insertions(+), 190 deletions(-) diff --git a/ckan/lib/activity_streams_session_extension.py b/ckan/lib/activity_streams_session_extension.py index decdeeb6cde..8a9d3791e1d 100644 --- a/ckan/lib/activity_streams_session_extension.py +++ b/ckan/lib/activity_streams_session_extension.py @@ -70,7 +70,7 @@ def before_commit(self, session): # object is a package. # Don't create activities for private datasets. - #if obj.private: + # if obj.private: # continue activities[obj.id] = activity @@ -106,7 +106,7 @@ def before_commit(self, session): continue # Don't create activities for private datasets. - #if package.private: + # if package.private: # continue if package.id in activities: diff --git a/ckan/lib/changes.py b/ckan/lib/changes.py index 87403cde0a4..e1ae63c6f81 100644 --- a/ckan/lib/changes.py +++ b/ckan/lib/changes.py @@ -1,10 +1,11 @@ ''' -Functions used by the helper function compare_pkg_dicts() to analyze the differences -between two versions of a dataset. +Functions used by the helper function compare_pkg_dicts() to analyze +the differences between two versions of a dataset. ''' from helpers import url_for + def _extras_to_dict(extras_list): ''' Takes a list of dictionaries with the following format: @@ -35,16 +36,17 @@ def _extras_to_dict(extras_list): return ret_dict -def _check_resource_changes(change_list, original, new, new_pkg, old_activity_id): + +def _check_resource_changes(change_list, original, new, new_pkg, + old_activity_id): ''' - Checks whether a dataset's resources have changed - whether new ones have been uploaded, - existing ones have been deleted, or existing ones have been edited. For existing - resources, checks whether their names, formats, and/or descriptions have changed, as well - as whether a new file has been uploaded for the resource. + Checks whether a dataset's resources have changed - whether new ones have + been uploaded, existing ones have been deleted, or existing ones have + been edited. For existing resources, checks whether their names, formats, + and/or descriptions have changed, as well as whether a new file has been + uploaded for the resource. ''' - # TODO: clean this up - # make a set of the resource IDs present in original and new original_resource_set = set() original_resource_dict = {} @@ -54,18 +56,21 @@ def _check_resource_changes(change_list, original, new, new_pkg, old_activity_id for resource in original['resources']: original_resource_set.add(resource['id']) - original_resource_dict[resource['id']] = {'name': resource['name'], - 'url': resource['url'], - 'description': resource['description'], - 'format': resource['format']} - + original_resource_dict[resource['id']] = { + 'name': resource['name'], + 'url': resource['url'], + 'description': resource['description'], + 'format': resource['format'] + } for resource in new['resources']: new_resource_set.add(resource['id']) - new_resource_dict[resource['id']] = {'name': resource['name'], - 'url': resource['url'], - 'description': resource['description'], - 'format': resource['format']} + new_resource_dict[resource['id']] = { + 'name': resource['name'], + 'url': resource['url'], + 'description': resource['description'], + 'format': resource['format'] + } # get the IDs of the resources that have been added between the versions new_resources = list(new_resource_set - original_resource_set) @@ -73,91 +78,151 @@ def _check_resource_changes(change_list, original, new, new_pkg, old_activity_id seq2 = ("", new_resource_dict[resource_id]['name'], "") - change_list.append(["Added resource", s.join(seq2), "to", new_pkg]) + change_list.append(["Added resource", + s.join(seq2), "to", + new_pkg]) # get the IDs of resources that have been deleted between versions deleted_resources = list(original_resource_set - new_resource_set) for resource_id in deleted_resources: seq2 = ("", + action="read", id=original['id'], resource_id=resource_id) + + "?activity_id=" + old_activity_id, "\">", original_resource_dict[resource_id]['name'], "") change_list.append(["Deleted resource", s.join(seq2), "from", new_pkg]) - # now check the resources that are in both and see if any have been changed - - # TODO: only one resource can be edited at a time like this right? - # so we could stop once we find the one that is edited + # now check the resources that are in both and see if any + # have been changed resources = new_resource_set.intersection(original_resource_set) for resource_id in resources: original_metadata = original_resource_dict[resource_id] new_metadata = new_resource_dict[resource_id] if original_metadata['name'] != new_metadata['name']: - seq2 = ("", + seq2 = ("", original_resource_dict[resource_id]['name'], "") - seq3 = ("", + seq3 = ("", new_resource_dict[resource_id]['name'], "") - change_list.append(["Renamed resource", s.join(seq2), "to", s.join(seq3), "in", new_pkg]) + change_list.append(["Renamed resource", s.join(seq2), + "to", s.join(seq3), "in", new_pkg]) + + # you can't remove a format, but if a resource's format isn't + # recognized, it won't have one set - # you can't remove a format, but if a resource's format isn't recognized, it won't have one set # if a format was not originally set and the user set one if not original_metadata['format'] and new_metadata['format']: - seq2 = ("", + seq2 = ("", new_resource_dict[resource_id]['name'], "") - seq3 = ("", + seq3 = ("", new_metadata['format'], "") - change_list.append(["Set format of resource", s.join(seq2), "to", s.join(seq3), "in", new_pkg]) + change_list.append(["Set format of resource", s.join(seq2), + "to", s.join(seq3), "in", new_pkg]) # if both versions have a format but the format changed elif original_metadata['format'] != new_metadata['format']: - seq2 = ("", + seq2 = ("", original_resource_dict[resource_id]['name'], "") - seq3 = ("", + seq3 = ("", new_metadata['format'], "") - seq4 = ("", + seq4 = ("", original_metadata['format'], "") - change_list.append(["Set format of resource", s.join(seq2), "to", s.join(seq3), "(previously", s.join(seq4) + ")", "in", new_pkg]) + change_list.append(["Set format of resource", + s.join(seq2), + "to", s.join(seq3), + "(previously", s.join(seq4) + ")", + "in", new_pkg]) # if the description changed - if not original_metadata['description'] and new_metadata['description']: - seq2 = ("", + if not original_metadata['description'] and + new_metadata['description']: + seq2 = ("", new_resource_dict[resource_id]['name'], "") - change_list.append(["Updated description of resource", s.join(seq2), "in", + change_list.append(["Updated description of resource", + s.join(seq2), "in", new_pkg, "to
            ", - "
            " + new_metadata['description'] + "
            "]) + "
            " + new_metadata['description'] + + "
            "]) # if there was a description but the user removed it - elif original_metadata['description'] and not new_metadata['description']: - seq2 = ("", + elif original_metadata['description'] and + not new_metadata['description']: + seq2 = ("", new_resource_dict[resource_id]['name'], "") - change_list.append(["Removed description from resource", s.join(seq2), "in", new_pkg]) + change_list.append(["Removed description from resource", + s.join(seq2), "in", new_pkg]) # if both have descriptions but they are different elif original_metadata['description'] != new_metadata['description']: - seq2 = ("", + seq2 = ("", new_resource_dict[resource_id]['name'], "") - change_list.append(["Updated description of resource", s.join(seq2), "in", - new_pkg, "from
            ", - "
            " + original_metadata['description'] + "
            ", + change_list.append(["Updated description of resource", + s.join(seq2), "in", + new_pkg, + "from
            ", + "
            " + + original_metadata['description'] + + "
            ", "to
            ", - "
            " + new_metadata['description'] + "
            "]) + "
            " + + new_metadata['description'] + + "
            "]) # check if the user uploaded a new file # TODO: use regular expressions to determine the actual name of the new and old files if original_metadata['url'] != new_metadata['url']: - seq2 = ("", + seq2 = ("", new_resource_dict[resource_id]['name'], "") - change_list.append(["Uploaded a new file to resource", s.join(seq2), "in", new_pkg]) + change_list.append(["Uploaded a new file to resource", + s.join(seq2), "in", new_pkg]) + def _check_metadata_changes(change_list, original, new, new_pkg): ''' @@ -191,7 +256,8 @@ def _check_metadata_changes(change_list, original, new, new_pkg): # if the visibility of the dataset changed if original['private'] != new['private']: - change_list.append(["Set visibility of", new_pkg, "to", "Private" if new['private'] else "Public"]) + change_list.append(["Set visibility of", new_pkg, "to", + "Private" if new['private'] else "Public"]) # if the description of the dataset changed if original['notes'] != new['notes']: @@ -209,11 +275,13 @@ def _check_metadata_changes(change_list, original, new, new_pkg): _license_change(change_list, original, new, new_pkg) # if the name of the dataset has changed - # this is only visible to the user via the dataset's URL, so display the change using that + # this is only visible to the user via the dataset's URL, + # so display the change using that if original['name'] != new['name']: _name_change(change_list, original, new) - # if the source URL (metadata value, not the actual URL of the dataset) has changed + # if the source URL (metadata value, not the actual URL of the dataset) + # has changed if original['url'] != new['url']: _source_url_change(change_list, original, new, new_pkg) @@ -221,8 +289,8 @@ def _check_metadata_changes(change_list, original, new, new_pkg): if original['version'] != new['version']: _version_change(change_list, original, new, new_pkg) - # check whether fields added by extensions or custom fields (in the "extras" field) - # have been changed + # check whether fields added by extensions or custom fields + # (in the "extras" field) have been changed _extension_fields(change_list, original, new, new_pkg) _extra_fields(change_list, original, new, new_pkg) @@ -236,25 +304,34 @@ def _title_change(change_list, original, new): seq2 = ("", new['title'], "") - change_list.append(["Changed title to", s.join(seq2), "(previously", original['title'] + ")"]) + change_list.append(["Changed title to", s.join(seq2), + "(previously", original['title'] + ")"]) + def _org_change(change_list, original, new, new_pkg): ''' - Appends a summary of a change to a dataset's organization between two versions - (original and new) to change_list. + Appends a summary of a change to a dataset's organization between + two versions (original and new) to change_list. ''' s = "" - seq2 = ("", + seq2 = ("", original['organization']['title'], "") - seq3 = ("", + seq3 = ("", new['organization']['title'], "") change_list.append(["Moved", new_pkg, - "from organization", - s.join(seq2), - "to organization", - s.join(seq3)]) + "from organization", + s.join(seq2), + "to organization", + s.join(seq3)]) + def _maintainer_change(change_list, original, new, new_pkg): ''' @@ -263,27 +340,36 @@ def _maintainer_change(change_list, original, new, new_pkg): ''' # if the original dataset had a maintainer if original['maintainer'] and new['maintainer']: - change_list.append(["Set maintainer of", new_pkg, "to", new['maintainer'], "(previously", original['maintainer'] + ")"]) + change_list.append(["Set maintainer of", new_pkg, + "to", new['maintainer'], + "(previously", original['maintainer'] + ")"]) elif not new['maintainer']: change_list.append(["Removed maintainer from", new_pkg]) else: change_list.append(["Set maintainer of", new_pkg, "to", new['maintainer']]) + def _maintainer_email_change(change_list, original, new, new_pkg): ''' - Appends a summary of a change to a dataset's maintainer e-mail address field - between two versions (original and new) to change_list. + Appends a summary of a change to a dataset's maintainer e-mail address + field between two versions (original and new) to change_list. ''' s = "" - seq2 = ("", new['maintainer_email'], "") + seq2 = ("", + new['maintainer_email'], "") # if the original dataset had a maintainer email if original['maintainer_email'] and new['maintainer_email']: - seq3 = ("", original['maintainer_email'], "") - change_list.append(["Set maintainer e-mail of", new_pkg, "to", s.join(seq2), "(previously", s.join(seq3) + ")"]) + seq3 = ("", + original['maintainer_email'], "") + change_list.append(["Set maintainer e-mail of", + new_pkg, "to", s.join(seq2), + "(previously", s.join(seq3) + ")"]) elif not new['maintainer_email']: change_list.append(["Removed maintainer e-mail from", new_pkg]) else: - change_list.append(["Set maintainer e-mail of", new_pkg, "to", s.join(seq2)]) + change_list.append(["Set maintainer e-mail of", + new_pkg, "to", s.join(seq2)]) + def _author_change(change_list, original, new, new_pkg): ''' @@ -292,27 +378,34 @@ def _author_change(change_list, original, new, new_pkg): ''' # if the original dataset had an author if original['author'] and new['author']: - change_list.append(["Set author of", new_pkg, "to", new['author'], "(previously", original['author'] + ")"]) + change_list.append(["Set author of", new_pkg, "to", new['author'], + "(previously", original['author'] + ")"]) elif not new['author']: change_list.append(["Removed author from", new_pkg]) else: change_list.append(["Set author of", new_pkg, "to", new['author']]) + def _author_email_change(change_list, original, new, new_pkg): ''' Appends a summary of a change to a dataset's author e-mail address field between two versions (original and new) to change_list. ''' s = "" - seq2 = ("", new['author_email'], "") + seq2 = ("", + new['author_email'], "") # if the original dataset had a author email if original['author_email'] and new['author_email']: - seq3 = ("", original['author_email'], "") - change_list.append(["Set author e-mail of", new_pkg, "to", s.join(seq2), "(previously", s.join(seq3) + ")"]) + seq3 = ("", + original['author_email'], "") + change_list.append(["Set author e-mail of", new_pkg, "to", + s.join(seq2), "(previously", s.join(seq3) + ")"]) elif not new['author_email']: change_list.append(["Removed author e-mail from", new_pkg]) else: - change_list.append(["Set author e-mail of", new_pkg, "to", s.join(seq2)]) + change_list.append(["Set author e-mail of", new_pkg, + "to", s.join(seq2)]) + def _description_change(change_list, original, new, new_pkg): ''' @@ -325,41 +418,70 @@ def _description_change(change_list, original, new, new_pkg): # if the original dataset had a description if original['notes'] and new['notes']: change_list.append(["Updated description of", new_pkg, - "from
            ", "
            " + original['notes'] + "
            ", - "to
            ", "
            " + new['notes'] + "
            "]) + "from
            ", + "
            " + + original['notes'] + + "
            ", + "to
            ", + "
            " + + new['notes'] + + "
            "]) elif not new['notes']: change_list.append(["Removed description from", new_pkg]) else: change_list.append(["Updated description of", new_pkg, - "to
            ", "
            " + new['notes'] + "
            "]) + "to
            ", + "
            " + + new['notes'] + + "
            "]) + def _tag_change(change_list, new_tags, original_tags, new_pkg): ''' - Appends a summary of a change to a dataset's tag list between two versions - (original and new) to change_list. + Appends a summary of a change to a dataset's tag list between two + versions (original and new) to change_list. ''' s = "" deleted_tags = original_tags - new_tags deleted_tags_list = list(deleted_tags) if len(deleted_tags) == 1: - seq2 = ("", + seq2 = ("", deleted_tags_list[0], "") change_list.append(["Removed tag", s.join(seq2), "from", new_pkg]) elif len(deleted_tags) > 1: - seq2 = ["
          • " + deleted_tags_list[i] + "
            • " + seq2 = ["
          • " + deleted_tags_list[i] + "
            • " for i in range(0, len(deleted_tags))] - change_list.append(["Removed the following tags from", new_pkg, "
              ", s.join(seq2), "
            "]) + change_list.append(["Removed the following tags from", new_pkg, + "
              ", s.join(seq2), "
            "]) added_tags = new_tags - original_tags added_tags_list = list(added_tags) if len(added_tags) == 1: - seq2 = ("", + seq2 = ("", added_tags_list[0], "") change_list.append(["Added tag", s.join(seq2), "to", new_pkg]) elif len(added_tags) > 1: - seq2 = ["
          • " + added_tags_list[i] + "
            • " + seq2 = ["
          • " + added_tags_list[i] + "
            • " for i in range(0, len(added_tags))] - change_list.append(["Added the following tags to", new_pkg, "
              ", s.join(seq2), "
            "]) + change_list.append(["Added the following tags to", new_pkg, + "
              ", s.join(seq2), "
            "]) + def _license_change(change_list, original, new, new_pkg): ''' @@ -371,72 +493,97 @@ def _license_change(change_list, original, new, new_pkg): seq3 = () # if the license has a URL, use it if 'license_url' in original and original['license_url']: - seq2 = ("", original['license_title'], "") + seq2 = ("", + original['license_title'], "") else: seq2 = (original['license_title']) if 'license_url' in new and new['license_url']: - seq3 = ("", new['license_title'], "") + seq3 = ("", + new['license_title'], "") else: seq3 = (new['license_title']) - change_list.append(["Changed the license of", new_pkg, "to", s.join(seq3), "(previously", s.join(seq2) + ")"]) + change_list.append(["Changed the license of", new_pkg, "to", + s.join(seq3), "(previously", s.join(seq2) + ")"]) + def _name_change(change_list, original, new): ''' - Appends a summary of a change to a dataset's name (and thus the URL it can - be accessed at) between two versions (original and new) to change_list. + Appends a summary of a change to a dataset's name (and thus the URL it + can be accessed at) between two versions (original and new) to + change_list. ''' s = "" - old_url = url_for(qualified=True, controller="dataset", - action="read", id=original['name']) - new_url = url_for(qualified=True, controller="dataset", - action="read", id=new['name']) + old_url = url_for(qualified=True, + controller="dataset", + action="read", + id=original['name']) + new_url = url_for(qualified=True, + controller="dataset", + action="read", + id=new['name']) seq2 = ("", old_url, "") seq3 = ("", new_url, "") - change_list.append(["Moved the dataset from", s.join(seq2), "to", s.join(seq3)]) + change_list.append(["Moved the dataset from", s.join(seq2), + "to", s.join(seq3)]) + def _source_url_change(change_list, original, new, new_pkg): ''' - Appends a summary of a change to a dataset's source URL (metadata field, not - its actual URL in the datahub) between two versions (original and new) to - change_list. + Appends a summary of a change to a dataset's source URL (metadata field, + not its actual URL in the datahub) between two versions (original and + new) to change_list. ''' s = "" seq2 = ("", original['url'], "") seq3 = ("", new['url'], "") if original['url'] and new['url']: - change_list.append(["Changed the source URL of", new_pkg, "from", s.join(seq2), "to", s.join(seq3)]) + change_list.append(["Changed the source URL of", new_pkg, + "from", s.join(seq2), "to", s.join(seq3)]) elif not new['url']: change_list.append(["Removed source URL from", new_pkg]) else: - change_list.append(["Changed the source URL of", new_pkg, "to", s.join(seq3)]) + change_list.append(["Changed the source URL of", + new_pkg, "to", s.join(seq3)]) + def _version_change(change_list, original, new, new_pkg): ''' - Appends a summary of a change to a dataset's version field (inputted by the user, - not from version control) between two versions (original and new) to change_list. + Appends a summary of a change to a dataset's version field (inputted + by the user, not from version control) between two versions (original + and new) to change_list. ''' if original['version'] and new['url']: - change_list.append(["Changed the version of", new_pkg, "from", original['version'], "to", new['version']]) + change_list.append(["Changed the version of", new_pkg, + "from", original['version'], + "to", new['version']]) elif not new['url']: change_list.append(["Removed version number from", new_pkg]) else: - change_list.append(["Changed the version of", new_pkg, "to", new['version']]) + change_list.append(["Changed the version of", new_pkg, + "to", new['version']]) + def _extension_fields(change_list, original, new, new_pkg): ''' - Checks whether any fields that have been added to the package dictionaries - by CKAN extensions have been changed between versions. If there have been - any changes between the two versions (original and new), a general summary - of the change is appended to change_list. This function does not produce - summaries for fields added or deleted by extensions, since these changes are - not triggered by the user in the web interface or API. + Checks whether any fields that have been added to the package + dictionaries by CKAN extensions have been changed between versions. + If there have been any changes between the two versions (original and + new), a general summary of the change is appended to change_list. This + function does not produce summaries for fields added or deleted by + extensions, since these changes are not triggered by the user in the web + interface or API. ''' # list of the default metadata fields for a dataset - # any fields that are not part of this list are custom fields added by a user or extension - fields = ['owner_org', 'maintainer', 'maintainer_email', 'relationships_as_object', 'private', 'num_tags', - 'id', 'metadata_created', 'metadata_modified', 'author', 'author_email', 'state', 'version', - 'license_id', 'type', 'resources', 'num_resources', 'tags', 'title', 'groups', 'creator_user_id', - 'relationships_as_subject', 'name', 'isopen', 'url', 'notes', 'license_title', 'extras', + # any fields that are not part of this list are custom fields added by a + #user or extension + fields = ['owner_org', 'maintainer', 'maintainer_email', + 'relationships_as_object', 'private', 'num_tags', + 'id', 'metadata_created', 'metadata_modified', + 'author', 'author_email', 'state', 'version', + 'license_id', 'type', 'resources', 'num_resources', + 'tags', 'title', 'groups', 'creator_user_id', + 'relationships_as_subject', 'name', 'isopen', 'url', + 'notes', 'license_title', 'extras', 'license_url', 'organization', 'revision_id'] fields_set = set(fields) @@ -445,29 +592,36 @@ def _extension_fields(change_list, original, new, new_pkg): original_set = set(original.keys()) new_set = set(new.keys()) - addl_fields_new = new_set - fields_set # set of additional fields in the new dictionary - addl_fields_original = original_set - fields_set # set of additional fields in the original dictionary - addl_fields = addl_fields_new.intersection(addl_fields_original) # set of additional fields in both + # set of additional fields in the new dictionary + addl_fields_new = new_set - fields_set + # set of additional fields in the original dictionary + addl_fields_original = original_set - fields_set + # set of additional fields in both + addl_fields = addl_fields_new.intersection(addl_fields_original) - # do NOT display a change if any additional fields have been added or deleted, - # since that is not a change made by the user from the web interface + # do NOT display a change if any additional fields have been + # added or deleted, since that is not a change made by the user + # from the web interface # if additional fields have been changed addl_fields_list = list(addl_fields) for field in addl_fields_list: if original[field] != new[field]: if original[field]: - change_list.append(["Changed value of field", field.capitalize(), "to", new[field], "(previously", original[field] + ")", "in", new_pkg]) + change_list.append(["Changed value of field", + field.capitalize(), "to", + new[field], "(previously", + original[field] + ")", "in", new_pkg]) else: - change_list.append(["Changed value of field", field.capitalize(), "to", new[field], "in", new_pkg]) + change_list.append(["Changed value of field", + field.capitalize(), "to", + new[field], "in", new_pkg]) + def _extra_fields(change_list, original, new, new_pkg): - # check the extras field to see if anything has been added, deleted, or changed - # that is where custom fields added via the web interface go - they do not become - # actual fields in a package dict ''' - Checks whether a user has added, removed, or changed any custom fields from - the web interface (or API?) and appends a summary of each change to + Checks whether a user has added, removed, or changed any custom fields + from the web interface (or API?) and appends a summary of each change to change_list. ''' @@ -476,8 +630,8 @@ def _extra_fields(change_list, original, new, new_pkg): extra_fields_new = _extras_to_dict(new['extras']) extra_new_set = set(extra_fields_new.keys()) - # if the original version has an extra fields, we need to compare the new version's - # extras to the original ones + # if the original version has an extra fields, we need + # to compare the new version'sextras to the original ones if 'extras' in original: extra_fields_original = _extras_to_dict(original['extras']) extra_original_set = set(extra_fields_original.keys()) @@ -486,61 +640,91 @@ def _extra_fields(change_list, original, new, new_pkg): new_fields = list(extra_new_set - extra_original_set) if len(new_fields) == 1: if extra_fields_new[new_fields[0]]: - change_list.append(["Added field", s.join(("", new_fields[0], "")), - "with value", s.join(("", extra_fields_new[new_fields[0]], "")), + change_list.append(["Added field", s.join(("", + new_fields[0], "")), + "with value", s.join(("", + extra_fields_new[new_fields[0]], + "")), "to", new_pkg]) else: - change_list.append(["Added field", s.join(("", new_fields[0], "")), + change_list.append(["Added field", s.join(("", + new_fields[0], "")), "to", new_pkg]) elif len(new_fields) > 1: - seq2 = ["
          • " + new_fields[i] + " with value " + extra_fields_new[new_fields[i]] + "
            • " if extra_fields_new[new_fields[i]] + seq2 = ["
          • " + new_fields[i] + " with value " + + extra_fields_new[new_fields[i]] + "
            • " + if extra_fields_new[new_fields[i]] else "
          • " + new_fields[i] + "
            • " for i in range(0, len(new_fields))] - change_list.append(["Added the following fields to", new_pkg, "
              ", s.join(seq2), "
            "]) + change_list.append(["Added the following fields to", + new_pkg, "
              ", s.join(seq2), "
            "]) # if some fields were deleted deleted_fields = list(extra_original_set - extra_new_set) if len(deleted_fields) == 1: - change_list.append(["Removed field", s.join(("", deleted_fields[0], "")), "from", new_pkg]) + change_list.append(["Removed field", s.join(("", + deleted_fields[0], "")), + "from", new_pkg]) elif len(deleted_fields) > 1: - seq2 = ["
          • " + deleted_fields[i] + "
            • " for i in range(0, len(deleted_fields))] - change_list.append(["Removed the following fields from", new_pkg, "
              ", s.join(seq2), "
            "]) + seq2 = ["
          • " + deleted_fields[i] + "
            • " + for i in range(0, len(deleted_fields))] + change_list.append(["Removed the following fields from", + new_pkg, "
              ", s.join(seq2), "
            "]) # if some existing fields were changed - extra_fields = list(extra_new_set.intersection(extra_original_set)) # list of extra fields in both the original and new versions + # list of extra fields in both the original and new versions + extra_fields = list(extra_new_set.intersection(extra_original_set)) for field in extra_fields: if extra_fields_original[field] != extra_fields_new[field]: if extra_fields_original[field]: - change_list.append(["Changed value of field", s.join(("", field, "")), - "to", s.join(("", extra_fields_new[field], "")), - "(previously", s.join(("", extra_fields_original[field], "")) + ")", + change_list.append(["Changed value of field", + s.join(("", field, "")), + "to", s.join(("", + extra_fields_new[field], "")), + "(previously", s.join(("", + extra_fields_original[field], + "")) + ")", "in", new_pkg]) else: - change_list.append(["Changed value of field", s.join(("", field, "")), - "to", s.join(("", extra_fields_new[field], "")), + change_list.append(["Changed value of field", + s.join(("", field, "")), + "to", s.join(("", + extra_fields_new[field], "")), "in", new_pkg]) - # if the original version didn't have an extras field, the user could only have added a field (not changed or deleted) + # if the original version didn't have an extras field, + # the user could only have added a field (not changed or deleted) else: new_fields = list(extra_new_set) if len(new_fields) == 1: if extra_fields_new[new_fields[0]]: - change_list.append(["Added field", s.join(("", new_fields[0], "")), - "with value", s.join(("", extra_fields_new[new_fields[0]], "")), + change_list.append(["Added field", s.join(("", + new_fields[0], "")), + "with value", s.join(("", + extra_fields_new[new_fields[0]], + "")), "to", new_pkg]) else: - change_list.append(["Added field", s.join(("", new_fields[0], "")), + change_list.append(["Added field", s.join(("", + new_fields[0], "")), "to", new_pkg]) elif len(new_fields) > 1: - seq2 = ["
          • " + new_fields[i] + " with value " + extra_fields_new[new_fields[i]] + "
            • " if extra_fields_new[new_fields[i]] + seq2 = ["
          • " + new_fields[i] + " with value " + + extra_fields_new[new_fields[i]] + "
            • " + if extra_fields_new[new_fields[i]] else "
          • " + new_fields[i] + "
            • " for i in range(0, len(new_fields))] - change_list.append(["Added the following fields to", new_pkg, "
              ", s.join(seq2), "
            "]) + change_list.append(["Added the following fields to", + new_pkg, "
              ", s.join(seq2), "
            "]) elif 'extras' in original: deleted_fields = _extras_to_dict(original['extras']).keys() if len(deleted_fields) == 1: - change_list.append(["Removed field", s.join(("", deleted_fields[0], "")), "from", new_pkg]) + change_list.append(["Removed field", s.join(("", + deleted_fields[0], "")), "from", + new_pkg]) elif len(deleted_fields) > 1: - seq2 = ["
          • " + deleted_fields[i] + "
            • " for i in range(0, len(deleted_fields))] - change_list.append(["Removed the following fields from", new_pkg, "
              ", s.join(seq2), "
            "]) + seq2 = ["
          • " + deleted_fields[i] + + "
            • " for i in range(0, len(deleted_fields))] + change_list.append(["Removed the following fields from", + new_pkg, "
              ", s.join(seq2), "
            "]) diff --git a/ckan/lib/helpers.py b/ckan/lib/helpers.py index b4f384b3118..88122286160 100644 --- a/ckan/lib/helpers.py +++ b/ckan/lib/helpers.py @@ -2705,33 +2705,45 @@ def compare_pkg_dicts(original, new, old_activity_id): s = "" seq1 = ("", - new['title'], "") + action="read", id=new['id']), "\">", + new['title'], "") new_pkg = s.join(seq1) _check_metadata_changes(change_list, original, new, new_pkg) - _check_resource_changes(change_list, original, new, new_pkg, old_activity_id) + _check_resource_changes(change_list, original, new, new_pkg, + old_activity_id) # if the dataset was updated but none of the fields we check were changed, # display a message stating that if len(change_list) == 0: - change_list.append(["No fields were updated. See metadata diff for more details."]) + change_list.append(["No fields were updated. \ + See metadata diff for more details."]) return change_list @core_helper def activity_list_select(pkg_activity_list, current_activity_id): ''' - Builds an HTML formatted list of options for the select lists on the "Changes" - summary page. + Builds an HTML formatted list of options for the select lists + on the "Changes" summary page. ''' select_list = [] for activity in pkg_activity_list: - entry = render_datetime(activity['timestamp'], with_hours=True, with_seconds=True) + entry = render_datetime(activity['timestamp'], + with_hours=True, + with_seconds=True) if activity['id'] == current_activity_id: - select_list.append("") + select_list.append("") else: - select_list.append("") + select_list.append("") return select_list diff --git a/ckan/views/dataset.py b/ckan/views/dataset.py index 0c5712e0cfc..c1231a6e59b 100644 --- a/ckan/views/dataset.py +++ b/ckan/views/dataset.py @@ -1104,7 +1104,8 @@ def changes(id, package_type=None): # changed, and we need a link to it which works pkg_id = activity_diff[u'activities'][1][u'data'][u'package'][u'id'] current_pkg_dict = get_action(u'package_show')(context, {u'id': pkg_id}) - pkg_activity_list = get_action(u'package_activity_list')(context, {u'id': pkg_id, u'limit': 100}) + pkg_activity_list = get_action(u'package_activity_list')(context, + {u'id': pkg_id, u'limit': 100}) return base.render( u'package/changes.html', { @@ -1114,11 +1115,13 @@ def changes(id, package_type=None): } ) + def change_range(package_type=None): ''' - Called when a user specifies a range of versions they want to look at changes between. - Verifies that the range is valid and finds the set of activity diffs for the changes - in the given version range, then re-renders changes.html with the list. + Called when a user specifies a range of versions they want to look at + changes between. Verifies that the range is valid and finds the set of + activity diffs for the changes in the given version range, then + re-renders changes.html with the list. ''' newest_id = h.get_request_param('newest_id') @@ -1129,9 +1132,16 @@ def change_range(package_type=None): u'user': g.user, u'auth_user_obj': g.userobj } - # check to ensure that the old activity is actually older than the new activity - old_activity = get_action(u'activity_show')(context, {u'id': oldest_id, u'include_data': False}) - new_activity = get_action('activity_show')(context, {'id': newest_id, u'include_data': False}) + # check to ensure that the old activity is actually older than + # the new activity + old_activity = get_action(u'activity_show')(context, { + u'id': oldest_id, + u'include_data': False + }) + new_activity = get_action('activity_show')(context, { + u'id': newest_id, + u'include_data': False + }) old_timestamp = old_activity['timestamp'] new_timestamp = new_activity['timestamp'] @@ -1142,9 +1152,9 @@ def change_range(package_type=None): time_diff = t2 - t1 # if the time difference is negative, just return the change that put us # at the more recent ID we were just looking at - # TODO: do something better here - go back to the previous page, display a warning - # that the user can't look at a sequence where the newest item is older than the - # oldest one, etc + # TODO: do something better here - go back to the previous page, + # display a warning that the user can't look at a sequence where + # the newest item is older than the oldest one, etc if time_diff.total_seconds() < 0: return changes(h.get_request_param('current_new_id')) @@ -1152,13 +1162,14 @@ def change_range(package_type=None): current_id = newest_id diff_list = [] - while done == False: + while not done: try: activity_diff = get_action(u'activity_diff')( context, {u'id': current_id, u'object_type': u'package', u'diff_type': u'html'}) except NotFound as e: - log.info(u'Activity not found: {} - {}'.format(str(e), current_id)) + log.info(u'Activity not found: {} - {}'.format(str(e), + current_id)) return base.abort(404, _(u'Activity not found')) except NotAuthorized: return base.abort(403, _(u'Unauthorized to view activity data')) @@ -1172,7 +1183,10 @@ def change_range(package_type=None): pkg_id = diff_list[0][u'activities'][1][u'data'][u'package'][u'id'] current_pkg_dict = get_action(u'package_show')(context, {u'id': pkg_id}) - pkg_activity_list = get_action(u'package_activity_list')(context, {u'id': pkg_id, u'limit': 100}) + pkg_activity_list = get_action(u'package_activity_list')(context, { + u'id': pkg_id, + u'limit': 100 + }) return base.render( u'package/changes.html', { From 9f3e213ac0778f126be091c731d76445336be4dd Mon Sep 17 00:00:00 2001 From: hayley-leblanc Date: Mon, 29 Jul 2019 11:38:05 +0200 Subject: [PATCH 13/47] Formatting --- ckan/lib/changes.py | 96 +++++++++++++++++++++---------------------- ckan/lib/helpers.py | 18 ++++---- ckan/views/dataset.py | 18 ++++---- 3 files changed, 66 insertions(+), 66 deletions(-) diff --git a/ckan/lib/changes.py b/ckan/lib/changes.py index e1ae63c6f81..e9927e353e3 100644 --- a/ckan/lib/changes.py +++ b/ckan/lib/changes.py @@ -1,11 +1,11 @@ +# encoding: utf-8 + ''' Functions used by the helper function compare_pkg_dicts() to analyze the differences between two versions of a dataset. ''' - from helpers import url_for - def _extras_to_dict(extras_list): ''' Takes a list of dictionaries with the following format: @@ -57,20 +57,18 @@ def _check_resource_changes(change_list, original, new, new_pkg, for resource in original['resources']: original_resource_set.add(resource['id']) original_resource_dict[resource['id']] = { - 'name': resource['name'], - 'url': resource['url'], - 'description': resource['description'], - 'format': resource['format'] - } + 'name': resource['name'], + 'url': resource['url'], + 'description': resource['description'], + 'format': resource['format']} for resource in new['resources']: new_resource_set.add(resource['id']) new_resource_dict[resource['id']] = { - 'name': resource['name'], - 'url': resource['url'], - 'description': resource['description'], - 'format': resource['format'] - } + 'name': resource['name'], + 'url': resource['url'], + 'description': resource['description'], + 'format': resource['format']} # get the IDs of the resources that have been added between the versions new_resources = list(new_resource_set - original_resource_set) @@ -100,17 +98,17 @@ def _check_resource_changes(change_list, original, new, new_pkg, if original_metadata['name'] != new_metadata['name']: seq2 = ("", original_resource_dict[resource_id]['name'], "") seq3 = ("", new_resource_dict[resource_id]['name'], "") change_list.append(["Renamed resource", s.join(seq2), @@ -122,39 +120,39 @@ def _check_resource_changes(change_list, original, new, new_pkg, # if a format was not originally set and the user set one if not original_metadata['format'] and new_metadata['format']: seq2 = ("", new_resource_dict[resource_id]['name'], "") seq3 = ("", + controller="organization", + action="read", + id=new['organization']['id']) + + "?res_format=" + new_metadata['format'], "\">", new_metadata['format'], "") change_list.append(["Set format of resource", s.join(seq2), "to", s.join(seq3), "in", new_pkg]) # if both versions have a format but the format changed elif original_metadata['format'] != new_metadata['format']: seq2 = ("", original_resource_dict[resource_id]['name'], "") seq3 = ("", new_metadata['format'], "") seq4 = ("", original_metadata['format'], "") change_list.append(["Set format of resource", @@ -164,13 +162,13 @@ def _check_resource_changes(change_list, original, new, new_pkg, "in", new_pkg]) # if the description changed - if not original_metadata['description'] and + if not original_metadata['description'] and \ new_metadata['description']: seq2 = ("", + controller="resource", + action="read", + id=new['id'], + resource_id=resource_id), "\">", new_resource_dict[resource_id]['name'], "") change_list.append(["Updated description of resource", s.join(seq2), "in", @@ -179,7 +177,7 @@ def _check_resource_changes(change_list, original, new, new_pkg, + ""]) # if there was a description but the user removed it - elif original_metadata['description'] and + elif original_metadata['description'] and \ not new_metadata['description']: seq2 = (""]) # check if the user uploaded a new file - # TODO: use regular expressions to determine the actual name of the new and old files + # TODO: use regular expressions to determine the actual name of the + # new and old files if original_metadata['url'] != new_metadata['url']: seq2 = ("" + - entry + - "") + activity['id'] + + "\" selected>" + + entry + + "") else: select_list.append("") + activity['id'] + + "\">" + + entry + + "") return select_list diff --git a/ckan/views/dataset.py b/ckan/views/dataset.py index c1231a6e59b..37f1698a653 100644 --- a/ckan/views/dataset.py +++ b/ckan/views/dataset.py @@ -1105,7 +1105,7 @@ def changes(id, package_type=None): pkg_id = activity_diff[u'activities'][1][u'data'][u'package'][u'id'] current_pkg_dict = get_action(u'package_show')(context, {u'id': pkg_id}) pkg_activity_list = get_action(u'package_activity_list')(context, - {u'id': pkg_id, u'limit': 100}) + {u'id': pkg_id, u'limit': 100}) return base.render( u'package/changes.html', { @@ -1135,13 +1135,11 @@ def change_range(package_type=None): # check to ensure that the old activity is actually older than # the new activity old_activity = get_action(u'activity_show')(context, { - u'id': oldest_id, - u'include_data': False - }) + u'id': oldest_id, + u'include_data': False}) new_activity = get_action('activity_show')(context, { - u'id': newest_id, - u'include_data': False - }) + u'id': newest_id, + u'include_data': False}) old_timestamp = old_activity['timestamp'] new_timestamp = new_activity['timestamp'] @@ -1166,7 +1164,7 @@ def change_range(package_type=None): try: activity_diff = get_action(u'activity_diff')( context, {u'id': current_id, u'object_type': u'package', - u'diff_type': u'html'}) + u'diff_type': u'html'}) except NotFound as e: log.info(u'Activity not found: {} - {}'.format(str(e), current_id)) @@ -1185,8 +1183,7 @@ def change_range(package_type=None): current_pkg_dict = get_action(u'package_show')(context, {u'id': pkg_id}) pkg_activity_list = get_action(u'package_activity_list')(context, { u'id': pkg_id, - u'limit': 100 - }) + u'limit': 100}) return base.render( u'package/changes.html', { @@ -1196,6 +1193,7 @@ def change_range(package_type=None): } ) + # deprecated def history(package_type, id): return h.redirect_to(u'dataset.activity', id=id) From 1575aba2bb6573fe607bc090064c0ce8f91af430 Mon Sep 17 00:00:00 2001 From: hayley-leblanc Date: Mon, 29 Jul 2019 16:17:22 +0200 Subject: [PATCH 14/47] Formatting to pass tests --- ckan/lib/changes.py | 611 +++++++++++++++++++++--------------------- ckan/lib/helpers.py | 16 +- ckan/views/dataset.py | 26 +- 3 files changed, 329 insertions(+), 324 deletions(-) diff --git a/ckan/lib/changes.py b/ckan/lib/changes.py index e9927e353e3..049d9c58b7e 100644 --- a/ckan/lib/changes.py +++ b/ckan/lib/changes.py @@ -6,6 +6,7 @@ ''' from helpers import url_for + def _extras_to_dict(extras_list): ''' Takes a list of dictionaries with the following format: @@ -52,42 +53,42 @@ def _check_resource_changes(change_list, original, new, new_pkg, original_resource_dict = {} new_resource_set = set() new_resource_dict = {} - s = "" + s = u"" for resource in original['resources']: original_resource_set.add(resource['id']) original_resource_dict[resource['id']] = { - 'name': resource['name'], - 'url': resource['url'], - 'description': resource['description'], - 'format': resource['format']} + u'name': resource['name'], + u'url': resource['url'], + u'description': resource['description'], + u'format': resource['format']} for resource in new['resources']: new_resource_set.add(resource['id']) new_resource_dict[resource['id']] = { - 'name': resource['name'], - 'url': resource['url'], - 'description': resource['description'], - 'format': resource['format']} + u'name': resource['name'], + u'url': resource['url'], + u'description': resource['description'], + u'format': resource['format']} # get the IDs of the resources that have been added between the versions new_resources = list(new_resource_set - original_resource_set) for resource_id in new_resources: - seq2 = ("", - new_resource_dict[resource_id]['name'], "") + seq2 = (u"", + new_resource_dict[resource_id]['name'], u"") change_list.append(["Added resource", - s.join(seq2), "to", + s.join(seq2), u"to", new_pkg]) # get the IDs of resources that have been deleted between versions deleted_resources = list(original_resource_set - new_resource_set) for resource_id in deleted_resources: - seq2 = ("", - original_resource_dict[resource_id]['name'], "") - change_list.append(["Deleted resource", s.join(seq2), "from", new_pkg]) + seq2 = (u"", + original_resource_dict[resource_id]['name'], u"") + change_list.append(["Deleted resource", s.join(seq2), u"from", new_pkg]) # now check the resources that are in both and see if any # have been changed @@ -97,130 +98,130 @@ def _check_resource_changes(change_list, original, new, new_pkg, new_metadata = new_resource_dict[resource_id] if original_metadata['name'] != new_metadata['name']: - seq2 = ("", - original_resource_dict[resource_id]['name'], "") - seq3 = ("", + original_resource_dict[resource_id]['name'], u"") + seq3 = (u"", - new_resource_dict[resource_id]['name'], "") + u"\">", + new_resource_dict[resource_id]['name'], u"") change_list.append(["Renamed resource", s.join(seq2), - "to", s.join(seq3), "in", new_pkg]) + u"to", s.join(seq3), u"in", new_pkg]) # you can't remove a format, but if a resource's format isn't # recognized, it won't have one set # if a format was not originally set and the user set one if not original_metadata['format'] and new_metadata['format']: - seq2 = ("", - new_resource_dict[resource_id]['name'], "") - seq3 = ("", + new_resource_dict[resource_id]['name'], u"") + seq3 = (u"", - new_metadata['format'], "") - change_list.append(["Set format of resource", s.join(seq2), - "to", s.join(seq3), "in", new_pkg]) + u"?res_format=" + new_metadata['format'], u"\">", + new_metadata['format'], u"") + change_list.append([u"Set format of resource", s.join(seq2), + u"to", s.join(seq3), u"in", new_pkg]) # if both versions have a format but the format changed elif original_metadata['format'] != new_metadata['format']: - seq2 = ("", - original_resource_dict[resource_id]['name'], "") - seq3 = ("", + original_resource_dict[resource_id]['name'], u"") + seq3 = (u"", - new_metadata['format'], "") - seq4 = ("", + new_metadata['format'], u"") + seq4 = (u"", - original_metadata['format'], "") - change_list.append(["Set format of resource", + + u"?res_format=" + original_metadata['format'], u"\">", + original_metadata['format'], u"") + change_list.append([u"Set format of resource", s.join(seq2), - "to", s.join(seq3), - "(previously", s.join(seq4) + ")", - "in", new_pkg]) + u"to", s.join(seq3), + u"(previously", s.join(seq4) + u")", + u"in", new_pkg]) # if the description changed if not original_metadata['description'] and \ new_metadata['description']: - seq2 = ("", - new_resource_dict[resource_id]['name'], "") - change_list.append(["Updated description of resource", - s.join(seq2), "in", - new_pkg, "to
            ", - "
            " + new_metadata['description'] - + "
            "]) + resource_id=resource_id), u"\">", + new_resource_dict[resource_id]['name'], u"") + change_list.append([u"Updated description of resource", + s.join(seq2), u"in", + new_pkg, u"to
            ", + u"
            " + new_metadata['description'] + + u"
            "]) # if there was a description but the user removed it elif original_metadata['description'] and \ not new_metadata['description']: - seq2 = ("", - new_resource_dict[resource_id]['name'], "") - change_list.append(["Removed description from resource", - s.join(seq2), "in", new_pkg]) + seq2 = (u"", + new_resource_dict[resource_id]['name'], u"") + change_list.append([u"Removed description from resource", + s.join(seq2), u"in", new_pkg]) # if both have descriptions but they are different elif original_metadata['description'] != new_metadata['description']: - seq2 = ("", - new_resource_dict[resource_id]['name'], "") - change_list.append(["Updated description of resource", - s.join(seq2), "in", + seq2 = (u"", + new_resource_dict[resource_id]['name'], u"") + change_list.append([u"Updated description of resource", + s.join(seq2), u"in", new_pkg, - "from
            ", - "
            " + + u"from
            ", + u"
            " + original_metadata['description'] + - "
            ", - "to
            ", - "
            " + + u"
            ", + u"to
            ", + u"
            " + new_metadata['description'] + - "
            "]) + u"
            "]) # check if the user uploaded a new file # TODO: use regular expressions to determine the actual name of the # new and old files if original_metadata['url'] != new_metadata['url']: - seq2 = ("", - new_resource_dict[resource_id]['name'], "") - change_list.append(["Uploaded a new file to resource", - s.join(seq2), "in", new_pkg]) + seq2 = (u"", + new_resource_dict[resource_id]['name'], u"") + change_list.append([u"Uploaded a new file to resource", + s.join(seq2), u"in", new_pkg]) def _check_metadata_changes(change_list, original, new, new_pkg): @@ -256,8 +257,8 @@ def _check_metadata_changes(change_list, original, new, new_pkg): # if the visibility of the dataset changed if original['private'] != new['private']: - change_list.append(["Set visibility of", new_pkg, "to", - "Private" if new['private'] else "Public"]) + change_list.append([u"Set visibility of", new_pkg, u"to", + u"Private" if new['private'] else u"Public"]) # if the description of the dataset changed if original['notes'] != new['notes']: @@ -295,17 +296,18 @@ def _check_metadata_changes(change_list, original, new, new_pkg): _extension_fields(change_list, original, new, new_pkg) _extra_fields(change_list, original, new, new_pkg) + def _title_change(change_list, original, new): ''' Appends a summary of a change to a dataset's title between two versions (original and new) to change_list. ''' - s = "" - seq2 = ("", - new['title'], "") - change_list.append(["Changed title to", s.join(seq2), - "(previously", original['title'] + ")"]) + s = u"" + seq2 = (u"", + new['title'], u"") + change_list.append([u"Changed title to", s.join(seq2), + u"(previously", original['title'] + u")"]) def _org_change(change_list, original, new, new_pkg): @@ -313,23 +315,23 @@ def _org_change(change_list, original, new, new_pkg): Appends a summary of a change to a dataset's organization between two versions (original and new) to change_list. ''' - s = "" - seq2 = ("", - original['organization']['title'], "") - seq3 = ("", - new['organization']['title'], "") - change_list.append(["Moved", new_pkg, - "from organization", + s = u"" + seq2 = (u"", + original['organization']['title'], u"") + seq3 = (u"", + new['organization']['title'], u"") + change_list.append([u"Moved", new_pkg, + u"from organization", s.join(seq2), - "to organization", + u"to organization", s.join(seq3)]) @@ -340,13 +342,14 @@ def _maintainer_change(change_list, original, new, new_pkg): ''' # if the original dataset had a maintainer if original['maintainer'] and new['maintainer']: - change_list.append(["Set maintainer of", new_pkg, - "to", new['maintainer'], - "(previously", original['maintainer'] + ")"]) + change_list.append([u"Set maintainer of", new_pkg, + u"to", new['maintainer'], + u"(previously", original['maintainer'] + u")"]) elif not new['maintainer']: - change_list.append(["Removed maintainer from", new_pkg]) + change_list.append([u"Removed maintainer from", new_pkg]) else: - change_list.append(["Set maintainer of", new_pkg, "to", new['maintainer']]) + change_list.append([u"Set maintainer of", new_pkg, u"to", + new['maintainer']]) def _maintainer_email_change(change_list, original, new, new_pkg): @@ -354,36 +357,36 @@ def _maintainer_email_change(change_list, original, new, new_pkg): Appends a summary of a change to a dataset's maintainer e-mail address field between two versions (original and new) to change_list. ''' - s = "" - seq2 = ("", - new['maintainer_email'], "") + s = u"" + seq2 = (u"", + new['maintainer_email'], u"") # if the original dataset had a maintainer email if original['maintainer_email'] and new['maintainer_email']: - seq3 = ("", - original['maintainer_email'], "") - change_list.append(["Set maintainer e-mail of", - new_pkg, "to", s.join(seq2), - "(previously", s.join(seq3) + ")"]) + seq3 = (u"", + original['maintainer_email'], u"") + change_list.append([u"Set maintainer e-mail of", + new_pkg, u"to", s.join(seq2), + u"(previously", s.join(seq3) + u")"]) elif not new['maintainer_email']: - change_list.append(["Removed maintainer e-mail from", new_pkg]) + change_list.append([u"Removed maintainer e-mail from", new_pkg]) else: - change_list.append(["Set maintainer e-mail of", - new_pkg, "to", s.join(seq2)]) + change_list.append([u"Set maintainer e-mail of", + new_pkg, u"to", s.join(seq2)]) def _author_change(change_list, original, new, new_pkg): ''' - Appends a summary of a change to a dataset's author field between two versions - (original and new) to change_list. + Appends a summary of a change to a dataset's author field between two + versions (original and new) to change_list. ''' # if the original dataset had an author if original['author'] and new['author']: - change_list.append(["Set author of", new_pkg, "to", new['author'], - "(previously", original['author'] + ")"]) + change_list.append([u"Set author of", new_pkg, u"to", new['author'], + u"(previously", original['author'] + u")"]) elif not new['author']: - change_list.append(["Removed author from", new_pkg]) + change_list.append([u"Removed author from", new_pkg]) else: - change_list.append(["Set author of", new_pkg, "to", new['author']]) + change_list.append([u"Set author of", new_pkg, u"to", new['author']]) def _author_email_change(change_list, original, new, new_pkg): @@ -391,49 +394,50 @@ def _author_email_change(change_list, original, new, new_pkg): Appends a summary of a change to a dataset's author e-mail address field between two versions (original and new) to change_list. ''' - s = "" - seq2 = ("", - new['author_email'], "") + s = u"" + seq2 = (u"", + new['author_email'], u"") # if the original dataset had a author email if original['author_email'] and new['author_email']: - seq3 = ("", - original['author_email'], "") - change_list.append(["Set author e-mail of", new_pkg, "to", - s.join(seq2), "(previously", s.join(seq3) + ")"]) + seq3 = (u"", + original['author_email'], u"") + change_list.append([u"Set author e-mail of", new_pkg, u"to", + s.join(seq2), u"(previously", s.join(seq3) + u")"]) elif not new['author_email']: - change_list.append(["Removed author e-mail from", new_pkg]) + change_list.append([u"Removed author e-mail from", new_pkg]) else: - change_list.append(["Set author e-mail of", new_pkg, - "to", s.join(seq2)]) + change_list.append([u"Set author e-mail of", new_pkg, + u"to", s.join(seq2)]) def _description_change(change_list, original, new, new_pkg): ''' - Appends a summary of a change to a dataset's description between two versions - (original and new) to change_list. + Appends a summary of a change to a dataset's description between two + versions (original and new) to change_list. ''' - # TODO: find a better way to format the descriptions along with the change summary + # TODO: find a better way to format the descriptions along with the + # change summary # if the original dataset had a description if original['notes'] and new['notes']: - change_list.append(["Updated description of", new_pkg, - "from
            ", - "
            " + + change_list.append([u"Updated description of", new_pkg, + u"from
            ", + u"
            " + original['notes'] + - "
            ", - "to
            ", - "
            " + + u"
            ", + u"to
            ", + u"
            " + new['notes'] + - "
            "]) + u"
            "]) elif not new['notes']: - change_list.append(["Removed description from", new_pkg]) + change_list.append([u"Removed description from", new_pkg]) else: - change_list.append(["Updated description of", new_pkg, - "to
            ", - "
            " + + change_list.append([u"Updated description of", new_pkg, + u"to
            ", + u"
            " + new['notes'] + - "
            "]) + u"
            "]) def _tag_change(change_list, new_tags, original_tags, new_pkg): @@ -441,46 +445,46 @@ def _tag_change(change_list, new_tags, original_tags, new_pkg): Appends a summary of a change to a dataset's tag list between two versions (original and new) to change_list. ''' - s = "" + s = u"" deleted_tags = original_tags - new_tags deleted_tags_list = list(deleted_tags) if len(deleted_tags) == 1: - seq2 = ("", - deleted_tags_list[0], "") - change_list.append(["Removed tag", s.join(seq2), "from", new_pkg]) + seq2 = (u"", + deleted_tags_list[0], u"") + change_list.append([u"Removed tag", s.join(seq2), u"from", new_pkg]) elif len(deleted_tags) > 1: - seq2 = ["
          • " + deleted_tags_list[i] + "
            • " - for i in range(0, len(deleted_tags))] - change_list.append(["Removed the following tags from", new_pkg, - "
              ", s.join(seq2), "
            "]) + seq2 = [u"
          • " + deleted_tags_list[i] + u"
            • " + for i in range(0, len(deleted_tags))] + change_list.append([u"Removed the following tags from", new_pkg, + u"
              ", s.join(seq2), u"
            "]) added_tags = new_tags - original_tags added_tags_list = list(added_tags) if len(added_tags) == 1: - seq2 = ("", - added_tags_list[0], "") - change_list.append(["Added tag", s.join(seq2), "to", new_pkg]) + seq2 = (u"", + added_tags_list[0], u"") + change_list.append([u"Added tag", s.join(seq2), u"to", new_pkg]) elif len(added_tags) > 1: - seq2 = ["
          • " + added_tags_list[i] + "
            • " + seq2 = [u"
          • " + added_tags_list[i] + u"
            • " for i in range(0, len(added_tags))] - change_list.append(["Added the following tags to", new_pkg, - "
              ", s.join(seq2), "
            "]) + change_list.append([u"Added the following tags to", new_pkg, + u"
              ", s.join(seq2), u"
            "]) def _license_change(change_list, original, new, new_pkg): @@ -488,22 +492,22 @@ def _license_change(change_list, original, new, new_pkg): Appends a summary of a change to a dataset's license between two versions (original and new) to change_list. ''' - s = "" + s = u"" seq2 = () seq3 = () # if the license has a URL, use it if 'license_url' in original and original['license_url']: - seq2 = ("", - original['license_title'], "") + seq2 = (u"", + original['license_title'], u"") else: seq2 = (original['license_title']) if 'license_url' in new and new['license_url']: - seq3 = ("", - new['license_title'], "") + seq3 = (u"", + new['license_title'], u"") else: seq3 = (new['license_title']) - change_list.append(["Changed the license of", new_pkg, "to", - s.join(seq3), "(previously", s.join(seq2) + ")"]) + change_list.append([u"Changed the license of", new_pkg, u"to", + s.join(seq3), u"(previously", s.join(seq2) + u")"]) def _name_change(change_list, original, new): @@ -512,19 +516,19 @@ def _name_change(change_list, original, new): can be accessed at) between two versions (original and new) to change_list. ''' - s = "" + s = u"" old_url = url_for(qualified=True, - controller="dataset", - action="read", - id=original['name']) + controller=u"dataset", + action=u"read", + id=original['name']) new_url = url_for(qualified=True, - controller="dataset", - action="read", - id=new['name']) - seq2 = ("", old_url, "") - seq3 = ("", new_url, "") - change_list.append(["Moved the dataset from", s.join(seq2), - "to", s.join(seq3)]) + controller=u"dataset", + action=u"read", + id=new['name']) + seq2 = (u"", old_url, u"") + seq3 = (u"", new_url, u"") + change_list.append([u"Moved the dataset from", s.join(seq2), + u"to", s.join(seq3)]) def _source_url_change(change_list, original, new, new_pkg): @@ -533,17 +537,17 @@ def _source_url_change(change_list, original, new, new_pkg): not its actual URL in the datahub) between two versions (original and new) to change_list. ''' - s = "" - seq2 = ("", original['url'], "") - seq3 = ("", new['url'], "") + s = u"" + seq2 = (u"", original['url'], u"") + seq3 = (u"", new['url'], u"") if original['url'] and new['url']: - change_list.append(["Changed the source URL of", new_pkg, - "from", s.join(seq2), "to", s.join(seq3)]) + change_list.append([u"Changed the source URL of", new_pkg, + u"from", s.join(seq2), u"to", s.join(seq3)]) elif not new['url']: - change_list.append(["Removed source URL from", new_pkg]) + change_list.append([u"Removed source URL from", new_pkg]) else: - change_list.append(["Changed the source URL of", - new_pkg, "to", s.join(seq3)]) + change_list.append([u"Changed the source URL of", + new_pkg, u"to", s.join(seq3)]) def _version_change(change_list, original, new, new_pkg): @@ -553,14 +557,14 @@ def _version_change(change_list, original, new, new_pkg): and new) to change_list. ''' if original['version'] and new['url']: - change_list.append(["Changed the version of", new_pkg, - "from", original['version'], - "to", new['version']]) + change_list.append([u"Changed the version of", new_pkg, + u"from", original['version'], + u"to", new['version']]) elif not new['url']: - change_list.append(["Removed version number from", new_pkg]) + change_list.append([u"Removed version number from", new_pkg]) else: - change_list.append(["Changed the version of", new_pkg, - "to", new['version']]) + change_list.append([u"Changed the version of", new_pkg, + u"to", new['version']]) def _extension_fields(change_list, original, new, new_pkg): @@ -575,16 +579,16 @@ def _extension_fields(change_list, original, new, new_pkg): ''' # list of the default metadata fields for a dataset # any fields that are not part of this list are custom fields added by a - #user or extension + # user or extension fields = ['owner_org', 'maintainer', 'maintainer_email', - 'relationships_as_object', 'private', 'num_tags', - 'id', 'metadata_created', 'metadata_modified', - 'author', 'author_email', 'state', 'version', - 'license_id', 'type', 'resources', 'num_resources', - 'tags', 'title', 'groups', 'creator_user_id', - 'relationships_as_subject', 'name', 'isopen', 'url', - 'notes', 'license_title', 'extras', - 'license_url', 'organization', 'revision_id'] + 'relationships_as_object', 'private', 'num_tags', + 'id', 'metadata_created', 'metadata_modified', + 'author', 'author_email', 'state', 'version', + 'license_id', 'type', 'resources', 'num_resources', + 'tags', 'title', 'groups', 'creator_user_id', + 'relationships_as_subject', 'name', 'isopen', 'url', + 'notes', 'license_title', 'extras', + 'license_url', 'organization', 'revision_id'] fields_set = set(fields) # if there are any fields from extensions that are in the new dataset and @@ -608,14 +612,14 @@ def _extension_fields(change_list, original, new, new_pkg): for field in addl_fields_list: if original[field] != new[field]: if original[field]: - change_list.append(["Changed value of field", - field.capitalize(), "to", - new[field], "(previously", - original[field] + ")", "in", new_pkg]) + change_list.append([u"Changed value of field", + field.capitalize(), u"to", + new[field], u"(previously", + original[field] + u")", u"in", new_pkg]) else: - change_list.append(["Changed value of field", - field.capitalize(), "to", - new[field], "in", new_pkg]) + change_list.append([u"Changed value of field", + field.capitalize(), u"to", + new[field], u"in", new_pkg]) def _extra_fields(change_list, original, new, new_pkg): @@ -625,7 +629,7 @@ def _extra_fields(change_list, original, new, new_pkg): change_list. ''' - s = "" + s = u"" if 'extras' in new: extra_fields_new = _extras_to_dict(new['extras']) extra_new_set = set(extra_fields_new.keys()) @@ -640,36 +644,36 @@ def _extra_fields(change_list, original, new, new_pkg): new_fields = list(extra_new_set - extra_original_set) if len(new_fields) == 1: if extra_fields_new[new_fields[0]]: - change_list.append(["Added field", s.join(("", - new_fields[0], "")), - "with value", s.join(("", - extra_fields_new[new_fields[0]], - "")), - "to", new_pkg]) + change_list.append([u"Added field", s.join((u"", + new_fields[0], u"")), + u"with value", s.join((u"", + extra_fields_new[new_fields[0]], + u"")), + u"to", new_pkg]) else: - change_list.append(["Added field", s.join(("", - new_fields[0], "")), - "to", new_pkg]) + change_list.append([u"Added field", s.join((u"", + new_fields[0], u"")), + u"to", new_pkg]) elif len(new_fields) > 1: - seq2 = ["
          • " + new_fields[i] + " with value " + - extra_fields_new[new_fields[i]] + "
            • " + seq2 = [u"
          • " + new_fields[i] + u" with value " + + extra_fields_new[new_fields[i]] + u"
            • " if extra_fields_new[new_fields[i]] - else "
          • " + new_fields[i] + "
            • " + else u"
          • " + new_fields[i] + u"
            • " for i in range(0, len(new_fields))] - change_list.append(["Added the following fields to", - new_pkg, "
              ", s.join(seq2), "
            "]) + change_list.append([u"Added the following fields to", + new_pkg, u"
              ", s.join(seq2), u"
            "]) # if some fields were deleted deleted_fields = list(extra_original_set - extra_new_set) if len(deleted_fields) == 1: - change_list.append(["Removed field", s.join(("", - deleted_fields[0], "")), - "from", new_pkg]) + change_list.append([u"Removed field", s.join((u"", + deleted_fields[0], u"")), + u"from", new_pkg]) elif len(deleted_fields) > 1: - seq2 = ["
          • " + deleted_fields[i] + "
            • " + seq2 = [u"
          • " + deleted_fields[i] + u"
            • " for i in range(0, len(deleted_fields))] - change_list.append(["Removed the following fields from", - new_pkg, "
              ", s.join(seq2), "
            "]) + change_list.append([u"Removed the following fields from", + new_pkg, u"
              ", s.join(seq2), u"
            "]) # if some existing fields were changed # list of extra fields in both the original and new versions @@ -677,20 +681,21 @@ def _extra_fields(change_list, original, new, new_pkg): for field in extra_fields: if extra_fields_original[field] != extra_fields_new[field]: if extra_fields_original[field]: - change_list.append(["Changed value of field", - s.join(("", field, "")), - "to", s.join(("", - extra_fields_new[field], "")), - "(previously", s.join(("", - extra_fields_original[field], - "")) + ")", - "in", new_pkg]) + change_list.append([u"Changed value of field", + s.join((u"", field, u"")), + u"to", s.join((u"", + extra_fields_new[field], + u"")), + u"(previously", s.join((u"", + extra_fields_original[field], + u"")) + u")", + u"in", new_pkg]) else: - change_list.append(["Changed value of field", - s.join(("", field, "")), - "to", s.join(("", - extra_fields_new[field], "")), - "in", new_pkg]) + change_list.append([u"Changed value of field", + s.join((u"", field, u"")), + u"to", s.join((u"", + extra_fields_new[field], u"")), + u"in", new_pkg]) # if the original version didn't have an extras field, # the user could only have added a field (not changed or deleted) @@ -698,33 +703,33 @@ def _extra_fields(change_list, original, new, new_pkg): new_fields = list(extra_new_set) if len(new_fields) == 1: if extra_fields_new[new_fields[0]]: - change_list.append(["Added field", s.join(("", - new_fields[0], "")), - "with value", s.join(("", + change_list.append([u"Added field", s.join((u"", + new_fields[0], u"")), + u"with value", s.join((u"", extra_fields_new[new_fields[0]], - "")), - "to", new_pkg]) + u"")), + u"to", new_pkg]) else: - change_list.append(["Added field", s.join(("", - new_fields[0], "")), - "to", new_pkg]) + change_list.append([u"Added field", s.join((u"", + new_fields[0], u"")), + u"to", new_pkg]) elif len(new_fields) > 1: - seq2 = ["
          • " + new_fields[i] + " with value " + - extra_fields_new[new_fields[i]] + "
            • " + seq2 = [u"
          • " + new_fields[i] + u" with value " + + extra_fields_new[new_fields[i]] + u"
            • " if extra_fields_new[new_fields[i]] - else "
          • " + new_fields[i] + "
            • " + else u"
          • " + new_fields[i] + u"
            • " for i in range(0, len(new_fields))] - change_list.append(["Added the following fields to", - new_pkg, "
              ", s.join(seq2), "
            "]) + change_list.append([u"Added the following fields to", + new_pkg, u"
              ", s.join(seq2), u"
            "]) elif 'extras' in original: deleted_fields = _extras_to_dict(original['extras']).keys() if len(deleted_fields) == 1: - change_list.append(["Removed field", s.join(("", - deleted_fields[0], "")), "from", + change_list.append([u"Removed field", s.join((u"", + deleted_fields[0], u"")), u"from", new_pkg]) elif len(deleted_fields) > 1: - seq2 = ["
          • " + deleted_fields[i] + - "
            • " for i in range(0, len(deleted_fields))] - change_list.append(["Removed the following fields from", - new_pkg, "
              ", s.join(seq2), "
            "]) + seq2 = [u"
          • " + deleted_fields[i] + + u"
            • " for i in range(0, len(deleted_fields))] + change_list.append([u"Removed the following fields from", + new_pkg, u"
              ", s.join(seq2), u"
            "]) diff --git a/ckan/lib/helpers.py b/ckan/lib/helpers.py index a229e3011d6..16b300ebc34 100644 --- a/ckan/lib/helpers.py +++ b/ckan/lib/helpers.py @@ -2737,15 +2737,15 @@ def activity_list_select(pkg_activity_list, current_activity_id): with_seconds=True) if activity['id'] == current_activity_id: select_list.append("") + activity['id'] + + "\" selected>" + + entry + + "") else: select_list.append("") + activity['id'] + + "\">" + + entry + + "") return select_list diff --git a/ckan/views/dataset.py b/ckan/views/dataset.py index 37f1698a653..3133c92fccd 100644 --- a/ckan/views/dataset.py +++ b/ckan/views/dataset.py @@ -1105,7 +1105,7 @@ def changes(id, package_type=None): pkg_id = activity_diff[u'activities'][1][u'data'][u'package'][u'id'] current_pkg_dict = get_action(u'package_show')(context, {u'id': pkg_id}) pkg_activity_list = get_action(u'package_activity_list')(context, - {u'id': pkg_id, u'limit': 100}) + {u'id': pkg_id, u'limit': 100}) return base.render( u'package/changes.html', { @@ -1124,8 +1124,8 @@ def change_range(package_type=None): re-renders changes.html with the list. ''' - newest_id = h.get_request_param('newest_id') - oldest_id = h.get_request_param('oldest_id') + newest_id = h.get_request_param(u'newest_id') + oldest_id = h.get_request_param(u'oldest_id') context = { u'model': model, u'session': model.Session, @@ -1137,15 +1137,15 @@ def change_range(package_type=None): old_activity = get_action(u'activity_show')(context, { u'id': oldest_id, u'include_data': False}) - new_activity = get_action('activity_show')(context, { + new_activity = get_action(u'activity_show')(context, { u'id': newest_id, u'include_data': False}) - old_timestamp = old_activity['timestamp'] - new_timestamp = new_activity['timestamp'] + old_timestamp = old_activity[u'timestamp'] + new_timestamp = new_activity[u'timestamp'] - t1 = datetime.strptime(old_timestamp, '%Y-%m-%dT%H:%M:%S.%f') - t2 = datetime.strptime(new_timestamp, '%Y-%m-%dT%H:%M:%S.%f') + t1 = datetime.strptime(old_timestamp, u'%Y-%m-%dT%H:%M:%S.%f') + t2 = datetime.strptime(new_timestamp, u'%Y-%m-%dT%H:%M:%S.%f') time_diff = t2 - t1 # if the time difference is negative, just return the change that put us @@ -1154,7 +1154,7 @@ def change_range(package_type=None): # display a warning that the user can't look at a sequence where # the newest item is older than the oldest one, etc if time_diff.total_seconds() < 0: - return changes(h.get_request_param('current_new_id')) + return changes(h.get_request_param(u'current_new_id')) done = False current_id = newest_id @@ -1164,10 +1164,10 @@ def change_range(package_type=None): try: activity_diff = get_action(u'activity_diff')( context, {u'id': current_id, u'object_type': u'package', - u'diff_type': u'html'}) + u'diff_type': u'html'}) except NotFound as e: log.info(u'Activity not found: {} - {}'.format(str(e), - current_id)) + current_id)) return base.abort(404, _(u'Activity not found')) except NotAuthorized: return base.abort(403, _(u'Unauthorized to view activity data')) @@ -1182,8 +1182,8 @@ def change_range(package_type=None): pkg_id = diff_list[0][u'activities'][1][u'data'][u'package'][u'id'] current_pkg_dict = get_action(u'package_show')(context, {u'id': pkg_id}) pkg_activity_list = get_action(u'package_activity_list')(context, { - u'id': pkg_id, - u'limit': 100}) + u'id': pkg_id, + u'limit': 100}) return base.render( u'package/changes.html', { From d815fcdfdd8c4dd6ca05d06080500f00274e5877 Mon Sep 17 00:00:00 2001 From: hayley-leblanc Date: Tue, 30 Jul 2019 08:42:19 +0200 Subject: [PATCH 15/47] Formatting --- ckan/cli/less.py | 2 +- ckan/cli/tracking.py | 4 +- ckan/lib/changes.py | 114 ++++++++++++++++++++++++------------------ ckan/lib/helpers.py | 18 +++---- ckan/views/dataset.py | 18 ++++--- 5 files changed, 86 insertions(+), 70 deletions(-) diff --git a/ckan/cli/less.py b/ckan/cli/less.py index 20945716a9e..28667f60dd4 100644 --- a/ckan/cli/less.py +++ b/ckan/cli/less.py @@ -61,7 +61,7 @@ def less(): directory = output[0].strip() if not directory: error_shout(u'Command "{}" returned nothing. Check that npm is ' - u'installed.'.format(' '.join(command))) + u'installed.'.format(u' '.join(command))) less_bin = os.path.join(directory, u'lessc') public = config.get(u'ckan.base_public_folder') diff --git a/ckan/cli/tracking.py b/ckan/cli/tracking.py index 73a1f6de653..cd1daeff849 100644 --- a/ckan/cli/tracking.py +++ b/ckan/cli/tracking.py @@ -202,8 +202,8 @@ def update_tracking_solr(engine, start_date): total = len(package_ids) not_found = 0 - click.echo('{} package index{} to be rebuilt starting from {}'.format( - total, '' if total < 2 else 'es', start_date) + click.echo(u'{} package index{} to be rebuilt starting from {}'.format( + total, u'' if total < 2 else u'es', start_date) ) from ckan.lib.search import rebuild diff --git a/ckan/lib/changes.py b/ckan/lib/changes.py index 049d9c58b7e..2582111b23e 100644 --- a/ckan/lib/changes.py +++ b/ckan/lib/changes.py @@ -88,7 +88,8 @@ def _check_resource_changes(change_list, original, new, new_pkg, action=u"read", id=original['id'], resource_id=resource_id) + u"?activity_id=" + old_activity_id, u"\">", original_resource_dict[resource_id]['name'], u"") - change_list.append(["Deleted resource", s.join(seq2), u"from", new_pkg]) + change_list.append( + ["Deleted resource", s.join(seq2), u"from", new_pkg]) # now check the resources that are in both and see if any # have been changed @@ -457,13 +458,15 @@ def _tag_change(change_list, new_tags, original_tags, new_pkg): deleted_tags_list[0], u"") change_list.append([u"Removed tag", s.join(seq2), u"from", new_pkg]) elif len(deleted_tags) > 1: - seq2 = [u"
          • " + deleted_tags_list[i] + u"
            • " - for i in range(0, len(deleted_tags))] - change_list.append([u"Removed the following tags from", new_pkg, + seq2 = [ + u"
          • " + deleted_tags_list[i] + u"
            • " + for i in range(0, len(deleted_tags)) + ] + change_list.append([u"Removed the following tags from", new_pkg, u"
              ", s.join(seq2), u"
            "]) added_tags = new_tags - original_tags @@ -496,12 +499,12 @@ def _license_change(change_list, original, new, new_pkg): seq2 = () seq3 = () # if the license has a URL, use it - if 'license_url' in original and original['license_url']: + if u'license_url' in original and original['license_url']: seq2 = (u"", original['license_title'], u"") else: seq2 = (original['license_title']) - if 'license_url' in new and new['license_url']: + if u'license_url' in new and new['license_url']: seq3 = (u"", new['license_title'], u"") else: @@ -517,14 +520,18 @@ def _name_change(change_list, original, new): change_list. ''' s = u"" - old_url = url_for(qualified=True, - controller=u"dataset", - action=u"read", - id=original['name']) - new_url = url_for(qualified=True, - controller=u"dataset", - action=u"read", - id=new['name']) + old_url = url_for( + qualified=True, + controller=u"dataset", + action=u"read", + id=original['name'] + ) + new_url = url_for( + qualified=True, + controller=u"dataset", + action=u"read", + id=new['name'] + ) seq2 = (u"", old_url, u"") seq3 = (u"", new_url, u"") change_list.append([u"Moved the dataset from", s.join(seq2), @@ -580,15 +587,17 @@ def _extension_fields(change_list, original, new, new_pkg): # list of the default metadata fields for a dataset # any fields that are not part of this list are custom fields added by a # user or extension - fields = ['owner_org', 'maintainer', 'maintainer_email', - 'relationships_as_object', 'private', 'num_tags', - 'id', 'metadata_created', 'metadata_modified', - 'author', 'author_email', 'state', 'version', - 'license_id', 'type', 'resources', 'num_resources', - 'tags', 'title', 'groups', 'creator_user_id', - 'relationships_as_subject', 'name', 'isopen', 'url', - 'notes', 'license_title', 'extras', - 'license_url', 'organization', 'revision_id'] + fields = [ + u'owner_org', u'maintainer', u'maintainer_email', + u'relationships_as_object', u'private', u'num_tags', + u'id', u'metadata_created', u'metadata_modified', + u'author', u'author_email', u'state', u'version', + u'license_id', u'type', u'resources', u'num_resources', + u'tags', u'title', u'groups', u'creator_user_id', + u'relationships_as_subject', u'name', u'isopen', u'url', + u'notes', u'license_title', u'extras', + u'license_url', u'organization', u'revision_id' + ] fields_set = set(fields) # if there are any fields from extensions that are in the new dataset and @@ -630,13 +639,13 @@ def _extra_fields(change_list, original, new, new_pkg): ''' s = u"" - if 'extras' in new: + if u'extras' in new: extra_fields_new = _extras_to_dict(new['extras']) extra_new_set = set(extra_fields_new.keys()) # if the original version has an extra fields, we need # to compare the new version'sextras to the original ones - if 'extras' in original: + if u'extras' in original: extra_fields_original = _extras_to_dict(original['extras']) extra_original_set = set(extra_fields_original.keys()) @@ -644,12 +653,13 @@ def _extra_fields(change_list, original, new, new_pkg): new_fields = list(extra_new_set - extra_original_set) if len(new_fields) == 1: if extra_fields_new[new_fields[0]]: - change_list.append([u"Added field", s.join((u"", - new_fields[0], u"")), - u"with value", s.join((u"", - extra_fields_new[new_fields[0]], - u"")), - u"to", new_pkg]) + change_list.append( + [u"Added field", s.join((u"", + new_fields[0], u"")), + u"with value", s.join((u"", + extra_fields_new[new_fields[0]], + u"")), u"to", new_pkg] + ) else: change_list.append([u"Added field", s.join((u"", new_fields[0], u"")), @@ -681,21 +691,25 @@ def _extra_fields(change_list, original, new, new_pkg): for field in extra_fields: if extra_fields_original[field] != extra_fields_new[field]: if extra_fields_original[field]: - change_list.append([u"Changed value of field", - s.join((u"", field, u"")), - u"to", s.join((u"", - extra_fields_new[field], - u"")), - u"(previously", s.join((u"", - extra_fields_original[field], - u"")) + u")", - u"in", new_pkg]) + change_list.append( + [u"Changed value of field", + s.join((u"", field, u"")), + u"to", s.join((u"", + extra_fields_new[field], + u"")), + u"(previously", s.join((u"", + extra_fields_original[field], + u"")) + u")", + u"in", new_pkg] + ) else: - change_list.append([u"Changed value of field", - s.join((u"", field, u"")), - u"to", s.join((u"", - extra_fields_new[field], u"")), - u"in", new_pkg]) + change_list.append( + [u"Changed value of field", + s.join((u"", field, u"")), + u"to", s.join((u"", + extra_fields_new[field], u"")), + u"in", new_pkg] + ) # if the original version didn't have an extras field, # the user could only have added a field (not changed or deleted) @@ -722,7 +736,7 @@ def _extra_fields(change_list, original, new, new_pkg): change_list.append([u"Added the following fields to", new_pkg, u"
              ", s.join(seq2), u"
            "]) - elif 'extras' in original: + elif u'extras' in original: deleted_fields = _extras_to_dict(original['extras']).keys() if len(deleted_fields) == 1: change_list.append([u"Removed field", s.join((u"", diff --git a/ckan/lib/helpers.py b/ckan/lib/helpers.py index 16b300ebc34..87fac47f7b6 100644 --- a/ckan/lib/helpers.py +++ b/ckan/lib/helpers.py @@ -2736,16 +2736,14 @@ def activity_list_select(pkg_activity_list, current_activity_id): with_hours=True, with_seconds=True) if activity['id'] == current_activity_id: - select_list.append("") + select_list.append( + "" + ) else: - select_list.append("") + select_list.append( + "" + ) return select_list diff --git a/ckan/views/dataset.py b/ckan/views/dataset.py index 3133c92fccd..e11a81ad16f 100644 --- a/ckan/views/dataset.py +++ b/ckan/views/dataset.py @@ -1104,8 +1104,10 @@ def changes(id, package_type=None): # changed, and we need a link to it which works pkg_id = activity_diff[u'activities'][1][u'data'][u'package'][u'id'] current_pkg_dict = get_action(u'package_show')(context, {u'id': pkg_id}) - pkg_activity_list = get_action(u'package_activity_list')(context, - {u'id': pkg_id, u'limit': 100}) + pkg_activity_list = get_action(u'package_activity_list')( + context, { + u'id': pkg_id, + u'limit': 100}) return base.render( u'package/changes.html', { @@ -1163,11 +1165,13 @@ def change_range(package_type=None): while not done: try: activity_diff = get_action(u'activity_diff')( - context, {u'id': current_id, u'object_type': u'package', - u'diff_type': u'html'}) + context, { + u'id': current_id, + u'object_type': u'package', + u'diff_type': u'html'}) except NotFound as e: log.info(u'Activity not found: {} - {}'.format(str(e), - current_id)) + current_id)) return base.abort(404, _(u'Activity not found')) except NotAuthorized: return base.abort(403, _(u'Unauthorized to view activity data')) @@ -1182,8 +1186,8 @@ def change_range(package_type=None): pkg_id = diff_list[0][u'activities'][1][u'data'][u'package'][u'id'] current_pkg_dict = get_action(u'package_show')(context, {u'id': pkg_id}) pkg_activity_list = get_action(u'package_activity_list')(context, { - u'id': pkg_id, - u'limit': 100}) + u'id': pkg_id, + u'limit': 100}) return base.render( u'package/changes.html', { From 9818ab067fe054ff99d5f650a8436ac33a616488 Mon Sep 17 00:00:00 2001 From: hayley-leblanc Date: Tue, 30 Jul 2019 08:58:18 +0200 Subject: [PATCH 16/47] Formatting --- ckan/lib/changes.py | 79 +++++++++++++++++++++++++++++-------------- ckan/views/dataset.py | 11 +++--- 2 files changed, 61 insertions(+), 29 deletions(-) diff --git a/ckan/lib/changes.py b/ckan/lib/changes.py index 2582111b23e..d37395761eb 100644 --- a/ckan/lib/changes.py +++ b/ckan/lib/changes.py @@ -459,10 +459,13 @@ def _tag_change(change_list, new_tags, original_tags, new_pkg): change_list.append([u"Removed tag", s.join(seq2), u"from", new_pkg]) elif len(deleted_tags) > 1: seq2 = [ - u"
          • " + deleted_tags_list[i] + u"
            • " for i in range(0, len(deleted_tags)) ] @@ -654,11 +657,18 @@ def _extra_fields(change_list, original, new, new_pkg): if len(new_fields) == 1: if extra_fields_new[new_fields[0]]: change_list.append( - [u"Added field", s.join((u"", - new_fields[0], u"")), - u"with value", s.join((u"", - extra_fields_new[new_fields[0]], - u"")), u"to", new_pkg] + [u"Added field", + s.join( + (u"", + new_fields[0], + u"") + ), + u"with value", + s.join( + (u"", + extra_fields_new[new_fields[0]], + u"") + ), u"to", new_pkg] ) else: change_list.append([u"Added field", s.join((u"", @@ -693,21 +703,35 @@ def _extra_fields(change_list, original, new, new_pkg): if extra_fields_original[field]: change_list.append( [u"Changed value of field", - s.join((u"", field, u"")), - u"to", s.join((u"", - extra_fields_new[field], - u"")), - u"(previously", s.join((u"", - extra_fields_original[field], - u"")) + u")", + s.join( + (u"", field, u"") + ), + u"to", + s.join( + (u"", + extra_fields_new[field], + u"") + ), + u"(previously", + s.join( + (u"", + extra_fields_original[field], + u"") + ) + u")", u"in", new_pkg] ) else: change_list.append( [u"Changed value of field", - s.join((u"", field, u"")), - u"to", s.join((u"", - extra_fields_new[field], u"")), + s.join( + (u"", field, u"") + ), + u"to", + s.join( + (u"", + extra_fields_new[field], + u"") + ), u"in", new_pkg] ) @@ -717,12 +741,17 @@ def _extra_fields(change_list, original, new, new_pkg): new_fields = list(extra_new_set) if len(new_fields) == 1: if extra_fields_new[new_fields[0]]: - change_list.append([u"Added field", s.join((u"", - new_fields[0], u"")), - u"with value", s.join((u"", - extra_fields_new[new_fields[0]], - u"")), - u"to", new_pkg]) + change_list.append( + [u"Added field", + s.join( + (u"", new_fields[0], u"") + ), + u"with value", + s.join( + (u"", extra_fields_new[new_fields[0]], u"") + ), + u"to", new_pkg] + ) else: change_list.append([u"Added field", s.join((u"", new_fields[0], u"")), diff --git a/ckan/views/dataset.py b/ckan/views/dataset.py index e11a81ad16f..c0644661807 100644 --- a/ckan/views/dataset.py +++ b/ckan/views/dataset.py @@ -1106,8 +1106,10 @@ def changes(id, package_type=None): current_pkg_dict = get_action(u'package_show')(context, {u'id': pkg_id}) pkg_activity_list = get_action(u'package_activity_list')( context, { - u'id': pkg_id, - u'limit': 100}) + u'id': pkg_id, + u'limit': 100 + } + ) return base.render( u'package/changes.html', { @@ -1170,8 +1172,9 @@ def change_range(package_type=None): u'object_type': u'package', u'diff_type': u'html'}) except NotFound as e: - log.info(u'Activity not found: {} - {}'.format(str(e), - current_id)) + log.info( + u'Activity not found: {} - {}'.format(str(e), current_id) + ) return base.abort(404, _(u'Activity not found')) except NotAuthorized: return base.abort(403, _(u'Unauthorized to view activity data')) From e566b2165f6fabab7cf7a6b7ef0a3f65769b3a67 Mon Sep 17 00:00:00 2001 From: hayley-leblanc Date: Tue, 30 Jul 2019 09:20:16 +0200 Subject: [PATCH 17/47] Formatting --- ckan/lib/changes.py | 79 +++++++++++++++++++++------------------------ 1 file changed, 36 insertions(+), 43 deletions(-) diff --git a/ckan/lib/changes.py b/ckan/lib/changes.py index d37395761eb..491853173a4 100644 --- a/ckan/lib/changes.py +++ b/ckan/lib/changes.py @@ -658,22 +658,24 @@ def _extra_fields(change_list, original, new, new_pkg): if extra_fields_new[new_fields[0]]: change_list.append( [u"Added field", - s.join( - (u"", - new_fields[0], - u"") - ), + s.join((u"", + new_fields[0], + u"")), u"with value", - s.join( - (u"", - extra_fields_new[new_fields[0]], - u"") - ), u"to", new_pkg] + s.join((u"", + extra_fields_new[new_fields[0]], + u"")), + u"to", new_pkg] ) else: - change_list.append([u"Added field", s.join((u"", - new_fields[0], u"")), - u"to", new_pkg]) + change_list.append( + [u"Added field", + s.join((u"", + new_fields[0], + u"") + ), + u"to", new_pkg] + ) elif len(new_fields) > 1: seq2 = [u"
          • " + new_fields[i] + u" with value " + extra_fields_new[new_fields[i]] + u"
            • " @@ -703,35 +705,26 @@ def _extra_fields(change_list, original, new, new_pkg): if extra_fields_original[field]: change_list.append( [u"Changed value of field", - s.join( - (u"", field, u"") - ), + s.join((u"", + field, u"")), u"to", - s.join( - (u"", - extra_fields_new[field], - u"") - ), + s.join((u"", + extra_fields_new[field], + u"")), u"(previously", - s.join( - (u"", - extra_fields_original[field], - u"") - ) + u")", - u"in", new_pkg] + s.join((u"", + extra_fields_original[field], + u"")) + + u")", u"in", new_pkg] ) else: change_list.append( [u"Changed value of field", - s.join( - (u"", field, u"") - ), + s.join((u"", field, u"")), u"to", - s.join( - (u"", - extra_fields_new[field], - u"") - ), + s.join((u"", + extra_fields_new[field], + u"")), u"in", new_pkg] ) @@ -743,18 +736,18 @@ def _extra_fields(change_list, original, new, new_pkg): if extra_fields_new[new_fields[0]]: change_list.append( [u"Added field", - s.join( - (u"", new_fields[0], u"") - ), + s.join((u"", new_fields[0], u"")), u"with value", - s.join( - (u"", extra_fields_new[new_fields[0]], u"") - ), + s.join((u"", + extra_fields_new[new_fields[0]], + u"")), u"to", new_pkg] ) else: - change_list.append([u"Added field", s.join((u"", - new_fields[0], u"")), + change_list.append([u"Added field", + s.join((u"", + new_fields[0], + u"")), u"to", new_pkg]) elif len(new_fields) > 1: seq2 = [u"
          • " + new_fields[i] + u" with value " + From 50d0d82af753e2200af102b17672441b65722e61 Mon Sep 17 00:00:00 2001 From: hayley-leblanc Date: Tue, 6 Aug 2019 12:48:50 +0200 Subject: [PATCH 18/47] Stop saving activity for private datasets --- ckan/lib/activity_streams_session_extension.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ckan/lib/activity_streams_session_extension.py b/ckan/lib/activity_streams_session_extension.py index decdeeb6cde..558f5e0ec33 100644 --- a/ckan/lib/activity_streams_session_extension.py +++ b/ckan/lib/activity_streams_session_extension.py @@ -70,8 +70,8 @@ def before_commit(self, session): # object is a package. # Don't create activities for private datasets. - #if obj.private: - # continue + if obj.private: + continue activities[obj.id] = activity @@ -106,8 +106,8 @@ def before_commit(self, session): continue # Don't create activities for private datasets. - #if package.private: - # continue + if package.private: + continue if package.id in activities: continue From 7ccd68a12d100e6650225a52809efd6037bcd14d Mon Sep 17 00:00:00 2001 From: hayley-leblanc Date: Wed, 7 Aug 2019 14:53:58 +0200 Subject: [PATCH 19/47] Move HTML for Changes page from changes.py to HTML files --- ckan/lib/changes.py | 654 ++++++++---------- ckan/lib/helpers.py | 6 +- .../package/snippets/change_item.html | 11 +- ckan/templates/snippets/changes/author.html | 25 + .../snippets/changes/author_email.html | 30 + .../snippets/changes/custom_fields.html | 82 +++ .../snippets/changes/delete_resource.html | 14 + .../snippets/changes/description.html | 29 + .../snippets/changes/extension_fields.html | 11 + ckan/templates/snippets/changes/license.html | 48 ++ .../snippets/changes/maintainer.html | 25 + .../snippets/changes/maintainer_email.html | 28 + ckan/templates/snippets/changes/name.html | 13 + ckan/templates/snippets/changes/new_file.html | 13 + .../snippets/changes/new_resource.html | 13 + ckan/templates/snippets/changes/org.html | 15 + ckan/templates/snippets/changes/private.html | 10 + .../snippets/changes/resource_desc.html | 40 ++ .../snippets/changes/resource_format.html | 35 + .../snippets/changes/resource_name.html | 19 + .../snippets/changes/source_url.html | 27 + ckan/templates/snippets/changes/tags.html | 58 ++ ckan/templates/snippets/changes/title.html | 10 + ckan/templates/snippets/changes/version.html | 26 + 24 files changed, 863 insertions(+), 379 deletions(-) create mode 100644 ckan/templates/snippets/changes/author.html create mode 100644 ckan/templates/snippets/changes/author_email.html create mode 100644 ckan/templates/snippets/changes/custom_fields.html create mode 100644 ckan/templates/snippets/changes/delete_resource.html create mode 100644 ckan/templates/snippets/changes/description.html create mode 100644 ckan/templates/snippets/changes/extension_fields.html create mode 100644 ckan/templates/snippets/changes/license.html create mode 100644 ckan/templates/snippets/changes/maintainer.html create mode 100644 ckan/templates/snippets/changes/maintainer_email.html create mode 100644 ckan/templates/snippets/changes/name.html create mode 100644 ckan/templates/snippets/changes/new_file.html create mode 100644 ckan/templates/snippets/changes/new_resource.html create mode 100644 ckan/templates/snippets/changes/org.html create mode 100644 ckan/templates/snippets/changes/private.html create mode 100644 ckan/templates/snippets/changes/resource_desc.html create mode 100644 ckan/templates/snippets/changes/resource_format.html create mode 100644 ckan/templates/snippets/changes/resource_name.html create mode 100644 ckan/templates/snippets/changes/source_url.html create mode 100644 ckan/templates/snippets/changes/tags.html create mode 100644 ckan/templates/snippets/changes/title.html create mode 100644 ckan/templates/snippets/changes/version.html diff --git a/ckan/lib/changes.py b/ckan/lib/changes.py index 491853173a4..87877686dd9 100644 --- a/ckan/lib/changes.py +++ b/ckan/lib/changes.py @@ -5,6 +5,8 @@ the differences between two versions of a dataset. ''' from helpers import url_for +import logging +log = logging.getLogger(__name__) def _extras_to_dict(extras_list): @@ -74,22 +76,23 @@ def _check_resource_changes(change_list, original, new, new_pkg, # get the IDs of the resources that have been added between the versions new_resources = list(new_resource_set - original_resource_set) for resource_id in new_resources: - seq2 = (u"", - new_resource_dict[resource_id]['name'], u"") - change_list.append(["Added resource", - s.join(seq2), u"to", - new_pkg]) + change_list.append({u'type': 'new_resource', + u'pkg_id': new_pkg['pkg_id'], + u'title': new_pkg['title'], + u'resource_name': + new_resource_dict[resource_id]['name'], + u'resource_id': resource_id}) # get the IDs of resources that have been deleted between versions deleted_resources = list(original_resource_set - new_resource_set) for resource_id in deleted_resources: - seq2 = (u"", - original_resource_dict[resource_id]['name'], u"") - change_list.append( - ["Deleted resource", s.join(seq2), u"from", new_pkg]) + change_list.append({u'type': 'delete_resource', + u'pkg_id': new_pkg['pkg_id'], + u'title': new_pkg['title'], + u'resource_id': resource_id, + u'resource_name': + original_resource_dict[resource_id]['name'], + u'old_activity_id': old_activity_id}) # now check the resources that are in both and see if any # have been changed @@ -99,130 +102,91 @@ def _check_resource_changes(change_list, original, new, new_pkg, new_metadata = new_resource_dict[resource_id] if original_metadata['name'] != new_metadata['name']: - seq2 = (u"", - original_resource_dict[resource_id]['name'], u"") - seq3 = (u"", - new_resource_dict[resource_id]['name'], u"") - change_list.append(["Renamed resource", s.join(seq2), - u"to", s.join(seq3), u"in", new_pkg]) + change_list.append({u'type': 'resource_name', + u'pkg_id': new_pkg['pkg_id'], + u'title': new_pkg['title'], + u'old_pkg_id': original['id'], + u'new_pkg_id': new['id'], + u'resource_id': resource_id, + u'old_resource_name': + original_resource_dict[resource_id]['name'], + u'new_resource_name': + new_resource_dict[resource_id]['name'], + u'old_activity_id': old_activity_id}) # you can't remove a format, but if a resource's format isn't # recognized, it won't have one set # if a format was not originally set and the user set one if not original_metadata['format'] and new_metadata['format']: - seq2 = (u"", - new_resource_dict[resource_id]['name'], u"") - seq3 = (u"", - new_metadata['format'], u"") - change_list.append([u"Set format of resource", s.join(seq2), - u"to", s.join(seq3), u"in", new_pkg]) + change_list.append({u'type': 'resource_format', + u'method': 'add', + u'pkg_id': new_pkg['pkg_id'], + u'title': new_pkg['title'], + u'resource_id': resource_id, + u'resource_name': + new_resource_dict[resource_id]['name'], + u'org_id':new['organization']['id'], + u'format': new_metadata['format']}) + # if both versions have a format but the format changed elif original_metadata['format'] != new_metadata['format']: - seq2 = (u"", - original_resource_dict[resource_id]['name'], u"") - seq3 = (u"", - new_metadata['format'], u"") - seq4 = (u"", - original_metadata['format'], u"") - change_list.append([u"Set format of resource", - s.join(seq2), - u"to", s.join(seq3), - u"(previously", s.join(seq4) + u")", - u"in", new_pkg]) + change_list.append({u'type': 'resource_format', + u'method': 'change', + u'pkg_id': new_pkg['pkg_id'], + u'title': new_pkg['title'], + u'resource_id': resource_id, + u'resource_name': + new_resource_dict[resource_id]['name'], + u'org_id':new['organization']['id'], + u'old_format': original_metadata['format'], + u'new_format': new_metadata['format']}) # if the description changed if not original_metadata['description'] and \ new_metadata['description']: - seq2 = (u"", - new_resource_dict[resource_id]['name'], u"") - change_list.append([u"Updated description of resource", - s.join(seq2), u"in", - new_pkg, u"to
            ", - u"
            " + new_metadata['description'] - + u"
            "]) + change_list.append({u'type': 'resource_desc', + u'method': 'add', + u'pkg_id': new_pkg['pkg_id'], + u'title': new_pkg['title'], + u'resource_id': resource_id, + u'resource_name': + new_resource_dict[resource_id]['name'], + u'new_desc': new_metadata['description']}) # if there was a description but the user removed it elif original_metadata['description'] and \ not new_metadata['description']: - seq2 = (u"", - new_resource_dict[resource_id]['name'], u"") - change_list.append([u"Removed description from resource", - s.join(seq2), u"in", new_pkg]) + change_list.append({u'type': 'resource_desc', + u'method': 'remove', + u'pkg_id': new_pkg['pkg_id'], + u'title': new_pkg['title'], + u'resource_id': resource_id, + u'resource_name': + new_resource_dict[resource_id]['name']}) # if both have descriptions but they are different elif original_metadata['description'] != new_metadata['description']: - seq2 = (u"", - new_resource_dict[resource_id]['name'], u"") - change_list.append([u"Updated description of resource", - s.join(seq2), u"in", - new_pkg, - u"from
            ", - u"
            " + - original_metadata['description'] + - u"
            ", - u"to
            ", - u"
            " + - new_metadata['description'] + - u"
            "]) + change_list.append({u'type': 'resource_desc', + u'method': 'change', + u'pkg_id': new_pkg['pkg_id'], + u'title': new_pkg['title'], + u'resource_id': resource_id, + u'resource_name': + new_resource_dict[resource_id]['name'], + u'new_desc': new_metadata['description'], + u'old_desc': old_metadata['description']}) # check if the user uploaded a new file # TODO: use regular expressions to determine the actual name of the # new and old files if original_metadata['url'] != new_metadata['url']: - seq2 = (u"", - new_resource_dict[resource_id]['name'], u"") - change_list.append([u"Uploaded a new file to resource", - s.join(seq2), u"in", new_pkg]) + change_list.append({u'type': 'new_file', + u'pkg_id': new_pkg['pkg_id'], + u'title': new_pkg['title'], + u'resource_id': resource_id, + u'resource_name': + new_resource_dict[resource_id]['name']}) def _check_metadata_changes(change_list, original, new, new_pkg): @@ -258,8 +222,9 @@ def _check_metadata_changes(change_list, original, new, new_pkg): # if the visibility of the dataset changed if original['private'] != new['private']: - change_list.append([u"Set visibility of", new_pkg, u"to", - u"Private" if new['private'] else u"Public"]) + change_list.append({u'type': 'private', u'pkg_id': new_pkg['pkg_id'], + u'title': new_pkg['title'], + u'new': 'Private' if bool(new['private']) else 'Public'}) # if the description of the dataset changed if original['notes'] != new['notes']: @@ -280,7 +245,7 @@ def _check_metadata_changes(change_list, original, new, new_pkg): # this is only visible to the user via the dataset's URL, # so display the change using that if original['name'] != new['name']: - _name_change(change_list, original, new) + _name_change(change_list, original, new, new_pkg) # if the source URL (metadata value, not the actual URL of the dataset) # has changed @@ -303,12 +268,8 @@ def _title_change(change_list, original, new): Appends a summary of a change to a dataset's title between two versions (original and new) to change_list. ''' - s = u"" - seq2 = (u"", - new['title'], u"") - change_list.append([u"Changed title to", s.join(seq2), - u"(previously", original['title'] + u")"]) + change_list.append({'type': 'title', 'id': new['name'], 'new_title': new['title'], + 'original_title': original['title']}) def _org_change(change_list, original, new, new_pkg): @@ -316,25 +277,12 @@ def _org_change(change_list, original, new, new_pkg): Appends a summary of a change to a dataset's organization between two versions (original and new) to change_list. ''' - s = u"" - seq2 = (u"", - original['organization']['title'], u"") - seq3 = (u"", - new['organization']['title'], u"") - change_list.append([u"Moved", new_pkg, - u"from organization", - s.join(seq2), - u"to organization", - s.join(seq3)]) - + change_list.append({u'type': u'org', u'pkg_id': new_pkg['pkg_id'], + u'title': new_pkg['title'], + u'original_org_id': original['organization']['id'], + u'original_org_title': original['organization']['title'], + u'new_org_id': new['organization']['id'], + u'new_org_title': new['organization']['title']}) def _maintainer_change(change_list, original, new, new_pkg): ''' @@ -343,14 +291,21 @@ def _maintainer_change(change_list, original, new, new_pkg): ''' # if the original dataset had a maintainer if original['maintainer'] and new['maintainer']: - change_list.append([u"Set maintainer of", new_pkg, - u"to", new['maintainer'], - u"(previously", original['maintainer'] + u")"]) + change_list.append({u'type': 'maintainer', u'method': 'change', + u'pkg_id': new_pkg['pkg_id'], + u'title': new_pkg['title'], 'new_maintainer': + new['maintainer'], 'old_maintainer': + original['maintainer']}) + # if they removed the maintainer elif not new['maintainer']: - change_list.append([u"Removed maintainer from", new_pkg]) + change_list.append({u'type': 'maintainer', u'pkg_id': new_pkg['pkg_id'], + u'title': new_pkg['title'], u'method': 'remove'}) + # if there wasn't one there before else: - change_list.append([u"Set maintainer of", new_pkg, u"to", - new['maintainer']]) + change_list.append({u'type': 'maintainer', u'pkg_id': new_pkg['pkg_id'], + u'title': new_pkg['title'], u'new_maintainer': + new['maintainer'], u'method': 'add'}) + def _maintainer_email_change(change_list, original, new, new_pkg): @@ -358,21 +313,21 @@ def _maintainer_email_change(change_list, original, new, new_pkg): Appends a summary of a change to a dataset's maintainer e-mail address field between two versions (original and new) to change_list. ''' - s = u"" - seq2 = (u"", - new['maintainer_email'], u"") # if the original dataset had a maintainer email if original['maintainer_email'] and new['maintainer_email']: - seq3 = (u"", - original['maintainer_email'], u"") - change_list.append([u"Set maintainer e-mail of", - new_pkg, u"to", s.join(seq2), - u"(previously", s.join(seq3) + u")"]) + change_list.append({u'type': 'maintainer_email', u'pkg_id': new_pkg['pkg_id'], + u'title': new_pkg['title'], 'new_maintainer_email': + new['maintainer_email'], 'old_maintainer_email': + original['maintainer_email'], u'method': 'change'}) + # if they removed the maintainer email elif not new['maintainer_email']: - change_list.append([u"Removed maintainer e-mail from", new_pkg]) + change_list.append({u'type': 'maintainer_email', u'pkg_id': new_pkg['pkg_id'], + u'title': new_pkg['title'], u'method': 'remove'}) + # if there wasn't one there before e else: - change_list.append([u"Set maintainer e-mail of", - new_pkg, u"to", s.join(seq2)]) + change_list.append({u'type': 'maintainer_email', u'pkg_id': new_pkg['pkg_id'], + u'title': new_pkg['title'], 'new_maintainer_email': + new['maintainer_email'], u'method': 'add'}) def _author_change(change_list, original, new, new_pkg): @@ -382,12 +337,19 @@ def _author_change(change_list, original, new, new_pkg): ''' # if the original dataset had an author if original['author'] and new['author']: - change_list.append([u"Set author of", new_pkg, u"to", new['author'], - u"(previously", original['author'] + u")"]) + change_list.append({u'type': 'author', u'pkg_id': new_pkg['pkg_id'], + u'title': new_pkg['title'], u'new_author': + new['author'], u'old_author': original['author'], + u'method': 'change'}) + # if they removed the author elif not new['author']: - change_list.append([u"Removed author from", new_pkg]) + change_list.append({u'type': 'author', u'pkg_id': new_pkg['pkg_id'], + u'title': new_pkg['title'], u'method': 'remove'}) + # if there wasn't one there before else: - change_list.append([u"Set author of", new_pkg, u"to", new['author']]) + change_list.append({u'type': 'author', u'pkg_id': new_pkg['pkg_id'], + u'title': new_pkg['title'], u'new_author': + new['author'], u'method': 'add'}) def _author_email_change(change_list, original, new, new_pkg): @@ -395,20 +357,21 @@ def _author_email_change(change_list, original, new, new_pkg): Appends a summary of a change to a dataset's author e-mail address field between two versions (original and new) to change_list. ''' - s = u"" - seq2 = (u"", - new['author_email'], u"") - # if the original dataset had a author email if original['author_email'] and new['author_email']: - seq3 = (u"", - original['author_email'], u"") - change_list.append([u"Set author e-mail of", new_pkg, u"to", - s.join(seq2), u"(previously", s.join(seq3) + u")"]) + change_list.append({u'type': 'author_email', u'pkg_id': new_pkg['pkg_id'], + u'title': new_pkg['title'], u'new_author_email': + new['author_email'], u'old_author_email': + original['author_email'], u'method': 'change'}) + # if they removed the author elif not new['author_email']: - change_list.append([u"Removed author e-mail from", new_pkg]) + change_list.append({u'type': 'author_email', u'pkg_id': + new_pkg['pkg_id'], u'title': new_pkg['title'], + u'method': 'remove'}) + # if there wasn't one there before else: - change_list.append([u"Set author e-mail of", new_pkg, - u"to", s.join(seq2)]) + change_list.append({u'type': 'author_email', u'pkg_id': new_pkg['pkg_id'], + u'title': new_pkg['title'], u'new_author_email': + new['author_email'], u'method': 'add'}) def _description_change(change_list, original, new, new_pkg): @@ -416,29 +379,21 @@ def _description_change(change_list, original, new, new_pkg): Appends a summary of a change to a dataset's description between two versions (original and new) to change_list. ''' - - # TODO: find a better way to format the descriptions along with the - # change summary - # if the original dataset had a description if original['notes'] and new['notes']: - change_list.append([u"Updated description of", new_pkg, - u"from
            ", - u"
            " + - original['notes'] + - u"
            ", - u"to
            ", - u"
            " + - new['notes'] + - u"
            "]) + change_list.append({u'type': 'description', u'pkg_id': + new_pkg['pkg_id'], u'title': new_pkg['title'], + u'new_desc': new['notes'], + u'old_desc': original['notes'], + u'method': 'change'}) elif not new['notes']: - change_list.append([u"Removed description from", new_pkg]) + change_list.append({u'type': 'description', u'pkg_id': + new_pkg['pkg_id'], u'title': new_pkg['title'], + 'method': 'remove'}) else: - change_list.append([u"Updated description of", new_pkg, - u"to
            ", - u"
            " + - new['notes'] + - u"
            "]) + change_list.append({u'type': 'description', u'pkg_id': + new_pkg['pkg_id'], u'title': new_pkg['title'], + u'new_desc': new['notes'], 'method': 'add'}) def _tag_change(change_list, new_tags, original_tags, new_pkg): @@ -446,51 +401,27 @@ def _tag_change(change_list, new_tags, original_tags, new_pkg): Appends a summary of a change to a dataset's tag list between two versions (original and new) to change_list. ''' - s = u"" deleted_tags = original_tags - new_tags deleted_tags_list = list(deleted_tags) if len(deleted_tags) == 1: - seq2 = (u"", - deleted_tags_list[0], u"") - change_list.append([u"Removed tag", s.join(seq2), u"from", new_pkg]) + change_list.append({u'type': 'tags', u'method': 'remove1', u'pkg_id': + new_pkg['pkg_id'], u'title': new_pkg['title'], + 'tag': deleted_tags_list[0]}) elif len(deleted_tags) > 1: - seq2 = [ - u"
          • " + deleted_tags_list[i] + u"
            • " - for i in range(0, len(deleted_tags)) - ] - change_list.append([u"Removed the following tags from", new_pkg, - u"
              ", s.join(seq2), u"
            "]) + change_list.append({u'type': 'tags', u'method': 'remove2', u'pkg_id': + new_pkg['pkg_id'], u'title': new_pkg['title'], + 'tags': deleted_tags_list}) added_tags = new_tags - original_tags added_tags_list = list(added_tags) if len(added_tags) == 1: - seq2 = (u"", - added_tags_list[0], u"") - change_list.append([u"Added tag", s.join(seq2), u"to", new_pkg]) + change_list.append({u'type': 'tags', u'method': 'add1', u'pkg_id': + new_pkg['pkg_id'], u'title': new_pkg['title'], + 'tag': added_tags_list[0]}) elif len(added_tags) > 1: - seq2 = [u"
          • " + added_tags_list[i] + u"
            • " - for i in range(0, len(added_tags))] - change_list.append([u"Added the following tags to", new_pkg, - u"
              ", s.join(seq2), u"
            "]) + change_list.append({u'type': 'tags', u'method': 'add2', u'pkg_id': + new_pkg['pkg_id'], u'title': new_pkg['title'], + 'tags': added_tags_list}) def _license_change(change_list, original, new, new_pkg): @@ -498,47 +429,30 @@ def _license_change(change_list, original, new, new_pkg): Appends a summary of a change to a dataset's license between two versions (original and new) to change_list. ''' - s = u"" - seq2 = () - seq3 = () - # if the license has a URL, use it + original_license_url = "" + new_license_url = "" + # if the license has a URL if u'license_url' in original and original['license_url']: - seq2 = (u"", - original['license_title'], u"") - else: - seq2 = (original['license_title']) + original_license_url = original['license_url'] if u'license_url' in new and new['license_url']: - seq3 = (u"", - new['license_title'], u"") - else: - seq3 = (new['license_title']) - change_list.append([u"Changed the license of", new_pkg, u"to", - s.join(seq3), u"(previously", s.join(seq2) + u")"]) + new_license_url = new['license_url'] + change_list.append({u'type': 'license', u'pkg_id': new_pkg['pkg_id'], + u'title': new_pkg['title'], + u'old_url': original_license_url, + u'new_url': new_license_url, 'new_title': + new['license_title'], 'old_title': + original['license_title']}) -def _name_change(change_list, original, new): +def _name_change(change_list, original, new, new_pkg): ''' Appends a summary of a change to a dataset's name (and thus the URL it can be accessed at) between two versions (original and new) to change_list. ''' - s = u"" - old_url = url_for( - qualified=True, - controller=u"dataset", - action=u"read", - id=original['name'] - ) - new_url = url_for( - qualified=True, - controller=u"dataset", - action=u"read", - id=new['name'] - ) - seq2 = (u"", old_url, u"") - seq3 = (u"", new_url, u"") - change_list.append([u"Moved the dataset from", s.join(seq2), - u"to", s.join(seq3)]) + change_list.append({u'type': 'name', u'pkg_id': new_pkg['pkg_id'], + u'title': new_pkg['title'], u'old_name': + original['name'], u'new_name': new['name']}) def _source_url_change(change_list, original, new, new_pkg): @@ -547,17 +461,24 @@ def _source_url_change(change_list, original, new, new_pkg): not its actual URL in the datahub) between two versions (original and new) to change_list. ''' - s = u"" - seq2 = (u"", original['url'], u"") - seq3 = (u"", new['url'], u"") + # if both old and new versions have source URLs if original['url'] and new['url']: - change_list.append([u"Changed the source URL of", new_pkg, - u"from", s.join(seq2), u"to", s.join(seq3)]) + change_list.append({u'type': 'source_url', u'method': 'change', + u'pkg_id': new_pkg['pkg_id'], u'title': + new_pkg['title'], u'new_url': new['url'], + u'old_url': original['url']}) + # if the user removed the source URL elif not new['url']: - change_list.append([u"Removed source URL from", new_pkg]) + change_list.append({u'type': 'source_url', u'method': 'remove', + u'pkg_id': new_pkg['pkg_id'], + u'title': new_pkg['title'], + u'old_url': original['url']}) + # if there wasn't one there before else: - change_list.append([u"Changed the source URL of", - new_pkg, u"to", s.join(seq3)]) + change_list.append({u'type': 'source_url', u'method': 'add', + u'pkg_id': new_pkg['pkg_id'], + u'title': new_pkg['title'], + u'new_url': new['url']}) def _version_change(change_list, original, new, new_pkg): @@ -566,15 +487,24 @@ def _version_change(change_list, original, new, new_pkg): by the user, not from version control) between two versions (original and new) to change_list. ''' - if original['version'] and new['url']: - change_list.append([u"Changed the version of", new_pkg, - u"from", original['version'], - u"to", new['version']]) - elif not new['url']: - change_list.append([u"Removed version number from", new_pkg]) + # if both old and new versions have version numbers + if original['version'] and new['version']: + change_list.append({u'type': 'version', u'method': 'change', + u'pkg_id': new_pkg['pkg_id'], u'title': + new_pkg['title'], 'old_version': + original['version'], 'new_version': + new['version']}) + # if the user removed the version number + elif not new['version']: + change_list.append({u'type': 'version', u'method': 'remove', + u'pkg_id': new_pkg['pkg_id'], + u'title': new_pkg['title']}) + # if there wasn't one there before else: - change_list.append([u"Changed the version of", new_pkg, - u"to", new['version']]) + change_list.append({u'type': 'version', u'method': 'add', + u'pkg_id': new_pkg['pkg_id'], + u'title': new_pkg['title'], + u'new_version': new['version']}) def _extension_fields(change_list, original, new, new_pkg): @@ -623,15 +553,11 @@ def _extension_fields(change_list, original, new, new_pkg): addl_fields_list = list(addl_fields) for field in addl_fields_list: if original[field] != new[field]: - if original[field]: - change_list.append([u"Changed value of field", - field.capitalize(), u"to", - new[field], u"(previously", - original[field] + u")", u"in", new_pkg]) - else: - change_list.append([u"Changed value of field", - field.capitalize(), u"to", - new[field], u"in", new_pkg]) + change_list.append({u'type': 'extension_fields', + u'pkg_id': new_pkg['pkg_id'], + u'title': new_pkg['title'], + u'field_name': field, + u'new_field': new[field]}) def _extra_fields(change_list, original, new, new_pkg): @@ -640,13 +566,12 @@ def _extra_fields(change_list, original, new, new_pkg): from the web interface (or API?) and appends a summary of each change to change_list. ''' - s = u"" if u'extras' in new: extra_fields_new = _extras_to_dict(new['extras']) extra_new_set = set(extra_fields_new.keys()) - # if the original version has an extra fields, we need + # if the original version has extra fields, we need # to compare the new version'sextras to the original ones if u'extras' in original: extra_fields_original = _extras_to_dict(original['extras']) @@ -656,46 +581,41 @@ def _extra_fields(change_list, original, new, new_pkg): new_fields = list(extra_new_set - extra_original_set) if len(new_fields) == 1: if extra_fields_new[new_fields[0]]: - change_list.append( - [u"Added field", - s.join((u"", - new_fields[0], - u"")), - u"with value", - s.join((u"", - extra_fields_new[new_fields[0]], - u"")), - u"to", new_pkg] - ) + change_list.append({u'type': 'custom_fields', + u'method': 'add1', + u'pkg_id': new_pkg['pkg_id'], + u'title': new_pkg['title'], + u'field_name': new_fields[0], + u'field_val': + extra_fields_new[new_fields[0]]}) else: - change_list.append( - [u"Added field", - s.join((u"", - new_fields[0], - u"") - ), - u"to", new_pkg] - ) + change_list.append({u'type': 'custom_fields', + u'method': 'add2', + u'pkg_id': new_pkg['pkg_id'], + u'title': new_pkg['title'], + u'field_name': new_fields[0]}) elif len(new_fields) > 1: - seq2 = [u"
          • " + new_fields[i] + u" with value " + - extra_fields_new[new_fields[i]] + u"
            • " - if extra_fields_new[new_fields[i]] - else u"
          • " + new_fields[i] + u"
            • " - for i in range(0, len(new_fields))] - change_list.append([u"Added the following fields to", - new_pkg, u"
              ", s.join(seq2), u"
            "]) + change_list.append({u'type': 'custom_fields', + u'method': 'add3', + u'pkg_id': new_pkg['pkg_id'], + u'title': new_pkg['title'], + u'fields': new_fields, + u'field_vals': extra_fields_new}) # if some fields were deleted deleted_fields = list(extra_original_set - extra_new_set) if len(deleted_fields) == 1: - change_list.append([u"Removed field", s.join((u"", - deleted_fields[0], u"")), - u"from", new_pkg]) + change_list.append({u'type': 'custom_fields', + u'method': 'remove1', + u'pkg_id': new_pkg['pkg_id'], + u'title': new_pkg['title'], + u'field_name': deleted_fields[0]}) elif len(deleted_fields) > 1: - seq2 = [u"
          • " + deleted_fields[i] + u"
            • " - for i in range(0, len(deleted_fields))] - change_list.append([u"Removed the following fields from", - new_pkg, u"
              ", s.join(seq2), u"
            "]) + change_list.append({u'type': 'custom_fields', + u'method': 'remove2', + u'pkg_id': new_pkg['pkg_id'], + u'title': new_pkg['title'], + u'fields': deleted_fields}) # if some existing fields were changed # list of extra fields in both the original and new versions @@ -703,30 +623,23 @@ def _extra_fields(change_list, original, new, new_pkg): for field in extra_fields: if extra_fields_original[field] != extra_fields_new[field]: if extra_fields_original[field]: - change_list.append( - [u"Changed value of field", - s.join((u"", - field, u"")), - u"to", - s.join((u"", - extra_fields_new[field], - u"")), - u"(previously", - s.join((u"", - extra_fields_original[field], - u"")) + - u")", u"in", new_pkg] - ) + change_list.append({u'type': 'custom_fields', + u'method': 'change1', + u'pkg_id': new_pkg['pkg_id'], + u'title': new_pkg['title'], + u'field_name': field, + u'field_val_old': + extra_fields_original[field], + u'field_val_new': + extra_fields_new[field]}) else: - change_list.append( - [u"Changed value of field", - s.join((u"", field, u"")), - u"to", - s.join((u"", - extra_fields_new[field], - u"")), - u"in", new_pkg] - ) + change_list.append({u'type': 'custom_fields', + u'method': 'change2', + u'pkg_id': new_pkg['pkg_id'], + u'title': new_pkg['title'], + u'field_name': field, + u'field_val_new': + extra_fields_new[field]}) # if the original version didn't have an extras field, # the user could only have added a field (not changed or deleted) @@ -734,38 +647,39 @@ def _extra_fields(change_list, original, new, new_pkg): new_fields = list(extra_new_set) if len(new_fields) == 1: if extra_fields_new[new_fields[0]]: - change_list.append( - [u"Added field", - s.join((u"", new_fields[0], u"")), - u"with value", - s.join((u"", - extra_fields_new[new_fields[0]], - u"")), - u"to", new_pkg] - ) + change_list.append({u'type': 'custom_fields', + u'method': 'add1', + u'pkg_id': new_pkg['pkg_id'], + u'title': new_pkg['title'], + u'field_name': new_fields[0], + u'field_val': + extra_fields_new[new_fields[0]]}) else: - change_list.append([u"Added field", - s.join((u"", - new_fields[0], - u"")), - u"to", new_pkg]) + change_list.append({u'type': 'custom_fields', + u'method': 'add2', + u'pkg_id': new_pkg['pkg_id'], + u'title': new_pkg['title'], + u'field_name': new_fields[0]}) + elif len(new_fields) > 1: - seq2 = [u"
          • " + new_fields[i] + u" with value " + - extra_fields_new[new_fields[i]] + u"
            • " - if extra_fields_new[new_fields[i]] - else u"
          • " + new_fields[i] + u"
            • " - for i in range(0, len(new_fields))] - change_list.append([u"Added the following fields to", - new_pkg, u"
              ", s.join(seq2), u"
            "]) + change_list.append({u'type': 'custom_fields', + u'method': 'add3', + u'pkg_id': new_pkg['pkg_id'], + u'title': new_pkg['title'], + u'fields': new_fields, + u'field_vals': extra_fields_new}) elif u'extras' in original: deleted_fields = _extras_to_dict(original['extras']).keys() if len(deleted_fields) == 1: - change_list.append([u"Removed field", s.join((u"", - deleted_fields[0], u"")), u"from", - new_pkg]) + change_list.append({u'type': 'custom_fields', + u'method': 'remove1', + u'pkg_id': new_pkg['pkg_id'], + u'title': new_pkg['title'], + u'field_name': deleted_fields[0]}) elif len(deleted_fields) > 1: - seq2 = [u"
          • " + deleted_fields[i] + - u"
            • " for i in range(0, len(deleted_fields))] - change_list.append([u"Removed the following fields from", - new_pkg, u"
              ", s.join(seq2), u"
            "]) + change_list.append({u'type': 'custom_fields', + u'method': 'remove2', + u'pkg_id': new_pkg['pkg_id'], + u'title': new_pkg['title'], + u'fields': deleted_fields}) diff --git a/ckan/lib/helpers.py b/ckan/lib/helpers.py index 87fac47f7b6..bd5914d08cf 100644 --- a/ckan/lib/helpers.py +++ b/ckan/lib/helpers.py @@ -2704,11 +2704,7 @@ def compare_pkg_dicts(original, new, old_activity_id): from changes import _check_metadata_changes, _check_resource_changes change_list = [] - s = "" - seq1 = ("", - new['title'], "") - new_pkg = s.join(seq1) + new_pkg = {u'pkg_id': new['id'], u'name': new['name'], u'title': new['title']} _check_metadata_changes(change_list, original, new, new_pkg) diff --git a/ckan/templates/package/snippets/change_item.html b/ckan/templates/package/snippets/change_item.html index 6512e9b34c6..341f0d4c422 100644 --- a/ckan/templates/package/snippets/change_item.html +++ b/ckan/templates/package/snippets/change_item.html @@ -3,17 +3,20 @@ {% set changes = h.compare_pkg_dicts(activity_diff.activities[0].data.package, activity_diff.activities[1].data.package, activity_diff.activities[0].id) %}
              {% for change in changes %} -
            • - {% for element in change %} + {% snippet "snippets/changes/{}.html".format( + change.type), ah=change %} +
              {% endfor %}
            {# button to show JSON metadata diff - not shown by default #} +
          diff --git a/ckan/templates/snippets/activities/changed_package.html b/ckan/templates/snippets/activities/changed_package.html index a1dd2155a41..4bef2b6d5fa 100644 --- a/ckan/templates/snippets/activities/changed_package.html +++ b/ckan/templates/snippets/activities/changed_package.html @@ -17,8 +17,6 @@ {{ _('Changes') }} -
          - Activity ID: {{ activity.id }} {% endif %}

          diff --git a/ckan/templates/snippets/changes/author.html b/ckan/templates/snippets/changes/author.html index d9aeb999e02..b46aa4463ac 100644 --- a/ckan/templates/snippets/changes/author.html +++ b/ckan/templates/snippets/changes/author.html @@ -20,6 +20,8 @@ pkg_url = h.url_for(controller='dataset', action='read', id=ah.pkg_id), dataset = ah.title )|safe }} + {% else %} + {{ _('No fields were updated. See the metadata diff for more details.') }} {% endif %}

          diff --git a/ckan/templates/snippets/changes/author_email.html b/ckan/templates/snippets/changes/author_email.html index a76b98fce61..4a86f26ae3d 100644 --- a/ckan/templates/snippets/changes/author_email.html +++ b/ckan/templates/snippets/changes/author_email.html @@ -25,6 +25,8 @@ pkg_url = h.url_for(controller='dataset', action='read', id=ah.pkg_id), dataset = ah.title )|safe }} + {% else %} + {{ _('No fields were updated. See the metadata diff for more details.') }} {% endif %}

          diff --git a/ckan/templates/snippets/changes/custom_fields.html b/ckan/templates/snippets/changes/custom_fields.html index bf84a35cea7..c1a56994241 100644 --- a/ckan/templates/snippets/changes/custom_fields.html +++ b/ckan/templates/snippets/changes/custom_fields.html @@ -77,6 +77,8 @@ {% endfor %}
        + {% else %} + {{ _('No fields were updated. See the metadata diff for more details.') }} {% endif %}

        diff --git a/ckan/templates/snippets/changes/description.html b/ckan/templates/snippets/changes/description.html index 99cdd9b2e38..0c531079302 100644 --- a/ckan/templates/snippets/changes/description.html +++ b/ckan/templates/snippets/changes/description.html @@ -24,6 +24,8 @@ pkg_url = h.url_for(controller='dataset', action='read', id=ah.pkg_id), dataset = ah.title )|safe }} + {% else %} + {{ _('No fields were updated. See the metadata diff for more details.') }} {% endif %}

        diff --git a/ckan/templates/snippets/changes/maintainer.html b/ckan/templates/snippets/changes/maintainer.html index 5edc399fc98..22285c5eb84 100644 --- a/ckan/templates/snippets/changes/maintainer.html +++ b/ckan/templates/snippets/changes/maintainer.html @@ -20,6 +20,8 @@ pkg_url = h.url_for(controller='dataset', action='read', id=ah.pkg_id), dataset = ah.title )|safe }} + {% else %} + {{ _('No fields were updated. See the metadata diff for more details.') }} {% endif %}

        diff --git a/ckan/templates/snippets/changes/maintainer_email.html b/ckan/templates/snippets/changes/maintainer_email.html index 07d13759b31..44cf5581ae1 100644 --- a/ckan/templates/snippets/changes/maintainer_email.html +++ b/ckan/templates/snippets/changes/maintainer_email.html @@ -23,6 +23,8 @@ pkg_url = h.url_for(controller='dataset', action='read', id=ah.pkg_id), dataset = ah.title )|safe }} + {% else %} + {{ _('No fields were updated. See the metadata diff for more details.') }} {% endif %}

        diff --git a/ckan/templates/snippets/changes/no_change.html b/ckan/templates/snippets/changes/no_change.html new file mode 100644 index 00000000000..0d1ae06f3b7 --- /dev/null +++ b/ckan/templates/snippets/changes/no_change.html @@ -0,0 +1,5 @@ +
      • +

        + {{ _('No fields were updated. See the metadata diff for more details.') }} +

        +
        • diff --git a/ckan/templates/snippets/changes/resource_desc.html b/ckan/templates/snippets/changes/resource_desc.html index 3d0cb26bec9..7ea932c6ace 100644 --- a/ckan/templates/snippets/changes/resource_desc.html +++ b/ckan/templates/snippets/changes/resource_desc.html @@ -35,6 +35,8 @@ new_desc = ah.new_desc, old_desc = ah.old_desc )|safe }} + {% else %} + {{ _('No fields were updated. See the metadata diff for more details.') }} {% endif %}

        diff --git a/ckan/templates/snippets/changes/resource_format.html b/ckan/templates/snippets/changes/resource_format.html index d37f0c7edd7..2224dada9b2 100644 --- a/ckan/templates/snippets/changes/resource_format.html +++ b/ckan/templates/snippets/changes/resource_format.html @@ -30,6 +30,8 @@ id=ah.org_id) + "?res_format=" + ah.old_format, old_format = ah.old_format )|safe }} + {% else %} + {{ _('No fields were updated. See the metadata diff for more details.') }} {% endif %}

        diff --git a/ckan/templates/snippets/changes/source_url.html b/ckan/templates/snippets/changes/source_url.html index a21e0b6238c..0f38ded253c 100644 --- a/ckan/templates/snippets/changes/source_url.html +++ b/ckan/templates/snippets/changes/source_url.html @@ -22,6 +22,8 @@ dataset = ah.title, new_url = ah.new_url )|safe }} + {% else %} + {{ _('No fields were updated. See the metadata diff for more details.') }} {% endif %}

        diff --git a/ckan/templates/snippets/changes/tags.html b/ckan/templates/snippets/changes/tags.html index 08ee85fb69c..9b3545c33ae 100644 --- a/ckan/templates/snippets/changes/tags.html +++ b/ckan/templates/snippets/changes/tags.html @@ -53,6 +53,8 @@ {% endfor %}
      + {% else %} + {{ _('No fields were updated. See the metadata diff for more details.') }} {% endif %}

      diff --git a/ckan/templates/snippets/changes/version.html b/ckan/templates/snippets/changes/version.html index 042ba6052fb..87a46924ad2 100644 --- a/ckan/templates/snippets/changes/version.html +++ b/ckan/templates/snippets/changes/version.html @@ -21,6 +21,8 @@ dataset = ah.title, new_version = ah.new_version )|safe }} + {% else %} + {{ _('No fields were updated. See the metadata diff for more details.') }} {% endif %}

      From 0b1ba761736a0736e2f35745d4370d4e49f899d0 Mon Sep 17 00:00:00 2001 From: hayley-leblanc Date: Thu, 8 Aug 2019 12:03:13 +0200 Subject: [PATCH 21/47] Formatting and fix error in changes.py --- .../lib/activity_streams_session_extension.py | 4 +- ckan/lib/changes.py | 222 +++++++++--------- ckan/lib/helpers.py | 11 +- 3 files changed, 125 insertions(+), 112 deletions(-) diff --git a/ckan/lib/activity_streams_session_extension.py b/ckan/lib/activity_streams_session_extension.py index 558f5e0ec33..3dec8a95173 100644 --- a/ckan/lib/activity_streams_session_extension.py +++ b/ckan/lib/activity_streams_session_extension.py @@ -71,7 +71,7 @@ def before_commit(self, session): # Don't create activities for private datasets. if obj.private: - continue + continue activities[obj.id] = activity @@ -107,7 +107,7 @@ def before_commit(self, session): # Don't create activities for private datasets. if package.private: - continue + continue if package.id in activities: continue diff --git a/ckan/lib/changes.py b/ckan/lib/changes.py index 87877686dd9..c3480ff4f3c 100644 --- a/ckan/lib/changes.py +++ b/ckan/lib/changes.py @@ -76,7 +76,7 @@ def _check_resource_changes(change_list, original, new, new_pkg, # get the IDs of the resources that have been added between the versions new_resources = list(new_resource_set - original_resource_set) for resource_id in new_resources: - change_list.append({u'type': 'new_resource', + change_list.append({u'type': u'new_resource', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], u'resource_name': @@ -86,7 +86,7 @@ def _check_resource_changes(change_list, original, new, new_pkg, # get the IDs of resources that have been deleted between versions deleted_resources = list(original_resource_set - new_resource_set) for resource_id in deleted_resources: - change_list.append({u'type': 'delete_resource', + change_list.append({u'type': u'delete_resource', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], u'resource_id': resource_id, @@ -102,7 +102,7 @@ def _check_resource_changes(change_list, original, new, new_pkg, new_metadata = new_resource_dict[resource_id] if original_metadata['name'] != new_metadata['name']: - change_list.append({u'type': 'resource_name', + change_list.append({u'type': u'resource_name', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], u'old_pkg_id': original['id'], @@ -120,33 +120,33 @@ def _check_resource_changes(change_list, original, new, new_pkg, # if a format was not originally set and the user set one if not original_metadata['format'] and new_metadata['format']: change_list.append({u'type': 'resource_format', - u'method': 'add', + u'method': u'add', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], u'resource_id': resource_id, u'resource_name': new_resource_dict[resource_id]['name'], - u'org_id':new['organization']['id'], + u'org_id': new['organization']['id'], u'format': new_metadata['format']}) # if both versions have a format but the format changed elif original_metadata['format'] != new_metadata['format']: - change_list.append({u'type': 'resource_format', - u'method': 'change', + change_list.append({u'type': u'resource_format', + u'method': u'change', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], u'resource_id': resource_id, u'resource_name': new_resource_dict[resource_id]['name'], - u'org_id':new['organization']['id'], + u'org_id': new['organization']['id'], u'old_format': original_metadata['format'], u'new_format': new_metadata['format']}) # if the description changed if not original_metadata['description'] and \ new_metadata['description']: - change_list.append({u'type': 'resource_desc', - u'method': 'add', + change_list.append({u'type': u'resource_desc', + u'method': u'add', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], u'resource_id': resource_id, @@ -157,8 +157,8 @@ def _check_resource_changes(change_list, original, new, new_pkg, # if there was a description but the user removed it elif original_metadata['description'] and \ not new_metadata['description']: - change_list.append({u'type': 'resource_desc', - u'method': 'remove', + change_list.append({u'type': u'resource_desc', + u'method': u'remove', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], u'resource_id': resource_id, @@ -167,21 +167,21 @@ def _check_resource_changes(change_list, original, new, new_pkg, # if both have descriptions but they are different elif original_metadata['description'] != new_metadata['description']: - change_list.append({u'type': 'resource_desc', - u'method': 'change', + change_list.append({u'type': u'resource_desc', + u'method': u'change', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], u'resource_id': resource_id, u'resource_name': new_resource_dict[resource_id]['name'], u'new_desc': new_metadata['description'], - u'old_desc': old_metadata['description']}) + u'old_desc': original_metadata['description']}) # check if the user uploaded a new file # TODO: use regular expressions to determine the actual name of the # new and old files if original_metadata['url'] != new_metadata['url']: - change_list.append({u'type': 'new_file', + change_list.append({u'type': u'new_file', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], u'resource_id': resource_id, @@ -222,9 +222,11 @@ def _check_metadata_changes(change_list, original, new, new_pkg): # if the visibility of the dataset changed if original['private'] != new['private']: - change_list.append({u'type': 'private', u'pkg_id': new_pkg['pkg_id'], + change_list.append({u'type': u'private', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], - u'new': 'Private' if bool(new['private']) else 'Public'}) + u'new': + u'Private' if bool(new['private']) \ + else u'Public'}) # if the description of the dataset changed if original['notes'] != new['notes']: @@ -268,8 +270,9 @@ def _title_change(change_list, original, new): Appends a summary of a change to a dataset's title between two versions (original and new) to change_list. ''' - change_list.append({'type': 'title', 'id': new['name'], 'new_title': new['title'], - 'original_title': original['title']}) + change_list.append({u'type': u'title', u'id': new['name'], + u'new_title': new['title'], + u'original_title': original['title']}) def _org_change(change_list, original, new, new_pkg): @@ -280,10 +283,12 @@ def _org_change(change_list, original, new, new_pkg): change_list.append({u'type': u'org', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], u'original_org_id': original['organization']['id'], - u'original_org_title': original['organization']['title'], + u'original_org_title': + original['organization']['title'], u'new_org_id': new['organization']['id'], u'new_org_title': new['organization']['title']}) + def _maintainer_change(change_list, original, new, new_pkg): ''' Appends a summary of a change to a dataset's maintainer field between two @@ -291,21 +296,22 @@ def _maintainer_change(change_list, original, new, new_pkg): ''' # if the original dataset had a maintainer if original['maintainer'] and new['maintainer']: - change_list.append({u'type': 'maintainer', u'method': 'change', + change_list.append({u'type': u'maintainer', u'method': u'change', u'pkg_id': new_pkg['pkg_id'], - u'title': new_pkg['title'], 'new_maintainer': - new['maintainer'], 'old_maintainer': + u'title': new_pkg['title'], u'new_maintainer': + new['maintainer'], u'old_maintainer': original['maintainer']}) # if they removed the maintainer elif not new['maintainer']: - change_list.append({u'type': 'maintainer', u'pkg_id': new_pkg['pkg_id'], - u'title': new_pkg['title'], u'method': 'remove'}) + change_list.append({u'type': u'maintainer', u'pkg_id': + new_pkg['pkg_id'], u'title': new_pkg['title'], + u'method':u 'remove'}) # if there wasn't one there before else: - change_list.append({u'type': 'maintainer', u'pkg_id': new_pkg['pkg_id'], - u'title': new_pkg['title'], u'new_maintainer': - new['maintainer'], u'method': 'add'}) - + change_list.append({u'type': 'maintainer', u'pkg_id': + new_pkg['pkg_id'], u'title': new_pkg['title'], + u'new_maintainer': new['maintainer'], + u'method': 'add'}) def _maintainer_email_change(change_list, original, new, new_pkg): @@ -315,19 +321,23 @@ def _maintainer_email_change(change_list, original, new, new_pkg): ''' # if the original dataset had a maintainer email if original['maintainer_email'] and new['maintainer_email']: - change_list.append({u'type': 'maintainer_email', u'pkg_id': new_pkg['pkg_id'], - u'title': new_pkg['title'], 'new_maintainer_email': - new['maintainer_email'], 'old_maintainer_email': - original['maintainer_email'], u'method': 'change'}) + change_list.append({u'type': u'maintainer_email', u'pkg_id': + new_pkg['pkg_id'], u'title': new_pkg['title'], + u'new_maintainer_email': new['maintainer_email'], + u'old_maintainer_email': + original['maintainer_email'], + u'method': u'change'}) # if they removed the maintainer email elif not new['maintainer_email']: - change_list.append({u'type': 'maintainer_email', u'pkg_id': new_pkg['pkg_id'], - u'title': new_pkg['title'], u'method': 'remove'}) + change_list.append({u'type': u'maintainer_email', u'pkg_id': + new_pkg['pkg_id'], u'title': new_pkg['title'], + u'method': u'remove'}) # if there wasn't one there before e else: - change_list.append({u'type': 'maintainer_email', u'pkg_id': new_pkg['pkg_id'], - u'title': new_pkg['title'], 'new_maintainer_email': - new['maintainer_email'], u'method': 'add'}) + change_list.append({u'type': u'maintainer_email', u'pkg_id': + new_pkg['pkg_id'], u'title': new_pkg['title'], + 'new_maintainer_email': new['maintainer_email'], + u'method': u'add'}) def _author_change(change_list, original, new, new_pkg): @@ -337,19 +347,19 @@ def _author_change(change_list, original, new, new_pkg): ''' # if the original dataset had an author if original['author'] and new['author']: - change_list.append({u'type': 'author', u'pkg_id': new_pkg['pkg_id'], + change_list.append({u'type': u'author', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], u'new_author': new['author'], u'old_author': original['author'], - u'method': 'change'}) + u'method': u'change'}) # if they removed the author elif not new['author']: - change_list.append({u'type': 'author', u'pkg_id': new_pkg['pkg_id'], - u'title': new_pkg['title'], u'method': 'remove'}) + change_list.append({u'type': u'author', u'pkg_id': new_pkg['pkg_id'], + u'title': new_pkg['title'], u'method': u'remove'}) # if there wasn't one there before else: - change_list.append({u'type': 'author', u'pkg_id': new_pkg['pkg_id'], + change_list.append({u'type': u'author', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], u'new_author': - new['author'], u'method': 'add'}) + new['author'], u'method': u'add'}) def _author_email_change(change_list, original, new, new_pkg): @@ -358,20 +368,22 @@ def _author_email_change(change_list, original, new, new_pkg): between two versions (original and new) to change_list. ''' if original['author_email'] and new['author_email']: - change_list.append({u'type': 'author_email', u'pkg_id': new_pkg['pkg_id'], - u'title': new_pkg['title'], u'new_author_email': - new['author_email'], u'old_author_email': - original['author_email'], u'method': 'change'}) + change_list.append({u'type': u'author_email', u'pkg_id': + new_pkg['pkg_id'], u'title': new_pkg['title'], + u'new_author_email': new['author_email'], + u'old_author_email': original['author_email'], + u'method': u'change'}) # if they removed the author elif not new['author_email']: - change_list.append({u'type': 'author_email', u'pkg_id': + change_list.append({u'type': u'author_email', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], - u'method': 'remove'}) + u'method': u'remove'}) # if there wasn't one there before else: - change_list.append({u'type': 'author_email', u'pkg_id': new_pkg['pkg_id'], - u'title': new_pkg['title'], u'new_author_email': - new['author_email'], u'method': 'add'}) + change_list.append({u'type': u'author_email', u'pkg_id': + new_pkg['pkg_id'], u'title': new_pkg['title'], + u'new_author_email': new['author_email'], + u'method': u'add'}) def _description_change(change_list, original, new, new_pkg): @@ -381,19 +393,19 @@ def _description_change(change_list, original, new, new_pkg): ''' # if the original dataset had a description if original['notes'] and new['notes']: - change_list.append({u'type': 'description', u'pkg_id': + change_list.append({u'type': u'description', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], u'new_desc': new['notes'], u'old_desc': original['notes'], - u'method': 'change'}) + u'method': u'change'}) elif not new['notes']: - change_list.append({u'type': 'description', u'pkg_id': + change_list.append({u'type': u'description', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], - 'method': 'remove'}) + u'method': u'remove'}) else: - change_list.append({u'type': 'description', u'pkg_id': + change_list.append({u'type': u'description', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], - u'new_desc': new['notes'], 'method': 'add'}) + u'new_desc': new['notes'], u'method': u'add'}) def _tag_change(change_list, new_tags, original_tags, new_pkg): @@ -404,24 +416,24 @@ def _tag_change(change_list, new_tags, original_tags, new_pkg): deleted_tags = original_tags - new_tags deleted_tags_list = list(deleted_tags) if len(deleted_tags) == 1: - change_list.append({u'type': 'tags', u'method': 'remove1', u'pkg_id': + change_list.append({u'type': u'tags', u'method': u'remove1', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], - 'tag': deleted_tags_list[0]}) + u'tag': deleted_tags_list[0]}) elif len(deleted_tags) > 1: - change_list.append({u'type': 'tags', u'method': 'remove2', u'pkg_id': + change_list.append({u'type': u'tags', u'method': u'remove2', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], - 'tags': deleted_tags_list}) + u'tags': deleted_tags_list}) added_tags = new_tags - original_tags added_tags_list = list(added_tags) if len(added_tags) == 1: - change_list.append({u'type': 'tags', u'method': 'add1', u'pkg_id': + change_list.append({u'type': u'tags', u'method': u'add1', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], - 'tag': added_tags_list[0]}) + u'tag': added_tags_list[0]}) elif len(added_tags) > 1: - change_list.append({u'type': 'tags', u'method': 'add2', u'pkg_id': + change_list.append({u'type': u'tags', u'method': u'add2', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], - 'tags': added_tags_list}) + u'tags': added_tags_list}) def _license_change(change_list, original, new, new_pkg): @@ -436,11 +448,11 @@ def _license_change(change_list, original, new, new_pkg): original_license_url = original['license_url'] if u'license_url' in new and new['license_url']: new_license_url = new['license_url'] - change_list.append({u'type': 'license', u'pkg_id': new_pkg['pkg_id'], + change_list.append({u'type': u'license', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], u'old_url': original_license_url, - u'new_url': new_license_url, 'new_title': - new['license_title'], 'old_title': + u'new_url': new_license_url, u'new_title': + new['license_title'], u'old_title': original['license_title']}) @@ -450,7 +462,7 @@ def _name_change(change_list, original, new, new_pkg): can be accessed at) between two versions (original and new) to change_list. ''' - change_list.append({u'type': 'name', u'pkg_id': new_pkg['pkg_id'], + change_list.append({u'type': u'name', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], u'old_name': original['name'], u'new_name': new['name']}) @@ -463,19 +475,19 @@ def _source_url_change(change_list, original, new, new_pkg): ''' # if both old and new versions have source URLs if original['url'] and new['url']: - change_list.append({u'type': 'source_url', u'method': 'change', + change_list.append({u'type': u'source_url', u'method': u'change', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], u'new_url': new['url'], u'old_url': original['url']}) # if the user removed the source URL elif not new['url']: - change_list.append({u'type': 'source_url', u'method': 'remove', + change_list.append({u'type': u'source_url', u'method': u'remove', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], u'old_url': original['url']}) # if there wasn't one there before else: - change_list.append({u'type': 'source_url', u'method': 'add', + change_list.append({u'type': u'source_url', u'method': u'add', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], u'new_url': new['url']}) @@ -489,19 +501,19 @@ def _version_change(change_list, original, new, new_pkg): ''' # if both old and new versions have version numbers if original['version'] and new['version']: - change_list.append({u'type': 'version', u'method': 'change', + change_list.append({u'type': 'version', u'method': u'change', u'pkg_id': new_pkg['pkg_id'], u'title': - new_pkg['title'], 'old_version': - original['version'], 'new_version': + new_pkg['title'], u'old_version': + original['version'], u'new_version': new['version']}) # if the user removed the version number elif not new['version']: - change_list.append({u'type': 'version', u'method': 'remove', + change_list.append({u'type': u'version', u'method': u'remove', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title']}) # if there wasn't one there before else: - change_list.append({u'type': 'version', u'method': 'add', + change_list.append({u'type': u'version', u'method': u'add', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], u'new_version': new['version']}) @@ -553,7 +565,7 @@ def _extension_fields(change_list, original, new, new_pkg): addl_fields_list = list(addl_fields) for field in addl_fields_list: if original[field] != new[field]: - change_list.append({u'type': 'extension_fields', + change_list.append({u'type': u'extension_fields', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], u'field_name': field, @@ -581,22 +593,22 @@ def _extra_fields(change_list, original, new, new_pkg): new_fields = list(extra_new_set - extra_original_set) if len(new_fields) == 1: if extra_fields_new[new_fields[0]]: - change_list.append({u'type': 'custom_fields', - u'method': 'add1', + change_list.append({u'type': u'custom_fields', + u'method': u'add1', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], u'field_name': new_fields[0], u'field_val': extra_fields_new[new_fields[0]]}) else: - change_list.append({u'type': 'custom_fields', - u'method': 'add2', + change_list.append({u'type': u'custom_fields', + u'method': u'add2', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], u'field_name': new_fields[0]}) elif len(new_fields) > 1: - change_list.append({u'type': 'custom_fields', - u'method': 'add3', + change_list.append({u'type': u'custom_fields', + u'method': u'add3', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], u'fields': new_fields, @@ -605,14 +617,14 @@ def _extra_fields(change_list, original, new, new_pkg): # if some fields were deleted deleted_fields = list(extra_original_set - extra_new_set) if len(deleted_fields) == 1: - change_list.append({u'type': 'custom_fields', - u'method': 'remove1', + change_list.append({u'type': u'custom_fields', + u'method': u'remove1', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], u'field_name': deleted_fields[0]}) elif len(deleted_fields) > 1: - change_list.append({u'type': 'custom_fields', - u'method': 'remove2', + change_list.append({u'type': u'custom_fields', + u'method': u'remove2', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], u'fields': deleted_fields}) @@ -623,8 +635,8 @@ def _extra_fields(change_list, original, new, new_pkg): for field in extra_fields: if extra_fields_original[field] != extra_fields_new[field]: if extra_fields_original[field]: - change_list.append({u'type': 'custom_fields', - u'method': 'change1', + change_list.append({u'type': u'custom_fields', + u'method': u'change1', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], u'field_name': field, @@ -633,8 +645,8 @@ def _extra_fields(change_list, original, new, new_pkg): u'field_val_new': extra_fields_new[field]}) else: - change_list.append({u'type': 'custom_fields', - u'method': 'change2', + change_list.append({u'type': u'custom_fields', + u'method': u'change2', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], u'field_name': field, @@ -647,23 +659,23 @@ def _extra_fields(change_list, original, new, new_pkg): new_fields = list(extra_new_set) if len(new_fields) == 1: if extra_fields_new[new_fields[0]]: - change_list.append({u'type': 'custom_fields', - u'method': 'add1', + change_list.append({u'type': u'custom_fields', + u'method': u'add1', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], u'field_name': new_fields[0], u'field_val': extra_fields_new[new_fields[0]]}) else: - change_list.append({u'type': 'custom_fields', - u'method': 'add2', + change_list.append({u'type': u'custom_fields', + u'method': u'add2', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], u'field_name': new_fields[0]}) elif len(new_fields) > 1: - change_list.append({u'type': 'custom_fields', - u'method': 'add3', + change_list.append({u'type': u'custom_fields', + u'method': u'add3', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], u'fields': new_fields, @@ -672,14 +684,14 @@ def _extra_fields(change_list, original, new, new_pkg): elif u'extras' in original: deleted_fields = _extras_to_dict(original['extras']).keys() if len(deleted_fields) == 1: - change_list.append({u'type': 'custom_fields', - u'method': 'remove1', + change_list.append({u'type': u'custom_fields', + u'method': u'remove1', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], u'field_name': deleted_fields[0]}) elif len(deleted_fields) > 1: - change_list.append({u'type': 'custom_fields', - u'method': 'remove2', + change_list.append({u'type': u'custom_fields', + u'method': u'remove2', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], u'fields': deleted_fields}) diff --git a/ckan/lib/helpers.py b/ckan/lib/helpers.py index cdbde7a72f1..26068333546 100644 --- a/ckan/lib/helpers.py +++ b/ckan/lib/helpers.py @@ -2701,15 +2701,16 @@ def compare_pkg_dicts(original, new, old_activity_id): keys added by extensions and custom fields added by the user in the web interface. - Returns a list of dictionaries, each of which corresponds to a change to the - dataset made in this revision. The dictionaries each contain a string - indicating the type of change made as well as other data necessary to - form a detailed summary of the change. + Returns a list of dictionaries, each of which corresponds to a change + to the dataset made in this revision. The dictionaries each contain a + string indicating the type of change made as well as other data necessary + to form a detailed summary of the change. ''' from changes import _check_metadata_changes, _check_resource_changes change_list = [] - new_pkg = {u'pkg_id': new['id'], u'name': new['name'], u'title': new['title']} + new_pkg = {u'pkg_id': new['id'], u'name': new['name'], + u'title': new['title']} _check_metadata_changes(change_list, original, new, new_pkg) From 4a990c670e611ace552289e1aae28dd037325fe9 Mon Sep 17 00:00:00 2001 From: hayley-leblanc Date: Thu, 8 Aug 2019 12:28:16 +0200 Subject: [PATCH 22/47] Fix syntax error --- ckan/lib/changes.py | 10 +++++----- ckan/lib/helpers.py | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/ckan/lib/changes.py b/ckan/lib/changes.py index c3480ff4f3c..8bcab3944b9 100644 --- a/ckan/lib/changes.py +++ b/ckan/lib/changes.py @@ -225,7 +225,7 @@ def _check_metadata_changes(change_list, original, new, new_pkg): change_list.append({u'type': u'private', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], u'new': - u'Private' if bool(new['private']) \ + u'Private' if bool(new['private']) else u'Public'}) # if the description of the dataset changed @@ -305,13 +305,13 @@ def _maintainer_change(change_list, original, new, new_pkg): elif not new['maintainer']: change_list.append({u'type': u'maintainer', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], - u'method':u 'remove'}) + u'method': u'remove'}) # if there wasn't one there before else: change_list.append({u'type': 'maintainer', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], u'new_maintainer': new['maintainer'], - u'method': 'add'}) + u'method': u'add'}) def _maintainer_email_change(change_list, original, new, new_pkg): @@ -382,8 +382,8 @@ def _author_email_change(change_list, original, new, new_pkg): else: change_list.append({u'type': u'author_email', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], - u'new_author_email': new['author_email'], - u'method': u'add'}) + u'new_author_email': new['author_email'], + u'method': u'add'}) def _description_change(change_list, original, new, new_pkg): diff --git a/ckan/lib/helpers.py b/ckan/lib/helpers.py index 26068333546..5c98ec9c7ac 100644 --- a/ckan/lib/helpers.py +++ b/ckan/lib/helpers.py @@ -2710,7 +2710,7 @@ def compare_pkg_dicts(original, new, old_activity_id): change_list = [] new_pkg = {u'pkg_id': new['id'], u'name': new['name'], - u'title': new['title']} + u'title': new['title']} _check_metadata_changes(change_list, original, new, new_pkg) From 0fb137344a24bcd604087b0133073a98a5166641 Mon Sep 17 00:00:00 2001 From: hayley-leblanc Date: Thu, 8 Aug 2019 14:02:30 +0200 Subject: [PATCH 23/47] Move scripts and CSS out of HTML, only show metadata diffs for most recent displayed change --- ckan/lib/changes.py | 12 +++--- ckan/lib/helpers.py | 7 +++- ckan/public/base/css/main.css | 15 ++++++++ .../javascript/modules/metadata-button.js | 38 +++++++++++++++++++ ckan/public/base/javascript/webassets.yml | 1 + ckan/templates/package/changes.html | 25 ++++++++---- .../package/snippets/change_item.html | 30 --------------- .../snippets/changes/description.html | 6 +-- .../snippets/changes/resource_desc.html | 6 +-- 9 files changed, 89 insertions(+), 51 deletions(-) create mode 100644 ckan/public/base/javascript/modules/metadata-button.js diff --git a/ckan/lib/changes.py b/ckan/lib/changes.py index 8bcab3944b9..dea71ddae12 100644 --- a/ckan/lib/changes.py +++ b/ckan/lib/changes.py @@ -119,7 +119,7 @@ def _check_resource_changes(change_list, original, new, new_pkg, # if a format was not originally set and the user set one if not original_metadata['format'] and new_metadata['format']: - change_list.append({u'type': 'resource_format', + change_list.append({u'type': u'resource_format', u'method': u'add', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], @@ -308,7 +308,7 @@ def _maintainer_change(change_list, original, new, new_pkg): u'method': u'remove'}) # if there wasn't one there before else: - change_list.append({u'type': 'maintainer', u'pkg_id': + change_list.append({u'type': u'maintainer', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], u'new_maintainer': new['maintainer'], u'method': u'add'}) @@ -336,7 +336,7 @@ def _maintainer_email_change(change_list, original, new, new_pkg): else: change_list.append({u'type': u'maintainer_email', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], - 'new_maintainer_email': new['maintainer_email'], + u'new_maintainer_email': new['maintainer_email'], u'method': u'add'}) @@ -441,8 +441,8 @@ def _license_change(change_list, original, new, new_pkg): Appends a summary of a change to a dataset's license between two versions (original and new) to change_list. ''' - original_license_url = "" - new_license_url = "" + original_license_url = u"" + new_license_url = u"" # if the license has a URL if u'license_url' in original and original['license_url']: original_license_url = original['license_url'] @@ -501,7 +501,7 @@ def _version_change(change_list, original, new, new_pkg): ''' # if both old and new versions have version numbers if original['version'] and new['version']: - change_list.append({u'type': 'version', u'method': u'change', + change_list.append({u'type': u'version', u'method': u'change', u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], u'old_version': original['version'], u'new_version': diff --git a/ckan/lib/helpers.py b/ckan/lib/helpers.py index 5c98ec9c7ac..915f74aff40 100644 --- a/ckan/lib/helpers.py +++ b/ckan/lib/helpers.py @@ -2709,8 +2709,11 @@ def compare_pkg_dicts(original, new, old_activity_id): from changes import _check_metadata_changes, _check_resource_changes change_list = [] - new_pkg = {u'pkg_id': new['id'], u'name': new['name'], - u'title': new['title']} + new_pkg = { + u'pkg_id': new['id'], + u'name': new['name'], + u'title': new['title'] + } _check_metadata_changes(change_list, original, new, new_pkg) diff --git a/ckan/public/base/css/main.css b/ckan/public/base/css/main.css index b841c71b530..439ae5988fa 100644 --- a/ckan/public/base/css/main.css +++ b/ckan/public/base/css/main.css @@ -10760,3 +10760,18 @@ iframe { .reduced-padding { padding: 3px 5px; } + +.select-time { + width: 250px; + display: inline; +} + +br.line-height2 { + line-height: 2; +} + +.select-div { + width:250px; + position:absolute; + right:30px; +} diff --git a/ckan/public/base/javascript/modules/metadata-button.js b/ckan/public/base/javascript/modules/metadata-button.js new file mode 100644 index 00000000000..609a8ec75e4 --- /dev/null +++ b/ckan/public/base/javascript/modules/metadata-button.js @@ -0,0 +1,38 @@ +/* Watches the "Show metadata diff" button on the Changes summary page. + * When the button is pressed, toggles the display of the metadata diff + * for the chronologically most recent revision on and off. + * + * target - a button to watch for changes (default: button) + * + */ + +ckan.module('metadata-button', function(jQuery) { + return { + options: { + target: 'button' + }, + + initialize: function () { + // Watch for our button to be clicked. + this.el.on('click', jQuery.proxy(this._onClick, this)); + }, + + _onClick: function(event) { + console.log("PRESSED THE BUTTON"); + var div = document.getElementById("metadata_diff"); + if (div.style.display === "none") { + div.style.display = "block"; + } + else { + div.style.display = "none"; + } + var btn = document.getElementById("metadata_button"); + if (btn.value === "Show metadata diff") { + btn.value = "Hide metadata diff"; + } + else { + btn.value = "Show metadata diff"; + } + } + } +}); diff --git a/ckan/public/base/javascript/webassets.yml b/ckan/public/base/javascript/webassets.yml index c52882973db..8ddcc33f974 100644 --- a/ckan/public/base/javascript/webassets.yml +++ b/ckan/public/base/javascript/webassets.yml @@ -57,6 +57,7 @@ ckan: - modules/media-grid.js - modules/image-upload.js - modules/followers-counter.js + - modules/metadata-button.js tracking: output: base/%(version)s_tracking.js diff --git a/ckan/templates/package/changes.html b/ckan/templates/package/changes.html index 4f3ae7252bb..dcaacb95f36 100644 --- a/ckan/templates/package/changes.html +++ b/ckan/templates/package/changes.html @@ -13,7 +13,7 @@ {% block primary %}
      -
      +
      {% block package_changes_header %}

      {{ _('Changes') }}

      @@ -21,21 +21,20 @@

      {{ _('Changes') }}

      {% set select_list1 = h.activity_list_select(pkg_activity_list, activity_diffs[-1].activities[0].id) %} {% set select_list2 = h.activity_list_select(pkg_activity_list, activity_diffs[0].activities[1].id) %} - + View changes from - 
                   {{ select_list1[1:]|safe }}
      to - 
                   {{ select_list2|safe }}
      -
      @@ -44,10 +43,22 @@

      {{ _('Changes') }}

      {% for i in range(activity_diffs|length) %} {% snippet "package/snippets/change_item.html", activity_diff=activity_diffs[i], pkg_dict=pkg_dict %} -
      + + {# TODO: display metadata for more than most recent change #} + {% if i == 0 %} + {# button to show JSON metadata diff for the most recent change - not shown by default #} + + + {% endif %} +
      {% endfor %} -
      {% endblock %} diff --git a/ckan/templates/package/snippets/change_item.html b/ckan/templates/package/snippets/change_item.html index 478d8c92cb1..727ee5d51b5 100644 --- a/ckan/templates/package/snippets/change_item.html +++ b/ckan/templates/package/snippets/change_item.html @@ -8,33 +8,3 @@
      {% endfor %}
    - -{# button to show JSON metadata diff - not shown by default #} - - - - - diff --git a/ckan/templates/snippets/changes/description.html b/ckan/templates/snippets/changes/description.html index 0c531079302..014fdaf3c49 100644 --- a/ckan/templates/snippets/changes/description.html +++ b/ckan/templates/snippets/changes/description.html @@ -2,8 +2,8 @@

    {% if ah.method == "change" %} {{ _('Updated description of {dataset} from -

    {old_desc}
    - to
    {new_desc} +
    {old_desc}
    + to
    {new_desc}
    ').format( pkg_url = h.url_for(controller='dataset', action='read', id=ah.pkg_id), dataset = ah.title, @@ -12,7 +12,7 @@ )|safe }} {% elif ah.method == "add" %} {{ _('Updated description of {dataset} - to
    {new_desc} + to
    {new_desc}
    ').format( pkg_url = h.url_for(controller='dataset', action='read', id=ah.pkg_id), dataset = ah.title, diff --git a/ckan/templates/snippets/changes/resource_desc.html b/ckan/templates/snippets/changes/resource_desc.html index 7ea932c6ace..1d837fd2063 100644 --- a/ckan/templates/snippets/changes/resource_desc.html +++ b/ckan/templates/snippets/changes/resource_desc.html @@ -3,7 +3,7 @@ {% if ah.method == "add" %} {{ _('Updated description of resource {resource_name} in {dataset} - to
    {new_desc}
    ' + to
    {new_desc}
    ' ).format( pkg_url = h.url_for(controller='dataset', action='read', id=ah.pkg_id), dataset = ah.title, @@ -24,8 +24,8 @@ {% elif ah.method == "change" %} {{ _('Updated description of resource {resource_name} in {dataset} - from
    {old_desc}
    - to
    {new_desc}
    ' + from
    {old_desc}
    + to
    {new_desc}
    ' ).format( pkg_url = h.url_for(controller='dataset', action='read', id=ah.pkg_id), dataset = ah.title, From c50374e52cefde0f792565141582c0901abb86ae Mon Sep 17 00:00:00 2001 From: David Read Date: Fri, 9 Aug 2019 16:56:02 +0100 Subject: [PATCH 24/47] Unit tests for changes.py in #4929 5 tests fail because they need additions to changes.py: * add & remove of owner_org * add/change/remove of resource_extras I've included a couple of other little improvements to changes.py too. --- ckan/lib/changes.py | 42 +- ckan/lib/helpers.py | 8 +- ckan/logic/action/patch.py | 1 + .../snippets/changes/resource_format.html | 14 +- ckan/tests/lib/test_changes.py | 696 ++++++++++++++++++ 5 files changed, 730 insertions(+), 31 deletions(-) create mode 100644 ckan/tests/lib/test_changes.py diff --git a/ckan/lib/changes.py b/ckan/lib/changes.py index dea71ddae12..fa4c50023ad 100644 --- a/ckan/lib/changes.py +++ b/ckan/lib/changes.py @@ -1,10 +1,10 @@ # encoding: utf-8 ''' -Functions used by the helper function compare_pkg_dicts() to analyze -the differences between two versions of a dataset. +Functions for generating a list of differences between two versions of a +dataset ''' -from helpers import url_for + import logging log = logging.getLogger(__name__) @@ -40,14 +40,14 @@ def _extras_to_dict(extras_list): return ret_dict -def _check_resource_changes(change_list, original, new, new_pkg, - old_activity_id): +def check_resource_changes(change_list, original, new, new_pkg, + old_activity_id): ''' - Checks whether a dataset's resources have changed - whether new ones have - been uploaded, existing ones have been deleted, or existing ones have - been edited. For existing resources, checks whether their names, formats, - and/or descriptions have changed, as well as whether a new file has been - uploaded for the resource. + Compares two versions of a dataset and records the changes between them + (just the resources) in change_list. e.g. resources that are added, changed + or deleted. For existing resources, checks whether their names, formats, + and/or descriptions have changed, as well as whether the url changed (e.g. + a new file has been uploaded for the resource). ''' # make a set of the resource IDs present in original and new @@ -55,7 +55,6 @@ def _check_resource_changes(change_list, original, new, new_pkg, original_resource_dict = {} new_resource_set = set() new_resource_dict = {} - s = u"" for resource in original['resources']: original_resource_set.add(resource['id']) @@ -126,7 +125,8 @@ def _check_resource_changes(change_list, original, new, new_pkg, u'resource_id': resource_id, u'resource_name': new_resource_dict[resource_id]['name'], - u'org_id': new['organization']['id'], + u'org_id': new['organization']['id'] + if new['organization'] else u'', u'format': new_metadata['format']}) # if both versions have a format but the format changed @@ -138,7 +138,8 @@ def _check_resource_changes(change_list, original, new, new_pkg, u'resource_id': resource_id, u'resource_name': new_resource_dict[resource_id]['name'], - u'org_id': new['organization']['id'], + u'org_id': new['organization']['id'] + if new['organization'] else u'', u'old_format': original_metadata['format'], u'new_format': new_metadata['format']}) @@ -177,7 +178,7 @@ def _check_resource_changes(change_list, original, new, new_pkg, u'new_desc': new_metadata['description'], u'old_desc': original_metadata['description']}) - # check if the user uploaded a new file + # check if the url changes (e.g. user uploaded a new file) # TODO: use regular expressions to determine the actual name of the # new and old files if original_metadata['url'] != new_metadata['url']: @@ -189,12 +190,10 @@ def _check_resource_changes(change_list, original, new, new_pkg, new_resource_dict[resource_id]['name']}) -def _check_metadata_changes(change_list, original, new, new_pkg): +def check_metadata_changes(change_list, original, new, new_pkg): ''' - Checks whether a dataset's metadata fields (fields in its package - dictionary not including resources) have changed between two consecutive - versions and puts a list of formatted summaries of these changes in - change_list. + Compares two versions of a dataset and records the changes between them + (excluding resources) in change_list. ''' # if the title has changed if original['title'] != new['title']: @@ -280,7 +279,9 @@ def _org_change(change_list, original, new, new_pkg): Appends a summary of a change to a dataset's organization between two versions (original and new) to change_list. ''' - change_list.append({u'type': u'org', u'pkg_id': new_pkg['pkg_id'], + change_list.append({u'type': u'org', + u'method': u'change', + u'pkg_id': new_pkg['pkg_id'], u'title': new_pkg['title'], u'original_org_id': original['organization']['id'], u'original_org_title': @@ -578,7 +579,6 @@ def _extra_fields(change_list, original, new, new_pkg): from the web interface (or API?) and appends a summary of each change to change_list. ''' - s = u"" if u'extras' in new: extra_fields_new = _extras_to_dict(new['extras']) extra_new_set = set(extra_fields_new.keys()) diff --git a/ckan/lib/helpers.py b/ckan/lib/helpers.py index 915f74aff40..1cf4e3a427c 100644 --- a/ckan/lib/helpers.py +++ b/ckan/lib/helpers.py @@ -2706,7 +2706,7 @@ def compare_pkg_dicts(original, new, old_activity_id): string indicating the type of change made as well as other data necessary to form a detailed summary of the change. ''' - from changes import _check_metadata_changes, _check_resource_changes + from changes import check_metadata_changes, check_resource_changes change_list = [] new_pkg = { @@ -2715,10 +2715,10 @@ def compare_pkg_dicts(original, new, old_activity_id): u'title': new['title'] } - _check_metadata_changes(change_list, original, new, new_pkg) + check_metadata_changes(change_list, original, new, new_pkg) - _check_resource_changes(change_list, original, new, new_pkg, - old_activity_id) + check_resource_changes(change_list, original, new, new_pkg, + old_activity_id) # if the dataset was updated but none of the fields we check were changed, # display a message stating that diff --git a/ckan/logic/action/patch.py b/ckan/logic/action/patch.py index f5150f58b7e..4a4e137893a 100644 --- a/ckan/logic/action/patch.py +++ b/ckan/logic/action/patch.py @@ -38,6 +38,7 @@ def package_patch(context, data_dict): 'session': context['session'], 'user': context['user'], 'auth_user_obj': context['auth_user_obj'], + 'ignore_auth': context.get('ignore_auth', False), } package_dict = _get_action('package_show')( diff --git a/ckan/templates/snippets/changes/resource_format.html b/ckan/templates/snippets/changes/resource_format.html index 2224dada9b2..8a242e39329 100644 --- a/ckan/templates/snippets/changes/resource_format.html +++ b/ckan/templates/snippets/changes/resource_format.html @@ -1,5 +1,10 @@

  • + {% set format_search_base_url = ( + h.url_for(controller="organization", action="read", id=ah.org_id) + if ah.org_id else + h.url_for(controller="dataset", action="search")) %} + {% if ah.method == "add" %} {{ _('Set format of resource {resource_name} to {format} in @@ -9,8 +14,7 @@ resource_url = h.url_for(controller='resource', action='read', id=ah.pkg_id, resource_id=ah.resource_id), resource_name = ah.resource_name, - format_url = h.url_for(controller="organization", action="read", - id=ah.org_id) + "?res_format=" + ah.format, + format_url = format_search_base_url + "?res_format=" + ah.format, format = ah.format )|safe }} {% elif ah.method == "change" %} @@ -23,11 +27,9 @@ resource_url = h.url_for(controller='resource', action='read', id=ah.pkg_id, resource_id=ah.resource_id), resource_name = ah.resource_name, - new_format_url = h.url_for(controller="organization", action="read", - id=ah.org_id) + "?res_format=" + ah.new_format, + new_format_url = format_search_base_url + "?res_format=" + ah.new_format, new_format = ah.new_format, - old_format_url = h.url_for(controller="organization", action="read", - id=ah.org_id) + "?res_format=" + ah.old_format, + old_format_url = format_search_base_url + "?res_format=" + ah.old_format, old_format = ah.old_format )|safe }} {% else %} diff --git a/ckan/tests/lib/test_changes.py b/ckan/tests/lib/test_changes.py new file mode 100644 index 00000000000..0ce3f082d1f --- /dev/null +++ b/ckan/tests/lib/test_changes.py @@ -0,0 +1,696 @@ +# encoding: utf-8 + +import copy + +from nose.tools import assert_equal as eq + +from ckan.lib.changes import check_metadata_changes, check_resource_changes +from ckan.tests import helpers +from ckan.tests.factories import Dataset, Organization, Group + + +def _new_pkg(new): + return { + u'pkg_id': new['id'], + u'name': new['name'], + u'title': new['title'] + } + + +class TestCheckMetadataChanges(object): + + def setup(self): + helpers.reset_db() + + def test_title(self): + changes = [] + original = Dataset() + new = helpers.call_action(u'package_patch', id=original['id'], + title=u'New title') + + check_metadata_changes(changes, original, new, _new_pkg(new)) + + assert len(changes) == 1, changes + eq(changes[0]['type'], u'title') + eq(changes[0]['original_title'], u'Test Dataset') + eq(changes[0]['new_title'], u'New title') + + def test_name(self): + changes = [] + original = Dataset() + new = helpers.call_action(u'package_patch', id=original['id'], + name=u'new-name') + + check_metadata_changes(changes, original, new, _new_pkg(new)) + + assert len(changes) == 1, changes + eq(changes[0]['type'], u'name') + eq(changes[0]['old_name'], original['name']) + eq(changes[0]['new_name'], u'new-name') + + def test_add_extra(self): + changes = [] + original = Dataset() + new = helpers.call_action( + u'package_patch', id=original['id'], + extras=[{u'key': u'subject', u'value': u'science'}]) + + check_metadata_changes(changes, original, new, _new_pkg(new)) + + assert len(changes) == 1, changes + eq(changes[0]['type'], u'custom_fields') + eq(changes[0]['method'], u'add1') + eq(changes[0]['field_name'], u'subject') + eq(changes[0]['field_val'], u'science') + + # TODO how to test 'add2'? + + def test_add_multiple_extras(self): + changes = [] + original = Dataset() + new = helpers.call_action( + u'package_patch', id=original['id'], + extras=[{u'key': u'subject', u'value': u'science'}, + {u'key': u'topic', u'value': u'wind'}]) + + check_metadata_changes(changes, original, new, _new_pkg(new)) + + assert len(changes) == 1, changes + eq(changes[0]['type'], u'custom_fields') + eq(changes[0]['method'], u'add3') + eq(set(changes[0]['fields']), set([u'subject', u'topic'])) + + def test_change_extra(self): + changes = [] + original = Dataset( + extras=[{u'key': u'subject', u'value': u'science'}, + {u'key': u'topic', u'value': u'wind'}]) + new = helpers.call_action( + u'package_patch', id=original['id'], + extras=[{u'key': u'subject', u'value': u'scientific'}, + {u'key': u'topic', u'value': u'wind'}]) + + check_metadata_changes(changes, original, new, _new_pkg(new)) + + assert len(changes) == 1, changes + eq(changes[0]['type'], u'custom_fields') + eq(changes[0]['method'], u'change1') + eq(changes[0]['field_name'], u'subject') + eq(changes[0]['field_val_old'], u'science') + eq(changes[0]['field_val_new'], u'scientific') + + def test_change_multiple_extras(self): + changes = [] + original = Dataset( + extras=[{u'key': u'subject', u'value': u'science'}, + {u'key': u'topic', u'value': u'wind'}]) + new = helpers.call_action( + u'package_patch', id=original['id'], + extras=[{u'key': u'subject', u'value': u'scientific'}, + {u'key': u'topic', u'value': u'rain'}]) + + check_metadata_changes(changes, original, new, _new_pkg(new)) + + assert len(changes) == 2, changes + for change in changes: + eq(change['type'], u'custom_fields') + eq(change['method'], u'change1') + if change['field_name'] == u'subject': + eq(change['field_val_new'], u'scientific') + else: + eq(changes[0]['field_name'], u'topic') + eq(change['field_val_new'], u'rain') + + # TODO how to test change2? + + def test_delete_extra(self): + changes = [] + original = Dataset( + extras=[{u'key': u'subject', u'value': u'science'}, + {u'key': u'topic', u'value': u'wind'}]) + new = helpers.call_action( + u'package_patch', id=original['id'], + extras=[{u'key': u'topic', u'value': u'wind'}]) + + check_metadata_changes(changes, original, new, _new_pkg(new)) + + assert len(changes) == 1, changes + eq(changes[0]['type'], u'custom_fields') + eq(changes[0]['method'], u'remove1') + eq(changes[0]['field_name'], u'subject') + + def test_delete_multiple_extras(self): + changes = [] + original = Dataset( + extras=[{u'key': u'subject', u'value': u'science'}, + {u'key': u'topic', u'value': u'wind'}, + {u'key': u'geography', u'value': u'global'}]) + new = helpers.call_action( + u'package_patch', id=original['id'], + extras=[{u'key': u'topic', u'value': u'wind'}]) + + check_metadata_changes(changes, original, new, _new_pkg(new)) + + assert len(changes) == 1, changes + eq(changes[0]['type'], u'custom_fields') + eq(changes[0]['method'], u'remove2') + eq(set(changes[0]['fields']), set((u'subject', u'geography'))) + + def test_add_maintainer(self): + changes = [] + original = Dataset() + new = helpers.call_action( + u'package_patch', id=original['id'], + maintainer=u'new maintainer') + + check_metadata_changes(changes, original, new, _new_pkg(new)) + + eq(changes[0]['type'], u'maintainer') + eq(changes[0]['method'], u'add') + eq(changes[0]['new_maintainer'], u'new maintainer') + + def test_change_maintainer(self): + changes = [] + original = Dataset(maintainer=u'first maintainer') + new = helpers.call_action( + u'package_patch', id=original['id'], + maintainer=u'new maintainer') + + check_metadata_changes(changes, original, new, _new_pkg(new)) + + eq(changes[0]['type'], u'maintainer') + eq(changes[0]['method'], u'change') + eq(changes[0]['old_maintainer'], u'first maintainer') + eq(changes[0]['new_maintainer'], u'new maintainer') + + def test_remove_maintainer(self): + changes = [] + original = Dataset(maintainer=u'first maintainer') + new = helpers.call_action( + u'package_patch', id=original['id'], + maintainer=u'') + + check_metadata_changes(changes, original, new, _new_pkg(new)) + + eq(changes[0]['type'], u'maintainer') + eq(changes[0]['method'], u'remove') + + def test_add_notes(self): + changes = [] + original = Dataset(notes=u'') + new = helpers.call_action( + u'package_patch', id=original['id'], + notes=u'new notes') + + check_metadata_changes(changes, original, new, _new_pkg(new)) + + eq(changes[0]['type'], u'description') + eq(changes[0]['method'], u'add') + eq(changes[0]['new_desc'], u'new notes') + + def test_change_notes(self): + changes = [] + original = Dataset(notes=u'first notes') + new = helpers.call_action( + u'package_patch', id=original['id'], + notes=u'new notes') + + check_metadata_changes(changes, original, new, _new_pkg(new)) + + eq(changes[0]['type'], u'description') + eq(changes[0]['method'], u'change') + eq(changes[0]['old_desc'], u'first notes') + eq(changes[0]['new_desc'], u'new notes') + + def test_remove_notes(self): + changes = [] + original = Dataset(notes=u'first notes') + new = helpers.call_action( + u'package_patch', id=original['id'], + notes=u'') + + check_metadata_changes(changes, original, new, _new_pkg(new)) + + eq(changes[0]['type'], u'description') + eq(changes[0]['method'], u'remove') + + @helpers.change_config(u'ckan.auth.create_unowned_dataset', True) + def test_add_org(self): + changes = [] + original = Dataset(owner_org=None) + new_org = Organization() + new = helpers.call_action( + u'package_patch', id=original['id'], + owner_org=new_org['id']) + + check_metadata_changes(changes, original, new, _new_pkg(new)) + + eq(changes[0]['type'], u'org') + eq(changes[0]['method'], u'add') + eq(changes[0]['new_org_id'], new_org['id']) + + def test_change_org(self): + changes = [] + old_org = Organization() + original = Dataset(owner_org=old_org['id']) + new_org = Organization() + new = helpers.call_action( + u'package_patch', id=original['id'], + owner_org=new_org['id']) + + check_metadata_changes(changes, original, new, _new_pkg(new)) + + eq(changes[0]['type'], u'org') + eq(changes[0]['method'], u'change') + eq(changes[0]['original_org_id'], original['organization']['id']) + eq(changes[0]['new_org_id'], new_org['id']) + + @helpers.change_config(u'ckan.auth.create_unowned_dataset', True) + def test_remove_org(self): + changes = [] + old_org = Organization() + original = Dataset(owner_org=old_org['id']) + new = helpers.call_action( + u'package_patch', id=original['id'], + owner_org=None) + + check_metadata_changes(changes, original, new, _new_pkg(new)) + + eq(changes[0]['type'], u'org') + eq(changes[0]['method'], u'remove') + + def test_make_private(self): + changes = [] + old_org = Organization() + original = Dataset(owner_org=old_org['id'], private=False) + new = helpers.call_action(u'package_patch', id=original['id'], + private=True) + + check_metadata_changes(changes, original, new, _new_pkg(new)) + + assert len(changes) == 1, changes + eq(changes[0]['type'], u'private') + eq(changes[0]['new'], u'Private') + + def test_make_public(self): + changes = [] + old_org = Organization() + original = Dataset(owner_org=old_org['id'], private=True) + new = helpers.call_action(u'package_patch', id=original['id'], + private=False) + + check_metadata_changes(changes, original, new, _new_pkg(new)) + + assert len(changes) == 1, changes + eq(changes[0]['type'], u'private') + eq(changes[0]['new'], u'Public') + + def test_add_tag(self): + changes = [] + original = Dataset(tags=[{u'name': u'rivers'}]) + new = helpers.call_action(u'package_patch', id=original['id'], + tags=[{u'name': u'rivers'}, + {u'name': u'oceans'}]) + + check_metadata_changes(changes, original, new, _new_pkg(new)) + + assert len(changes) == 1, changes + eq(changes[0]['type'], u'tags') + eq(changes[0]['method'], u'add1') + eq(changes[0]['tag'], u'oceans') + + def test_add_multiple_tags(self): + changes = [] + original = Dataset(tags=[{u'name': u'rivers'}]) + new = helpers.call_action(u'package_patch', id=original['id'], + tags=[{u'name': u'rivers'}, + {u'name': u'oceans'}, + {u'name': u'streams'}]) + + check_metadata_changes(changes, original, new, _new_pkg(new)) + + assert len(changes) == 1, changes + eq(changes[0]['type'], u'tags') + eq(changes[0]['method'], u'add2') + eq(set(changes[0]['tags']), set((u'oceans', u'streams'))) + + def test_delete_tag(self): + changes = [] + original = Dataset(tags=[{u'name': u'rivers'}, + {u'name': u'oceans'}]) + new = helpers.call_action(u'package_patch', id=original['id'], + tags=[{u'name': u'rivers'}]) + + check_metadata_changes(changes, original, new, _new_pkg(new)) + + assert len(changes) == 1, changes + eq(changes[0]['type'], u'tags') + eq(changes[0]['method'], u'remove1') + eq(changes[0]['tag'], u'oceans') + + def test_remove_multiple_tags(self): + changes = [] + original = Dataset(tags=[{u'name': u'rivers'}, + {u'name': u'oceans'}, + {u'name': u'streams'}]) + new = helpers.call_action(u'package_patch', id=original['id'], + tags=[{u'name': u'rivers'}]) + + check_metadata_changes(changes, original, new, _new_pkg(new)) + + assert len(changes) == 1, changes + eq(changes[0]['type'], u'tags') + eq(changes[0]['method'], u'remove2') + eq(set(changes[0]['tags']), set((u'oceans', u'streams'))) + + def test_add_url(self): + changes = [] + original = Dataset() + new = helpers.call_action( + u'package_patch', id=original['id'], + url=u'new url') + + check_metadata_changes(changes, original, new, _new_pkg(new)) + + eq(changes[0]['type'], u'source_url') + eq(changes[0]['method'], u'add') + eq(changes[0]['new_url'], u'new url') + + def test_change_url(self): + changes = [] + original = Dataset(url=u'first url') + new = helpers.call_action( + u'package_patch', id=original['id'], + url=u'new url') + + check_metadata_changes(changes, original, new, _new_pkg(new)) + + eq(changes[0]['type'], u'source_url') + eq(changes[0]['method'], u'change') + eq(changes[0]['old_url'], u'first url') + eq(changes[0]['new_url'], u'new url') + + def test_remove_url(self): + changes = [] + original = Dataset(url=u'first url') + new = helpers.call_action( + u'package_patch', id=original['id'], + url=u'') + + check_metadata_changes(changes, original, new, _new_pkg(new)) + + eq(changes[0]['type'], u'source_url') + eq(changes[0]['method'], u'remove') + + def test_add_version(self): + changes = [] + original = Dataset() + new = helpers.call_action( + u'package_patch', id=original['id'], + version=u'new version') + + check_metadata_changes(changes, original, new, _new_pkg(new)) + + eq(changes[0]['type'], u'version') + eq(changes[0]['method'], u'add') + eq(changes[0]['new_version'], u'new version') + + def test_change_version(self): + changes = [] + original = Dataset(version=u'first version') + new = helpers.call_action( + u'package_patch', id=original['id'], + version=u'new version') + + check_metadata_changes(changes, original, new, _new_pkg(new)) + + eq(changes[0]['type'], u'version') + eq(changes[0]['method'], u'change') + eq(changes[0]['old_version'], u'first version') + eq(changes[0]['new_version'], u'new version') + + def test_remove_version(self): + changes = [] + original = Dataset(version=u'first version') + new = helpers.call_action( + u'package_patch', id=original['id'], + version=u'') + + check_metadata_changes(changes, original, new, _new_pkg(new)) + + eq(changes[0]['type'], u'version') + eq(changes[0]['method'], u'remove') + + +class TestCheckResourceChanges(object): + + def setup(self): + helpers.reset_db() + + def test_add_resource(self): + changes = [] + original = Dataset() + new = helpers.call_action( + u'package_patch', id=original['id'], + resources=[{u'url': u'http://example.com/image.png', + u'format': u'png', + u'name': u'Image 1'}]) + + check_resource_changes(changes, original, new, _new_pkg(new), u'fake') + + assert len(changes) == 1, changes + eq(changes[0]['type'], u'new_resource') + eq(changes[0]['resource_name'], u'Image 1') + + def test_add_multiple_resources(self): + changes = [] + original = Dataset() + new = helpers.call_action( + u'package_patch', id=original['id'], + resources=[{u'url': u'http://example.com/image.png', + u'format': u'png', + u'name': u'Image 1'}, + {u'url': u'http://example.com/image2.png', + u'format': u'png', + u'name': u'Image 2'}]) + + check_resource_changes(changes, original, new, _new_pkg(new), u'fake') + + assert len(changes) == 2, changes + eq(changes[0]['type'], u'new_resource') + eq(changes[1]['type'], u'new_resource') + if changes[0]['resource_name'] == u'Image 1': + eq(changes[1]['resource_name'], u'Image 2') + else: + eq(changes[1]['resource_name'], u'Image 1') + eq(changes[0]['resource_name'], u'Image 2') + + def test_change_resource_url(self): + changes = [] + original = Dataset( + resources=[{u'url': u'http://example.com/image.png', + u'format': u'png', + u'name': u'Image 1'}, + {u'url': u'http://example.com/image.png', + u'format': u'png', + u'name': u'Image 2'}]) + new = copy.deepcopy(original) + new['resources'][1][u'url'] = u'http://example.com/image_changed.png' + new = helpers.call_action(u'package_update', **new) + + check_resource_changes(changes, original, new, _new_pkg(new), u'fake') + + assert len(changes) == 1, changes + eq(changes[0]['type'], u'new_file') + eq(changes[0]['resource_name'], u'Image 2') + + def test_change_resource_format(self): + changes = [] + original = Dataset( + resources=[{u'url': u'http://example.com/image.png', + u'format': u'png', + u'name': u'Image 1'}, + {u'url': u'http://example.com/image.png', + u'format': u'png', + u'name': u'Image 2'}]) + new = copy.deepcopy(original) + new['resources'][1]['format'] = u'jpg' + new = helpers.call_action(u'package_update', **new) + + check_resource_changes(changes, original, new, _new_pkg(new), u'fake') + + assert len(changes) == 1, changes + eq(changes[0]['type'], u'resource_format') + eq(changes[0]['resource_name'], u'Image 2') + + def test_change_resource_name(self): + changes = [] + original = Dataset( + resources=[{u'url': u'http://example.com/image.png', + u'format': u'png', + u'name': u'Image 1'}, + {u'url': u'http://example.com/image.png', + u'format': u'png', + u'name': u'Image 2'}]) + new = copy.deepcopy(original) + new['resources'][1]['name'] = u'Image changed' + new = helpers.call_action(u'package_update', **new) + + check_resource_changes(changes, original, new, _new_pkg(new), u'fake') + + assert len(changes) == 1, changes + eq(changes[0]['type'], u'resource_name') + eq(changes[0]['old_resource_name'], u'Image 2') + eq(changes[0]['new_resource_name'], u'Image changed') + + def test_change_resource_description(self): + changes = [] + original = Dataset( + resources=[{u'url': u'http://example.com/image.png', + u'format': u'png', + u'name': u'Image 1', + u'description': u'First image'}, + {u'url': u'http://example.com/image.png', + u'format': u'png', + u'name': u'Image 2', + u'description': u'Second image'}]) + new = copy.deepcopy(original) + new['resources'][1]['description'] = u'changed' + new = helpers.call_action(u'package_update', **new) + + check_resource_changes(changes, original, new, _new_pkg(new), u'fake') + + assert len(changes) == 1, changes + eq(changes[0]['type'], u'resource_desc') + eq(changes[0]['method'], u'change') + eq(changes[0]['resource_name'], u'Image 2') + + def test_add_resource_extra(self): + changes = [] + original = Dataset( + resources=[{u'url': u'http://example.com/image.png', + u'format': u'png', + u'name': u'Image 1'}]) + new = copy.deepcopy(original) + new['resources'][0]['new key'] = u'new value' + new = helpers.call_action(u'package_update', **new) + + check_resource_changes(changes, original, new, _new_pkg(new), u'fake') + + assert len(changes) == 1, changes + eq(changes[0]['type'], u'resource_extra') + eq(changes[0]['method'], u'add') + eq(changes[0]['key'], u'new key') + eq(changes[0]['value'], u'new value') + + def test_change_resource_extra(self): + changes = [] + original = Dataset( + resources=[{u'url': u'http://example.com/image.png', + u'format': u'png', + u'name': u'Image 1', + u'key1': u'value1'}]) + new = copy.deepcopy(original) + new['resources'][0]['key1'] = u'new value' + new = helpers.call_action(u'package_update', **new) + + check_resource_changes(changes, original, new, _new_pkg(new), u'fake') + + assert len(changes) == 1, changes + eq(changes[0]['type'], u'resource_extra') + eq(changes[0]['method'], u'change') + eq(changes[0]['key'], u'key1') + eq(changes[0]['value_old'], u'value1') + eq(changes[0]['value_new'], u'new value') + + def test_remove_resource_extra(self): + changes = [] + original = Dataset( + resources=[{u'url': u'http://example.com/image.png', + u'format': u'png', + u'name': u'Image 1', + u'key1': u'value1'}]) + new = copy.deepcopy(original) + del new['resources'][0]['key1'] + new = helpers.call_action(u'package_update', **new) + + check_resource_changes(changes, original, new, _new_pkg(new), u'fake') + + assert len(changes) == 1, changes + eq(changes[0]['type'], u'resource_extra') + eq(changes[0]['method'], u'remove') + eq(changes[0]['key'], u'key1') + + def test_change_multiple_resources(self): + changes = [] + original = Dataset( + resources=[{u'url': u'http://example.com/image.png', + u'format': u'png', + u'name': u'Image 1'}, + {u'url': u'http://example.com/image.png', + u'format': u'png', + u'name': u'Image 2'}, + {u'url': u'http://example.com/image.png', + u'format': u'png', + u'name': u'Image 3'}]) + new = copy.deepcopy(original) + new['resources'][0]['name'] = u'changed-1' + new['resources'][1]['name'] = u'changed-2' + new = helpers.call_action(u'package_update', **new) + + check_resource_changes(changes, original, new, _new_pkg(new), u'fake') + + assert len(changes) == 2, changes + eq(changes[0]['type'], u'resource_name') + eq(changes[1]['type'], u'resource_name') + if changes[0]['old_resource_name'] == u'Image 1': + eq(changes[0]['new_resource_name'], u'changed-1') + else: + eq(changes[0]['old_resource_name'], u'Image 2') + eq(changes[0]['new_resource_name'], u'changed-2') + + def test_delete_resource(self): + changes = [] + original = Dataset( + resources=[{u'url': u'http://example.com/image.png', + u'format': u'png', + u'name': u'Image 1'}, + {u'url': u'http://example.com/image.png', + u'format': u'png', + u'name': u'Image 2'}]) + new = copy.deepcopy(original) + del new['resources'][0] + new = helpers.call_action(u'package_update', **new) + + check_resource_changes(changes, original, new, _new_pkg(new), u'fake') + + assert len(changes) == 1, changes + eq(changes[0]['type'], u'delete_resource') + eq(changes[0]['resource_name'], u'Image 1') + + def test_delete_multiple_resources(self): + changes = [] + original = Dataset( + resources=[{u'url': u'http://example.com/image.png', + u'format': u'png', + u'name': u'Image 1'}, + {u'url': u'http://example.com/image.png', + u'format': u'png', + u'name': u'Image 2'}, + {u'url': u'http://example.com/image.png', + u'format': u'png', + u'name': u'Image 3'}]) + new = copy.deepcopy(original) + del new['resources'][1] + del new['resources'][0] + new = helpers.call_action(u'package_update', **new) + + check_resource_changes(changes, original, new, _new_pkg(new), u'fake') + + assert len(changes) == 2, changes + eq(changes[0]['type'], u'delete_resource') + if changes[0]['resource_name'] == u'Image 1': + eq(changes[1]['resource_name'], u'Image 2') + else: + eq(changes[0]['resource_name'], u'Image 2') + eq(changes[1]['resource_name'], u'Image 1') From ac4c9259bfe05cd1ba9ffc4fb269bc70fb1105e8 Mon Sep 17 00:00:00 2001 From: David Read Date: Fri, 9 Aug 2019 18:03:54 +0100 Subject: [PATCH 25/47] Added escaping to activity_list_select and test --- ckan/lib/helpers.py | 22 +++++++------- ckan/templates/package/changes.html | 4 +-- ckan/tests/lib/test_helpers.py | 46 +++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 12 deletions(-) diff --git a/ckan/lib/helpers.py b/ckan/lib/helpers.py index 1cf4e3a427c..6d13f09bae2 100644 --- a/ckan/lib/helpers.py +++ b/ckan/lib/helpers.py @@ -36,6 +36,7 @@ from werkzeug.routing import BuildError as FlaskRouteBuildError import i18n from six import string_types, text_type +import jinja2 import ckan.exceptions import ckan.model as model @@ -2735,19 +2736,20 @@ def activity_list_select(pkg_activity_list, current_activity_id): on the "Changes" summary page. ''' select_list = [] + template = jinja2.Template( + u'', + autoescape=True) for activity in pkg_activity_list: entry = render_datetime(activity['timestamp'], with_hours=True, with_seconds=True) - if activity['id'] == current_activity_id: - select_list.append( - "" - ) - else: - select_list.append( - "" - ) + select_list.append(Markup( + template + .render(activity_id=activity['id'], timestamp=entry, + selected='selected' + if activity['id'] == current_activity_id + else '') + )) return select_list diff --git a/ckan/templates/package/changes.html b/ckan/templates/package/changes.html index dcaacb95f36..89d4a0e6cb5 100644 --- a/ckan/templates/package/changes.html +++ b/ckan/templates/package/changes.html @@ -27,12 +27,12 @@

    {{ _('Changes') }}

    View changes from to diff --git a/ckan/tests/lib/test_helpers.py b/ckan/tests/lib/test_helpers.py index 9f7db4d8c0e..9c25c2fbc61 100644 --- a/ckan/tests/lib/test_helpers.py +++ b/ckan/tests/lib/test_helpers.py @@ -790,3 +790,49 @@ def helper_as_attribute(self): def helper_as_item(self): return base.render('tests/helper_as_item.html') + + +class TestActivityListSelect(object): + + def setup(self): + helpers.reset_db() + + def test_simple(self): + pkg_activity = { + 'id': 'id1', + 'timestamp': datetime.datetime(2018, 2, 1, 10, 58, 59), + } + + out = h.activity_list_select([pkg_activity], '') + + html = out[0] + eq_(str(html), + '') + assert hasattr(html, '__html__') # shows it is safe Markup + + def test_selected(self): + pkg_activity = { + 'id': 'id1', + 'timestamp': datetime.datetime(2018, 2, 1, 10, 58, 59), + } + + out = h.activity_list_select([pkg_activity], 'id1') + + html = out[0] + print html + eq_(str(html), + '') + assert hasattr(html, '__html__') # shows it is safe Markup + + def test_escaping(self): + pkg_activity = { + 'id': '">', # hacked somehow + 'timestamp': datetime.datetime(2018, 2, 1, 10, 58, 59), + } + + out = h.activity_list_select([pkg_activity], '') + + html = out[0] + assert str(html).startswith(u'