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

Update Schema Config to include new geo fields and COG picklists #5257

Merged
merged 28 commits into from
Oct 23, 2024

Conversation

sharadsw
Copy link
Contributor

@sharadsw sharadsw commented Sep 3, 2024

Fixes #5215
Fixes #5289
Fixes #5292
Fixes #5293

NOTE: COG -> cojo has been removed and COG -> childCojos is renamed to COG -> children

This migration updates the data model for COG and Schema Config entries for pre-geo tables and creates picklists for COGTypes.

Data model changes:
Removed COG -> cojo
Added COG -> children

Schema Config changes:
Added StorageTreeDef -> institution
Added COG -> children
Removed CollectionObject -> collectionObjectType (duplicate)
Removed COG -> cojo

Creates a picklist for COGType -> type and updates an existing incorrect picklist for COG -> COGType

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

Test new geo fields

StorageTreeDef -> institution
CollectionObjectGroup -> children
  • Verify above fields exist in Schema Config

Test COG picklists in Workbench

  • Create a workbench dataset with base table CollectionObjectGroup
  • Map a column to CogType -> name and to CogType -> type
  • Click save
  • Verify CogType -> name consists of all CogTypes added in the db as table records
  • Verify CogType -> type consists of values: Discrete, Consolidated, Drill Core

Test COG picklists in Data Entry

Here are some viewdefs that can be used to test the picklists in data entry

CollectionObjectGroup

<viewdef name="CollectionObjectGroup" class="edu.ku.brc.specify.datamodel.CollectionObjectGroup" type="form" gettable="edu.ku.brc.af.ui.forms.DataGetterForObj" settable="edu.ku.brc.af.ui.forms.DataSetterForObj">
	<desc>The CollectionObjectGroup Table</desc>
	<enableRules/>
	<columnDef>p,2px,p,2px,p,2px,p,2px,p,2px,p,p:g</columnDef>
	<rowDef auto="true" cell="p" sep="2px"/>
	<rows>
		<row>
			<cell type="label" labelfor="1"/>
			<cell type="field" id="1" name="name" uitype="text"/>
			<cell type="label" labelfor="2"/>
			<cell type="field" id="2" name="cogtype" uitype="combobox"/>
		</row>
	</rows>
</viewdef>

CollectionObjectGroupType

<viewdef name="CollectionObjectGroupType" class="edu.ku.brc.specify.datamodel.CollectionObjectGroupType" type="form" gettable="edu.ku.brc.af.ui.forms.DataGetterForObj" settable="edu.ku.brc.af.ui.forms.DataSetterForObj">
	<desc>The CollectionObjectGroupType Table</desc>
	<enableRules/>
	<columnDef>p,2px,p,2px,p,2px,p,2px,p,2px,p,p:g</columnDef>
	<rowDef auto="true" cell="p" sep="2px"/>
	<rows>
		<row>
			<cell type="label" labelfor="1"/>
			<cell type="field" id="1" name="name" uitype="text"/>
			<cell type="label" labelfor="2"/>
			<cell type="field" id="2" name="type" uitype="combobox"/>
		</row>
	</rows>
</viewdef>

@sharadsw sharadsw added this to the 7.9.8 milestone Sep 3, 2024
@sharadsw sharadsw changed the base branch from production to issue-5114 September 3, 2024 19:13
@sharadsw sharadsw changed the base branch from issue-5114 to production September 3, 2024 19:14
@sharadsw sharadsw changed the title Update Schema Config to include new fields in existing tables Update Schema Config to include new geo fields in existing tables Sep 3, 2024
@sharadsw
Copy link
Contributor Author

sharadsw commented Sep 9, 2024

TODO: Add Sp7 tables
The tables all look like they're only internally used. I don't think we need to add these to Schema Config

