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.

Sign up for GitHub

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

Jump to bottom

Fix bulk_update #534

Merged
merged 15 commits into from
Dec 20, 2019
Merged

Fix bulk_update #534

merged 15 commits into from
Dec 20, 2019

Conversation

satels
Copy link
Contributor

@satels satels commented Dec 16, 2019

Close #507 and double issue #533

Stranger6667
Stranger6667 requested changes Dec 16, 2019
View reviewed changes
Copy link
Collaborator

@Stranger6667 Stranger6667 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 your contribution! Could you please add a test case and a changelog entry for it?

@codecov-io
Copy link

codecov-io commented Dec 16, 2019 •
edited
Loading

Codecov Report

Merging #534 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #534      +/-   ##
==========================================
+ Coverage   98.07%   98.07%   +<.01%     
==========================================
  Files          29       29              
  Lines         883      884       +1     
  Branches      153      153              
==========================================
+ Hits          866      867       +1     
  Misses         12       12              
  Partials        5        5
Impacted Files Coverage Δ
djmoney/models/managers.py 99.29% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2db971c...43ea086. Read the comment docs.

@satels
Copy link
Contributor Author

satels commented Dec 16, 2019

Yes, of course

@satels
Copy link
Contributor Author

satels commented Dec 16, 2019

Done

@satels
Copy link
Contributor Author

satels commented Dec 16, 2019

I should add the record to changelog also

@satels
Copy link
Contributor Author

satels commented Dec 16, 2019

I also changed the version

Stranger6667
Stranger6667 requested changes Dec 17, 2019
View reviewed changes
djmoney/__init__.py Outdated Show resolved Hide resolved
docs/changes.rst Outdated Show resolved Hide resolved
tests/test_models.py Outdated Show resolved Hide resolved
tests/test_models.py Outdated Show resolved Hide resolved
tests/test_models.py Outdated Show resolved Hide resolved
benjaoming
benjaoming reviewed Dec 17, 2019
View reviewed changes
README.rst Show resolved Hide resolved
benjaoming
benjaoming approved these changes Dec 17, 2019
View reviewed changes
Copy link
Contributor

@benjaoming benjaoming left a comment

Choose a reason for hiding this comment

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

Thanks so much @satels for the thorough work, I think when you address @Stranger6667 's feedback, this could be well-baked and ready to come out of the oven 🍰

@satels
Copy link
Contributor Author

satels commented Dec 17, 2019

I will make changes tomorrow.

@satels
Copy link
Contributor Author

satels commented Dec 20, 2019

Ok, this pull request is ready to merge

@Stranger6667
Copy link
Collaborator

Thanks! Could you, please, rebase? I can't do automatic rebase due to conflicts

@satels
Copy link
Contributor Author

satels commented Dec 20, 2019

I did rebase for that

benjaoming
benjaoming approved these changes Dec 20, 2019
View reviewed changes
Copy link
Contributor

@benjaoming benjaoming left a comment

Choose a reason for hiding this comment

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

Some really well-baked stuff, congrats on the first PR in django-money @satels 🎉

docs/changes.rst Outdated Show resolved Hide resolved
@satels @benjaoming
Update docs/changes.rst
43ea086 
Co-Authored-By: Benjamin Balder Bach <benjaoming@gmail.com>
@Stranger6667 Stranger6667 merged commit e46109f into django-money:master Dec 20, 2019
@cloudcode-hungary
Copy link

Guys, is there any eta to release the 1.1 version to pypi? I would really appreciate to have this fix.

@benjaoming
Copy link
Contributor

Reviewed the changes and 1.0...master seems fine. Wondering why it got bumped to 1.1 since the current changes don't have anything backwards incompatible or big big features.

But I guess it's fine, we can be generous with the version numbers now that we're past 1.0 :)

@benjaoming
Copy link
Contributor

@Stranger6667 shall we?

@Stranger6667
Copy link
Collaborator

Let's do it :)

@benjaoming
Copy link
Contributor

Cool, pushing some buttons now

stianjensen added a commit to stianjensen/django-money that referenced this pull request Jul 11, 2022
@stianjensen
Handle Coalesce in updates
eb90eb8 
Modeled on django-money#534, which
I see came out of a similar issue as I've been seeing now:
"AttributeError: 'Coalesce' object has no attribute 'rhs'
@stianjensen stianjensen mentioned this pull request Jul 11, 2022
Merged
stianjensen added a commit to stianjensen/django-money that referenced this pull request Jul 11, 2022
@stianjensen
Handle Coalesce in updates
be159a7 
Modeled on django-money#534, which
I see came out of a similar issue as I've been seeing now:
"AttributeError: 'Coalesce' object has no attribute 'rhs'
stianjensen added a commit to stianjensen/django-money that referenced this pull request Jul 11, 2022
@stianjensen
Handle Coalesce in updates
7e72ec3 
Modeled on django-money#534, which
I see came out of a similar issue as I've been seeing now:
"AttributeError: 'Coalesce' object has no attribute 'rhs'
stianjensen added a commit to stianjensen/django-money that referenced this pull request Jul 11, 2022
@stianjensen
Handle Coalesce in updates
03f68d0 
Modeled on django-money#534, which
I see came out of a similar issue as I've been seeing now:
"AttributeError: 'Coalesce' object has no attribute 'rhs'
stianjensen added a commit to stianjensen/django-money that referenced this pull request Jul 11, 2022
@stianjensen
Handle Coalesce in updates
c7cb67e 
Modeled on django-money#534, which
I see came out of a similar issue as I've been seeing now:
"AttributeError: 'Coalesce' object has no attribute 'rhs'
Stranger6667 pushed a commit that referenced this pull request Oct 28, 2022
@stianjensen @Stranger6667
Handle Coalesce in updates
667342f 
Modeled on #534, which
I see came out of a similar issue as I've been seeing now:
"AttributeError: 'Coalesce' object has no attribute 'rhs'
SunShiny712 pushed a commit to SunShiny712/Django-moeny that referenced this pull request Aug 16, 2023
@stianjensen @Stranger6667
Handle Coalesce in updates
6033141 
Modeled on django-money/django-money#534, which
I see came out of a similar issue as I've been seeing now:
"AttributeError: 'Coalesce' object has no attribute 'rhs'
alexkoren-dev pushed a commit to alexkoren-dev/django_money that referenced this pull request Oct 3, 2023
Handle Coalesce in updates
53c1a5a 
Modeled on django-money/django-money#534, which
I see came out of a similar issue as I've been seeing now:
"AttributeError: 'Coalesce' object has no attribute 'rhs'
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benjaoming benjaoming benjaoming approved these changes

@Stranger6667 Stranger6667 Stranger6667 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Bulk update on models containing MoneyField gives error
5 participants
@satels @codecov-io @Stranger6667 @cloudcode-hungary @benjaoming