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

Mutliple datatypes by property in template, and multiple properties with different names (laminas) #1597

Closed
wants to merge 37 commits into from

Conversation

Daniel-KM
Copy link
Contributor

Rebased on develop (#1595), to implement 1 and 2 of #1565.

@zerocrates
Copy link
Member

This one is the basis for all the other template-related PRs you're doing, correct?

So, to clarify what this is doing:

  • it changes dataType from a simple string to a JSON column, to allow each property to support multiple data types
  • and it removes the unique constraint from the (resource_template_id, property_id) pair so you can specify more than one "template property" for the same actual underlying property within a single template
  • to make the reverse lookup work with this, you have the resource template API validate the template so that no data type can appear in multiple "template properties" for the same property, so when looking at a stored value, you find which property template to apply by looking and both the property ID and the data type of the value
  • there's also a somewhat separate feature of altering the set of "default" types that can be chosen among for properties with no data type restriction

Do I have that right? Do the specific use cases you're targeting need both of those things, or just one? I could see either existing on its own, or together as you have them.

Since this really touches the metadata system for Omeka S fairly directly, I think I have to defer to, or at least solicit input from, @jimsafley on the desirability of these features. I do believe his Numeric Data Types module is the only one I'm aware of that would encounter breakage from this: I believe it currently looks at the database to find which properties are using the numeric types.

I don't have any major immediate remaining issues on the implementation side in general... I'm a little concerned about the Javascript side of things, though: it looks like you're matching up fields to newly-applied templates based on whether the set of data types is a complete match... this seems like it would result in "orphaned" fields when switching between templates that don't agree in their groupings of the types, in a way that doesn't match how the system will end up matching those values. With the features you have here I don't see a way around having to possibly move values from one field to another when switching templates, which unless I'm misreading you don't seem to have a provision for. I'm also not seeing what exactly happens with the "new value" UI when a multi-type template is set for a property, but I may just be missing that somehow.

@Daniel-KM Daniel-KM mentioned this pull request Aug 26, 2020
10 tasks
@Daniel-KM
Copy link
Contributor Author

Daniel-KM commented Aug 27, 2020

In fact, there are two main improvements to support #1565: multiple data types and template settings. They are not related, but for simplicity, I based settings improvements on this pr, but I can rebase the second on develop. The same for the "create a new item during editing an item" #1608, it's not related (and I don't based it on it anyway).

Ideally, I prefer to work on modules (it's quicker, lighter and it's easy with Omeka), but for the data types, I can't. For the support of settings, I think it is possible to move it in a module, but it is simpler if Omeka supports template settings natively. The same for language selector in resource form: if settings are managed (#1610), it can be a separated module.

About data types, the feature comes from the need to have dcterms:subject and dcterms:creator from multiple databases with value suggest (omeka-s-modules/ValueSuggest#39). But the use case is larger, since we need to let user choose not only multiple value suggest endpoints, but for a literal or a uri too.

The point is only to remove all the code that forces to have only one datatype by template property and unique properties by template, and there are many places for that. So it explains the fact that there are many changes. I didn't checked numeric data types, because it is a module and it can be updated according to the core, but I think it's working as it. Anyway a good time to do it with laminas upgrade.

  • it changes dataType from a simple string to a JSON column, to allow each property to support multiple data types

Yes. It can be a separate table, but it was simpler to do it with a json.

  • and it removes the unique constraint from the (resource_template_id, property_id) pair so you can specify more than one "template property" for the same actual underlying property within a single template

Yes, it allows to support complex templates.

  • to make the reverse lookup work with this, you have the resource template API validate the template so that no data type can appear in multiple "template properties" for the same property, so when looking at a stored value, you find which property template to apply by looking and both the property ID and the data type of the value

Yes, this is the key feature that allows to keep compatibility with current working process and to forbid some non-sense templates.

  • there's also a somewhat separate feature of altering the set of "default" types that can be chosen among for properties with no data type restriction

The default type may appear in the multi-select template property form when loaded, but it does not exist. This is the same than current setting: the choice is the default text/uri/resource, but in fact, there is a new main setting that let admin define the default list of data types, where it is possible to add another data type. This point can be separated from this pr.

Anyway, it only changes the form in the admin board, but not in public interface.

About the js, when a template is applied, all values follow the rules in the templates, but if there is no template property (orphaned), it is reset to the default state, like if the user selected a new property from the right sidebar. If there is a template property but with different data type, the value is kept as it. It works fine because the data type cannot be duplicated with the same property in a template, so this is the key feature to keep current working process. No data is lost when switching any template, but empty default values are removed.

Of course, anything can be improved if needed.

@Daniel-KM
Copy link
Contributor Author

I published a version of the resource template settings independent from this pull request in #1614 .

Daniel Berthereau and others added 16 commits September 3, 2020 00:00
@Daniel-KM
Copy link
Contributor Author

I reset this pr and divided it into multiple branches, so here is now only a merge of them. Only the resource form is based on the settings js, except all branches are independant.

Features are the same: to allow multiple data types by property (for example to allow to fill as a literal a creator who is not present in a value suggest database); to allow duplicate properties (except with same datatype), so it's possible to have a literal spatial with a specific label and a geonames one with another one.

@Daniel-KM
Copy link
Contributor Author

A simpler version (without multiple properties) is pr in #1634.

@Daniel-KM
Copy link
Contributor Author

The feature "multiple data types by property" is managed via pr #1634 and the feature "multiple labels for property" will be managed directly by the module Advanced Resource Template.

@Daniel-KM Daniel-KM closed this Sep 18, 2020
@Daniel-KM Daniel-KM deleted the feature/template_datatypes_3 branch September 18, 2020 13:59
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