Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

📊🖌️ Table Designer #7882

Merged
merged 152 commits into from May 3, 2024
Merged

📊🖌️ Table Designer #7882

merged 152 commits into from May 3, 2024

Conversation

wardi
Copy link
Contributor

@wardi wardi commented Oct 30, 2023

Fixes #6118

Proposed fixes:

Table Designer lets users define a schema and validation rules for their data. This ensures high quality data for sharing, building applications and growth over time. API documentation is automatically generated for ease of integration with existing or custom-built applications.

  • Table Designer option on resource url/upload control url_type
    resource-data-button

  • automatic creation of datatable view for new Table Designer resources
    table-with-data

  • add/delete columns and edit schema via Data Dictionary page
    data-dictionary
    add-field-options

  • add individual rows with an auto-generated form based on the schema
    add-row

  • primary keys and required columns fully supported
    duplicate-pk

  • data validation enforced by postgresql triggers, rendered as friendly errors in forms
    invalid-int

  • extended datatable preview with "edit row" and "delete rows" buttons for managing data
    edit-button
    delete-button
    delete-confirm

  • automatic API documentation for upsert/delete with examples from real data when available*
    api-docs

*arguably could be a separate PR that applies to all R/W datastore resources

This feature has been developed alongside two external ckan extensions:

  • ckanext-excelforms for bulk loading data using generated Excel templates
    edit-in-excel

  • ckanext-dsaudit for an audit trail of Table Designer resource datastore changes in the activity feed
    dsaudit-stream

Many supporting parts of this work have already been merged ❤️

Remaining features include: defining custom field types, custom validation rules and generating choice lists dynamically

Tests and documentation also need to be developed.

Features:

  • includes tests covering changes
  • includes updated documentation
  • includes user-visible changes
  • includes API changes
  • includes bugfix for possible backport

wardi added 30 commits July 14, 2021 12:33
@wardi wardi added this to the CKAN 2.11 milestone Apr 23, 2024
Copy link
Member

@amercader amercader left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wardi I did a first pass. This is really impressive stuff. One of the biggest recent features in CKAN by potential and work involved by far. I need to review more in depth a couple of places but in general it looks really solid and ready to be included in 2.11, but...

... the UI in its current form really lets it down. I believe with some moderate effort we can really make a difference in how users interact with it. I really want to propose some changes instead of just complaining so give me some time to come up with some proposals, but I need to understand better how Table Designer and DataTables interact.

