Skip to content

Conversation

@mrsaicharan1
Copy link
Member

Fixes #6280

Short description of what this resolves:

Adds api checks for ticket prices on the server

Changes proposed in this pull request:

  • Added check at before_create_object & before_update_object

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.

@codecov
Copy link

codecov bot commented Aug 1, 2019

Codecov Report

Merging #6281 into development will decrease coverage by 0.02%.
The diff coverage is 12.5%.

Impacted file tree graph

@@               Coverage Diff               @@
##           development    #6281      +/-   ##
===============================================
- Coverage        65.42%   65.39%   -0.03%     
===============================================
  Files              287      287              
  Lines            14727    14735       +8     
===============================================
+ Hits              9635     9636       +1     
- Misses            5092     5099       +7
Impacted Files Coverage Δ
app/api/tickets.py 45.13% <12.5%> (-2.49%) ⬇️

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 250b920...2c57240. Read the comment docs.

@mrsaicharan1 mrsaicharan1 force-pushed the decimal-val branch 5 times, most recently from 6250724 to 1af787e Compare August 1, 2019 13:26
@shreyanshdwivedi shreyanshdwivedi mentioned this pull request Aug 1, 2019
6 tasks
@mrsaicharan1 mrsaicharan1 force-pushed the decimal-val branch 5 times, most recently from 1122557 to aa0359b Compare August 5, 2019 09:43
@mrsaicharan1
Copy link
Member Author

@shreyanshdwivedi @iamareebjamal @uds5501 Please have a look.

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.

LGTM 👍

@mrsaicharan1 mrsaicharan1 force-pushed the decimal-val branch 3 times, most recently from d10706e to ec29cc4 Compare August 14, 2019 17:03
Copy link
Member

@iamareebjamal iamareebjamal left a comment

Choose a reason for hiding this comment

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

:l

Copy link
Member

@iamareebjamal iamareebjamal left a comment

Choose a reason for hiding this comment

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

Build failing

@mrsaicharan1 mrsaicharan1 force-pushed the decimal-val branch 3 times, most recently from ad3a021 to a21bb4b Compare August 15, 2019 04:45
@mrsaicharan1
Copy link
Member Author

@iamareebjamal The DELETE test for the tickets was failing as the price & type are not required for a DELETE request. Any suggestions on how I could go about this?

@mrsaicharan1
Copy link
Member Author

mrsaicharan1 commented Aug 15, 2019

According to me, I think that that the condition

if not data.get('price') or not data.get('type'):
        raise UnprocessableEntity({}, "Type/price of ticket is missing")

should only be present under before_create_object as the updation of a ticket might not always involve type and price. What do you think? @uds5501 @shreyanshdwivedi
@iamareebjamal

@shreyanshdwivedi
Copy link
Member

I agree @mrsaicharan1
Someone can update only name or any other attribute and this check will create error

@mrsaicharan1
Copy link
Member Author

@iamareebjamal @uds5501 @shreyanshdwivedi Fixed the check to be compatible with the DELETE operation as well. Ready for another review.

@iamareebjamal iamareebjamal merged commit 1a0953d into fossasia:development Aug 15, 2019
@mrsaicharan1
Copy link
Member Author

@iamareebjamal This fix seems to be buggy with donation tickets. I will revert this for now and see what's wrong.

mrsaicharan1 added a commit to mrsaicharan1/open-event-server that referenced this pull request Aug 15, 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.

Server checks for ticket prices are missing

4 participants