Skip to content

Conversation

@uds5501
Copy link
Contributor

@uds5501 uds5501 commented Aug 16, 2019

Fixes #6368

Changes proposed in this pull request:

  • set default value of event invoice objects to "due"

Checklist

  • I have read the Contribution & Best practices Guide and my PR follows them.
  • My branch is up-to-date with the Upstream development branch.
  • The unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
  • All the functions created/modified in this PR contain relevant docstrings.

mrsaicharan1
mrsaicharan1 previously approved these changes Aug 16, 2019
@iamareebjamal
Copy link
Member

iamareebjamal commented Aug 16, 2019

Shouldn't you also write a migration for existing ones with pending status? And also for new ones with default status

@shreyanshdwivedi
Copy link
Member

I think currently the event invoice not completely in function so maybe they are not in db too
@uds5501 please fix travis

@uds5501 uds5501 dismissed stale reviews from mrsaicharan1 and shreyanshdwivedi via ca751bf August 17, 2019 11:36
@uds5501
Copy link
Contributor Author

uds5501 commented Aug 17, 2019

@iamareebjamal I have included the migration, please review now

@codecov
Copy link

codecov bot commented Aug 17, 2019

Codecov Report

Merging #6369 into development will not change coverage.
The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff              @@
##           development    #6369   +/-   ##
============================================
  Coverage        65.29%   65.29%           
============================================
  Files              287      287           
  Lines            14775    14775           
============================================
  Hits              9647     9647           
  Misses            5128     5128
Impacted Files Coverage Δ
app/models/event_invoice.py 57.53% <0%> (ø) ⬆️

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 c576251...bb8e882. Read the comment docs.

Copy link
Member

@mrsaicharan1 mrsaicharan1 left a comment

Choose a reason for hiding this comment

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

I think this migration would do. Did you test it out by running it locally though?

@uds5501
Copy link
Contributor Author

uds5501 commented Aug 17, 2019

@mrsaicharan1 yes, tested it locally, it's working

@uds5501 uds5501 merged commit 1c21c35 into fossasia:development Aug 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Generated event invoices have a default state of 'pending'

4 participants