Backport 8309

changes/198.backport.feature

@@ -0,0 +1,5 @@
+- Saving a draft dataset is now called "Publish" and an "Unpublish" button is available
+to move an active dataset back to the draft state.
+- Consistent display of draft state across dataset pages.
+- Error handling is now done on publish, so validators that only apply certain rules to fields for published datasets (e.g. required-only-when-published) will be properly displayed instead of causing a server error.
+- templates/package/new_package_form.html content has been merged into its parent template templates/package/snippets/package_form.html and is now marked as deprecated.

changes/8308.feature

@@ -0,0 +1,5 @@
+- Saving a draft dataset is now called "Publish" and an "Unpublish" button is available
+to move an active dataset back to the draft state.
+- Consistent display of draft state across dataset pages.
+- Error handling is now done on publish, so validators that only apply certain rules to fields for published datasets (e.g. required-only-when-published) will be properly displayed instead of causing a server error.
+- templates/package/new_package_form.html content has been merged into its parent template templates/package/snippets/package_form.html and is now marked as deprecated.

ckan/lib/plugins.py

@@ -406,7 +406,7 @@ def resource_template(self) -> str:
         return 'package/resource_read.html'
 
     def package_form(self) -> str:
-        return 'package/new_package_form.html'
+        return 'package/snippets/package_form.html'
 
     def resource_form(self) -> str:
         return 'package/snippets/resource_form.html'

ckan/templates/package/new_package_form.html

@@ -1,27 +1,6 @@
 {% extends 'package/snippets/package_form.html' %}
 
