diff --git a/.github/workflows/changelog.yml b/.github/workflows/changelog.yml deleted file mode 100644 index e8f596e..0000000 --- a/.github/workflows/changelog.yml +++ /dev/null @@ -1,30 +0,0 @@ -name: Changelog - -on: - pull_request: - branches: [main] - paths: - - 'lib/**' - - 'tests/**' - - 'composer.json' - -jobs: - check: - runs-on: ubuntu-latest - name: Check CHANGELOG updated - - steps: - - uses: actions/checkout@v6 - with: - fetch-depth: 0 - - - name: Verify CHANGELOG.md was updated - run: | - git fetch origin ${{ github.base_ref }} --depth=1 - if git diff --name-only origin/${{ github.base_ref }} HEAD | grep -q 'CHANGELOG.md'; then - echo "CHANGELOG.md was updated" - exit 0 - else - echo "ERROR: CHANGELOG.md was not updated. Please document your changes." - exit 1 - fi diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d2c8132..903ae15 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -11,15 +11,15 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - php-version: ['8.1', '8.2', '8.3', '8.4'] + php-version: ['8.1', '8.2', '8.3', '8.4', '8.5'] name: PHP ${{ matrix.php-version }} steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7.0.1 - name: Setup PHP - uses: shivammathur/setup-php@v2 + uses: shivammathur/setup-php@2.37.2 with: php-version: ${{ matrix.php-version }} coverage: none @@ -29,7 +29,7 @@ jobs: - name: Cache Composer packages id: composer-cache - uses: actions/cache@v5 + uses: actions/cache@v6.1.0 with: path: vendor key: ${{ runner.os }}-php-${{ matrix.php-version }}-${{ hashFiles('**/composer.lock') }} @@ -47,10 +47,10 @@ jobs: name: Static Analysis steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7.0.1 - name: Setup PHP - uses: shivammathur/setup-php@v2 + uses: shivammathur/setup-php@2.37.2 with: php-version: '8.1' coverage: none @@ -66,10 +66,10 @@ jobs: name: Code Style steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7.0.1 - name: Setup PHP - uses: shivammathur/setup-php@v2 + uses: shivammathur/setup-php@2.37.2 with: php-version: '8.1' coverage: none diff --git a/.github/workflows/pr-summary.yml b/.github/workflows/pr-summary.yml deleted file mode 100644 index 930c7b2..0000000 --- a/.github/workflows/pr-summary.yml +++ /dev/null @@ -1,49 +0,0 @@ -name: Auto Generate PR Summary - -on: - pull_request: - types: [opened, synchronize] - -jobs: - generate-summary: - runs-on: ubuntu-latest - permissions: - contents: read - pull-requests: write - - steps: - - name: Checkout code - uses: actions/checkout@v6 - with: - fetch-depth: 0 - - - name: Set up Python - uses: actions/setup-python@v6 - with: - python-version: '3.11' - - - name: Install dependencies - run: pip install requests - - - name: Get PR Diff - run: | - git diff origin/${{ github.base_ref }}...HEAD > pr_diff.txt - - - name: Generate Summary with Ollama Cloud - id: ollama - env: - OLLAMA_API_KEY: ${{ secrets.OLLAMA_API_KEY }} - GH_TOKEN: ${{ github.token }} - run: | - gh pr view ${{ github.event.pull_request.number }} --json body -q .body > current_body.md || touch current_body.md - python scripts/generate_pr_summary.py pr_diff.txt current_body.md > summary.md - - echo "SUMMARY<> $GITHUB_ENV - cat summary.md >> $GITHUB_ENV - echo "EOF" >> $GITHUB_ENV - - - name: Update PR Description - env: - GH_TOKEN: ${{ github.token }} - run: | - gh pr edit ${{ github.event.pull_request.number }} --body "$SUMMARY" diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml deleted file mode 100644 index 9f90fdd..0000000 --- a/.github/workflows/release.yml +++ /dev/null @@ -1,43 +0,0 @@ -name: Release - -on: - push: - tags: - - 'v*' - -permissions: - contents: write - -jobs: - create-release: - runs-on: ubuntu-latest - name: Create GitHub Release - - steps: - - uses: actions/checkout@v6 - - - name: Extract version from tag - id: version - run: echo "VERSION=${GITHUB_REF#refs/tags/}" >> "$GITHUB_OUTPUT" - - - name: Extract release notes from CHANGELOG - id: notes - run: | - VERSION="${GITHUB_REF#refs/tags/v}" - # Extract section for this version from CHANGELOG.md - awk "/^## \\[$VERSION\\]/ {flag=1; next} /^## \\[/ {flag=0} flag" CHANGELOG.md > release_notes.md - if [ ! -s release_notes.md ]; then - echo "No release notes found for $VERSION in CHANGELOG.md" - exit 1 - fi - echo "notes<> "$GITHUB_OUTPUT" - cat release_notes.md >> "$GITHUB_OUTPUT" - echo "EOF" >> "$GITHUB_OUTPUT" - - - name: Create GitHub Release - uses: softprops/action-gh-release@v2 - with: - name: ${{ steps.version.outputs.VERSION }} - body_path: release_notes.md - draft: false - prerelease: false diff --git a/CHANGELOG.md b/CHANGELOG.md index 74220cb..c95e963 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,22 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [2.1.0] - 2026-09-01 + +### Fixed + +- Coerce money fields (`price`, `discount`, `total_price_override`, capture/refund `amount`, `total`, `debt`, `subtotal_override`, `total_tax_override`, `total_discount_override`, `total_override`) to integers before API calls — floating point noise from `ringgit × 100` conversions (e.g. `0.29 * 100 = 28.999999999999996`) no longer produces fractional JSON that the API rejects with 400 "A valid integer is required." +- Values within 1e-9 of an integer are rounded to it; genuine fractional sen (e.g. `108.5`) now throws `InvalidMoneyValueException` instead of being silently truncated by the builder's implicit int cast (previously `28.999…` became `28` sen — a wrong charge amount) + +### Added + +- `Chip\Support\Money::coerce()` shared money coercion helper +- `Chip\Exception\InvalidMoneyValueException` for money values that cannot be safely sent to the API + +### Removed + +- Remove auto PR summary workflow and script (`pr-summary.yml`, `generate_pr_summary.py`) — the Ollama endpoint returns 410 Gone and the workflow overwrote PR descriptions with its error output + ## [2.0.2] - 2026-05-18 ### Added diff --git a/lib/Builder/PurchaseBuilder.php b/lib/Builder/PurchaseBuilder.php index f8689a9..5b7101d 100644 --- a/lib/Builder/PurchaseBuilder.php +++ b/lib/Builder/PurchaseBuilder.php @@ -6,6 +6,7 @@ use Chip\Model\Product; use Chip\Model\Purchase; use Chip\Model\PurchaseDetails; +use Chip\Support\Money; class PurchaseBuilder { @@ -328,21 +329,21 @@ public function clientBankCode(?string $bankCode): self public function addProduct( string $name, - int $price, + int|float|string $price, float|string $quantity = 1.0, - ?int $discount = null, + int|float|string|null $discount = null, ?string $taxPercent = null, ?string $category = null, - ?int $totalPriceOverride = null + int|float|string|null $totalPriceOverride = null ): self { $product = new Product(); $product->name = $name; - $product->price = $price; + $product->price = Money::coerce($price); $product->quantity = is_string($quantity) ? $quantity : (string) $quantity; - $product->discount = $discount; + $product->discount = $discount === null ? null : Money::coerce($discount); $product->tax_percent = $taxPercent; $product->category = $category; - $product->total_price_override = $totalPriceOverride; + $product->total_price_override = $totalPriceOverride === null ? null : Money::coerce($totalPriceOverride); $this->purchase->purchase->products[] = $product; @@ -356,37 +357,37 @@ public function notes(string $notes): self return $this; } - public function debt(int $debt): self + public function debt(int|float|string $debt): self { - $this->purchase->purchase->debt = $debt; + $this->purchase->purchase->debt = Money::coerce($debt); return $this; } - public function subtotalOverride(int $subtotalOverride): self + public function subtotalOverride(int|float|string $subtotalOverride): self { - $this->purchase->purchase->subtotal_override = $subtotalOverride; + $this->purchase->purchase->subtotal_override = Money::coerce($subtotalOverride); return $this; } - public function totalTaxOverride(int $totalTaxOverride): self + public function totalTaxOverride(int|float|string $totalTaxOverride): self { - $this->purchase->purchase->total_tax_override = $totalTaxOverride; + $this->purchase->purchase->total_tax_override = Money::coerce($totalTaxOverride); return $this; } - public function totalDiscountOverride(int $totalDiscountOverride): self + public function totalDiscountOverride(int|float|string $totalDiscountOverride): self { - $this->purchase->purchase->total_discount_override = $totalDiscountOverride; + $this->purchase->purchase->total_discount_override = Money::coerce($totalDiscountOverride); return $this; } - public function totalOverride(int $totalOverride): self + public function totalOverride(int|float|string $totalOverride): self { - $this->purchase->purchase->total_override = $totalOverride; + $this->purchase->purchase->total_override = Money::coerce($totalOverride); return $this; } diff --git a/lib/Exception/InvalidMoneyValueException.php b/lib/Exception/InvalidMoneyValueException.php new file mode 100644 index 0000000..51bf94d --- /dev/null +++ b/lib/Exception/InvalidMoneyValueException.php @@ -0,0 +1,17 @@ +name = $data['name'] ?? null; $product->quantity = $data['quantity'] ?? null; - $product->price = $data['price'] ?? null; - $product->discount = $data['discount'] ?? null; + $product->price = isset($data['price']) ? Money::coerce($data['price']) : null; + $product->discount = isset($data['discount']) ? Money::coerce($data['discount']) : null; $product->tax_percent = $data['tax_percent'] ?? null; $product->category = $data['category'] ?? null; - $product->total_price_override = $data['total_price_override'] ?? null; + $product->total_price_override = isset($data['total_price_override']) ? Money::coerce($data['total_price_override']) : null; return $product; } @@ -59,7 +62,22 @@ public static function fromArray(array $data): self #[\ReturnTypeWillChange] public function jsonSerialize() { - return array_filter((array) $this, [$this, 'allow_non_null']); + // Coerce at serialization time too: public properties mean callers may + // assign raw values (e.g. 0.29 * 100 float noise) directly without the + // builder. Money fields must reach the API as integers. + $data = (array) $this; + + foreach (['price', 'discount', 'total_price_override'] as $moneyField) { + if ($data[$moneyField] !== null) { + try { + $data[$moneyField] = Money::coerce($data[$moneyField]); + } catch (InvalidMoneyValueException $e) { + throw new InvalidMoneyValueException("Product->{$moneyField}: " . $e->getMessage(), 0, $e); + } + } + } + + return array_filter($data, [$this, 'allow_non_null']); } /** diff --git a/lib/Model/PurchaseDetails.php b/lib/Model/PurchaseDetails.php index 1e624ba..59adcb7 100644 --- a/lib/Model/PurchaseDetails.php +++ b/lib/Model/PurchaseDetails.php @@ -2,6 +2,9 @@ namespace Chip\Model; +use Chip\Exception\InvalidMoneyValueException; +use Chip\Support\Money; + class PurchaseDetails implements \JsonSerializable { /** @@ -131,14 +134,14 @@ public static function fromArray(array $data): self fn (array $p) => Product::fromArray($p), $data['products'] ?? [] ); - $details->total = $data['total'] ?? 0; + $details->total = Money::coerce($data['total'] ?? 0); $details->language = $data['language'] ?? ''; $details->notes = $data['notes'] ?? ''; - $details->debt = $data['debt'] ?? 0; - $details->subtotal_override = $data['subtotal_override'] ?? 0; - $details->total_tax_override = $data['total_tax_override'] ?? 0; - $details->total_discount_override = $data['total_discount_override'] ?? 0; - $details->total_override = $data['total_override'] ?? 0; + $details->debt = Money::coerce($data['debt'] ?? 0); + $details->subtotal_override = Money::coerce($data['subtotal_override'] ?? 0); + $details->total_tax_override = Money::coerce($data['total_tax_override'] ?? 0); + $details->total_discount_override = Money::coerce($data['total_discount_override'] ?? 0); + $details->total_override = Money::coerce($data['total_override'] ?? 0); $details->request_client_details = $data['request_client_details'] ?? []; $details->timezone = $data['timezone'] ?? ''; $details->due_strict = $data['due_strict'] ?? false; @@ -155,6 +158,21 @@ public static function fromArray(array $data): self #[\ReturnTypeWillChange] public function jsonSerialize() { - return array_filter((array) $this); + // Coerce money fields at serialization time: callers may assign raw + // values (e.g. ringgit * 100 float noise) directly to the public + // properties. Money fields must reach the API as integers. + $data = (array) $this; + + foreach (['total', 'debt', 'subtotal_override', 'total_tax_override', 'total_discount_override', 'total_override'] as $moneyField) { + if ($data[$moneyField] !== null) { + try { + $data[$moneyField] = Money::coerce($data[$moneyField]); + } catch (InvalidMoneyValueException $e) { + throw new InvalidMoneyValueException("PurchaseDetails->{$moneyField}: " . $e->getMessage(), 0, $e); + } + } + } + + return array_filter($data); } } diff --git a/lib/Resource/PurchasesResource.php b/lib/Resource/PurchasesResource.php index 6edbde9..a2911e1 100644 --- a/lib/Resource/PurchasesResource.php +++ b/lib/Resource/PurchasesResource.php @@ -6,6 +6,7 @@ use Chip\Http\ClientInterface; use Chip\Model\Purchase; +use Chip\Support\Money; final class PurchasesResource { @@ -43,11 +44,11 @@ public function release(string $purchaseId): Purchase return Purchase::fromArray((array) $response); } - public function capture(string $purchaseId, ?int $amount = null): Purchase + public function capture(string $purchaseId, int|float|string|null $amount = null): Purchase { $options = []; if ($amount !== null) { - $options['json'] = ['amount' => $amount]; + $options['json'] = ['amount' => Money::coerce($amount)]; } $response = $this->client->request('POST', "purchases/$purchaseId/capture/", $options); @@ -71,11 +72,11 @@ public function deleteRecurringToken(string $purchaseId): Purchase return Purchase::fromArray((array) $response); } - public function refund(string $purchaseId, ?int $amount = null): Purchase + public function refund(string $purchaseId, int|float|string|null $amount = null): Purchase { $options = []; if ($amount !== null) { - $options['json'] = ['amount' => $amount]; + $options['json'] = ['amount' => Money::coerce($amount)]; } $response = $this->client->request('POST', "purchases/$purchaseId/refund/", $options); diff --git a/lib/Support/Money.php b/lib/Support/Money.php new file mode 100644 index 0000000..169623e --- /dev/null +++ b/lib/Support/Money.php @@ -0,0 +1,74 @@ + str: - url = os.getenv("OLLAMA_API_URL", "https://api.ollama.com/api/generate") - - prompt = f"""You are a senior software engineer. Please review the following git diff and generate a Pull Request description. - -Current PR Body (if any): -{current_body} - -Git Diff: -{diff_text} - -Please generate a Pull Request description that follows this exact format: - -## What does this change? -[Provide a detailed explanation of WHAT the problem was and HOW this change solves it. Focus on the 'why' and 'how'.] - -## Asana / Jira / Trello task link - - -## How to test -[Provide step-by-step instructions to help others verify the change. Suggest specific tests based on the modified files.] - -## Potential Risks & Senior Review Items -[Identify potential side effects, performance implications, security considerations, or architectural concerns. Highlight specific areas where a senior engineer should focus their review.] - -## Is this PR warrant an automatic approval? -[Yes/No. Provide a brief justification based on the complexity and risk of the changes.] - -## Images - - -Important: -- If the 'Current PR Body' already contains information (like task links or images), PRESERVE them in the new summary. -- Fill in the 'What does this change?', 'How to test', 'Potential Risks & Senior Review Items', and 'Is this PR warrant an automatic approval?' sections based on the provided diff. -- Keep the other sections exactly as shown (with their HTML comments/placeholders) so the user can fill them in manually if needed. -- Return ONLY the markdown content. -""" - - headers = { - "Authorization": f"Bearer {api_key}", - "Content-Type": "application/json", - } - - data = { - "model": model, - "prompt": prompt, - "stream": False, - } - - try: - response = requests.post(url, headers=headers, json=data, timeout=90) - response.raise_for_status() - result = response.json() - return result.get("response", "Could not generate summary.") - except requests.exceptions.RequestException as e: - return f"Error calling Ollama API: {e!s}" - - -def main() -> int: - if len(sys.argv) < 2: - print("Usage: python generate_pr_summary.py [current_body_file]") - return 1 - - diff_file = sys.argv[1] - current_body_file = sys.argv[2] if len(sys.argv) > 2 else None - - api_key = os.getenv("OLLAMA_API_KEY") - if not api_key: - print("Error: OLLAMA_API_KEY environment variable not set.") - return 1 - - if not os.path.exists(diff_file): - print(f"Error: File {diff_file} not found.") - return 1 - - with open(diff_file, encoding="utf-8") as f: - diff_text = f.read() - - current_body = "" - if current_body_file and os.path.exists(current_body_file): - with open(current_body_file, encoding="utf-8") as f: - current_body = f.read() - - # Limit diff size to avoid token limits - if len(diff_text) > 50000: - diff_text = diff_text[:50000] + "\n\n... (diff truncated for size) ..." - - summary = generate_summary(diff_text, current_body, api_key) - print(summary) - return 0 - - -if __name__ == "__main__": - sys.exit(main()) diff --git a/tests/MoneyCoercionTest.php b/tests/MoneyCoercionTest.php new file mode 100644 index 0000000..00db0a3 --- /dev/null +++ b/tests/MoneyCoercionTest.php @@ -0,0 +1,180 @@ +assertSame(108, Money::coerce(108)); + $this->assertSame(0, Money::coerce(0)); + } + + public function testCoercePassesThroughNull(): void + { + $this->assertNull(Money::coerce(null)); + } + + public function testCoerceIntValuedFloat(): void + { + $this->assertSame(108, Money::coerce(108.0)); + $this->assertSame(10800, Money::coerce(10800.0)); + } + + public function testCoerceFloatRoundingNoise(): void + { + // 0.29 * 100 in binary floating point is 28.999999999999996 — a whole + // number in intent, fractional in representation. This exact value was + // rejected by the API with 400 "A valid integer is required.". + $this->assertSame(29, Money::coerce(0.29 * 100)); + $this->assertSame(29, Money::coerce(28.999999999999996)); + } + + public function testCoerceNumericStrings(): void + { + $this->assertSame(108, Money::coerce('108')); + $this->assertSame(108, Money::coerce('108.00')); + $this->assertSame(10800, Money::coerce('108.00' * 100)); + } + + public function testCoerceRejectsGenuineFraction(): void + { + $this->expectException(InvalidMoneyValueException::class); + Money::coerce(108.5); + } + + public function testCoerceRejectsNonNumeric(): void + { + $this->expectException(InvalidMoneyValueException::class); + Money::coerce('12abc'); + } + + public function testCoerceRejectsNanAndInfinity(): void + { + $this->expectException(InvalidMoneyValueException::class); + Money::coerce(NAN); + } + + // ===== Product serialization (covers direct model usage, v1-style apps) ===== + + public function testProductSerializesFloatNoiseAsInteger(): void + { + $product = new \Chip\Model\Product(); + $product->name = 'X'; + $product->price = 0.29 * 100; // @phpstan-ignore assign.propertyType (raw app input on purpose) + $product->quantity = '1'; + + $decoded = json_decode((string) json_encode($product), true); + $this->assertSame(29, $decoded['price']); + } + + public function testProductSerializeRejectsGenuineFraction(): void + { + $product = new \Chip\Model\Product(); + $product->name = 'X'; + $product->price = 108.5; // @phpstan-ignore assign.propertyType (raw app input on purpose) + + $this->expectException(InvalidMoneyValueException::class); + $this->expectExceptionMessage('price'); + json_encode($product); + } + + public function testProductFromArrayCoercesMoneyFields(): void + { + $product = Product::fromArray([ + 'name' => 'X', + 'price' => '108.00', + 'discount' => 2.0, + 'total_price_override' => 106.0, + ]); + + $this->assertSame(108, $product->price); + $this->assertSame(2, $product->discount); + $this->assertSame(106, $product->total_price_override); + } + + // ===== PurchaseDetails ===== + + public function testPurchaseDetailsSerializesOverrideFloatsAsIntegers(): void + { + $details = new PurchaseDetails(); + $details->total = 100.0 * 100; // @phpstan-ignore assign.propertyType (raw app input on purpose) + $details->total_override = '999.00'; // @phpstan-ignore assign.propertyType (raw app input on purpose) + + $decoded = json_decode((string) json_encode($details), true); + $this->assertSame(10000, $decoded['total']); + $this->assertSame(999, $decoded['total_override']); + } + + public function testPurchaseDetailsSerializeRejectsFractionalOverride(): void + { + $details = new PurchaseDetails(); + $details->total_override = 108.25; // @phpstan-ignore assign.propertyType (raw app input on purpose) + + $this->expectException(InvalidMoneyValueException::class); + $this->expectExceptionMessage('total_override'); + json_encode($details); + } + + // ===== Builder ===== + + public function testBuilderAcceptsFloatNoisePrice(): void + { + $purchase = PurchaseBuilder::create() + ->currency('MYR') + ->addProduct('X', 0.29 * 100) + ->build(); + + $this->assertSame(29, $purchase->purchase->products[0]->price); + $this->assertStringNotContainsString('28.9', (string) json_encode($purchase)); + } + + public function testBuilderAcceptsStringPrice(): void + { + $purchase = PurchaseBuilder::create() + ->currency('MYR') + ->addProduct('X', '108.00') + ->build(); + + $this->assertSame(108, $purchase->purchase->products[0]->price); + } + + public function testBuilderRejectsFractionalPrice(): void + { + $this->expectException(InvalidMoneyValueException::class); + PurchaseBuilder::create()->addProduct('X', 108.999); + } + + public function testBuilderCoercesDiscountAndOverride(): void + { + $purchase = PurchaseBuilder::create() + ->currency('MYR') + ->addProduct('X', 100, 1.0, 1.0, null, null, 99.0000000001) + ->build(); + + $product = $purchase->purchase->products[0]; + $this->assertSame(1, $product->discount); + $this->assertSame(99, $product->total_price_override); + } + + public function testBuilderCoercesTotalOverrideMoney(): void + { + $purchase = PurchaseBuilder::create() + ->currency('MYR') + ->addProduct('X', 100) + ->totalOverride('1000.00') + ->build(); + + $decoded = json_decode((string) json_encode($purchase), true); + $this->assertSame(1000, $decoded['purchase']['total_override']); + } +}