Missing Sp tables in Schema Config:
specifyuser_spprincipal
splibraryrole (added in geo migration)
splibraryrolepolicy (added in geo migration)
spprincipal_sppermission
sprole (added in geo migration)
sprolepolicy (added in geo migration)
spstynthy
spuserpolicy (added in geo migration)
spuserrole (added in geo migration)
sp_schema_mapping

NOTE: Spattachmentdataset already exists in Schema config but the table it refers to in the db is called attachmentdataset

ALL missing tables in Schema Config - Most of these seem to be only be internally used:
attachmentdataset (exists as Spattachmentdataset)
auth_group
auth_group_permissions
auth_permission
autonumsch_coll
autonumsch_div
autonumsch_dsp
collectionobjecttype_collectionobjecttypeid
collectionobjecttype_CollectionObjectTypeID
continentcodes
countryinfo
django_content_type
django_migrations
django_session
dwc_ento
ento_web_portal
exup
geoname
hibernate_unique_key
ios_colobjagents
ios_colobjbio
ios_colobjchron
ios_colobjcnts
ios_colobjgeo
ios_colobjlitho
ios_geogeo_cnt
ios_geogeo_cty
ios_geoloc
ios_geoloc_cnt
ios_geoloc_cty
ios_taxon_pid
localityupdate
localityupdaterowresult
notifications_message
project_colobj
sgrbatchmatchresultitem
sgrbatchmatchresultset
sgrmatchconfiguration
specifyuser_spprincipal
splibraryrole
splibraryrolepolicy
spprincipal_sppermission
sprole
sprolepolicy
spstynthy
spuserpolicy
spuserrole
sp_schema_mapping
uniquenessrule_fields

@sharadsw sharadsw changed the title Update Schema Config to include new geo fields in existing tables Update Schema Config to include SP tables, new geo fields, and COG picklists Sep 12, 2024
@sharadsw sharadsw marked this pull request as ready for review September 12, 2024 16:27
@sharadsw sharadsw changed the title Update Schema Config to include SP tables, new geo fields, and COG picklists Update Schema Config to include new geo fields and COG picklists Sep 12, 2024
@sharadsw
Copy link
Contributor Author

sharadsw commented Sep 30, 2024

TODO: Verify picklist params, check all usages of the default cogtype picklist

specifyweb/specify/migrations/0004_schema_config_update.py Outdated Show resolved Hide resolved
specifyweb/specify/migrations/0004_schema_config_update.py Outdated Show resolved Hide resolved
specifyweb/specify/migrations/0002_geo.py Outdated Show resolved Hide resolved
@sharadsw sharadsw requested review from a team and removed request for a team October 21, 2024 20:35
Base automatically changed from issue-5317 to production October 21, 2024 21:11
@sharadsw sharadsw requested review from emenslin and a team October 22, 2024 16:27
@sharadsw
Copy link
Contributor Author

@emenslin Thank you! Turns out some of those fields were already added in a previous migration. I have removed the duplicate fields and changed the testing instructions. Also, the formatter being invalid is expected. The formatter will be added in #5258

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

Test new geo fields

StorageTreeDef -> institution
CollectionObjectGroup -> children
  • Verify above fields exist in Schema Config

Test COG picklists in Workbench

  • Create a workbench dataset with base table CollectionObjectGroup
  • Map a column to CogType -> name and to CogType -> type
  • Click save
  • Verify CogType -> name consists of all CogTypes added in the db as table records
  • Verify CogType -> type consists of values: Discrete, Consolidated, Drill Core

Test COG picklists in Data Entry

Here are some viewdefs that can be used to test the picklists in data entry

CollectionObjectGroup

<viewdef name="CollectionObjectGroup" class="edu.ku.brc.specify.datamodel.CollectionObjectGroup" type="form" gettable="edu.ku.brc.af.ui.forms.DataGetterForObj" settable="edu.ku.brc.af.ui.forms.DataSetterForObj">
	<desc>The CollectionObjectGroup Table</desc>
	<enableRules/>
	<columnDef>p,2px,p,2px,p,2px,p,2px,p,2px,p,p:g</columnDef>
	<rowDef auto="true" cell="p" sep="2px"/>
	<rows>
		<row>
			<cell type="label" labelfor="1"/>
			<cell type="field" id="1" name="name" uitype="text"/>
			<cell type="label" labelfor="2"/>
			<cell type="field" id="2" name="cogtype" uitype="combobox"/>
		</row>
	</rows>
