From 71169ba5c30d440bd66213aeaa2b355050ae4e2b Mon Sep 17 00:00:00 2001 From: Uddeshya Singh Date: Sat, 2 Mar 2019 15:45:46 +0530 Subject: [PATCH 1/6] feat : add conflict check for TicketFee - implement conflict checks for currency-country combinations - change TicketFee Factory model - add Unprocessable Entity exception for API --- app/api/ticket_fees.py | 26 +++++++++++++++++++++++++- tests/hook_main.py | 1 + 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/app/api/ticket_fees.py b/app/api/ticket_fees.py index 04fa5254c4..edfb606fb8 100644 --- a/app/api/ticket_fees.py +++ b/app/api/ticket_fees.py @@ -1,7 +1,9 @@ from flask_rest_jsonapi import ResourceDetail, ResourceList +from sqlalchemy.orm.exc import NoResultFound from app import db from app.api.bootstrap import api +from app.api.helpers.exceptions import ConflictException, UnprocessableEntity from app.api.schema.ticket_fees import TicketFeesSchema from app.models.ticket_fee import TicketFees @@ -10,10 +12,32 @@ class TicketFeeList(ResourceList): """ List and create TicketFees """ + def before_post(self, args, kwargs, data): + """ + before post method to check for existing currency-country combination + :param args: + :param kwargs: + :param data: + :return: + """ + if data['country'] and data['currency']: + try: + TicketFees.query.filter_by(country=data['country'], currency=data['currency']).one() + except NoResultFound: + pass + else: + raise ConflictException( + {'pointer': ''}, + "({}-{}) Combination already exists".format(data['currency'], data['country'])) + else: + raise UnprocessableEntity({'source': ''}, "Country or Currency is missing") + decorators = (api.has_permission('is_admin'),) schema = TicketFeesSchema data_layer = {'session': db.session, - 'model': TicketFees} + 'model': TicketFees, + 'methods': {'before_post': before_post} + } class TicketFeeDetail(ResourceDetail): diff --git a/tests/hook_main.py b/tests/hook_main.py index 09fbab8a10..905b3fab48 100644 --- a/tests/hook_main.py +++ b/tests/hook_main.py @@ -1709,6 +1709,7 @@ def ticket_fees_post(transaction): """ with stash['app'].app_context(): ticket_fees = TicketFeesFactory() + ticket_fees.country = 'US' db.session.add(ticket_fees) db.session.commit() From 0095c1c33c4500a0ad10a1a307502410e580fc02 Mon Sep 17 00:00:00 2001 From: Uddeshya Singh Date: Sun, 3 Mar 2019 12:18:26 +0530 Subject: [PATCH 2/6] update error messages --- app/api/ticket_fees.py | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/app/api/ticket_fees.py b/app/api/ticket_fees.py index edfb606fb8..eef97b6ac1 100644 --- a/app/api/ticket_fees.py +++ b/app/api/ticket_fees.py @@ -20,17 +20,20 @@ def before_post(self, args, kwargs, data): :param data: :return: """ - if data['country'] and data['currency']: - try: - TicketFees.query.filter_by(country=data['country'], currency=data['currency']).one() - except NoResultFound: - pass + if 'country' in data and 'currency' in data: + if data['country'] and data['currency']: + try: + TicketFees.query.filter_by(country=data['country'], currency=data['currency']).one() + except NoResultFound: + pass + else: + raise ConflictException( + {'pointer': ''}, + "({}-{}) Combination already exists".format(data['currency'], data['country'])) else: - raise ConflictException( - {'pointer': ''}, - "({}-{}) Combination already exists".format(data['currency'], data['country'])) + raise UnprocessableEntity({'source': ''}, "Country or Currency cannot be NULL") else: - raise UnprocessableEntity({'source': ''}, "Country or Currency is missing") + raise UnprocessableEntity({'source': ''}, "Country or Currency Attribute is missing") decorators = (api.has_permission('is_admin'),) schema = TicketFeesSchema From d522e78aaa147a55152ec85fa71183843edd029d Mon Sep 17 00:00:00 2001 From: Uddeshya Singh Date: Thu, 7 Mar 2019 12:11:13 +0530 Subject: [PATCH 3/6] remove before_post hook --- app/api/ticket_fees.py | 28 +--------------------------- 1 file changed, 1 insertion(+), 27 deletions(-) diff --git a/app/api/ticket_fees.py b/app/api/ticket_fees.py index eef97b6ac1..e4a6fd9980 100644 --- a/app/api/ticket_fees.py +++ b/app/api/ticket_fees.py @@ -1,9 +1,7 @@ from flask_rest_jsonapi import ResourceDetail, ResourceList -from sqlalchemy.orm.exc import NoResultFound from app import db from app.api.bootstrap import api -from app.api.helpers.exceptions import ConflictException, UnprocessableEntity from app.api.schema.ticket_fees import TicketFeesSchema from app.models.ticket_fee import TicketFees @@ -12,34 +10,10 @@ class TicketFeeList(ResourceList): """ List and create TicketFees """ - def before_post(self, args, kwargs, data): - """ - before post method to check for existing currency-country combination - :param args: - :param kwargs: - :param data: - :return: - """ - if 'country' in data and 'currency' in data: - if data['country'] and data['currency']: - try: - TicketFees.query.filter_by(country=data['country'], currency=data['currency']).one() - except NoResultFound: - pass - else: - raise ConflictException( - {'pointer': ''}, - "({}-{}) Combination already exists".format(data['currency'], data['country'])) - else: - raise UnprocessableEntity({'source': ''}, "Country or Currency cannot be NULL") - else: - raise UnprocessableEntity({'source': ''}, "Country or Currency Attribute is missing") - decorators = (api.has_permission('is_admin'),) schema = TicketFeesSchema data_layer = {'session': db.session, - 'model': TicketFees, - 'methods': {'before_post': before_post} + 'model': TicketFees } From 422809edd81684a7a2147e772cf4f7f8239d1c5b Mon Sep 17 00:00:00 2001 From: Uddeshya Singh Date: Thu, 7 Mar 2019 12:13:58 +0530 Subject: [PATCH 4/6] add schema validation for ticket_fee --- app/api/schema/ticket_fees.py | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/app/api/schema/ticket_fees.py b/app/api/schema/ticket_fees.py index d980c3f2e8..908f4b3146 100644 --- a/app/api/schema/ticket_fees.py +++ b/app/api/schema/ticket_fees.py @@ -1,9 +1,12 @@ -from marshmallow import validate +from marshmallow import validate, validates_schema from marshmallow_jsonapi import fields from marshmallow_jsonapi.flask import Schema +from sqlalchemy.orm.exc import NoResultFound from app.api.helpers.static import PAYMENT_CURRENCY_CHOICES from app.api.helpers.utilities import dasherize +from app.api.helpers.exceptions import ConflictException, UnprocessableEntity +from app.models.ticket_fee import TicketFees class TicketFeesSchema(Schema): @@ -19,6 +22,25 @@ class Meta: self_view_kwargs = {'id': ''} inflect = dasherize + @validates_schema(pass_original=True) + def validate_currency_country(self, data, original_data): + if 'country' in data and 'currency' in data: + if data['country'] and data['currency']: + try: + TicketFees.query.filter_by(country=data['country'], currency=data['currency']).one() + except NoResultFound: + pass + else: + # modifications for PATCH request + if 'id' not in original_data: + raise ConflictException( + {'pointer': ''}, + "({}-{}) Combination already exists".format(data['currency'], data['country'])) + else: + raise UnprocessableEntity({'source': ''}, "Country or Currency cannot be NULL") + else: + raise UnprocessableEntity({'source': ''}, "Country or Currency Attribute is missing") + id = fields.Integer(dump_only=True) currency = fields.Str(validate=validate.OneOf(choices=PAYMENT_CURRENCY_CHOICES), allow_none=True) country = fields.String(allow_none=True) From 80742a3a907b13bd6c07e3dc7ac22f718475741a Mon Sep 17 00:00:00 2001 From: Uddeshya Singh Date: Tue, 21 May 2019 19:41:43 +0530 Subject: [PATCH 5/6] resolve merge conflicts and resolve hound issues --- app/api/schema/ticket_fees.py | 28 +++-------------------- app/api/ticket_fees.py | 42 ++++++++++++++++++++++++++++++++--- 2 files changed, 42 insertions(+), 28 deletions(-) diff --git a/app/api/schema/ticket_fees.py b/app/api/schema/ticket_fees.py index 908f4b3146..ec8896f1f4 100644 --- a/app/api/schema/ticket_fees.py +++ b/app/api/schema/ticket_fees.py @@ -1,12 +1,9 @@ -from marshmallow import validate, validates_schema +from marshmallow import validate from marshmallow_jsonapi import fields from marshmallow_jsonapi.flask import Schema -from sqlalchemy.orm.exc import NoResultFound from app.api.helpers.static import PAYMENT_CURRENCY_CHOICES from app.api.helpers.utilities import dasherize -from app.api.helpers.exceptions import ConflictException, UnprocessableEntity -from app.models.ticket_fee import TicketFees class TicketFeesSchema(Schema): @@ -22,27 +19,8 @@ class Meta: self_view_kwargs = {'id': ''} inflect = dasherize - @validates_schema(pass_original=True) - def validate_currency_country(self, data, original_data): - if 'country' in data and 'currency' in data: - if data['country'] and data['currency']: - try: - TicketFees.query.filter_by(country=data['country'], currency=data['currency']).one() - except NoResultFound: - pass - else: - # modifications for PATCH request - if 'id' not in original_data: - raise ConflictException( - {'pointer': ''}, - "({}-{}) Combination already exists".format(data['currency'], data['country'])) - else: - raise UnprocessableEntity({'source': ''}, "Country or Currency cannot be NULL") - else: - raise UnprocessableEntity({'source': ''}, "Country or Currency Attribute is missing") - id = fields.Integer(dump_only=True) - currency = fields.Str(validate=validate.OneOf(choices=PAYMENT_CURRENCY_CHOICES), allow_none=True) - country = fields.String(allow_none=True) + currency = fields.Str(validate=validate.OneOf(choices=PAYMENT_CURRENCY_CHOICES), allow_none=False) + country = fields.String(allow_none=False) service_fee = fields.Float(validate=lambda n: n >= 0, allow_none=True) maximum_fee = fields.Float(validate=lambda n: n >= 0, allow_none=True) diff --git a/app/api/ticket_fees.py b/app/api/ticket_fees.py index e4a6fd9980..f561f9da19 100644 --- a/app/api/ticket_fees.py +++ b/app/api/ticket_fees.py @@ -1,26 +1,62 @@ from flask_rest_jsonapi import ResourceDetail, ResourceList - +from sqlalchemy.orm.exc import NoResultFound from app import db from app.api.bootstrap import api from app.api.schema.ticket_fees import TicketFeesSchema from app.models.ticket_fee import TicketFees +from app.api.helpers.exceptions import ConflictException class TicketFeeList(ResourceList): """ List and create TicketFees """ + + def before_post(self, args, kwargs, data): + """ + method to check for existing country currency combination + :param args: + :param kwargs: + :param data: + :return: + """ + if (data['country'] and data['currency']): + try: + already_existing_combination = TicketFees.query.filter_by(country=data['country'], + currency=data['currency']).one() + if already_existing_combination: + raise ConflictException({'pointer': 'data/attributes/country'}, 'Combination Exists') + except NoResultFound: + pass + decorators = (api.has_permission('is_admin'),) schema = TicketFeesSchema data_layer = {'session': db.session, - 'model': TicketFees - } + 'model': TicketFees} class TicketFeeDetail(ResourceDetail): """ ticket_fee detail by id """ + + def before_patch(self, args, kwargs, data): + """ + method to check if patched combination id is same as new one + :param args: + :param kwargs: + :param data: + :return: + """ + if (data['country'] and data['currency']): + try: + already_existing_combination = TicketFees.query.filter_by(country=data['country'], + currency=data['currency']).one() + if already_existing_combination.id != kwargs['id']: + raise ConflictException({'pointer': 'data/attributes/country'}, 'Combination Exists') + except NoResultFound: + pass + decorators = (api.has_permission('is_admin'),) schema = TicketFeesSchema data_layer = {'session': db.session, From 1257bb0f9468674c5190bdcfaefe85bc55ce9124 Mon Sep 17 00:00:00 2001 From: Uddeshya Singh Date: Mon, 3 Jun 2019 23:05:03 +0530 Subject: [PATCH 6/6] remove api checks and make fields unique --- app/api/ticket_fees.py | 38 ---------------------------- app/models/ticket_fee.py | 4 +-- migrations/versions/cb4ccda76a2b_.py | 30 ++++++++++++++++++++++ 3 files changed, 32 insertions(+), 40 deletions(-) create mode 100644 migrations/versions/cb4ccda76a2b_.py diff --git a/app/api/ticket_fees.py b/app/api/ticket_fees.py index f561f9da19..aae6a16f00 100644 --- a/app/api/ticket_fees.py +++ b/app/api/ticket_fees.py @@ -1,34 +1,14 @@ from flask_rest_jsonapi import ResourceDetail, ResourceList -from sqlalchemy.orm.exc import NoResultFound from app import db from app.api.bootstrap import api from app.api.schema.ticket_fees import TicketFeesSchema from app.models.ticket_fee import TicketFees -from app.api.helpers.exceptions import ConflictException class TicketFeeList(ResourceList): """ List and create TicketFees """ - - def before_post(self, args, kwargs, data): - """ - method to check for existing country currency combination - :param args: - :param kwargs: - :param data: - :return: - """ - if (data['country'] and data['currency']): - try: - already_existing_combination = TicketFees.query.filter_by(country=data['country'], - currency=data['currency']).one() - if already_existing_combination: - raise ConflictException({'pointer': 'data/attributes/country'}, 'Combination Exists') - except NoResultFound: - pass - decorators = (api.has_permission('is_admin'),) schema = TicketFeesSchema data_layer = {'session': db.session, @@ -39,24 +19,6 @@ class TicketFeeDetail(ResourceDetail): """ ticket_fee detail by id """ - - def before_patch(self, args, kwargs, data): - """ - method to check if patched combination id is same as new one - :param args: - :param kwargs: - :param data: - :return: - """ - if (data['country'] and data['currency']): - try: - already_existing_combination = TicketFees.query.filter_by(country=data['country'], - currency=data['currency']).one() - if already_existing_combination.id != kwargs['id']: - raise ConflictException({'pointer': 'data/attributes/country'}, 'Combination Exists') - except NoResultFound: - pass - decorators = (api.has_permission('is_admin'),) schema = TicketFeesSchema data_layer = {'session': db.session, diff --git a/app/models/ticket_fee.py b/app/models/ticket_fee.py index 803a40adce..bd8d76cdd7 100644 --- a/app/models/ticket_fee.py +++ b/app/models/ticket_fee.py @@ -10,8 +10,8 @@ class TicketFees(db.Model): __tablename__ = 'ticket_fees' id = db.Column(db.Integer, primary_key=True) - currency = db.Column(db.String) - country = db.Column(db.String) + currency = db.Column(db.String, unique=True) + country = db.Column(db.String, unique=True) service_fee = db.Column(db.Float) maximum_fee = db.Column(db.Float) diff --git a/migrations/versions/cb4ccda76a2b_.py b/migrations/versions/cb4ccda76a2b_.py new file mode 100644 index 0000000000..e7b8ba1907 --- /dev/null +++ b/migrations/versions/cb4ccda76a2b_.py @@ -0,0 +1,30 @@ +"""empty message + +Revision ID: cb4ccda76a2b +Revises: 0e80c49a6e28 +Create Date: 2019-06-03 22:57:01.500527 + +""" + +from alembic import op +import sqlalchemy as sa +import sqlalchemy_utils + + +# revision identifiers, used by Alembic. +revision = 'cb4ccda76a2b' +down_revision = '0e80c49a6e28' + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.create_unique_constraint(None, 'ticket_fees', ['country']) + op.create_unique_constraint(None, 'ticket_fees', ['currency']) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_constraint(None, 'ticket_fees', type_='unique') + op.drop_constraint(None, 'ticket_fees', type_='unique') + # ### end Alembic commands ###