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

Treat configurable embedded relationships as to-one when applicable #6193

Open
wants to merge 6 commits into
base: production
Choose a base branch
from

Conversation

melton-jason
Copy link
Contributor

@melton-jason melton-jason commented Feb 4, 2025

Fixes #6154

There are two possible relationships which can be configured as embedded:

  • CollectionObject -> collectingEvent
    • Dependent on the Collection -> IsEmbeddedCollectingEvent field

And one of:

  • CollectionObject -> paleoContext
  • Locality -> paleoContext
  • CollectingEvent -> paleoContext

This is dependent on the Discipline -> IsPaleoContextEmbedded and Discipline -> PaleoContextChildTable fields.

For a more information, view the related Speciforum docs: https://discourse.specifysoftware.org/t/shared-vs-embedded-records-configuration/1712

The datamodel however still has a static definition of the relationship type: e.g., CollectingEvent -> collectionObjects is always considered a one-to-many relationship regardless of whether CollectionObject -> collectingEvent is embedded or not (the same applies for the PaleoContext relationships).
This means that without changes to how the datamodel is constructed/defined, it is up to the application to enforce that only one related record can be added through the reverse of an embedded to-one relationship.

This PR makes it so that after fetching the domain and datamodel information, the frontend overrides the relationship type for the reverse of embedded relationships - from one-to-many to zero-to-one.
I used zero-to-one inestead of one-to-one because the frontend ORM still treats the realtionships as to-many when sending requests to the backend: there are semantic differences between to-one and to-many relationships, and the backend still expects these to be formatted as to-many.

This change is a global change on the frontend! Similar changes can be made to the backend if needed, but none are included in this PR at the time of writing this.

Checklist

  • Self-review the PR after opening it to make sure the changes look good and
    self-explanatory (or properly documented)
  • Add automated tests
  • Add relevant issue to release milestone
  • Add relevant documentation (Tester - Dev)

Testing instructions

  • Find a database which has Collection -> IsEmbeddedCollectingEvent equal to True (or edit the Collection to make CollectingEvents embedded)

  • Create a new CollectionObject with a CollectingEvent, or find a CollectingEvent which is already associated with a CollectionObject

  • Show the collectionObjects relationship on the CollectingEvent form via a Subview

  • Ensure you can not add another CollectionObject to the CollectingEvent through any means (the plus button, the query button, or via subview or querycombobox from CollecitonObject -> collectingEvent)

  • Ensure you can still upload CollectingEvents via the workbench, and that only one CollectionObject is permitted to be mapped through the collectionObjects relationship

  • In a version of Specify which is not this branch (e.g., production), ensure that a WorkBench mapping using CollectingEvent as a base table which includes the collectionObjects relationship created on this branch preserves its mapping and can still be uploaded

  • Find a database which has Discipline -> IsPaleoContextEmbedded equal to True (or edit the Discipline to make PaleoContexts embedded)

  • For future use, note the Discipline's PaleoContextChildTable

    • This means the <PaleoContextChildTable> -> paleoContext relationship should not be shared (i.e., embedded)
  • Create a new record of the PaleoContextChildTable, or find a PaleoContext record which is already associated with a PaleoContextChildTable record

  • Show the reverse of the <PaleoContextChildTable> -> paleoContext relationship (and at least one other of the possible relationships) on the PaleoContext form via a Subview

    • The possible relationships are:
      • PaleoContext -> collectingEvents
      • PaleoContext -> collectionObjects
      • PaleoContext -> localities
  • Ensure you can not add another PaleoContextChildTable to the PaleoContext through any means (the plus button, the query button, or via subview or querycombobox from PaleoContextChildTable -> paleoContext)

  • Ensure the other relationships on PaleoContext which are not embdedded (collectingEvents, collectionObjects, localities) are not restricted and can still associate with more than one of the PaleoContextChildTable records

  • Ensure you can still upload PaleoContexts via the workbench, and that only one PaleoContextChildTable is permitted to be mapped through the corresponding relationship

  • In a version of Specify which is not this branch (e.g., production), ensure that a WorkBench mapping using PaleoContext as a base table which includes the embdedded relationship created on this branch preserves its mapping and can still be uploaded

@melton-jason melton-jason added this to the 7.10 milestone Feb 4, 2025
@melton-jason melton-jason requested review from a team February 4, 2025 01:47
Copy link
Collaborator

@emenslin emenslin left a comment

Choose a reason for hiding this comment

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

  • Ensure you can not add another CollectionObject to the CollectingEvent through any means (the plus button, the query button, or via subview or querycombobox from CollecitonObject -> collectingEvent)

  • Ensure you can still upload CollectingEvents via the workbench, and that only one CollectionObject is permitted to be mapped through the collectionObjects relationship

  • Ensure you can not add another PaleoContextChildTable to the PaleoContext through any means (the plus button, the query button, or via subview or querycombobox from PaleoContextChildTable -> paleoContext)

  • Ensure the other relationships on PaleoContext which are not embdedded (collectingEvents, collectionObjects, localities) are not restricted and can still associate with more than one of the PaleoContextChildTable records

  • Ensure you can still upload PaleoContexts via the workbench, and that only one PaleoContextChildTable is permitted to be mapped through the corresponding relationship


Cannot add a CO to a CE

02-04_09.31.mp4

Also I was a bit confused on the paleo context section, could you maybe reword some of the checks or provide more information so I can make sure I properly test them?

Also I could not get this db to load in production so I removed those checks but when I retest this later I will go back through and verify that is working.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Dev Attention Needed
Development

Successfully merging this pull request may close these issues.

Multiple COs can be added to an embedded CE using CO subview
5 participants