Skip to content

add batch total volume to cocktail sheet#21

Merged
bpolack merged 1 commit intomainfrom
total-volume-on-recipes
Jul 29, 2025
Merged

add batch total volume to cocktail sheet#21
bpolack merged 1 commit intomainfrom
total-volume-on-recipes

Conversation

@bpolack
Copy link
Owner

@bpolack bpolack commented Jul 29, 2025

No description provided.

@bpolack bpolack requested a review from Copilot July 29, 2025 04:36
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds functionality to display the total volume of a cocktail batch when the batch value is greater than 1. The feature calculates the combined volume of all volume-based ingredients (ml/oz) and displays it as a badge below the ingredients list.

  • Adds batch total volume calculation using useMemo for performance optimization
  • Displays total volume badge when batch value is not 1
  • Minor code style fix (quote consistency)

ingredient.units,
user?.preferredUnits,
)
units = volume.units || 'ml'
Copy link

Copilot AI Jul 29, 2025

Choose a reason for hiding this comment

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

The units variable is being overwritten on each iteration. This means the final units will be determined by the last ingredient processed, which could be inconsistent. Consider determining units based on the user's preferred units or the majority of ingredients instead.

Suggested change
units = volume.units || 'ml'
if (volume.units) unitsArray.push(volume.units)

Copilot uses AI. Check for mistakes.
units = volume.units || 'ml'
return total + volume.value
} else {
// Ignore non-volume ingredients
Copy link

Copilot AI Jul 29, 2025

Choose a reason for hiding this comment

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

The comment should be more specific about what constitutes 'non-volume ingredients'. Consider listing examples like 'dashes', 'pinches', 'garnishes', etc. for better code documentation.

Suggested change
// Ignore non-volume ingredients
// Ignore non-volume ingredients (e.g., dashes, pinches, garnishes)

Copilot uses AI. Check for mistakes.
Comment on lines +87 to +91
const totalValue = data.ingredients.reduce((total, ingredient) => {
if (ingredient.units === 'oz' || ingredient.units === 'ml') {
const volume = convertToPreferredUnits(
ingredient.amount
? new Prisma.Decimal(ingredient.amount).toNumber() * batchValue
Copy link

Copilot AI Jul 29, 2025

Choose a reason for hiding this comment

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

Creating a new Prisma.Decimal instance on each iteration could be inefficient. Consider converting the amount to number once and reusing it, or handle the Decimal multiplication directly without converting to number first.

Suggested change
const totalValue = data.ingredients.reduce((total, ingredient) => {
if (ingredient.units === 'oz' || ingredient.units === 'ml') {
const volume = convertToPreferredUnits(
ingredient.amount
? new Prisma.Decimal(ingredient.amount).toNumber() * batchValue
const batchDecimal = new Prisma.Decimal(batchValue);
const totalValue = data.ingredients.reduce((total, ingredient) => {
if (ingredient.units === 'oz' || ingredient.units === 'ml') {
const volume = convertToPreferredUnits(
ingredient.amount
? new Prisma.Decimal(ingredient.amount).mul(batchDecimal).toNumber()

Copilot uses AI. Check for mistakes.
@bpolack bpolack merged commit dc3a02d into main Jul 29, 2025
0 of 2 checks passed
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.

2 participants