Skip to content

feat: Model & schema validations for donations #6062

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
Merged
merged 2 commits into from
Jun 19, 2019
Merged

feat: Model & schema validations for donations #6062

merged 2 commits into from
Jun 19, 2019

Conversation

@mrsaicharan1
Copy link
Member

@mrsaicharan1 mrsaicharan1 commented Jun 18, 2019 •
edited
Loading

Fixes #6061

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.

Short description of what this resolves:

  • Adds schemas & fields for donations

Changes proposed in this pull request:

  • Migration file
  • Schema & model additions
  • Added validation checks

houndci-bot
houndci-bot reviewed Jun 18, 2019
View reviewed changes
@auto-label auto-label bot added the feature label Jun 18, 2019
@mrsaicharan1
Copy link
Member Author

mrsaicharan1 commented Jun 18, 2019

@uds5501 @shreyanshdwivedi Please review.

uds5501
uds5501 reviewed Jun 18, 2019
View reviewed changes
@codecov
Copy link

codecov bot commented Jun 18, 2019 •
edited
Loading

Codecov Report

Merging #6062 into development will increase coverage by <.01%.
The diff coverage is 77.77%.

Impacted file tree graph

@@               Coverage Diff               @@
##           development    #6062      +/-   ##
===============================================
+ Coverage        66.12%   66.12%   +<.01%     
===============================================
  Files              285      285              
  Lines            14089    14098       +9     
===============================================
+ Hits              9316     9323       +7     
- Misses            4773     4775       +2
Impacted Files Coverage Δ
app/models/ticket.py 72.91% <100%> (+1.17%) ⬆️
app/api/schema/tickets.py 86.84% <60%> (-1.9%) ⬇️

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 d78839b...eb18960. Read the comment docs.

shreyanshdwivedi
shreyanshdwivedi suggested changes Jun 18, 2019
View reviewed changes
Copy link
Member

@shreyanshdwivedi shreyanshdwivedi left a comment

Choose a reason for hiding this comment

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

@mrsaicharan1 I think there should be a validation check so that min_price <= max_price.

houndci-bot
houndci-bot reviewed Jun 18, 2019
View reviewed changes
@mrsaicharan1
Copy link
Member Author

mrsaicharan1 commented Jun 18, 2019

@mrsaicharan1 I think there should be a validation check so that min_price <= max_price.

Yeah. Made it.

@mrsaicharan1
Copy link
Member Author

mrsaicharan1 commented Jun 19, 2019

@uds5501 @shreyanshdwivedi @iamareebjamal Please review.

uds5501
uds5501 suggested changes Jun 19, 2019
View reviewed changes
shreyanshdwivedi
shreyanshdwivedi reviewed Jun 19, 2019
View reviewed changes
abhinavk96
abhinavk96 suggested changes Jun 19, 2019
View reviewed changes
migrations/versions/43e8c59337ae_.py

def upgrade():
# ### commands auto generated by Alembic - please adjust! ###
op.add_column('tickets', sa.Column('max_price', sa.Float(), nullable=True))
Copy link
Contributor

@abhinavk96 abhinavk96 Jun 19, 2019

Choose a reason for hiding this comment

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

here it's nullable false, I don't see any specification in the model file.

houndci-bot
houndci-bot reviewed Jun 19, 2019
View reviewed changes
@mrsaicharan1 mrsaicharan1 force-pushed the donation-fields branch 3 times, most recently from 5b0b792 to 9e4f058 Compare June 19, 2019 11:10
@fossasia fossasia deleted a comment Jun 19, 2019
@mrsaicharan1
Copy link
Member Author

mrsaicharan1 commented Jun 19, 2019

@CosmicCoder96 I've made the requested changes. Please review.

@mrsaicharan1 mrsaicharan1 changed the title feat: Model & schema additions for donations feat: Model & schema validations for donations Jun 19, 2019
iamareebjamal
iamareebjamal reviewed Jun 19, 2019
View reviewed changes
iamareebjamal
iamareebjamal reviewed Jun 19, 2019
View reviewed changes
@mrsaicharan1
Copy link
Member Author

mrsaicharan1 commented Jun 19, 2019

@iamareebjamal Oh okay! I thought only boolean fields required a server_default. Understood now. Updated it.

iamareebjamal
iamareebjamal reviewed Jun 19, 2019
View reviewed changes
@fossasia fossasia deleted a comment Jun 19, 2019
@mrsaicharan1
changed nullable value
eb18960 
Updated validations & corrected pointer info

Updated revision head

provided defaults
@fossasia fossasia deleted a comment Jun 19, 2019
@abhinavk96 abhinavk96 merged commit 636c8ba into fossasia:development Jun 19, 2019
@iamareebjamal iamareebjamal mentioned this pull request Jun 19, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@iamareebjamal iamareebjamal iamareebjamal approved these changes

+4 more reviewers

@houndci-bot houndci-bot houndci-bot left review comments

@shreyanshdwivedi shreyanshdwivedi shreyanshdwivedi requested changes

@uds5501 uds5501 uds5501 requested changes

@abhinavk96 abhinavk96 abhinavk96 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

feature

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

feat: Max/Min price fields for donation tickets

6 participants

@mrsaicharan1 @iamareebjamal @houndci-bot @abhinavk96 @uds5501 @shreyanshdwivedi