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

Inventory transaction resources #385

Merged
merged 10 commits into from
Jul 10, 2023
Merged

Inventory transaction resources #385

merged 10 commits into from
Jul 10, 2023

Conversation

cookeac
Copy link
Collaborator

@cookeac cookeac commented Apr 12, 2023

Defines a generalised icarInventoryTransactionResource and its specific versions, icarFeedTransactionResource and icarMedicineTransactionResource. Resolves #369

Note - update to exampleURLScheme included, but I know that I will need to merge these changes with the changes in #381 before this can be merged.

Defines a generalised icarInventoryTransactionResource and its specific versions, icarFeedTransactionResource and icarMedicineTransactionResource.
Resolves adewg#369
@cookeac cookeac linked an issue Apr 12, 2023 that may be closed by this pull request
@erwinspeybroeck
Copy link
Collaborator

Andrew, a general remark from my side.
You have a generic API for inventory-transactions: GET /locations/{location-scheme}/{location-id}/inventory-transactions and specific ones for feed and medicines. But in the specific ones also the complete list of family products is taken [ Animal Feeds, Animal Reproductive Products, Veterinary Supplies, Seed and Plant Material, Fertilisers and Nutrients, Pest Control Products, Other Animal Products, Milking Supplies, Fencing Supplies, Water System Supplies ], while one would expect this to be limited to e.g. Animal Feeds in case of feed.
Not a blocking remark, I don't know if we can have a fixed family ...

@cookeac
Copy link
Collaborator Author

cookeac commented Apr 19, 2023

Andrew, a general remark from my side. You have a generic API for inventory-transactions: GET /locations/{location-scheme}/{location-id}/inventory-transactions and specific ones for feed and medicines. But in the specific ones also the complete list of family products is taken [ Animal Feeds, Animal Reproductive Products, Veterinary Supplies, Seed and Plant Material, Fertilisers and Nutrients, Pest Control Products, Other Animal Products, Milking Supplies, Fencing Supplies, Water System Supplies ], while one would expect this to be limited to e.g. Animal Feeds in case of feed. Not a blocking remark, I don't know if we can have a fixed family ...

You're exactly right, and I was aware of this. The only thing I could think was documentation - that if you were using the feed or medicine resource type, the family should be set to a constant value. I'll ask my team if there is any way to use the "default" keyword if the property is already defined.

Copy link
Collaborator

@AndreasSchultzGEA AndreasSchultzGEA left a comment

Choose a reason for hiding this comment

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

Hello Andrew, I'm sorry but I couldn't finish the review due to missing time.

cookeac and others added 5 commits June 22, 2023 17:27
Inventory transactions are already in the example URL scheme. Adding the medicines ones to the health scheme, and the feed ones to the feed scheme.
@cookeac
Copy link
Collaborator Author

cookeac commented Jul 10, 2023

Reviewed at previous meetings; merging.

@cookeac cookeac merged commit a1dfb63 into adewg:Develop Jul 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Inventory to MedicineRessource?
3 participants