Skip to content

Fix in LastStatementId #13

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.

Sign up for GitHub

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

Open
byjg wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Fix in LastStatementId #13

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
c53d947
Refactor fund operations to optimize statement handling and add bulkE…
byjg Sep 4, 2025
780e052
Refactor statement handling to use named parameters and update constr…
byjg Sep 6, 2025
b565228
Refactor statement handling to use named parameters and update constr…
byjg Sep 6, 2025
0abf61b
Add observer support for updateFunds using bulkExecute.
byjg Sep 6, 2025
3480c54
Refactor fund operations to include `capAtZero` feature, update const…
byjg Sep 6, 2025
0e2543a
Refactor fund operations to include `capAtZero` feature, update const…
byjg Sep 6, 2025
21747a3
Refactor fund operations to return `StatementEntity` instead of `Stat…
byjg Sep 6, 2025
9fc3993
Refactor fund operations to return `StatementEntity` instead of `Stat…
byjg Sep 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .github/workflows/phpunit.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,5 +50,6 @@ jobs:
with:
folder: php
project: ${{ github.event.repository.name }}
secrets: inherit
secrets:
DOC_TOKEN: ${{ secrets.DOC_TOKEN }}

4 changes: 2 additions & 2 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,10 @@ $statementBLL->withdrawFunds($statement);

// Add 50 USD hold to the account
$statement = new StatementDTO($accountId, 50);
$statementId = $statementBLL->reserveFundsForDeposit($statement);
$reserve = $statementBLL->reserveFundsForDeposit($statement);