</viewdef>

CollectionObjectGroupType

<viewdef name="CollectionObjectGroupType" class="edu.ku.brc.specify.datamodel.CollectionObjectGroupType" type="form" gettable="edu.ku.brc.af.ui.forms.DataGetterForObj" settable="edu.ku.brc.af.ui.forms.DataSetterForObj">
	<desc>The CollectionObjectGroupType Table</desc>
	<enableRules/>
	<columnDef>p,2px,p,2px,p,2px,p,2px,p,2px,p,p:g</columnDef>
	<rowDef auto="true" cell="p" sep="2px"/>
	<rows>
		<row>
			<cell type="label" labelfor="1"/>
			<cell type="field" id="1" name="name" uitype="text"/>
			<cell type="label" labelfor="2"/>
			<cell type="field" id="2" name="type" uitype="combobox"/>
		</row>
	</rows>
</viewdef>

CollectionObjectGroup -> children does not show up in the schema config but it is present in the database schema. Everything else looks good though!

Screenshot 2024-10-22 115817

@sharadsw
Copy link
Contributor Author

@emenslin Looks like the migration was either skipped due to it being run already previously or incomplete on certain databases. I have manually reverted the migration on kufish. Should hopefully work if you create a new instance of this branch on the test panel. Can you try testing it with another db as well just in case?

Copy link
Contributor

@pashiav pashiav left a comment

Choose a reason for hiding this comment

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

Test new geo fields

StorageTreeDef -> institution
CollectionObjectGroup -> children
  • Verify above fields exist in Schema Config

Test COG picklists in Workbench

  • Create a workbench dataset with base table CollectionObjectGroup
  • Map a column to CogType -> name and to CogType -> type
  • Click save
  • Verify CogType -> name consists of all CogTypes added in the db as table records
  • Verify CogType -> type consists of values: Discrete, Consolidated, Drill Core

Test COG picklists in Data Entry

Here are some viewdefs that can be used to test the picklists in data entry

CollectionObjectGroup

<viewdef name="CollectionObjectGroup" class="edu.ku.brc.specify.datamodel.CollectionObjectGroup" type="form" gettable="edu.ku.brc.af.ui.forms.DataGetterForObj" settable="edu.ku.brc.af.ui.forms.DataSetterForObj">
	<desc>The CollectionObjectGroup Table</desc>
	<enableRules/>
	<columnDef>p,2px,p,2px,p,2px,p,2px,p,2px,p,p:g</columnDef>
	<rowDef auto="true" cell="p" sep="2px"/>
	<rows>
		<row>
			<cell type="label" labelfor="1"/>
			<cell type="field" id="1" name="name" uitype="text"/>
			<cell type="label" labelfor="2"/>
			<cell type="field" id="2" name="cogtype" uitype="combobox"/>
		</row>
	</rows>
</viewdef>

CollectionObjectGroupType

<viewdef name="CollectionObjectGroupType" class="edu.ku.brc.specify.datamodel.CollectionObjectGroupType" type="form" gettable="edu.ku.brc.af.ui.forms.DataGetterForObj" settable="edu.ku.brc.af.ui.forms.DataSetterForObj">
	<desc>The CollectionObjectGroupType Table</desc>
	<enableRules/>
	<columnDef>p,2px,p,2px,p,2px,p,2px,p,2px,p,p:g</columnDef>
	<rowDef auto="true" cell="p" sep="2px"/>
	<rows>
		<row>
			<cell type="label" labelfor="1"/>
			<cell type="field" id="1" name="name" uitype="text"/>
			<cell type="label" labelfor="2"/>
			<cell type="field" id="2" name="type" uitype="combobox"/>
		</row>
	</rows>
