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

Clear only determination when COType is changing #5201

Merged
merged 18 commits into from
Aug 15, 2024
Merged

Conversation

CarolineDenis
Copy link
Contributor

@CarolineDenis CarolineDenis commented Aug 7, 2024

Fixes #5199
Fixes #5198

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

Fixes #5199

  • Open CO from
  • Add COType
  • Add Determination
  • Add other fields
  • Save
  • Change the COType
  • Verify Determination has been erased but other fields have kept their value

Fixes #5198

  • Open CO from
  • Add COType
  • Add Determination
  • Save
  • Add CE
  • Verify Determination has not been erased

@CarolineDenis CarolineDenis added this to the 7.9.7 milestone Aug 7, 2024
@CarolineDenis CarolineDenis requested review from acwhite211, sharadsw and a team August 7, 2024 16:27
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

  • Open CO from
  • Add COType
  • Add Determination
  • Add other fields
  • Save
  • Change the COType
  • Verify Determination as been erased but other fields have kept their value

Looks good, only determinations are removed when COType is changed

@emenslin emenslin requested a review from a team August 7, 2024 17:02
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.

Testing instructions

  • Open CO from
  • Add COType
  • Add Determination
  • Add other fields
  • Save
  • Change the COType
  • Verify Determination as been erased but other fields have kept their value

Looks good! Only fields from the Determination table are cleared.

Screen.Recording.2024-08-07.at.11.58.12.AM.mov

@lexiclevenger lexiclevenger requested a review from a team August 7, 2024 17:03
Copy link
Contributor

@alesan99 alesan99 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

Fixes #5199

  • Open CO from
  • Add COType
  • Add Determination
  • Add other fields
  • Save
  • Change the COType
  • Verify Determination as been erased but other fields have kept their value

Fixes #5198

  • Go to a record with a 'Collection Object Type'
  • Add a determination if not present
  • Save
  • Add a dependent Collecting Event via the subview
  • See the determination(s) are not deleted

Issue #5199 is fixed but #5198 is still happening to me.
Seems to only happen after a refresh, not when immediately adding a CE after saving.

chrome_O8urjoLJ6v.mp4

EDIT: #5198 is no longer in the scope of this PR, so its working as expected 👍

@CarolineDenis CarolineDenis requested review from melton-jason and removed request for acwhite211 August 12, 2024 16:41
Copy link
Contributor

@melton-jason melton-jason left a comment

Choose a reason for hiding this comment

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

Functionally, this is fine as long as this is acceptable for the requirements. I'd love to see a test for the clearing functionality! 😃

Building a little more on the requirements, I will caution that if it is never acceptable to have determinations on a CollectionObject which are associated to taxon records from a different taxon tree, then this is not a complete solution.
Whether intentional or not, a user can still have determinations which have taxons that do not exist on the collectionObjectType -> taxontreedef.

@combs-a combs-a changed the title Clear only determination when COType is chnaging Clear only determination when COType is changing Aug 14, 2024
Copy link
Collaborator

@combs-a combs-a 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

Fixes #5199

  • Open CO from
  • Add COType
  • Add Determination
  • Add other fields
  • Save
  • Change the COType
  • Verify Determination has been erased but other fields have kept their value
14_funnyturtles.mp4

Fixes #5198

  • Open CO from
  • Add COType
  • Add Determination
  • Save
  • Add CE
  • Verify Determination has not been erased
14_addevent.mp4

Looks good, is working!

@CarolineDenis CarolineDenis merged commit 159cb83 into production Aug 15, 2024
9 checks passed
@CarolineDenis CarolineDenis deleted the issue-5199 branch August 15, 2024 14:20
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.

Form Clearing when COType is changing Adding a Collecting Event record to a CO deletes all Determinations
7 participants