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

COG business rules: isPrimary and isSubstrate #5275

Merged
merged 177 commits into from
Nov 5, 2024
Merged

COG business rules: isPrimary and isSubstrate #5275

merged 177 commits into from
Nov 5, 2024

Conversation

sharadsw
Copy link
Contributor

@sharadsw sharadsw commented Sep 13, 2024

Fixes #5246

Business Logic:

  • In each COG, isPrimary and isSubstrate can only be set to True for a single CO.
  • COJOs that link to a child COG instead of a child CO cannot check either the isPrimary or isSubstrate fields.
    • (Will need to be done with conditional forms)
  • When checked, IsPrimary is set to True for one CO in a COG. All other COJO records will have IsPrimary set to False.
  • When no CO has IsPrimary set to True, it is NULL for all COJO records in the COG.
    • Default behavior?
  • The first COJO CO will automatically have isPrimary set to True when the COG type is 'consolidated'

NOTE:
This PR adds the possibility to do an empty value check in conditional rendering using the keyword _EMPTY. This was already possible if we use an incomplete condition like <rows condition="childcog="> but it's an unintentional feature and may not seem very clear to users. Empty value checks can now be done with a condition like this:

<rows condition="childcog=_EMPTY">
...
</rows>

The condition will hold true whenever a relation is null or a field has a blank value.

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

Use following viewdefs

  1. Import these viewdefs: https://github.com/user-attachments/files/17590977/better.common.views.xml.zip
  2. Update the COG and COJO viewdef to the ones below

COG viewdef

<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>Collection Object Group Form</desc>
	<enableRules/>
	<columnDef>100px,2px,178px,5px,108px,2px,130px,5px,100px,2px,195px,0px,15px,p:g</columnDef>
	<columnDef os="lnx">115px,2px,196px,2px,113px,2px,150px,2px,115px,5px,215px,0px,15px,p:g</columnDef>
	<columnDef os="mac">130px,2px,216px,2px,133px,2px,170px,2px,135px,5px,235px,0px,15px,p:g</columnDef>
	<columnDef os="exp">p,2px,p:g,2px,p:g(2),5px,p:g,2px,p:g(2),5px:g,p,2px,p:g,0px,p,p:g</columnDef>
	<rowDef auto="true" cell="p" sep="2px"/>
	<rows>
		<row>
			<cell type="label" labelfor="name"/>
			<cell type="field" id="name" name="name" uitype="text"/>
			<cell type="label" label="Type"/>
			<cell type="field" id="cogType" name="cogType" uitype="combobox"/>
			<cell type="label" label="Parent COG"/>
			<cell type="field" id="parentCojo" name="parentCojo" uitype="querycbx" readonly="true"/>
		</row>
		<row>
			<cell type="subview" id="cojo" name="children" uitype="subview" isrequired="false" defaulttype="table" colspan="13"/>
		</row>
	</rows>
</viewdef>

COJO viewdef

<viewdef name="CollectionObjectGroupJoin" class="edu.ku.brc.specify.datamodel.CollectionObjectGroupJoin" type="form" gettable="edu.ku.brc.af.ui.forms.DataGetterForObj" settable="edu.ku.brc.af.ui.forms.DataSetterForObj">
	<desc>The Collection Object Group Join Form</desc>
	<enableRules/>
	<columnDef>50px,2px,30px,5px,50px,2px,30px,5px,50px,2px,100px,0px,50px,2px,100px,p:g</columnDef>
	<rowDef auto="true" cell="p" sep="2px"/>
	<rows>
		<row>
			<!-- 			<cell type="label" labelfor="1"/>
			<cell type="field" id="1" name="isPrimary" uitype="checkbox" default="false"/>
			<cell type="label" labelfor="2"/>
			<cell type="field" id="2" name="isSubstrate" uitype="checkbox" default="false"/> -->
			<!--<cell type="label" labelfor="3"/>
      <cell type="field" id="3" name="precedence" uitype="text"/>-->
			<cell type="label" labelfor="5"/>
			<cell type="field" id="5" name="childCog" uitype="querycbx" readonly="true" colspan="3"/>
			<cell type="label" labelfor="6"/>
			<cell type="field" id="6" name="childCo" uitype="querycbx" readonly="true" colspan="3"/>
		</row>
		<row>
			<cell type="separator" label="Child Attributes" colspan="20"/>
		</row>
		<row>
			<cell type="panel" colspan="20">
				<rows>
					<row/>
					<!-- Show nothing if no Collection Object Group (COG) or Collection Object (CO) child exists for this COJO record -->
				</rows>
				<rows condition="childCog=_EMPTY">
					<row>
						<cell type="label" labelfor="2" label="isPrimary"/>
						<cell type="field" id="2" name="isPrimary" uitype="checkbox"/>
						<cell type="label" labelfor="3" label="isSubstrate"/>
						<cell type="field" id="3" name="isSubstrate" uitype="checkbox"/>
					</row>
				</rows>
			</cell>
		</row>
		<row>
			<cell type="panel" colspan="15">
				<rows>
					<row/>
					<!-- Show nothing if no Collection Object Group (COG) or Collection Object (CO) child exists for this COJO record -->
				</rows>
				<rows condition="childCo=_EMPTY">
					<row>
						<cell type="label" labelfor="2" label="isPrimary"/>
						<cell type="field" id="2" name="isPrimary" uitype="checkbox" readonly="true"/>
						<cell type="label" labelfor="3" label="isSubstrate"/>
						<cell type="field" id="3" name="isSubstrate" uitype="checkbox" readonly="true"/>
					</row>
				</rows>
			</cell>
		</row>
		<!-- 		<row>
			<cell type="label" labelfor="7"/>
			<cell type="field" id="7" name="version" uitype="text"/>
			<cell type="label" labelfor="8"/>
			<cell type="field" id="8" name="timestampCreated" uitype="text"/>
			<cell type="label" labelfor="9"/>
			<cell type="field" id="9" name="timestampModified" uitype="text"/>
		</row>
		<row>
			<cell type="label" labelfor="10"/>
			<cell type="field" id="10" name="text1" uitype="text"/>
			<cell type="label" labelfor="11"/>
			<cell type="field" id="11" name="text2" uitype="text"/>
			<cell type="label" labelfor="12"/>
			<cell type="field" id="12" name="text3" uitype="text"/>
		</row>
		<row>
			<cell type="label" labelfor="13"/>
			<cell type="field" id="13" name="integer1" uitype="text"/>
			<cell type="label" labelfor="14"/>
			<cell type="field" id="14" name="integer2" uitype="text"/>
			<cell type="label" labelfor="15"/>
			<cell type="field" id="15" name="integer3" uitype="text"/>
		</row>
		<row>
			<cell type="label" labelfor="16"/>
			<cell type="field" id="16" name="Yesno1" uitype="checkbox"/>
			<cell type="label" labelfor="17"/>
			<cell type="field" id="17" name="Yesno2" uitype="checkbox"/>
			<cell type="label" labelfor="18"/>
			<cell type="field" id="18" name="Yesno3" uitype="checkbox"/>
		</row> -->
	</rows>
</viewdef>
  • Go to Data Entry > CollectionObjectGroup (navigate through the URL if hidden)
  • Add children to the COG
  • Verify only 1 CO child can be set as isPrimary or isSubstrate
  • Verify isPrimary and isSubstrate are read-only for children that link to COGs
  • Verify isPrimary for the first CO child gets checked when cogtype changes to a Consolidated one (you may have to create COGtypes in your db)

maxpatiiuk and others added 30 commits March 8, 2023 15:50
Triggered by dfaa5d0 on branch refs/heads/issue-114
Triggered by 1359149 on branch refs/heads/issue-114
This is not likely the behavior we want, so this is something to be discussed.
See related #3127
@sharadsw sharadsw changed the base branch from issue-5185 to production November 1, 2024 14:19
@CarolineDenis CarolineDenis requested a review from a team November 1, 2024 14:45
@sharadsw sharadsw marked this pull request as ready for review November 1, 2024 15:50
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.

  • Go to Data Entry > CollectionObjectGroup (navigate through the URL if hidden)
  • Add children to the COG
  • Verify only 1 CO child can be set as isPrimary or isSubstrate
  • Verify isPrimary and isSubstrate are read-only for children that link to COGs
  • Verify isPrimary for the first CO child gets checked when cogtype changes to a Consolidated one (you may have to create COGtypes in your db)

Looks good, I did notice some weird behavior with the isprimary behavior when the cogtype is consolidated. When you change the cogtype to consolidated it does check the first isPrimary box, but if you save and then try to select another isPrimary box it appears like it has selected more than one but when you save again it only selects one. Not sure if this is the expected behavior but it is a little confusing when this happens

chrome_PiiPh5lGKa.mp4

@sharadsw
Copy link
Contributor Author

sharadsw commented Nov 1, 2024

@emenslin Looks like all COG children in that video link to COGs (which means primary/substrate needs to be readonly). Can you try again with this COJO viewdef? With this viewdef, primary and substrate will only be visible when you expand the subview but it should handle the mentioned issues

<viewdef name="CollectionObjectGroupJoin" class="edu.ku.brc.specify.datamodel.CollectionObjectGroupJoin" type="form" gettable="edu.ku.brc.af.ui.forms.DataGetterForObj" settable="edu.ku.brc.af.ui.forms.DataSetterForObj">
	<desc>The Collection Object Group Join Form</desc>
	<enableRules/>
	<columnDef>50px,2px,30px,5px,50px,2px,30px,5px,50px,2px,100px,0px,50px,2px,100px,p:g</columnDef>
	<rowDef auto="true" cell="p" sep="2px"/>
	<rows>
		<row>
			<!-- 			<cell type="label" labelfor="1"/>
			<cell type="field" id="1" name="isPrimary" uitype="checkbox" default="false"/>
			<cell type="label" labelfor="2"/>
			<cell type="field" id="2" name="isSubstrate" uitype="checkbox" default="false"/> -->
			<!--<cell type="label" labelfor="3"/>
      <cell type="field" id="3" name="precedence" uitype="text"/>-->
			<cell type="label" labelfor="5"/>
			<cell type="field" id="5" name="childCog" uitype="querycbx" readonly="true" colspan="3"/>
			<cell type="label" labelfor="6"/>
			<cell type="field" id="6" name="childCo" uitype="querycbx" readonly="true" colspan="3"/>
		</row>
		<row>
			<cell type="separator" label="Child Attributes" colspan="20"/>
		</row>
		<row>
			<cell type="panel" colspan="20">
				<rows>
					<row/>
					<!-- Show nothing if no Collection Object Group (COG) or Collection Object (CO) child exists for this COJO record -->
				</rows>
				<rows condition="childCog=_EMPTY">
					<row>
						<cell type="label" labelfor="2" label="isPrimary"/>
						<cell type="field" id="2" name="isPrimary" uitype="checkbox"/>
						<cell type="label" labelfor="3" label="isSubstrate"/>
						<cell type="field" id="3" name="isSubstrate" uitype="checkbox"/>
					</row>
				</rows>
			</cell>
		</row>
		<row>
			<cell type="panel" colspan="15">
				<rows>
					<row/>
					<!-- Show nothing if no Collection Object Group (COG) or Collection Object (CO) child exists for this COJO record -->
				</rows>
				<rows condition="childCo=_EMPTY">
					<row>
						<cell type="label" labelfor="2" label="isPrimary"/>
						<cell type="field" id="2" name="isPrimary" uitype="checkbox" readonly="true"/>
						<cell type="label" labelfor="3" label="isSubstrate"/>
						<cell type="field" id="3" name="isSubstrate" uitype="checkbox" readonly="true"/>
					</row>
				</rows>
			</cell>
		</row>
		<!-- 		<row>
			<cell type="label" labelfor="7"/>
			<cell type="field" id="7" name="version" uitype="text"/>
			<cell type="label" labelfor="8"/>
			<cell type="field" id="8" name="timestampCreated" uitype="text"/>
			<cell type="label" labelfor="9"/>
			<cell type="field" id="9" name="timestampModified" uitype="text"/>
		</row>
		<row>
			<cell type="label" labelfor="10"/>
			<cell type="field" id="10" name="text1" uitype="text"/>
			<cell type="label" labelfor="11"/>
			<cell type="field" id="11" name="text2" uitype="text"/>
			<cell type="label" labelfor="12"/>
			<cell type="field" id="12" name="text3" uitype="text"/>
		</row>
		<row>
			<cell type="label" labelfor="13"/>
			<cell type="field" id="13" name="integer1" uitype="text"/>
			<cell type="label" labelfor="14"/>
			<cell type="field" id="14" name="integer2" uitype="text"/>
			<cell type="label" labelfor="15"/>
			<cell type="field" id="15" name="integer3" uitype="text"/>
		</row>
		<row>
			<cell type="label" labelfor="16"/>
			<cell type="field" id="16" name="Yesno1" uitype="checkbox"/>
			<cell type="label" labelfor="17"/>
			<cell type="field" id="17" name="Yesno2" uitype="checkbox"/>
			<cell type="label" labelfor="18"/>
			<cell type="field" id="18" name="Yesno3" uitype="checkbox"/>
		</row> -->
	</rows>
</viewdef>

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.

  • Go to Data Entry > CollectionObjectGroup (navigate through the URL if hidden)
  • Add children to the COG
  • Verify only 1 CO child can be set as isPrimary or isSubstrate
  • Verify isPrimary and isSubstrate are read-only for children that link to COGs
  • Verify isPrimary for the first CO child gets checked when cogtype changes to a Consolidated one (you may have to create COGtypes in your db)

When cogtype changes to a consolidated one and all the children are COGs (all childCo are empty), isPrimary still gets checked for the first one. I believe this is the incorrect behavior but it is a little confusing so please correct me if I am wrong.

chrome_Px2WisygVH.mp4

@sharadsw
Copy link
Contributor Author

sharadsw commented Nov 1, 2024

Thanks! Pushed a fix for that bug

@pashiav pashiav requested a review from a team November 4, 2024 14:12
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.

  • Go to Data Entry > CollectionObjectGroup (navigate through the URL if hidden)
  • Add children to the COG
  • Verify only 1 CO child can be set as isPrimary or isSubstrate
  • Verify isPrimary and isSubstrate are read-only for children that link to COGs
  • Verify isPrimary for the first CO child gets checked when cogtype changes to a Consolidated one (you may have to create COGtypes in your db)

From #5275 (review):

  • When cogtype changes to a consolidated one and all the children are COGs (all childCo are empty), isPrimary still gets checked for the first one. I believe this is the incorrect behavior but it is a little confusing so please correct me if I am wrong.
    —Fixed!

Everything looks good!

@pashiav pashiav requested a review from a team November 4, 2024 15:43
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.

  • Go to Data Entry > CollectionObjectGroup (navigate through the URL if hidden)
  • Add children to the COG
  • Verify only 1 CO child can be set as isPrimary or isSubstrate
  • Verify isPrimary and isSubstrate are read-only for children that link to COGs
  • Verify isPrimary for the first CO child gets checked when cogtype changes to a Consolidated one (you may have to create COGtypes in your db)

Looks good, the previous issue is now fixed!

@sharadsw sharadsw merged commit e3cd0bc into production Nov 5, 2024
12 checks passed
@sharadsw sharadsw deleted the issue-5246 branch November 5, 2024 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Add business logic for isPrimary and isSubstrate in COGs
8 participants