Subscribed email searches #170
Conversation
… the database. Migration file and changes to elasticSearch.
…iptions in the DB.
zeeguu/api/endpoints/search.py
Outdated
| user = User.find_by_id(flask.g.user_id) | ||
| receive_email = True | ||
| SearchSubscription.update_receive_email(db_session, user, search, receive_email) | ||
| Search.update_receive_email(db_session, search_terms, receive_email) |
There was a problem hiding this comment.
I do not understand this. Why does Search have a receive_email attribute? It should only be the Subscription that knows about this.
There was a problem hiding this comment.
I have added it to Search because you use search to show data in the UI. So if it is not added, I will not be able to see the changes made to the receive_email. Most endpoints use this, return json_result(search.as_dictionary()), they do not use search_subscription.
There was a problem hiding this comment.
Ah! In that case we should really refactor the UI to use Subscription instead of Search. I already observed that we should do that in the PR for the Web, but I thought we can delay it for after Wednesday. But in this case we should prioritize this straight away.
There was a problem hiding this comment.
Do you think you can do it, or should we try to do a brief pair-programming session this afternoon to fix this?
There was a problem hiding this comment.
Actually, we already have a meeting tomorrow @ 10am. We could use that too. But it might be a bit more than one hour.
There was a problem hiding this comment.
I would appreciate some help to fix this. Tomorrows meeting works fine for me, and I can do more than one hour.
zeeguu/api/endpoints/search.py
Outdated
| receive_email = False | ||
| SearchSubscription.update_receive_email(db_session, user, search, receive_email) | ||
|
|
||
| Search.update_receive_email(db_session, search_terms, receive_email) |
There was a problem hiding this comment.
this should also not be needed!
| previous_day_datetime = current_datetime - datetime.timedelta(days=1) | ||
|
|
||
| for subscription in all_subscriptions: | ||
| for search in all_searches: |
There was a problem hiding this comment.
you should be able to simply say subscription.search because the search is defined inside of the SearchSubscription class as a relationship! Thus, you don't have to iterate over all the searches to find the matching search. Just say subscriptio.search.
There was a problem hiding this comment.
So like this instead?
for subscription in all_subscriptions:
articles = article_search_for_user(subscription.user_id, 2, subscription.search, page = 0)
new_articles_found = [article for article in articles if article.published > previous_day_datetime]
if new_articles_found:
user = User.find_by_id(subscription.user_id)
send_mail_new_articles_search(user.email, search)
There was a problem hiding this comment.
Yes, use subscription.search. And also in your last line send_mail_new_articles_search(user.email, search) you'll probably have to use the same.
Added the endpoints for subscribing to email notifications for subscribed searches.
Added two migration files, one for adding columns for the receive_email in search_subscription and Search. And one for deleting all entries in the tables search and search_subscription.