Feature:add new formatters for Tickets#494
Conversation
andrestejerina97
force-pushed
the
feature/add-ticket-formatters
branch
from
d325475 to
4b7ad9c
Compare
There was a problem hiding this comment.
@andrestejerina97 please review comments. Thanks
| ); | ||
|
|
||
| case IAuditStrategy::EVENT_ENTITY_DELETION: | ||
| $quantity_used = ($subject->getQuantityAvailable() ?? 0) - ($quantity_available ?? 0); |
There was a problem hiding this comment.
In line 41 we have $quantity_available = $subject->getQuantityAvailable() ?? 0;
This will make $quantity_used always calculate to 0.
There was a problem hiding this comment.
Thank you. Please review changes @martinquiroga-exo
caseylocker
left a comment
caseylocker
left a comment
There was a problem hiding this comment.
This is looking good but shouldn't we see the new formatters registered in config/audit_log.php?
Also, SummitTaxTypeAuditLogFormatter and SummitRefundPolicyTypeAuditLogFormatter are scaling rates incorrectly by multiplying by 100 before rendering, but these are already stored as percentage points (e.g., 21.0, 90.5). This causes audit logs to report inflated values (e.g., 2100% instead of 21%). Remove the extra * 100 in formatter output. The corresponding tests (SummitTaxTypeAuditLogFormatterTest, SummitRefundPolicyTypeAuditLogFormatterTest) also use decimal-fraction mock values
(e.g., 0.21, 1.0, 0.5) instead of real-world percentage-point values (e.g., 21.0, 100.0, 50.0).
|
Ready to review @caseylocker |
caseylocker
left a comment
caseylocker
left a comment
There was a problem hiding this comment.
LGTM but needs to have the merge conflicts fixed before merging.
andrestejerina97
force-pushed
the
feature/add-ticket-formatters
branch
from
2697d22 to
2db0afa
Compare
| use models\summit\PrePaidSummitRegistrationDiscountCode; | ||
| use Illuminate\Support\Facades\Log; | ||
|
|
||
| class PrePaidSummitRegistrationDiscountCodeAuditLogFormatter extends AbstractAuditLogFormatter |
There was a problem hiding this comment.
@andrestejerina97 i do think that here we do need to include more info related to the discount code
including domain specific fields like rate / amount and so
smarcet
left a comment
smarcet
left a comment
There was a problem hiding this comment.
@andrestejerina97 please review coments
4 participants
ref: https://app.clickup.com/t/86b8acw9f