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

Added weight quick form #507

Open
wants to merge 10 commits into
base: 2.x
Choose a base branch
from
Open

Added weight quick form #507

wants to merge 10 commits into from

Conversation

Skipper-is
Copy link
Contributor

Added the quick form module for weighing animals.

Need to still sort the setting of default units - Currently it includes the id of the unit taxonomy.
Also - uninstall hook - need to clean up the state after through _uninstall hook.

Added the quick form module for weighing animals
@mstenta
Copy link
Member

mstenta commented Feb 26, 2022

Nice thanks for starting this @Skipper-is !

Need to still sort the setting of default units - Currently it includes the id of the unit taxonomy.

One thought I have: maybe we should present the user with some hard-coded options, based on their system of measurement. For example, if they use metric, show a dropdown with "kg" and "g", and if they use "us/imperial" then show "lbs" and "oz".

We should coordinate this decision with @paul121 and how the Animal Weight Report module is going to work: https://drupal.org/project/farm_animal_weight - to make sure that logs created with this quick form work with that.

Other things we'll need to do before we can merge this:

I'll try to look at this closer in the near future to see if there's any other feedback... happy to help push this forward - when I have time. :-)

Tidied whitespace issues up
Code sniffer didn't pick these up last time...
Removed unused use statements
Fixed order
Hidden whitespace
@mstenta
Copy link
Member

mstenta commented Feb 26, 2022

Automated tests (we should include tests for all quick forms that farmOS core provides)

To whomever it may concern: in my 2.x-quick-birth branch I just created a base class that can be used for quick form kernel tests, and refactored the Planting quick form tests to extend from it. So we can use that here too.

First run of tests
@Skipper-is
Copy link
Contributor Author

Have attempted to mirror the testing of the quick-birth form, with additional assertEquals functions.

Whitespace removal...
@mstenta
Copy link
Member

mstenta commented Feb 27, 2022

That's awesome @Skipper-is - thanks! I'll add this to my list to review after the birth form.

@mstenta
Copy link
Member

mstenta commented Jan 9, 2024

Sorry for the long silence on this! I developed a plan in my head but never commented here to propose it. :-)

I propose we implement this in the farmOS Animal Weight module here: https://www.drupal.org/project/farm_animal_weight

During farmOS v2 development, we split all of the animal weight stuff out to this new contrib module, so it can serve as the shepherd of that convention. Makes perfect sense for it to provide a quick form too IMO!

My plan was to transfer @Skipper-is's work over to that repo, but I never got around to it. Once we move it over, we can close this core PR. I didn't want to close it until then so I don't forget.

How does that sound?

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

2 participants