Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
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
HP-1751 Create configurations for billing types #93
base: master
Are you sure you want to change the base?
HP-1751 Create configurations for billing types #93
Changes from 54 commits
01dc6ff
f819552
f5292a4
ccb0b9c
9f7c0da
83d242f
0e26ed5
ff69913
1ac3f7d
99371a4
01e778a
e95efbc
1e731a9
ef63f8f
e72ae0f
94f7fb3
93c0ea8
7972157
e967f1b
944f07d
821f83e
9bc5a8e
b5065b0
0247a8b
e2fd487
48a032e
bc62b19
ea5c85f
02b3aee
a7d1c34
baab974
18c7690
e65d27d
fea22bd
f6c5c1c
9b8bb1d
d9a5ef7
2bb5310
881679f
1acbb7b
0c060ab
93686bb
77b9efe
9dad8c6
285ae11
afe5a62
57394fe
b649b44
2cc1971
6411325
0328f42
1f7fb11
1cfa210
c75b5d4
27c16d1
6808daf
413f729
d7d838a
2c6a195
6f0783b
0e0bc7c
75ee687
9b25ddf
7d09102
c00ea1e
4ce2b38
7269bf9
03ee8dc
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add input validation and fix return type hint
The method needs validation for the representation class and updated return type documentation.
Apply these improvements:
📝 Committable suggestion
🧰 Tools
🪛 GitHub Check: PHP 8.3
[failure] 42-42: UndefinedDocblockClass
src/product/BillingRegistry.php:42:16: UndefinedDocblockClass: Docblock-defined class, interface or enum named hiqdev\billing\registry\invoice\RepresentationInterface does not exist (see https://psalm.dev/200)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Refactor behavior management for better maintainability
The behavior management implementation needs several improvements:
Apply these improvements:
📝 Committable suggestion
🧰 Tools
🪛 GitHub Check: PHP 8.3
[failure] 104-104: UndefinedClass
src/product/BillingRegistry.php:104:22: UndefinedClass: Class, interface or enum named hiqdev\billing\registry\behavior\TariffTypeBehaviorCollection does not exist (see https://psalm.dev/019)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add import for AggregateInterface and improve error message.
The
AggregateInterface
is used but not imported. Also, enhance the error message for better debugging.Apply these changes:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Integration verification reveals potential issues
The codebase analysis shows a potential mismatch in the implementation. While
InvoiceDescriptionsBuilder
callsrepresentInvoice()
on price types, the actual implementation shows:BillingRegistry::priceTypes()
exists but returns a GeneratorPriceTypesCollection
andPriceTypeDefinition
PriceTypeDefinition
hasinvoiceRepresentation()
method, but norepresentInvoice()
methodrepresentInvoice()
methodThis suggests that either:
representInvoice()
method is missing from the implementation🔗 Analysis chain
Verify integration with dependent components.
Let's ensure the required methods exist in the dependent classes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
Length of output: 204
Script:
Length of output: 4025
Script:
Length of output: 1992
Script:
Length of output: 1407
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add error handling and consider using array_map.
The build method could benefit from several improvements:
📝 Committable suggestion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolve Psalm’s missing template param error.
Static analysis indicates that
TariffTypeDefinition
is missing a template parameter expected byTariffTypeDefinitionInterface
. Consult the interface definition to determine the correct template syntax, for example:📝 Committable suggestion
🧰 Tools
🪛 GitHub Check: PHP 8.3
[failure] 10-10:
src/product/TariffTypeDefinition.php:10:39: MissingTemplateParam: hiqdev\php\billing\product\TariffTypeDefinition has missing template params when extending hiqdev\php\billing\product\TariffTypeDefinitionInterface, expecting 1 (see https://psalm.dev/182)
🪛 GitHub Actions: Psalm Static Analysis
[error] 10-10: MissingTemplateParam: hiqdev\php\billing\product\TariffTypeDefinition has missing template params when extending hiqdev\php\billing\product\TariffTypeDefinitionInterface, expecting 1 (see https://psalm.dev/182)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implement validation in the
end()
methodThe
end()
method contains a comment about validating the tariff type and locking its state, but doesn't implement any actual validation logic. This should be implemented to ensure tariff types are properly configured before use.📝 Committable suggestion