4158 Add validation to disallow negative quantity for kit items.

[aniketpatidar]

Resolves #4158

Description

This PR fixes the validation for the quantity of items in a kit. Previously, negative values were allowed for the quantity, which does not make sense. This change ensures that only positive values are allowed for the quantity of items in a kit.

Type of change

• Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

To test this change:

1. Go to Inventory > Kits
2. Choose "New Kit"
3. Enter a kit with an item having a negative value
4. Save the kit

It gives you an error.

• Negative quantities are disallowed for kit item quantities.
• Added tests to validate the functionality.

[cielf]

On a quick testing, I am seeing a problem -- in that if we have an error, the line items are disappearing (and they shouldn't). That's a separate, pre-existing issue, though.

[cielf]

@aniketpatidar Thanks for the PR!

My main role is looking out for the end-user's interest, rather than making sure the technical aspects are perfect.

I'd like to see a friendlier error message here -- the person trying to enter a kit might not understand what they did wrong when they see "Line items.quantity must be a positive number, Line items is invalid," as we don't use that wording in the user interface.

For a model of friendlier errors, I recommend you look in a couple of places:
1/ On kit, If the value for kit is negative, we have a friendlier message: "Value in cents must be greater than or equal to 0",
2/ A better model for this case is on distributions. If we enter a negative item quantity there we give an error like this: "Sorry, we weren't able to save the distribution. Validation failed: Inventory Adult Briefs (Large/X-Large)'s quantity needs to be at least 1"

So, if you could, please, make the error something like "Sorry, we weren't able to save the kit. Validation failed: [Item name]'s quantity needs to be at least 1" that would be ideal.

Thanks again.

[aniketpatidar]

Sure, I'll make those adjustments to the error message in the PR. Thanks for the detailed feedback! @cielf

[aniketpatidar]

This looks good to me What do you think about this? 'Line items are invalid' come from this line we are adding to our model kit:
validates_associated: line_items

[cielf]

That will be better than what you had, but I would prefer it without the "Line Items is invalid". Can you model it after how it is handled with the distributions (which, I believe, also deals with line items, but is not showing that 'line items is invalid')?

Thanks.

[aniketpatidar]

I will make these changes as per your suggestion. Thanks!

[aniketpatidar]

I've made the updates as per your suggestion.

[cielf]

Looks good, functionality-wise, to me. @dorner - can you take a look from a technical p.o.v.?

[aniketpatidar]

I'll make those changes. Thanks!

[aniketpatidar]

I've made the updates as per your suggestion.
@dorner

[dorner]

Looks good, thanks! In the future, please don't force-push the branch (or do a sync with rebase) since it blows away the history of the review.

[aniketpatidar]

Got it, noted for the future. Thanks for the heads-up! @dorner

[cielf]

Looks good to me too. Note: I have added fixing the disappearing info on error to our long list of things to write up for people to fix.