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

Daily Liveweight Gain statistic for weights #309

Closed
wants to merge 2 commits into from
Closed

Daily Liveweight Gain statistic for weights #309

wants to merge 2 commits into from

Conversation

Skipper-is
Copy link
Contributor

Two main changes here:
First - Rectified missing translates to the weight module

Second - Added two new functions - farm_livestock_weight_dlwg() and farm_livestock_weight_all()

farm_livestock_weight_all() - Returns an array of all weights previously recorded for the asset, with the value, units, label and timestamp.

farm_livestock_weight_dlwg() - Returns the latest daily liveweight gain for assets with more than 2 weights recorded. DLWG (Daily Live Weight Gain) is calculated from the two most recent logs:
DLWG = Weight difference / Time difference
The function returns an array of: timestamp of most recent record, timestamp of the previous record, value of DLWG, and the units.

The function will return an empty array if there is only 1 (or 0) weight recorded for the asset, and will return empty if the units are different.

The Daily Liveweight Gain function is called on the asset page - and shown below the most recent weight for the asset, and is also shown in the "Weight" tab.

...I think this is pulling the most recent farmOS
Added function to calculate daily liveweight gain, and to get all weight records.
Added translation
Added farm_livestock_weight_all() function
@paul121
Copy link
Member

paul121 commented Jul 31, 2020

@Skipper-is just looked this over, I think this would a great addition! I created a branch with your commit and made a few follow up commits here: 7.x-1.x...paul121:dlwg @mstenta this should be nearly ready to merge.

Changes:

  • Return the actual log object with farm_livestock_weight_all and farm_livestock_weight_dlwg (will make the helper functions more useful in the future)
  • Change the text from "Daily Liveweight Gain since Date: ## units/day" to read "Daily Liveweight Gain: ## units/day (observed between Date and Date)" where the dates are links to the weight logs.
    • @Skipper-is what do you think about this? "since" makes it seem like the animal is currently growing at this rate which isn't necessarily true. Giving the "observed between" date range is more explicit.
  • Generate and return markup for the DLWG text inside farm_livestock_weight_dlwg function. Generating markup could also be implemented in a second function but because the current implementations just want markup in the end this seems fine for now.

@Skipper-is
Copy link
Contributor Author

I agree that the between date and date is much clearer than since.
Looks good

@mstenta
Copy link
Member

mstenta commented Aug 28, 2020

Hi @Skipper-is and @paul121 - finally getting around to looking at this. :-)

This is looking good! I was going to try to include this in the 7.x-1.5 release, but there are a few things I want to think more on (mostly minor), and really pushing to get 7.x-1.5 out before I take a week off. So I think I'll pick this back up when I'm back and we can get it ready for inclusion in 7.x-1.6. Thanks for your patience @Skipper-is and both of your work on it!

I have rebased the current work on top of the latest 7.x-1.x commit, pushed to my fork, and added a few changes myself: 7.x-1.x...mstenta:dlwg

My changes are mostly small improvements to the translation strings (separating markup and whitespace out of them, so it is only the text itself, with the exception of the one string that contains links, see https://drupal.stackexchange.com/questions/96622/how-to-use-t-function-for-a-text-with-anchor-links), as well as a few small coding standards/conventions/whitespace tweaks.

One other thing we might consider is merging the farm_livestock_weight_all() function into farm_livestock_weight() and simply adding a $single = TRUE default argument to the function. They are mostly the same, otherwise. If I have a chance to try that out I will.

mstenta added a commit that referenced this pull request Nov 26, 2020
@mstenta
Copy link
Member

mstenta commented Nov 26, 2020

I went ahead and merged this. Thanks again @Skipper-is and @paul121 - for the code and the patience! :-)

One other thing we might consider is merging the farm_livestock_weight_all() function into farm_livestock_weight() and simply adding a $single = TRUE default argument to the function. They are mostly the same, otherwise. If I have a chance to try that out I will.

In looking at this again... I decided to leave them as separate functions. If we want to merge them in the future we can always do so and simply deprecate farm_livestock_weight_all().

@mstenta mstenta closed this Nov 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants