-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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.
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
feat: Expire initializing attendees automatically #7050
feat: Expire initializing attendees automatically #7050
Conversation
status='pending', | ||
created_at=datetime.datetime.now(timezone.utc) - timedelta(minutes=15), | ||
) | ||
attendees_new = AttendeeSubFactory.create_batch(2, order=order_new) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
local variable 'attendees_new' is assigned to but never used
status='pending', | ||
created_at=datetime.datetime.now(timezone.utc) - timedelta(minutes=45), | ||
) | ||
attendees_old = AttendeeSubFactory.create_batch(3, order=order_old) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
local variable 'attendees_old' is assigned to but never used
assert len(order_new.ticket_holders) == 2 | ||
|
||
|
||
def test_expire_pending_tickets(db): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
redefinition of unused 'db' from line 12
) | ||
attendees_old = AttendeeSubFactory.create_batch(3, order=order_old) | ||
order_new = OrderSubFactory() | ||
attendees_new = AttendeeSubFactory.create_batch(2, order=order_new) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
local variable 'attendees_new' is assigned to but never used
created_at=datetime.datetime.now(timezone.utc) | ||
- timedelta(minutes=order_expiry_time) | ||
) | ||
attendees_old = AttendeeSubFactory.create_batch(3, order=order_old) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
local variable 'attendees_old' is assigned to but never used
assert ticket_holder != None | ||
|
||
|
||
def test_delete_ticket_holders_with_no_order_id(db): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
redefinition of unused 'db' from line 12
attendee_id = attendee.id | ||
delete_ticket_holders_no_order_id() | ||
ticket_holder = TicketHolder.query.get(attendee_id) | ||
assert ticket_holder != None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
comparison to None should be 'if cond is not None:'
test_ticket_holder = AttendeeFactory() | ||
test_ticket_holder.event = test_event | ||
test_ticket_holder.order = test_order | ||
def test_delete_ticket_holder_with_valid_order_id(db): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
redefinition of unused 'db' from line 12
attendee_id = attendee.id | ||
delete_ticket_holders_no_order_id() | ||
ticket_holder = TicketHolder.query.get(attendee_id) | ||
assert ticket_holder != None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
comparison to None should be 'if cond is not None:'
|
||
test_event = EventFactoryBasic(state='published') | ||
def test_delete_ticket_holder_created_currently(db): | ||
"""Method to test not deleting ticket holders with no order id but created within expiry time""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
line too long (100 > 90 characters)
TicketFeesFactory(service_fee=10.23, maximum_fee=11) | ||
|
||
test_event = EventFactoryBasic(state='published') | ||
def test_delete_ticket_holder_created_currently(db): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
redefinition of unused 'db' from line 12
from tests.factories.event_invoice import EventInvoiceFactory | ||
from tests.factories.order import OrderFactory | ||
from tests.factories.event_invoice import EventInvoiceFactory, EventInvoiceSubFactory | ||
from tests.factories.order import OrderFactory, OrderSubFactory |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'tests.factories.order.OrderFactory' imported but unused
from tests.factories.event import EventFactoryBasic | ||
from tests.factories.event_invoice import EventInvoiceFactory | ||
from tests.factories.order import OrderFactory | ||
from tests.factories.event_invoice import EventInvoiceFactory, EventInvoiceSubFactory |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'tests.factories.event_invoice.EventInvoiceFactory' imported but unused
from tests.all.integration.utils import OpenEventLegacyTestCase | ||
from tests.factories.attendee import AttendeeFactory | ||
from tests.factories.attendee import ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'tests.factories.attendee.AttendeeFactory' imported but unused
db.session.commit() | ||
order_expiry_time = get_settings()['order_expiry_time'] | ||
TicketHolder.query.filter( | ||
TicketHolder.order_id == None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
comparison to None should be 'if cond is None:'
app/api/helpers/scheduled_jobs.py
Outdated
db.session.commit() | ||
except: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not use bare except:
, it also catches unexpected events like memory errors, interrupts, system exit, and so on. Prefer except Exception:
. If you're sure what you're doing, be explicit and write except BaseException:
.
do not use bare 'except'
order_expiry_time = get_settings()['order_expiry_time'] | ||
query = db.session.query(Order.id).filter( | ||
Order.status == 'initializing', | ||
Order.paid_via == None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
comparison to None should be 'if cond is None:'
Codecov Report
@@ Coverage Diff @@
## development #7050 +/- ##
===============================================
+ Coverage 61.15% 61.17% +0.01%
===============================================
Files 260 260
Lines 12881 12884 +3
===============================================
+ Hits 7878 7882 +4
+ Misses 5003 5002 -1
Continue to review full report at Codecov.
|
Here is an overview of what got changed by this pull request: Complexity increasing per file
==============================
- tests/all/integration/api/helpers/test_scheduled_jobs.py 3
See the complete overview on Codacy |
This pull request introduces 1 alert when merging 77d97c9 into 98aca7d - view on LGTM.com new alerts:
|
Currently, this is handled through Order List and Order GET API which is really bad. It should instead be handled through scheduled job like pending tickets are handled