Some quick things I noticed:

  • Once I finished creating the fields it was difficult to know how to add data to the table. My first instinct was to fill the text field for filtering columns that DataTables adds. I had to go to the docs to notice the "Add row" link below the table. Can this be moved next to the "Edit row", "Delete row buttons" ?
    Screenshot 2024-04-26 at 14-45-44 Test table desiger - td2 - CKAN

  • Regardless of placement, when there are no fields defined the "Add row" button should be hidden/disabled. Otherwise you get an exception if you click it now

  • Edit / Delete rows buttons should not be shown for unauthorized users. You can currently click them and an exception is shown

  • Can we disable the "Edit row" button if no rows are selected? Sorry, I saw that it disabled, the disabled style is just not that much different from the active one.

  • Maybe this is difficult to solve but when you have defined a schema but have no rows the examples in the "Data API" popup are not updated with the schema and show the "watershed", "survey" stuff. I guess you use actual rows to create the examples but maybe some placeholders can be used? It would be useful to have examples to populate an empty table via the API.

})
if any(v['view_type'] == 'datatables_view' for v in views):
return
get_action('resource_view_create')({}, {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This raises an exception if the datatables_view plugin is not loaded in ckan.plugins.

Granted, I jumped to try to the plugin without reading the docs where datatables_view is mentioned but there will be others like me, so perhaps we can check here that the plugin is loaded and add a warning?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a few places where the documented plugins are sensitive to other plugins being enabled and in the right order. Would it be better to fail on startup if something is missing or in the wrong order? Would checking the plugin list block things we don't want to, like an alternate implementation of datatables (I believe @jqnatividad is working on a version separate from the core one)?

.. _table-designer-web-forms:

--------------------------------------------
Creating and updating rows with the web form
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps between "Creating and updating rows with the web form" and "Creating and updating rows with ckanext-excelforms" it makes sense to have section briefly describing the process of using DataStore API calls directly to populate the resource table, as this will be how other third party tools or extensions can integrate with it. Perhaps some example datastore_create calls, some validation errors, etc

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

heading changes make sense, but hope the larger documentation changes won't be a blocker. I don't want to duplicate datastore API documentation (maybe that could be improved instead?) need to think on this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, that can be done later

doc/maintaining/table-designer.rst Outdated Show resolved Hide resolved
- uses the CKAN DataStore database as the primary data source
- allows rows to be updated without re-loading all data
- builds data schemas with custom types and constraints in the :ref:`data_dictionary` form
- enables linked data with simple and composite primary keys
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This "linked data" may be confusing. Do you mean that enables relationships between table designer resource tables (or regular datastore tables)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It means that table designer lets you create reliable primary keys that can't have duplicates, so you can use this key as a reference from other tables.
There isn't a built-in Foreign Key type in table designer to automatically reference other tables but that could be added in a future version, or in an IColumnTypes plugin.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sense, I'd just replace "linked data" to avoid confusion with the semantic sense. What about "enables referencing other tables"


.. note::

For column types and constraints we use a dummy gettext function
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not clear what this refers to. Is it for strings like error = _('Rating must be between 1 and 5')?
If so how they get extracted later?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, exactly. They get translated when rendered and we know the requesting user's language.

ckanext/tabledesigner/cli.py Outdated Show resolved Hide resolved
ckanext/tabledesigner/cli.py Outdated Show resolved Hide resolved
@wardi
Copy link
Contributor Author

wardi commented Apr 30, 2024

Add button is moved to the datatable view. All buttons are displayed only when the user has permission, excelforms interface updated too.

There's also a new alert directing users to the Data Dictionary page when no fields are defined.

@wardi
Copy link
Contributor Author

wardi commented Apr 30, 2024

@amercader re UI terribleness, I've just wrapped the data dictionary editing form (both for normal resources and table designer resources) in an accordion to more cleanly separate the fields and I tweaked some of the button layout too.

I've also updated the docs and fixed the crash when datatables_view is not enabled by adding another alert that will also be displayed if someone deletes the Table view from a table designer resource.

Please let me know if anything else is blocking this PR

@wardi
Copy link
Contributor Author

wardi commented Apr 30, 2024

re: examples for API when no data is present, that's a good idea and should not be too hard to do. I did define some example values in the ColumnType classes that could stand in for the real values when they're not available.

@wardi
Copy link
Contributor Author

wardi commented May 1, 2024

@amercader I updated the API example generation code to use the example values from the ColumType classes and have a second fall-back for when fields are defined but no data is available. Failing tests are from master and a flakey cypress test.

Comment on lines 5 to 6
{% asset 'ckanext-tabledesigner/fields' %}
{% asset 'ckanext-tabledesigner/datatables_buttons' %}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we load these just in the templates they are needed rather than the base one? Otherwise they get loaded in all pages. Maybe ckanext/tabledesigner/templates/datastore/dictionary.html and ckanext/tabledesigner/templates/package/resource_read.html? That seem to work fine for the CSS asset

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For sure. I was following recommendation from the docs https://docs.ckan.org/en/2.10/theming/webassets.html#adding-css-and-javascript-files-using-webassets (assuming including all js in the base means the bundle of JS will be cached better?) but it makes more sense to me to only include them in the pages that need them.

@amercader
Copy link
Member

@wardi thanks, the UI changes really make a big difference!

I wanted to push a small CSS fix and noticed that the CSS assets were broken. Do you mind adding this change yourself? Sorry, I can't push to this repo with my current network.
Once this is done this is good to go.

diff --git a/ckanext/tabledesigner/assets/style.css b/ckanext/tabledesigner/assets/style.css
deleted file mode 100644
index 963159508..000000000
--- a/ckanext/tabledesigner/assets/style.css
+++ /dev/null
@@ -1,5 +0,0 @@
-/*
-body {
-  border-radius: 0;
-}
-*/
diff --git a/ckanext/tabledesigner/assets/tabledesigner.css b/ckanext/tabledesigner/assets/tabledesigner.css
new file mode 100644
index 000000000..8153c7d04
--- /dev/null
+++ b/ckanext/tabledesigner/assets/tabledesigner.css
@@ -0,0 +1,3 @@
+#tabledesigner-add {
+  margin-top: 10px;
+}
diff --git a/ckanext/tabledesigner/assets/webassets.yml b/ckanext/tabledesigner/assets/webassets.yml
index 1e938832e..9ec7a2197 100644
--- a/ckanext/tabledesigner/assets/webassets.yml
+++ b/ckanext/tabledesigner/assets/webassets.yml
@@ -1,12 +1,7 @@
-# tabledesigner-css:
-#   filter: cssrewrite
-#   output: ckanext-tabledesigner/%(version)s-tabledesigner.css
-#   contents:
-#     - css/tabledesigner.css
 tabledesigner_css:
   output: ckanext-tabledesigner/%(version)s_tabledesigner_css.css
   contents:
-  - styles/tabledesigner.css
+  - tabledesigner.css
 
 fields:
   filters: rjsmin
diff --git a/ckanext/tabledesigner/templates/datastore/dictionary.html b/ckanext/tabledesigner/templates/datastore/dictionary.html
index 06158d2f6..900432f76 100644
--- a/ckanext/tabledesigner/templates/datastore/dictionary.html
+++ b/ckanext/tabledesigner/templates/datastore/dictionary.html
@@ -4,6 +4,9 @@
   {% if res.url_type != 'tabledesigner' %}
     {{ super() }}
   {% else %}
+
+    {% asset "ckanext-tabledesigner/tabledesigner_css" %}
+
     {% snippet "tabledesigner/snippets/design_fields.html",
       fields=fields,
       data=data,
diff --git a/ckanext/tabledesigner/templates/package/resource_read.html b/ckanext/tabledesigner/templates/package/resource_read.html
index 01e16a319..008d1c88a 100644
--- a/ckanext/tabledesigner/templates/package/resource_read.html
+++ b/ckanext/tabledesigner/templates/package/resource_read.html
@@ -1,6 +1,9 @@
 {% ckan_extends %}
 
 {% block data_preview %}
+
+  {% asset "ckanext-tabledesigner/tabledesigner_css" %}
+
   {% if
       res.url_type == 'tabledesigner'
       and res.datastore_active %}

@amercader amercader merged commit e0eb92a into master May 3, 2024
7 of 8 checks passed
@amercader amercader deleted the 6118-table-designer branch May 3, 2024 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Spec: Table Designer Feature
5 participants