HP-1928: array => string type #64
Conversation
|
Warning Rate limit exceeded@bladeroot has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 51 minutes and 3 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe recent changes primarily focus on the Changes
Sequence Diagram(s) (Beta)sequenceDiagram
participant Client
participant SaleFactory
participant Sale
participant Database
Client->>SaleFactory: createSale(data)
SaleFactory->>Sale: new Sale(id, target, customer, plan, time, data)
Sale->>Sale: setData(data)
Sale->>Database: save()
Sale-->>SaleFactory: Sale instance
SaleFactory-->>Client: Sale instance
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- src/sale/Sale.php (3 hunks)
- tests/behat/bootstrap/BillingContext.php (3 hunks)
Additional context used
PHPStan
src/sale/Sale.php
27-27: Class hiqdev\php\billing\sale\Sale implements unknown interface hiqdev\php\billing\sale\SaleInterface. (interface.notFound)
Learn more at https://phpstan.org/user-guide/discovering-symbols
60-60: Parameter $target of method hiqdev\php\billing\sale\Sale::__construct() has invalid type hiqdev\php\billing\target\TargetInterface. (class.notFound)
61-61: Parameter $customer of method hiqdev\php\billing\sale\Sale::__construct() has invalid type hiqdev\php\billing\customer\CustomerInterface. (class.notFound)
62-62: Parameter $plan of method hiqdev\php\billing\sale\Sale::__construct() has invalid type hiqdev\php\billing\plan\PlanInterface. (class.notFound)
112-112: Instantiated class hiqdev\php\billing\Exception\InvariantException not found. (class.notFound)
Learn more at https://phpstan.org/user-guide/discovering-symbols
116-116: Instantiated class hiqdev\php\billing\Exception\ConstraintException not found. (class.notFound)
Learn more at https://phpstan.org/user-guide/discovering-symbols
128-128: Instantiated class hiqdev\php\billing\Exception\CannotReassignException not found. (class.notFound)
Learn more at https://phpstan.org/user-guide/discovering-symbols
140-140: Function json_validate not found. (function.notFound)
Learn more at https://phpstan.org/user-guide/discovering-symbols
146-146: Call to static method encode() on an unknown class yii\helpers\Json. (class.notFound)
Learn more at https://phpstan.org/user-guide/discovering-symbols
155-155: Call to static method decode() on an unknown class yii\helpers\Json. (class.notFound)
Learn more at https://phpstan.org/user-guide/discovering-symbolstests/behat/bootstrap/BillingContext.php
20-20: Class hiqdev\php\billing\tests\behat\bootstrap\BillingContext extends unknown class hiqdev\php\billing\tests\behat\bootstrap\BaseContext. (class.notFound)
Learn more at https://phpstan.org/user-guide/discovering-symbols
22-22: Class hiqdev\php\billing\tests\behat\bootstrap\BillingContext uses unknown trait BehatExpectException\ExpectException. (trait.notFound)
Learn more at https://phpstan.org/user-guide/discovering-symbols
39-39: Access to an undefined property hiqdev\php\billing\tests\behat\bootstrap\BillingContext::$builder. (property.notFound)
Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property
47-47: Access to an undefined property hiqdev\php\billing\tests\behat\bootstrap\BillingContext::$builder. (property.notFound)
Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property
55-55: Access to an undefined property hiqdev\php\billing\tests\behat\bootstrap\BillingContext::$builder. (property.notFound)
Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property
66-66: Access to an undefined property hiqdev\php\billing\tests\behat\bootstrap\BillingContext::$builder. (property.notFound)
Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property
74-74: Access to an undefined property hiqdev\php\billing\tests\behat\bootstrap\BillingContext::$builder. (property.notFound)
Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property
124-124: Access to an undefined property hiqdev\php\billing\tests\behat\bootstrap\BillingContext::$builder. (property.notFound)
Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property
133-133: Access to an undefined property hiqdev\php\billing\tests\behat\bootstrap\BillingContext::$builder. (property.notFound)
Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property
140-140: Instantiated class hiqdev\php\billing\tests\behat\bootstrap\PendingException not found. (class.notFound)
Learn more at https://phpstan.org/user-guide/discovering-symbols
151-151: Call to static method assertCount() on an unknown class PHPUnit\Framework\Assert. (class.notFound)
Learn more at https://phpstan.org/user-guide/discovering-symbols
151-151: Undefined variable: $sales (variable.undefined)
160-160: Access to an undefined property hiqdev\php\billing\tests\behat\bootstrap\BillingContext::$builder. (property.notFound)
Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property
166-166: Parameter $usesTable of method hiqdev\php\billing\tests\behat\bootstrap\BillingContext::purchaseTargetWithInitialUses() has invalid type Behat\Gherkin\Node\TableNode. (class.notFound)
177-177: Call to an undefined method hiqdev\php\billing\tests\behat\bootstrap\BillingContext::mayFail(). (method.notFound)
178-178: Access to an undefined property hiqdev\php\billing\tests\behat\bootstrap\BillingContext::$builder. (property.notFound)
Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property
188-188: Access to an undefined property hiqdev\php\billing\tests\behat\bootstrap\BillingContext::$builder. (property.notFound)
Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property
196-196: Access to an undefined property hiqdev\php\billing\tests\behat\bootstrap\BillingContext::$builder. (property.notFound)
Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property
204-204: Access to an undefined property hiqdev\php\billing\tests\behat\bootstrap\BillingContext::$builder. (property.notFound)
Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property
213-213: Access to an undefined property hiqdev\php\billing\tests\behat\bootstrap\BillingContext::$builder. (property.notFound)
Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property
221-221: Access to an undefined property hiqdev\php\billing\tests\behat\bootstrap\BillingContext::$builder. (property.notFound)
Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property
230-230: Access to an undefined property hiqdev\php\billing\tests\behat\bootstrap\BillingContext::$builder. (property.notFound)
Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property
242-242: Call to static method assertSame() on an unknown class PHPUnit\Framework\Assert. (class.notFound)
Learn more at https://phpstan.org/user-guide/discovering-symbols
243-243: Call to static method assertSame() on an unknown class PHPUnit\Framework\Assert. (class.notFound)
Learn more at https://phpstan.org/user-guide/discovering-symbols
244-244: Call to static method assertEquals() on an unknown class PHPUnit\Framework\Assert. (class.notFound)
Learn more at https://phpstan.org/user-guide/discovering-symbols
245-245: Call to static method assertSame() on an unknown class PHPUnit\Framework\Assert. (class.notFound)
Learn more at https://phpstan.org/user-guide/discovering-symbols
246-246: Call to static method assertEquals() on an unknown class PHPUnit\Framework\Assert. (class.notFound)
Learn more at https://phpstan.org/user-guide/discovering-symbols
247-247: Call to static method assertEquals() on an unknown class PHPUnit\Framework\Assert. (class.notFound)
Learn more at https://phpstan.org/user-guide/discovering-symbols
249-249: Call to static method assertEquals() on an unknown class PHPUnit\Framework\Assert. (class.notFound)
Learn more at https://phpstan.org/user-guide/discovering-symbols
253-253: Method hiqdev\php\billing\tests\behat\bootstrap\BillingContext::findBill() has invalid return type hiqdev\php\billing\bill\BillInterface. (class.notFound)
255-255: Access to an undefined property hiqdev\php\billing\tests\behat\bootstrap\BillingContext::$builder. (property.notFound)
Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property
267-267: Access to an undefined property hiqdev\php\billing\tests\behat\bootstrap\BillingContext::$builder. (property.notFound)
Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property
273-273: Call to static method assertEquals() on an unknown class PHPUnit\Framework\Assert. (class.notFound)
Learn more at https://phpstan.org/user-guide/discovering-symbols
281-281: Call to static method assertEquals() on an unknown class PHPUnit\Framework\Assert. (class.notFound)
Learn more at https://phpstan.org/user-guide/discovering-symbols
300-300: Call to static method assertNotNull() on an unknown class PHPUnit\Framework\Assert. (class.notFound)
Learn more at https://phpstan.org/user-guide/discovering-symbols
301-301: Call to static method assertSame() on an unknown class PHPUnit\Framework\Assert. (class.notFound)
Learn more at https://phpstan.org/user-guide/discovering-symbols
302-302: Call to static method assertSame() on an unknown class PHPUnit\Framework\Assert. (class.notFound)
Learn more at https://phpstan.org/user-guide/discovering-symbols
303-303: Call to static method assertEquals() on an unknown class PHPUnit\Framework\Assert. (class.notFound)
Learn more at https://phpstan.org/user-guide/discovering-symbols
304-304: Call to static method assertSame() on an unknown class PHPUnit\Framework\Assert. (class.notFound)
Learn more at https://phpstan.org/user-guide/discovering-symbols
305-305: Call to static method assertEquals() on an unknown class PHPUnit\Framework\Assert. (class.notFound)
Learn more at https://phpstan.org/user-guide/discovering-symbols
306-306: Call to static method assertEquals() on an unknown class PHPUnit\Framework\Assert. (class.notFound)
Learn more at https://phpstan.org/user-guide/discovering-symbols
309-309: Method hiqdev\php\billing\tests\behat\bootstrap\BillingContext::findCharge() has invalid return type hiqdev\php\billing\charge\ChargeInterface. (class.notFound)
325-325: Method hiqdev\php\billing\tests\behat\bootstrap\BillingContext::getNextCharge() has invalid return type hiqdev\php\billing\charge\ChargeInterface. (class.notFound)
398-398: Access to an undefined property hiqdev\php\billing\tests\behat\bootstrap\BillingContext::$builder. (property.notFound)
Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property
398-398: Call to an undefined method hiqdev\php\billing\tests\behat\bootstrap\BillingContext::mayFail(). (method.notFound)
406-406: Access to an undefined property hiqdev\php\billing\tests\behat\bootstrap\BillingContext::$builder. (property.notFound)
Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property
406-406: Call to an undefined method hiqdev\php\billing\tests\behat\bootstrap\BillingContext::mayFail(). (method.notFound)
414-414: Access to an undefined property hiqdev\php\billing\tests\behat\bootstrap\BillingContext::$builder. (property.notFound)
Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property
437-437: Call to static method fail() on an unknown class PHPUnit\Framework\Assert. (class.notFound)
Learn more at https://phpstan.org/user-guide/discovering-symbols
445-445: Access to an undefined property hiqdev\php\billing\tests\behat\bootstrap\BillingContext::$builder. (property.notFound)
Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property
449-449: Call to static method assertCount() on an unknown class PHPUnit\Framework\Assert. (class.notFound)
Learn more at https://phpstan.org/user-guide/discovering-symbols
457-457: Call to an undefined method hiqdev\php\billing\tests\behat\bootstrap\BillingContext::assertCaughtExceptionMatches(). (method.notFound)
465-465: Access to an undefined property hiqdev\php\billing\tests\behat\bootstrap\BillingContext::$builder. (property.notFound)
Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property
473-473: Access to an undefined property hiqdev\php\billing\tests\behat\bootstrap\BillingContext::$builder. (property.notFound)
Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property
479-479: Parameter $usesTable of method hiqdev\php\billing\tests\behat\bootstrap\BillingContext::targetHasTheFollowingUses() has invalid type Behat\Gherkin\Node\TableNode. (class.notFound)
482-482: Access to an undefined property hiqdev\php\billing\tests\behat\bootstrap\BillingContext::$builder. (property.notFound)
Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property
483-483: Call to static method assertCount() on an unknown class PHPUnit\Framework\Assert. (class.notFound)
Learn more at https://phpstan.org/user-guide/discovering-symbols
486-486: Call to static method assertSame() on an unknown class PHPUnit\Framework\Assert. (class.notFound)
Learn more at https://phpstan.org/user-guide/discovering-symbols
490-490: Call to static method assertEquals() on an unknown class PHPUnit\Framework\Assert. (class.notFound)
Learn more at https://phpstan.org/user-guide/discovering-symbols
Additional comments not posted (5)
src/sale/Sale.php (3)
56-56: Change in data type for$dataproperty from array to string.This change aligns with the PR's objective to handle JSON string conversion and validation.
64-71: Constructor now accepts$dataas a parameter and sets it usingsetData.Ensure that all instantiations of
Saleare updated to pass the correct data type.
153-155:getDatamethod now returns an array decoded from the JSON string stored in$data.This change supports the new data handling strategy as described in the PR summary.
Tools
PHPStan
155-155: Call to static method decode() on an unknown class yii\helpers\Json. (class.notFound)
Learn more at https://phpstan.org/user-guide/discovering-symbolstests/behat/bootstrap/BillingContext.php (2)
191-197: Added methodrecalculateAutoTariffto handle auto tariff recalculations for a target.This addition enhances the testing capabilities by allowing automated tariff recalculations in test scenarios.
Tools
PHPStan
196-196: Access to an undefined property hiqdev\php\billing\tests\behat\bootstrap\BillingContext::$builder. (property.notFound)
Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property
242-249: Enhanced assertions inbillWithTimeto include detailed checks for bill properties.These changes improve the robustness of the tests by ensuring that all bill properties match expected values.
Tools
PHPStan
242-242: Call to static method assertSame() on an unknown class PHPUnit\Framework\Assert. (class.notFound)
Learn more at https://phpstan.org/user-guide/discovering-symbols
243-243: Call to static method assertSame() on an unknown class PHPUnit\Framework\Assert. (class.notFound)
Learn more at https://phpstan.org/user-guide/discovering-symbols
244-244: Call to static method assertEquals() on an unknown class PHPUnit\Framework\Assert. (class.notFound)
Learn more at https://phpstan.org/user-guide/discovering-symbols
245-245: Call to static method assertSame() on an unknown class PHPUnit\Framework\Assert. (class.notFound)
Learn more at https://phpstan.org/user-guide/discovering-symbols
246-246: Call to static method assertEquals() on an unknown class PHPUnit\Framework\Assert. (class.notFound)
Learn more at https://phpstan.org/user-guide/discovering-symbols
247-247: Call to static method assertEquals() on an unknown class PHPUnit\Framework\Assert. (class.notFound)
Learn more at https://phpstan.org/user-guide/discovering-symbols
249-249: Call to static method assertEquals() on an unknown class PHPUnit\Framework\Assert. (class.notFound)
Learn more at https://phpstan.org/user-guide/discovering-symbols
src/sale/Sale.php
Outdated
| @@ -17,6 +17,7 @@ | |||
| use hiqdev\php\billing\Exception\InvariantException; | |||
| use hiqdev\php\billing\plan\PlanInterface; | |||
| use hiqdev\php\billing\target\TargetInterface; | |||
| use yii\helpers\Json; | |||
There was a problem hiding this comment.
This package does not depend on Yii.
src/sale/Sale.php
Outdated
| @@ -52,22 +53,22 @@ class Sale implements SaleInterface | |||
|
|
|||
| protected ?DateTimeImmutable $closeTime = null; | |||
|
|
|||
| protected ?array $data = null; | |||
| protected ?string $data = null; | |||
There was a problem hiding this comment.
Why do you prefer to store data as a JSON string instead of an array?
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/sale/Sale.php (2 hunks)
Additional context used
PHPStan
src/sale/Sale.php
27-27: Class hiqdev\php\billing\sale\Sale implements unknown interface hiqdev\php\billing\sale\SaleInterface. (interface.notFound)
Learn more at https://phpstan.org/user-guide/discovering-symbols
60-60: Parameter $target of method hiqdev\php\billing\sale\Sale::__construct() has invalid type hiqdev\php\billing\target\TargetInterface. (class.notFound)
61-61: Parameter $customer of method hiqdev\php\billing\sale\Sale::__construct() has invalid type hiqdev\php\billing\customer\CustomerInterface. (class.notFound)
62-62: Parameter $plan of method hiqdev\php\billing\sale\Sale::__construct() has invalid type hiqdev\php\billing\plan\PlanInterface. (class.notFound)
112-112: Instantiated class hiqdev\php\billing\Exception\InvariantException not found. (class.notFound)
Learn more at https://phpstan.org/user-guide/discovering-symbols
116-116: Instantiated class hiqdev\php\billing\Exception\ConstraintException not found. (class.notFound)
Learn more at https://phpstan.org/user-guide/discovering-symbols
128-128: Instantiated class hiqdev\php\billing\Exception\CannotReassignException not found. (class.notFound)
Learn more at https://phpstan.org/user-guide/discovering-symbols
Additional comments not posted (2)
src/sale/Sale.php (2)
133-135: Verify and ensuresetDatamethod correctly handles JSON conversion.The
setDatamethod should convert array inputs to JSON strings, ensuring consistency with thegetDatamethod. Please verify its implementation and ensure it aligns with the changes made to thegetDatamethod.
Line range hint
27-27: Verify project configuration for static analysis tool.The static analysis tool has reported several issues regarding unknown classes and interfaces. This might be due to the tool not being properly configured with the project's dependencies or context. Please verify the configuration to ensure accurate analysis.
Also applies to: 60-62, 112-128
src/sale/Sale.php
Outdated
| @@ -17,6 +17,7 @@ | |||
| use hiqdev\php\billing\Exception\InvariantException; | |||
| use hiqdev\php\billing\plan\PlanInterface; | |||
| use hiqdev\php\billing\target\TargetInterface; | |||
| use yii\helpers\Json; | |||
There was a problem hiding this comment.
Remove unnecessary import of yii\helpers\Json.
This package does not depend on Yii, as noted by SilverFire in a previous comment. Please remove this import to avoid confusion and potential errors.
| public function getData(): ?array | ||
| { | ||
| return $this->data; | ||
| return !empty($this->data) ? $this->data : null; |
There was a problem hiding this comment.
Update getData method to handle JSON strings.
The getData method should return an array decoded from the JSON string stored in $data, as per the PR's objective and the AI-generated summary. Currently, it still treats $data as an array.
- return !empty($this->data) ? $this->data : null;
+ return !empty($this->data) ? json_decode($this->data, true) : null;Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| public function getData(): ?array | |
| { | |
| return $this->data; | |
| return !empty($this->data) ? $this->data : null; | |
| public function getData(): ?array | |
| { | |
| return !empty($this->data) ? json_decode($this->data, true) : null; |
| Assert::assertSame($type, $bill->getType()->getName(), "Bill type mismatch: expected $type, got {$bill->getType()->getName()}"); | ||
| Assert::assertSame($target, $bill->getTarget()->getFullName(), "Bill target mismatch: expected $target, got {$bill->getTarget()->getFullName()}"); | ||
| Assert::assertEquals(bcmul($sum, 100), $bill->getSum()->getAmount(), "Bill sum mismatch: expected $sum, got {$bill->getSum()->getAmount()}"); | ||
| Assert::assertSame($currency, $bill->getSum()->getCurrency()->getCode(), "Bill currency mismatch: expected $currency, got {$bill->getSum()->getCurrency()->getCode()}"); | ||
| Assert::assertEquals((float)$quantity, (float)$bill->getQuantity()->getQuantity(), "Bill quantity mismatch: expected $quantity, got {$bill->getQuantity()->getQuantity()}"); | ||
| Assert::assertEquals(strtolower($unit), strtolower($bill->getQuantity()->getUnit()->getName()), "Bill unit mismatch: expected $unit, got {$bill->getQuantity()->getUnit()->getName()}"); | ||
| if ($time) { | ||
| Assert::assertEquals(new DateTimeImmutable($time), $bill->getTime()); | ||
| Assert::assertEquals(new DateTimeImmutable($time), $bill->getTime(), "Bill time mismatch: expected $time, got {$bill->getTime()->format(DATE_ATOM)}"); |
There was a problem hiding this comment.
It seems you've mixed different PRs
| if (str_contains($time, 'pm')) { | ||
| $time = str_replace('pm', 'm', $time); | ||
| $time = date($time, strtotime('-1 month')); | ||
| } |
| /** | ||
| * @Given /recalculate autotariff for target (\S+)( +at (\S+))?$/ | ||
| */ | ||
| public function recalculateAutoTariff(string $target, string $time = null): void | ||
| { | ||
| $this->builder->clientSetAutoTariff($target, $time); | ||
| } | ||
|
|
Summary by CodeRabbit
New Features
$reasonto the sale creation data transfer object.Bug Fixes
Tests