Skip to content
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

Refactor delete...() methods #284

Merged
merged 6 commits into from
Jan 14, 2024
Merged

Refactor delete...() methods #284

merged 6 commits into from
Jan 14, 2024

Conversation

Tigrov
Copy link
Member

@Tigrov Tigrov commented Dec 29, 2023

  • Remove redundant false return type in ActiveRecordInterface::delelte() method
  • Refactor and move ActiveRecord::deleteInternal() to BaseActiveRecord::deleteInternal()
  • Add optimistic lock tests on delete and after delete.
Q A
Is bugfix?
New feature?
Breaks BC?
Fixed issues -

Copy link

codecov bot commented Dec 29, 2023

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (fb7f4a8) 87.79% compared to head (f3177f5) 88.63%.

Files Patch % Lines
src/BaseActiveRecord.php 84.61% 2 Missing ⚠️
src/ActiveRecord.php 50.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master     #284      +/-   ##
============================================
+ Coverage     87.79%   88.63%   +0.83%     
+ Complexity      580      575       -5     
============================================
  Files             7        7              
  Lines          1311     1302       -9     
============================================
+ Hits           1151     1154       +3     
+ Misses          160      148      -12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

what-the-diff bot commented Dec 29, 2023

PR Summary

  • Updates to ActiveRecord.php, ActiveRecordInterface.php, and BaseActiveRecord.php
    The existing delete() methods in these files were updated to always return an integer value. Previously, they could return either false (representing an unsuccessful delete) or an integer (representing the number of deleted records). Now, they only return an integer, which simplifies the tracking of delete operations and makes the code more consistent. Additionally, minor code cleanup was performed for efficiency and readability.

  • Introduction of deleteInternal() in BaseActiveRecord.php
    A new method, deleteInternal(), was added to handle the actual deletion logic. This makes the code more straightforward and gathers the deletion operations into a single location.

  • Additional test methods in ActiveQueryTest.php
    Two new test methods, testOptimisticLockOnDelete() and testOptimisticLockAfterDelete(), were added. These tests will ensure the code is working as expected when handling deletion operations. This contributes to improving overall code reliability and predictability.

  • Minor correction in Cat.php
    There was a minor fix made to a division operation in the file. This update ensures the accuracy of the operation.

Please note that while some changes might come off as minor, they contribute significantly to the overall performance and reliability of the application.

@Tigrov Tigrov added the status:under development Someone is working on a pull request. label Dec 29, 2023
@Tigrov Tigrov added status:code review The pull request needs review. and removed status:under development Someone is working on a pull request. labels Jan 2, 2024
# Conflicts:
#	src/ActiveRecord.php
#	src/BaseActiveRecord.php
@Tigrov Tigrov merged commit d2fa237 into master Jan 14, 2024
55 of 57 checks passed
@Tigrov Tigrov deleted the refactor-delete branch January 14, 2024 02:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:code review The pull request needs review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants