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 update...() methods #282

Merged
merged 11 commits into from
Jan 14, 2024
Merged

Refactor update...() methods #282

merged 11 commits into from
Jan 14, 2024

Conversation

Tigrov
Copy link
Member

@Tigrov Tigrov commented Dec 24, 2023

  • Fix bug: ActiveRecord::save() without changes should return true;
  • Fix bug with return type of ActiveRecord::getOldPrimaryKey() method;
  • Remove redundant false return type in ActiveRecordInterface::update() method;
  • Refactor BaseActiveRecord::updateAttributes();
  • Refactor BaseActiveRecord::updateInternal();
  • Refactor BaseActiveRecord::updateCounters().
Q A
Is bugfix? ✔️
New feature?
Breaks BC?
Fixed issues -

Copy link

codecov bot commented Dec 24, 2023

Codecov Report

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

Comparison is base (2ba318b) 87.70% compared to head (f3648db) 88.02%.

Files Patch % Lines
src/BaseActiveRecord.php 92.30% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master     #282      +/-   ##
============================================
+ Coverage     87.70%   88.02%   +0.31%     
+ Complexity      595      587       -8     
============================================
  Files             7        7              
  Lines          1326     1319       -7     
============================================
- Hits           1163     1161       -2     
+ Misses          163      158       -5     

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

Copy link

what-the-diff bot commented Dec 24, 2023

PR Summary

  • Refined issue handling in psalm.xml
    The addition of an issueHandlers section helps suppress the MixedAssignment error level, increasing code inspection efficacy.
  • Standardized return types in ActiveRecord.php and BaseActiveRecord.php
    Several methods in these two files have had their return types altered. Specifically, certain methods which originally could return false|int now strictly return int, or have specific conditions guiding their output. This increases the predictability of these methods' behaviours.
  • Improved documentation in ActiveRecordInterface.php and BaseActiveRecord.php
    Several methods in these files now have documented return types, making it easier for developers to understand what to expect upon the method's execution.
  • Updated attribute handling in BaseActiveRecord.php
    The updateAttributes method now employs setAttribute to update attribute values, leading to improved handling and functionality of attributes.
  • Enhanced error handling in BaseActiveRecord.php
    The updateInternal method now throws a StaleObjectException, rather than returning false. This makes error capturing and handling less ambiguous and more efficient.
  • New test case in ActiveRecordTest.php
    Added testSaveWithoutChanges, a new test case for Agile automation testing processes. This increases overall test coverage, ensuring that the function works as expected, even in edge case scenarios.

@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
@Tigrov Tigrov merged commit c5f92e5 into master Jan 14, 2024
56 of 57 checks passed
@Tigrov Tigrov deleted the refactor-update branch January 14, 2024 01:57
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.

5 participants