Skip to content

Add Datamodel and Django Model definitions #4764

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

Merged
merged 64 commits into from
Jun 14, 2024
Merged

Add Datamodel and Django Model definitions #4764

merged 64 commits into from
Jun 14, 2024

Conversation

acwhite211
Copy link
Member

@acwhite211 acwhite211 commented Apr 9, 2024

Fixes #4358 and #2813

This PR adds datamodel and Django model code definitions, instead of relying on reading the 'specify_datamodel.xml' file from Specify 6. So, the schema is now statically defined in our python Django code, instead of dynamically be created at runtime. There is also code that can generated the static python code from the 'specify_datamodel.xml', so we can regenerate the code again in the future easily if we need to. Unit tests have been created to ensure the equivalence between the old and new datamodels. There was another branch django-schema that had a lot of code from the development process, but I cleaned it up and simplified it into this branch. There were some problems with adding the sql alchemy schema model classes, this isn't a pressing matter so this is something we can do in the future.

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

Testing instructions

Unfortunately there isn't anything specific to test. I would say the best way to go about testing this from the UX side is to play around with forms and queries, and make sure no database table or field related errors occur. Also, for the sp7_only tables, test them out in the query builder.

sp7_only tables:

  • Spuserexternalid
  • Spattachmentdataset
  • UniquenessRule
  • UniquenessRuleField
  • Message
  • Spmerging
  • UserPolicy
  • Role
  • LibraryRole
  • UserRole
  • RolePolicy
  • LibraryRolePolicy
  • Spdataset

PR Testing Checklist:

  • QB works on tables like Agent, Collecting Object, and Locality
  • QB works on the new sp7_only tables
  • The Database Schema page works without errors (UserTools->DatabaseSchema) localhost/specify/data-model
  • Test out simple agent merging to make sure it still works properly

@acwhite211 acwhite211 added the 2 - Database/Schema Issues that are related to the underlying database and schema label Apr 9, 2024
@acwhite211 acwhite211 added this to the 7.9.6 milestone Apr 9, 2024
@acwhite211 acwhite211 self-assigned this Apr 9, 2024
@acwhite211 acwhite211 marked this pull request as draft April 9, 2024 17:21
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.

Testing instructions

Unfortunately there isn't anything specific to test. I would say the best way to go about testing this from the UX side is to play around with forms and queries, and make sure no database table or field related errors occur. Also, for the sp7_only tables, test them out in the query builder.

sp7_only tables:

  • Spuserexternalid
  • Spattachmentdataset
  • UniquenessRule
  • UniquenessRuleField
  • Message
  • Spmerging
  • UserPolicy
  • Role
  • LibraryRole
  • UserRole
  • RolePolicy
  • LibraryRolePolicy
  • Spdataset

PR Testing Checklist:

  • QB works on tables like Agent, Collecting Object, and Locality
  • QB works on the new sp7_only tables
  • The Database Schema page works without errors (UserTools->DatabaseSchema) localhost/specify/data-model
  • Test out simple agent merging to make sure it still works properly

Query does not work on Spattachmentdataset

KPc3gvrf3N.mp4

Specify 7 Crash Report - 2024-06-11T17_15_46.725Z.txt

Another issue with LibraryRolePolicy

kP77G78M0u.mp4

Specify 7 Crash Report - 2024-06-11T17_21_55.182Z.txt

Other than those 2 tables every other sp7_only table worked for me

@Areyes42 Areyes42 self-requested a review June 11, 2024 20:59
Copy link
Contributor

@Areyes42 Areyes42 left a comment

Choose a reason for hiding this comment

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

Testing instructions

Unfortunately there isn't anything specific to test. I would say the best way to go about testing this from the UX side is to play around with forms and queries, and make sure no database table or field related errors occur. Also, for the sp7_only tables, test them out in the query builder.

sp7_only tables:

  • Spuserexternalid
  • Spattachmentdataset
  • UniquenessRule
  • UniquenessRuleField
  • Message
  • Spmerging
  • UserPolicy
  • Role
  • LibraryRole
  • UserRole
  • RolePolicy
  • LibraryRolePolicy
  • Spdataset

PR Testing Checklist:

  • QB works on tables like Agent, Collecting Object, and Locality
  • QB works on the new sp7_only tables
  • The Database Schema page works without errors (UserTools->DatabaseSchema) localhost/specify/data-model
  • Test out simple agent merging to make sure it still works properly

I was able to recreate the issue with Spattachmentdataset as @emenslin mentioned.
Screenshot 2024-06-11 at 3 28 45 PM

Also, I get an 'Unable to find SpLocaleConatiner for Role' error when trying to access any of the sp7_only tables in Schema Config
Screenshot 2024-06-11 at 3 48 30 PM
Specify 7 Crash Report - 2024-06-11T21_03_51.256Z.txt