// Accept the hold
$statementBLL->acceptFundsById($statementId);
$statementBLL->acceptFundsById($reserve->getStatementId());
```

## Installation
Expand Down
Binary file removed composer.phar
View file
Open in desktop
Binary file not shown.
2 changes: 2 additions & 0 deletions db/migrations/down/00002-delete-laststatementid.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
alter table account
drop column laststatementid;
5 changes: 5 additions & 0 deletions db/migrations/down/00003-delete-constraint.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
ALTER TABLE statement
DROP CONSTRAINT statement_chk_value_nonnegative;

ALTER TABLE account
DROP CONSTRAINT account_chk_value_nonnegative;
3 changes: 3 additions & 0 deletions db/migrations/up/00003-add-laststatementid.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
alter table account
add laststatementid int null;

8 changes: 8 additions & 0 deletions db/migrations/up/00004-add-constraint.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
ALTER TABLE statement
ADD CONSTRAINT statement_chk_value_nonnegative
CHECK (netbalance >= 0);
Copy link
Copy Markdown

@korbit-ai korbit-ai Bot Nov 7, 2025

Choose a reason for hiding this comment

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

Imprecise constraint naming category Readability

Tell me more
What is the issue?

The constraint name suggests 'nonnegative' but the logic checks for 'greater than or equal to zero', which could be more explicitly stated in the constraint name.

Why this matters

When other developers encounter this code, they need to check the actual constraint logic to confirm what 'nonnegative' means. A more precise name would make the behavior immediately clear.

Suggested change ∙ Feature Preview
ALTER TABLE statement
    ADD CONSTRAINT statement_chk_netbalance_gte_zero
        CHECK (netbalance >= 0);

ALTER TABLE account
    ADD CONSTRAINT account_chk_netbalance_gte_zero
        CHECK (netbalance >= 0);
Provide feedback to improve future suggestions

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.

Comment on lines +2 to +3
Copy link
Copy Markdown

@korbit-ai korbit-ai Bot Nov 7, 2025

Choose a reason for hiding this comment

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

Misleading constraint name for non-negative check category Functionality

Tell me more
What is the issue?

The constraint name 'statement_chk_value_nonnegative' suggests it's checking for non-negative values, but the CHECK condition allows zero values (>= 0) rather than strictly positive values (> 0).

Why this matters

This naming inconsistency could mislead future developers about the actual constraint behavior, potentially causing incorrect assumptions when writing application logic or additional constraints.

Suggested change ∙ Feature Preview

Either rename the constraint to reflect that zero is allowed (e.g., 'statement_chk_netbalance_not_negative') or change the condition to CHECK (netbalance > 0) if truly non-negative values are required:

ALTER TABLE statement
    ADD CONSTRAINT statement_chk_netbalance_not_negative
        CHECK (netbalance >= 0);
Provide feedback to improve future suggestions

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.


ALTER TABLE account
ADD CONSTRAINT account_chk_value_nonnegative
CHECK (netbalance >= 0);
Comment on lines +6 to +7
Copy link
Copy Markdown

@korbit-ai korbit-ai Bot Nov 7, 2025

Choose a reason for hiding this comment

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

Misleading constraint name for non-negative check category Functionality

Tell me more
What is the issue?

The constraint name 'account_chk_value_nonnegative' suggests it's checking for non-negative values, but the CHECK condition allows zero values (>= 0) rather than strictly positive values (> 0).

Why this matters

This naming inconsistency could mislead future developers about the actual constraint behavior, potentially causing incorrect assumptions when writing application logic or additional constraints.

Suggested change ∙ Feature Preview

Either rename the constraint to reflect that zero is allowed (e.g., 'account_chk_netbalance_not_negative') or change the condition to CHECK (netbalance > 0) if truly non-negative values are required:

ALTER TABLE account
    ADD CONSTRAINT account_chk_netbalance_not_negative
        CHECK (netbalance >= 0);
Provide feedback to improve future suggestions

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.


47 changes: 17 additions & 30 deletions phpunit.xml.dist
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,34 +4,21 @@ To change this license header, choose License Headers in Project Properties.
To change this template file, choose Tools | Templates
and open the template in the editor.
-->

<!-- see http://www.phpunit.de/wiki/Documentation -->
<phpunit bootstrap="./vendor/autoload.php"
colors="true"
testdox="true"
convertErrorsToExceptions="true"
convertNoticesToExceptions="true"
convertWarningsToExceptions="true"
convertDeprecationsToExceptions="true"
stopOnFailure="false">


<php>
<ini name="display_errors" value="On" />
<ini name="display_startup_errors" value="On" />
<ini name="error_reporting" value="E_ALL" />
</php>

<filter>
<whitelist>
<directory>./src</directory>
</whitelist>
</filter>

<testsuites>
<testsuite name="Test Suite">
<directory>./tests/</directory>
</testsuite>
</testsuites>

</phpunit>
<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" bootstrap="./vendor/autoload.php" colors="true" testdox="true" convertErrorsToExceptions="true" convertNoticesToExceptions="true" convertWarningsToExceptions="true" convertDeprecationsToExceptions="true" stopOnFailure="false" xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/9.3/phpunit.xsd">
<coverage>
<include>
<directory>./src</directory>
</include>
</coverage>
<php>
<ini name="display_errors" value="On"/>
<ini name="display_startup_errors" value="On"/>
<ini name="error_reporting" value="E_ALL"/>
</php>
<testsuites>
<testsuite name="Test Suite">
<directory>./tests/</directory>
</testsuite>
</testsuites>
</phpunit>
24 changes: 8 additions & 16 deletions src/Bll/AccountBLL.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,30 +254,26 @@ public function closeAccount(int $accountId): ?int
* @param int $accountId
* @param float $balance
* @param string $description
* @return int|null
* @return StatementEntity
* @throws AccountException
* @throws AmountException
* @throws InvalidArgumentException
* @throws OrmBeforeInvalidException
* @throws OrmInvalidFieldsException
* @throws RepositoryReadOnlyException
* @throws StatementException
* @throws UpdateConstraintException
* @throws \ByJG\MicroOrm\Exception\InvalidArgumentException
*/
public function partialBalance(int $accountId, float $balance, string $description = "Partial Balance"): ?int
public function partialBalance(int $accountId, float $balance, string $description = "Partial Balance"): StatementEntity
{
$account = $this->getById($accountId);

$amount = $balance - $account->getNetBalance();

if ($amount >= 0) {
$statementId = $this->statementBLL->addFunds(StatementDTO::create($accountId, $amount)->setDescription($description));
$statement = $this->statementBLL->addFunds(StatementDTO::create($accountId, $amount)->setDescription($description));
} else {
$statementId = $this->statementBLL->withdrawFunds(StatementDTO::create($accountId, abs($amount))->setDescription($description));
$statement = $this->statementBLL->withdrawFunds(StatementDTO::create($accountId, abs($amount))->setDescription($description));
}

return $statementId;
return $statement;
}

/**
Expand All @@ -288,11 +284,7 @@ public function partialBalance(int $accountId, float $balance, string $descripti
* @throws AccountException
* @throws AmountException
* @throws InvalidArgumentException
* @throws OrmBeforeInvalidException
* @throws OrmInvalidFieldsException
* @throws RepositoryReadOnlyException
* @throws StatementException
* @throws UpdateConstraintException
* @throws \ByJG\MicroOrm\Exception\InvalidArgumentException
*/
public function transferFunds(int $accountSource, int $accountTarget, float $amount): array
Expand All @@ -315,10 +307,10 @@ public function transferFunds(int $accountSource, int $accountTarget, float $amo
$statementTargetDTO->setReferenceId($refSource);
$statementTargetDTO->setDescription('Transfer from account id ' . $accountSource);

$statementSourceId = $this->statementBLL->withdrawFunds($statementSourceDTO);
$statementTargetId = $this->statementBLL->addFunds($statementTargetDTO);
$statementSource = $this->statementBLL->withdrawFunds($statementSourceDTO);
$statementTarget = $this->statementBLL->addFunds($statementTargetDTO);

return [ $statementSourceId, $statementTargetId ];
return [ $statementSource, $statementTarget ];
}

public function getRepository(): AccountRepository
Expand Down
4 changes: 2 additions & 2 deletions src/Bll/AccountTypeBLL.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@ public function __construct(AccountTypeRepository $accountTypeRepository)
* Se o ID não for passado, então devolve todos os AccountTypes.
*
* @param string $accountTypeId Opcional. Se não for passado obtém todos
* @return AccountTypeEntity|AccountTypeEntity[]
* @return AccountTypeEntity|AccountTypeEntity[]|null
* @throws \ByJG\MicroOrm\Exception\InvalidArgumentException
*/
public function getById(string $accountTypeId): array|AccountTypeEntity
public function getById(string $accountTypeId): array|AccountTypeEntity|null
{
return $this->accountTypeRepository->getById($accountTypeId);
}
Expand Down
Loading
Loading