-{% block stages %}
-  {% if form_style != 'edit' %}
-    {{ super() }}
-  {% endif %}
-{% endblock %}
+{# this template exists for backwards compatibility with old
+   IDatasetForm plugins and may be removed in a future release.
 
-{% block save_button_text %}
-  {% if form_style != 'edit' %}
-    {{ super() }}
-  {% else %}
-    {{ h.humanize_entity_type('package', pkg_dict.type, 'update label') or _('Update Dataset') }}
-  {% endif %}
-{% endblock %}
-
-{% block cancel_button %}
-  {% if form_style != 'edit' %}
-    {{ super() }}
-  {% endif %}
-{% endblock %}
-
-{% block delete_button %}
-  {% if form_style == 'edit' and h.check_access('package_delete', {'id': pkg_dict.id}) %}
-    {{ super() }}
-  {% endif %}
-{% endblock %}
+   Please extend package/snippets/package_form.html instead. #}

ckan/templates/package/new_resource.html

@@ -18,6 +18,7 @@
 
 {% block secondary_content %}
   {% snippet 'package/snippets/resource_help.html' %}
+  {% snippet 'package/snippets/resources.html', pkg=pkg %}
 {% endblock %}
 
 {% block scripts %}

ckan/templates/package/read.html

@@ -13,10 +13,10 @@ <h1>
       {% block page_heading %}
         {{ h.dataset_display_name(pkg) }}
         {% if pkg.state.startswith('draft') %}
-          [{{ _('Draft') }}]
+          <span class="badge bg-info">{{ _('Draft') }}</span>
         {% endif %}
         {% if pkg.state == 'deleted' %}
-          [{{ _('Deleted') }}]
+          <span class="badge bg-info">{{ _('Deleted') }}</span>
         {% endif %}
       {% endblock %}
     </h1>

ckan/templates/package/snippets/package_form.html

@@ -6,7 +6,9 @@
 <form id="dataset-edit" class="dataset-form" method="post" action="{{ action }}" data-module="basic-form" novalidate>
   {{ h.csrf_input() }}
   {% block stages %}
-    {{ h.snippet('package/snippets/stages.html', stages=stage, dataset_type=dataset_type) }}
+    {% if form_style != 'edit' %}
+      {% snippet 'package/snippets/stages.html', stages=stage, dataset_type=dataset_type %}
+    {% endif %}
   {% endblock %}
   {# (canada fork only): add ckan_phase block #}
   {% block ckan_phase %}
@@ -40,12 +42,25 @@
         </p>
       {% endblock %}
       {% block delete_button %}
-        {% if h.check_access('package_delete', {'id': data.id}) and not data.state == 'deleted' %}
+        {% if form_style == 'edit' and h.check_access('package_delete', {'id': data.id}) and not data.state == 'deleted' %}
           <a class="btn btn-danger pull-left" href="{% url_for dataset_type ~ '.delete', id=data.id %}" data-module="confirm-action" data-module-content="{{ h.humanize_entity_type('package', dataset_type, 'delete confirmation') or _('Are you sure you want to delete this dataset?') }}">{% block delete_button_text %}{{ _('Delete') }}{% endblock %}</a>
         {% endif %}
       {% endblock %}
+      {% block publish_button %}
+        {% if form_style == 'edit' %}
+          <button class="btn btn-warning" name="save" value="go-metadata-unpublish" type="submit">{% block publish_button_text %}{{ _('Unpublish') }}{% endblock %}</button>
+        {% endif %}
+      {% endblock %}
       {% block save_button %}
-        <button class="btn btn-primary" type="submit" name="save">{% block save_button_text %}{{ _('Next: Add Data') }}{% endblock %}</button>
+        <button class="btn btn-primary" type="submit" name="save">
+          {%- block save_button_text -%}
+            {%- if form_style == 'edit' -%}
+              {{ h.humanize_entity_type('package', pkg_dict.type, 'update label') or _('Update Dataset') }}
+            {%- else -%}
+              {{ _('Next: Add Data') }}
+            {%- endif -%}
+          {%- endblock -%}
+        </button>
       {% endblock %}
     </div>
   {% endblock %}

ckan/templates/package/snippets/resource_form.html

@@ -92,7 +92,7 @@
     {% endblock %}
     {% if stage %}
       {% block save_button %}
-        <button class="btn btn-primary" name="save" value="go-metadata" type="submit">{% block save_button_text %}{{ _('Finish') }}{% endblock %}</button>
+        <button class="btn btn-primary" name="save" value="go-metadata" type="submit">{% block save_button_text %}{{ _('Publish') }}{% endblock %}</button>
       {% endblock %}
     {% else %}
       {% block add_button %}

ckan/templates/package/snippets/resources.html

@@ -55,7 +55,7 @@ <h2 class="module-heading"><i class="fa fa-bars"></i> {{ _("Resources") }}</h2>
   {% endblock %}
 {% endif %}
 
-{% if can_edit and not is_activity_archive %}{# (canada fork only): prevent Resource edit/manage links when in activity archive "view" #}
+{% if can_edit and active and not is_activity_archive %}
   <div class="module-content">
     {% link_for _('Add new resource'), named_route=pkg.type ~ '_resource.new', id=pkg.name, class_='btn btn-light btn-sm', icon='plus' %}
   </div>

ckan/tests/controllers/test_package.py

@@ -142,7 +142,7 @@ def test_first_page_creates_draft_package(self, app, user):
 
     def test_resource_required(self, app, user):
         url = url_for("dataset.new")
-        name = "one-resource-required"
+        name = "resource-data-required"
         env = {"Authorization": user["token"]}
         response = app.post(url, extra_environ=env, data={
             "name": name,
@@ -153,9 +153,9 @@ def test_resource_required(self, app, user):
         response = app.post(location, extra_environ=env, data={
             "id": "",
             "url": "",
-            "save": "go-metadata",
+            "save": "again",
         })
-        assert "You must add at least one data resource" in response
+        assert "No resource data entered" in response
 
     def test_complete_package_with_one_resource(self, app, user):
         url = url_for("dataset.new")

ckan/views/dataset.py

@@ -574,10 +574,25 @@ def post(self, package_type: str) -> Union[Response, str]:
                     )
 
                     # redirect to add dataset resources
-                    url = h.url_for(
-                        u'{}_resource.new'.format(package_type),
-                        id=pkg_dict[u'name']
-                    )
+                    try:
+                        last_added_resource = pkg_dict[u'resources'][-1]
+                    except IndexError:
+                        last_added_resource = None
+                    if last_added_resource and request.form[u'save'] == "go-resources":
+                        url = h.url_for(
+                            u'{}_resource.edit'.format(package_type),
+                            id=pkg_dict.get('id'),
+                            resource_id=last_added_resource.get('id'))
+                    elif request.form[u'save'] == "go-metadata-preview":
+                        url = h.url_for(
+                            u'{}.read'.format(package_type),
+                            id=pkg_dict.get('id')
+                        )
+                    else:
+                        url = h.url_for(
+                            u'{}_resource.new'.format(package_type),
+                            id=pkg_dict[u'name']
+                        )
                     return h.redirect_to(url)
                 # Make sure we don't index this dataset
                 if request.form[u'save'] not in [
@@ -749,6 +764,9 @@ def post(self, package_type: str, id: str) -> Union[Response, str]:
                 del data_dict[u'_ckan_phase']
                 del data_dict[u'save']
             data_dict['id'] = id
+            if request.form.get('save', None) == 'go-metadata-unpublish':
+                data_dict['state'] = 'draft'
+
             pkg_dict = get_action(u'package_update')(context, data_dict)
 
             return _form_save_redirect(

ckan/views/resource.py

@@ -4,7 +4,7 @@
 import cgi
 import json
 import logging
-from typing import Any, cast, Optional, Union
+from typing import Any, Optional, Union, cast
 
 from werkzeug.wrappers.response import Response as WerkzeugResponse
 import flask
@@ -25,7 +25,7 @@
     _get_pkg_template, _get_package_type, _setup_template_variables
 )
 
-from ckan.types import Context, Response
+from ckan.types import Context, Response, ErrorDict
 
 Blueprint = flask.Blueprint
 NotFound = logic.NotFound
@@ -213,35 +213,31 @@ def post(self, package_type: str, id: str) -> Union[str, Response]:
                 data_provided = True
                 break
 
-        if not data_provided and save_action != u"go-dataset-complete":
-            if save_action == u'go-dataset':
+        if not data_provided and save_action != "go-dataset-complete":
+            if save_action == 'again':
+                errors: dict[str, Any] = {}
+                error_summary = {_('Error'): _('No resource data entered')}
+                return self.get(package_type, id, data, errors, error_summary)
+            if save_action == 'go-dataset':
                 # go to final stage of adddataset
                 return h.redirect_to(u'{}.edit'.format(package_type), id=id)
-            # see if we have added any resources
             try:
-                data_dict = get_action(u'package_show')(context, {u'id': id})
+                get_action('package_patch')(
+                    logic.fresh_context(context),
+                    {'id': id, 'state': 'active'}
+                )
             except NotAuthorized:
                 return base.abort(403, _(u'Unauthorized to update dataset'))
             except NotFound:
                 return base.abort(
                     404,
                     _(u'The dataset {id} could not be found.').format(id=id)
                 )
-            if not len(data_dict[u'resources']):
-                # no data so keep on page
-                msg = _(u'You must add at least one data resource')
-                # On new templates do not use flash message
-
-                errors: dict[str, Any] = {}
-                error_summary = {_(u'Error'): msg}
+            except ValidationError as e:
+                errors = cast(
+                    "list[ErrorDict]", e.error_dict.get('resources', [{}]))[-1]
+                error_summary = e.error_summary
                 return self.get(package_type, id, data, errors, error_summary)
-
-            # XXX race condition if another user edits/deletes
-            data_dict = get_action(u'package_show')(context, {u'id': id})
-            get_action(u'package_update')(
-                cast(Context, dict(context, allow_state_change=True)),
-                dict(data_dict, state=u'active')
-            )
             return h.redirect_to(u'{}.read'.format(package_type), id=id)
 
         data[u'package_id'] = id
@@ -266,12 +262,16 @@ def post(self, package_type: str, id: str) -> Union[str, Response]:
                 404, _(u'The dataset {id} could not be found.').format(id=id)
             )
         if save_action == u'go-metadata':
-            # XXX race condition if another user edits/deletes
-            data_dict = get_action(u'package_show')(context, {u'id': id})
-            get_action(u'package_update')(
-                cast(Context, dict(context, allow_state_change=True)),
-                dict(data_dict, state=u'active')
-            )
+            try:
+                get_action('package_patch')(
+                    logic.fresh_context(context),
+                    {'id': id, 'state': 'active'}
+                )
+            except ValidationError as e:
+                errors = cast(
+                    "list[ErrorDict]", e.error_dict.get('resources', [{}]))[-1]
+                error_summary = e.error_summary
+                return self.get(package_type, id, data, errors, error_summary)
             return h.redirect_to(u'{}.read'.format(package_type), id=id)
         elif save_action == u'go-dataset':
             # go to first stage of add dataset