</viewdef>

Great work! I didn't find any issues that were a part of this pr.

Comments:
For future @specify/ux-testing:

  1. Add the form definition for COG in App Resources, or else the name text field will be defaulted into a picklist because of the workbench picklist.
  • @sharadsw: "The field defaults to the workbench picklist created in this PR if the form definition does not explicitly set it as a text field."
  1. "Formatter will be invalid until we implement defaults for geo":
    image

  2. Having multiple COGs with the same name is not reflected in the workbench picklist. WorkBench picklist only displays unique COGtype names, ignoring duplicates #5346
    image
    image

@pashiav pashiav requested a review from a team October 23, 2024 15:29
Copy link
Member

@grantfitzsimmons grantfitzsimmons left a comment

Choose a reason for hiding this comment

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

Test new geo fields

StorageTreeDef -> institution
CollectionObjectGroup -> children
  • Verify above fields exist in Schema Config

Test COG picklists in Workbench

  • Create a workbench dataset with base table CollectionObjectGroup
  • Map a column to CogType -> name and to CogType -> type
  • Click save
  • Verify CogType -> name consists of all CogTypes added in the db as table records
  • Verify CogType -> type consists of values: Discrete, Consolidated, Drill Core

This would have saved me a lot of time before the demo this morning. Nice work!

Test COG picklists in Data Entry

Here are some viewdefs that can be used to test the picklists in data entry

CollectionObjectGroup

<viewdef name="CollectionObjectGroup" class="edu.ku.brc.specify.datamodel.CollectionObjectGroup" type="form" gettable="edu.ku.brc.af.ui.forms.DataGetterForObj" settable="edu.ku.brc.af.ui.forms.DataSetterForObj">
	<desc>The CollectionObjectGroup Table</desc>
	<enableRules/>
	<columnDef>p,2px,p,2px,p,2px,p,2px,p,2px,p,p:g</columnDef>
	<rowDef auto="true" cell="p" sep="2px"/>
	<rows>
		<row>
			<cell type="label" labelfor="1"/>
			<cell type="field" id="1" name="name" uitype="text"/>
			<cell type="label" labelfor="2"/>
			<cell type="field" id="2" name="cogtype" uitype="combobox"/>
		</row>
	</rows>
</viewdef>

CollectionObjectGroupType

<viewdef name="CollectionObjectGroupType" class="edu.ku.brc.specify.datamodel.CollectionObjectGroupType" type="form" gettable="edu.ku.brc.af.ui.forms.DataGetterForObj" settable="edu.ku.brc.af.ui.forms.DataSetterForObj">
	<desc>The CollectionObjectGroupType Table</desc>
	<enableRules/>
	<columnDef>p,2px,p,2px,p,2px,p,2px,p,2px,p,p:g</columnDef>
	<rowDef auto="true" cell="p" sep="2px"/>
	<rows>
		<row>
			<cell type="label" labelfor="1"/>
			<cell type="field" id="1" name="name" uitype="text"/>
			<cell type="label" labelfor="2"/>
			<cell type="field" id="2" name="type" uitype="combobox"/>
		</row>
	</rows>
</viewdef>

A little different, but functional. That was the intention of that issue.

Yes, as long as the field is on the form, which is not unlike existing issues elsewhere (e.g. this error happens when you remove AccessionNumber (or any other required field) from the form and try to save)

image

@grantfitzsimmons
Copy link
Member

specify@91f75de1bd1b:/opt/specify7$ ve/bin/python manage.py migrate specify 0006_fix_tectonic_tree_fields

Operations to perform:
  Target specific migration: 0006_fix_tectonic_tree_fields, from specify
Running migrations:
  Rendering model states... DONE
  Unapplying specify.0007_schema_config_update... OK

It's even reversible. 💯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
8 participants