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

Implement Exchange Out, Disposal, Gift, and Deaccession Virtual Calculated Fields #4824

Merged
merged 21 commits into from
May 22, 2024

Conversation

acwhite211
Copy link
Member

@acwhite211 acwhite211 commented Apr 24, 2024

Fixes #2521

Implements the calculations done in Specify 6 into Specify 7 as virtual fields for the following:
Exchange Out:

  • Total Preps (Sum of all associated exchange out preparations)
  • Total Items (Sum of all associated collection object preparations)

Disposal:

  • Total Preps (Sum of all associated disposal preparations)
  • Total Items (Sum of all associated collection object preparations)

Gift:

  • Total Preps (Sum of all associated gift preparations)
  • Total Items (Sum of all associated collection object preparations)

Deaccession’s

  • Total Preps (Sum of all associated disposal, gift, and exchange out preparations)
  • Total Items (Sum of all associated collection object preparations)

Note: This PR is also taking the opportunity to take the existing hard-coded sql code in the calculated_fields.py file, and re-implementing it using the Django ORM.

Checklist

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

Testing instructions

  • Go to the forms of ExchangeOut, Disposal, Gift, and Deaccession.
  • Play around with adding and deleting preparations
  • Ensure Total Preps and Total Items display accurate values

@acwhite211 acwhite211 added this to the 7.9.6 milestone Apr 24, 2024
@acwhite211 acwhite211 self-assigned this Apr 24, 2024
@acwhite211
Copy link
Member Author

I found the file https://github.com/specify/specify7/blob/production/specifyweb/specify/calculated_fields.py where many of the already implemented virtual calculated fields are coded. I think it will be good to extend this file, instead of using business rules. Although this file uses manually created sql code, so it would be ideal to use the Django ORM in this file where possible.

@acwhite211 acwhite211 marked this pull request as ready for review April 24, 2024 21:18
specifyweb/specify/calculated_fields.py Outdated Show resolved Hide resolved
@CarolineDenis CarolineDenis requested a review from a team May 16, 2024 13:50
Copy link

@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

  • Go to the forms of ExchangeOut, Disposal, Gift, and Deaccession.
  • Play around with adding and deleting preparations
  • Ensure Total Preps and Total Items display accurate values

Everything looks good! Total Preps and Total Items fields are properly calculated in each form.

@Areyes42 Areyes42 requested a review from a team May 16, 2024 16:22
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

  • Go to the forms of ExchangeOut, Disposal, Gift, and Deaccession.
  • Play around with adding and deleting preparations
  • Ensure Total Preps and Total Items display accurate values

Looks good!
Screenshot 2024-05-16 155218

@emenslin emenslin requested a review from a team May 16, 2024 20:55
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

  • Go to the forms of ExchangeOut, Disposal, Gift, and Deaccession.
  • Play around with adding and deleting preparations
  • Ensure Total Preps and Total Items display accurate values

Looks good! The fields are now functioning for each interaction.

This branch seems to be causing some sort of issue in the Fitz_NHMD_Oct_2023 database, but that is the only database I've found to be affected. I will go ahead and approve it, but could someone take a look and verify whether it is an issue specific to the database or the PR? This error is thrown when opening disposal forms in this branch:

Specify 7 Crash Report - 2024-05-16T21_28_17.993Z.txt

https://fitznhmdoct2023-issue-2521-2.test.specifysystems.org/specify/view/disposal/2/

@sharadsw sharadsw requested a review from realVinayak May 21, 2024 19:52
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.

Looks good!

@sharadsw sharadsw merged commit e00ea8b into production May 22, 2024
9 checks passed
@sharadsw sharadsw deleted the issue-2521 branch May 22, 2024 18:57
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 calculated fields for Exchange Out, Disposal, Gift, and Deaccession
8 participants