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

Deferred fields patch #1393

Merged
merged 29 commits into from
Nov 20, 2024
Merged

Conversation

JordanHyatt
Copy link
Contributor

@JordanHyatt JordanHyatt commented Sep 16, 2024

Description

Provided a patch that allows objects to be deleted when they have deferred fields present

Related Issue

This is related to issue #678

Motivation and Context

This allows users of simple_history to delete objects without having to consider the consequences of deferring fields in their query

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have run the pre-commit run command to format and lint.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have added my name and/or github handle to AUTHORS.rst
  • I have added my change to CHANGES.rst
  • All new and existing tests passed.

@JordanHyatt
Copy link
Contributor Author

JordanHyatt commented Sep 16, 2024

I wanted to work on a new feature so I needed to branch my fork and re-submit this PR

@jwaschkau jwaschkau mentioned this pull request Sep 17, 2024
11 tasks
Copy link
Contributor

@tim-schilling tim-schilling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR @JordanHyatt. I apologize for taking so long to get to this. I think once it's cleaned up, it's an easy thing for us to merge in.

.python-version Outdated Show resolved Hide resolved
CHANGES.rst Outdated Show resolved Hide resolved
CHANGES.rst Outdated Show resolved Hide resolved
docs/signals.rst Outdated Show resolved Hide resolved
@JordanHyatt
Copy link
Contributor Author

All requested changes have been pushed! (except for maybe the signals.rst issue?)

tim-schilling
tim-schilling previously approved these changes Nov 20, 2024
Copy link
Contributor

@tim-schilling tim-schilling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your help!

@JordanHyatt
Copy link
Contributor Author

@tim-schilling sorry I added in a last second piece of logic that will skip loading the fields if its not a history enabled model.

Copy link

codecov bot commented Nov 20, 2024

Codecov Report

Attention: Patch coverage is 83.33333% with 2 lines in your changes missing coverage. Please review.

Project coverage is 96.73%. Comparing base (4a8756a) to head (b01aeac).
Report is 3 commits behind head on master.

Files with missing lines Patch % Lines
simple_history/models.py 83.33% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1393      +/-   ##
==========================================
- Coverage   96.81%   96.73%   -0.09%     
==========================================
  Files          24       24              
  Lines        1446     1470      +24     
  Branches      236      240       +4     
==========================================
+ Hits         1400     1422      +22     
  Misses         25       25              
- Partials       21       23       +2     

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


🚨 Try these New Features:

@JordanHyatt
Copy link
Contributor Author

JordanHyatt commented Nov 20, 2024

I think we are about there, let me know what you think @tim-schilling!

Copy link
Contributor

@tim-schilling tim-schilling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@tim-schilling tim-schilling merged commit 6c10248 into jazzband:master Nov 20, 2024
17 checks passed
@JordanHyatt JordanHyatt deleted the deferred-fields-patch branch November 20, 2024 22:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants