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

Don't store empty repeating subfields #415

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

amercader
Copy link
Member

When implementing repeating subfields without any mandatory subfields, if empty values are sent from the form an "empty" item is stored, with all its properties empties:

{
    "name": "test-dataset",
    "publisher": [
        {
            "email": "",
            "name": "",
            "type": "",
        }
    ]
}

The changes in expand_form_composite() fix the issue on the scheming side, preparing a data_dict with an empty list for that field:

{
    "name": "test-dataset",
    "publisher": []
}

Sadly, on the CKAN side the navl functions drop the field entirely (somewhere along make_full_schema and get_all_key_combinations) and it doesn't get stored as an extra, so it's missing from the resulting dataset dict.

{
    "name": "test-dataset",
}

@wardi Ideally we would want an empty list in the resulting dataset, but do you think a missing field is an improvement?

When implementing repeating subfields without any mandatory subfields,
if empty values are sent from the form an "empty" item is stored, with
all its properties empties:

```
{
    "name": "test-dataset",
    "publisher": [
        {
            "email": "",
            "name": "",
            "type": "",
        }
    ]
}
```

The changes in `expand_form_composite()` fix the issue on the scheming
side, preparing a `data_dict` with an empty list for that field:

```
{
    "name": "test-dataset",
    "publisher": []
}
```

Sadly, the navl functions drop the field entirely and it doesn't get
stored as an extra, so it's missing from the resulting dataset dict

```
{
    "name": "test-dataset",
}
```
@wardi
Copy link
Contributor

wardi commented Jun 20, 2024

It would be nicer to be able to store an empty list, but a missing value is an improvement over a list with one object with blank fields for sure.

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.

2 participants