Lastly, there is a small typo on the default label for description on LibraryRole
Screenshot 2024-06-11 at 4 19 19 PM

@acwhite211
Copy link
Member Author

Thanks @specify/testing, the issues found can be re-tested

  • Spattachmentdataset QB fix data field
  • Description typo in LibraryRole

This issue will be fixed in a later PR

  • 'Unable to find SpLocaleConatiner for Role' error when trying to access any of the sp7_only tables in Schema Config

@realVinayak
Copy link
Contributor

realVinayak commented Jun 13, 2024

^Could you add a unit test for the dataset? Too encourage it, it is not too complicated. just a couple of lines will do.
Here, test_create_record_set in DataSetTests, use the test_session_context. see usage test_collection_object_count for reference.

with test_session_context() as session:
     query = session.query(models.Spdataset.data).limit(1) # this is SQL model (not django), do the import approriately
     assert list(query) = ["[["1"], ["2"], ["3"]]"]

that's it^

EDIT: In retrorespect, the above won't work (bc the JSON serializer is specific to query builder). Will need to add the column after passing it through _fieldformat. See FormatterAggregatorTests and utilize it

@grantfitzsimmons
Copy link
Member

Some possible table descriptions (feel free to make edits for accuracy/clarification):


SpUserExternalId
Stores provider identifiers and tokens for users who sign in using Single Sign On (SSO).

SpAttachmentDataSet
Holds attachment data sets.

UniquenessRule
Stores table names in the data model that have uniqueness rules configured for each discipline.

UniquenessRuleField
Stores field names in the data model that have uniqueness rules configured for each discipline, linked to UniquenessRule records.

NotificationsMessage
Stores user notifications.

SpMerging
Tracks record and task IDs of records being merged.

SpUserPolicy
Records permissions for a user within a collection.

SpUserRole
Records roles associated with Specify users.

SpRole
Stores names, descriptions, and collection information for user-created roles.

SpRolePolicy
Stores resource and action permissions for user-created roles within a collection.

SpLibraryRole
Stores names and descriptions of default roles that can be added to any collection.

SpLibraryRolePolicy
Stores resource and action permissions for library roles within a collection.

SpDataSet
Stores Specify Data Sets created during bulk import using the WorkBench, typically through spreadsheet uploads.


Something to consider– we probably shouldn't let anyone aside from institution admins edit the spuser* tables to prevent full access users from giving themselves administrative permissions, even if they have permission to create, edit, delete in all tables.

Copy link
Collaborator

@lexiclevenger lexiclevenger left a comment

Choose a reason for hiding this comment

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

PR Testing Checklist:

  • QB works on tables like Agent, Collecting Object, and Locality
  • QB works on the new sp7_only tables
  • The Database Schema page works without errors (UserTools->DatabaseSchema) localhost/specify/data-model
  • Test out simple agent merging to make sure it still works properly

  • Spattachmentdataset QB fix data field
  • Description typo in LibraryRole

Looks great!

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.

Testing instructions

Unfortunately there isn't anything specific to test. I would say the best way to go about testing this from the UX side is to play around with forms and queries, and make sure no database table or field related errors occur. Also, for the sp7_only tables, test them out in the query builder.

sp7_only tables:

  • Spuserexternalid
  • Spattachmentdataset
  • UniquenessRule
  • UniquenessRuleField
  • Message
  • Spmerging
  • UserPolicy
  • Role
  • LibraryRole
  • UserRole
  • RolePolicy
  • LibraryRolePolicy
  • Spdataset

PR Testing Checklist:

  • QB works on tables like Agent, Collecting Object, and Locality
  • QB works on the new sp7_only tables
  • The Database Schema page works without errors (UserTools->DatabaseSchema) localhost/specify/data-model
  • Test out simple agent merging to make sure it still works properly

Everything within this pr seems to be working great!

@Areyes42 Areyes42 self-requested a review June 13, 2024 19:36
Copy link
Contributor

@Areyes42 Areyes42 left a comment

Choose a reason for hiding this comment

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

Testing instructions

PR Testing Checklist:

  • QB works on tables like Agent, Collecting Object, and Locality
  • QB works on the new sp7_only tables
  • The Database Schema page works without errors (UserTools->DatabaseSchema) localhost/specify/data-model
  • Test out simple agent merging to make sure it still works properly
  • Spattachmentdataset QB fix data field
  • Description typo in LibraryRole

Retested, everything looks good now! 🤸

@acwhite211 acwhite211 merged commit 4c0e0c8 into production Jun 14, 2024
9 checks passed
@acwhite211 acwhite211 deleted the issue-4358 branch June 14, 2024 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 - Database/Schema Issues that are related to the underlying database and schema
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Django Schema Independence
8 participants