search

specify / specify7

Specify 7
https://www.specifysoftware.org/products/specify-7/
GNU General Public License v2.0
66 stars 36 forks source link

Allow right side of HostTaxon to be from any taxon tree #2675

Open maxpatiiuk opened 1 year ago

maxpatiiuk commented 1 year ago

Currently, HostTaxon request explicitly specifying the Collection Relationship name.

Need to add support for omitting relationship name - which should be interpreted as right side being from any collection

melton-jason commented 1 year ago

How would I go about testing this? Does this involve the Host Taxon ID field in the CollectingEventAttribute table?

maxpatiiuk commented 1 year ago

Find a database that has that plugin configured, or configure it (https://discourse.specifysoftware.org/t/create-the-host-taxon-id-field/737)

After you verified that it works, remove relname=PlantHost from the form definition.

Before removing relname=PlantHost, the query combo box would have allowed searching taxa only in the discipline to which the relationship was created. After removing relname=PlantHost, the query combo box should allow searching taxa in any discipline

melton-jason commented 1 year ago

Everything works correctly when relname is defined on the form. However, when relname is missing, the Query ComboBox only searches on the current Discipline's Taxa.

Furthermore, the PURHerb database on the testpanel only has two disciplines. As testing for this continues, I have uploaded a database with three different disciplines and taxon trees (https://issue2675-issue-2675.test.specifysystems.org/specify/)

Tested on: https://purherb-issue-2675.test.specifysystems.org/specify/view/collectingevent/new/ https://issue2675-issue-2675.test.specifysystems.org/specify/view/collectingevent/new/

maxpatiiuk commented 1 year ago

Fixed

grantfitzsimmons commented 1 year ago
image image

Left Side: Arthur Fungarium Right Side: Host

Abies exists only in the Arthur Fungarium's discipline's tree.

Arthur Fungarium collection:

<cell type="field" id="55" uitype="plugin" name="this" initialize="name=HostTaxonPlugin;"/>
image

From what I understand, this should be searching the taxon tree in the Host collection, but it is only searching the current discipline's tree.

Host collection:

<cell type="field" id="55" uitype="plugin" name="this" initialize="name=HostTaxonPlugin;relname=HostPlant"/>
image
melton-jason commented 1 year ago

I can confirm @grantfitzsimmons findings in https://issue2675-issue-2675.test.specifysystems.org/specify/. When relname in the Plugin on the form definition is not supplied, the Query Combo Box searches only the current discipline's Taxon tree.

maxpatiiuk commented 1 year ago

There is a complication here. Back-end query builder endpoint does not seem to support not-scoping the results - if front-end didn't tell it to scope results to some collection, it scopes them to current collection.

To make it worse, if user does not have permission to some collections, query builder should theoretically only return results from the collections the user has access to, thus, it's not just a matter of disabling scoping completely.

I don't know how high of a priority this issue is for the original requesting institution, but this is much larger in scope than we expected. I belive it's lower priority than the business rule improvements and other changes @melton-jason is doing right now.

The simplest (though not very satisfying solution) would be to only allow HostTaxon to not be scoped to discipline for users that have read access to Taxon tree in all collections. In that case, we can just completely disable scoping for the query builder (easier than scoping to several collections)

melton-jason commented 1 year ago

If the requesting institution requires these changes, I wouldn't be against pursuing the 'simple' solution: disable scoping for the Query Builder. Do you mean de-scoping the Query Builder for this specific instance, or as a whole? If we do entirely de-scope the Query Builder, we would probably need preferences or some other way to filter results by collection, discipline, etc. Do we have the infrastructure in place for viewing records from a separate discipline/collection?

maxpatiiuk commented 1 year ago

Just this case. Currently, query builder endpoint accepts a collection id. If it's not provided, it scopes to current collection - good, let's leave it like that as it's the most common case. Instead, either add a magic value (i.e, -1) or a separate field scoping: false) that runs unscoped query (but not before checking the permissions in all collections - every place where query builder checks for permissions needs to be refactored to instead accept an array of collection ids.

If we do entirely de-scope the Query Builder, we would probably need preferences or some other way to filter results by collection, discipline, etc.

Not doing this

Do we have the infrastructure in place for viewing records from a separate discipline/collection?

No. We talked about it. Conclusion: very hard, out of scope. not possible while we support sp6

melton-jason commented 1 year ago

Ok, that is still a little work but infinitely more manageable! We can probably get some feedback from the institution that requested this regarding this solution/implementation and proceed from there.

grantfitzsimmons commented 1 year ago

Who requested this?

maxpatiiuk commented 1 year ago

Don't know. Theresa would know

grantfitzsimmons commented 1 year ago

This should be fixed by https://github.com/specify/specify7/pull/3431 once it is finished

emenslin commented 3 months ago

I was not able to remove relname from the form definition without it throwing an error and the field becomes invalid. This is what happened when I tried to remove the attribute:

https://github.com/user-attachments/assets/51001a9f-083f-4910-8c52-1b3691a83356

The original issue seems to be fixed but I'll leave this open for now as there is a lot of information in here and I imagine there is still some sort of underlying problem.