Skip to content
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

Cron job for marking event invoices due is wrong #6494

Closed
iamareebjamal opened this issue Oct 3, 2019 · 6 comments · Fixed by #6510
Closed

Cron job for marking event invoices due is wrong #6494

iamareebjamal opened this issue Oct 3, 2019 · 6 comments · Fixed by #6510

Comments

@iamareebjamal
Copy link
Member

iamareebjamal commented Oct 3, 2019

PR #6166 is wrong. The query fails in every case

def event_invoices_mark_due():
from app import current_app as app
with app.app_context():
db.session.query(EventInvoice).\
filter(EventInvoice.status == 'upcoming',
EventInvoice.event.ends_at >= datetime.datetime.now(),
(EventInvoice.created_at + datetime.timedelta(days=30) <=
datetime.datetime.now())).\
update({'status': 'due'})
db.session.commit()

This line is not valid SQLAlchemy syntax:

EventInvoice.event.ends_at >= datetime.datetime.now(),

Neither 'InstrumentedAttribute' object nor 'Comparator' object associated with EventInvoice.event has an attribute 'ends_at'

It should be db.session.query(EventInvoice).join(Event).filter(EventInvoice.status == 'upcoming', Event.ends_at >= datetime.datetime.now()).all()

Each run of this task is failing

Fix it and write tests to ensure it is working

@arpit551
Copy link
Member

arpit551 commented Oct 3, 2019

@iamareebjamal I would like to take this one if no one is working on it.

@iamareebjamal
Copy link
Member Author

Go ahead. This is high priority issue so please try to submit a PR within few days

@arpit551
Copy link
Member

arpit551 commented Oct 5, 2019

@iamareebjamal are there any tests written for scheduled jobs?

@iamareebjamal
Copy link
Member Author

For some of them, it might be written, I'm not sure

@mrsaicharan1
Copy link
Member

@arpit551 If you're pre-occupied elsewhere, I'll go ahead & fix this.

@iamareebjamal
Copy link
Member Author

@mrsaicharan1 Please go ahead, this is high priority

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants