-
Notifications
You must be signed in to change notification settings - Fork 81
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
Alerts for job posts based on filtersets [WIP] #435
base: main
Are you sure you want to change the base?
Changes from 9 commits
23519c0
620d045
3c457f0
df699d7
7e87088
3681656
75c8976
3583eb3
661d6c7
9903e76
c085782
dfe052e
ed3a605
0f9e3ac
9d0432d
20512a2
da775ca
30993bd
27c2104
0623088
1219f55
7a818d0
f82ee05
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
# -*- coding: utf-8 -*- | ||
|
||
from . import job_alerts # NOQA |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
# -*- coding: utf-8 -*- | ||
|
||
from flask_mail import Message | ||
from flask import render_template | ||
from flask_rq import job | ||
from html2text import html2text | ||
from premailer import transform as email_transform | ||
from hasjob import mail | ||
from hasjob.models import db, JobPost, JobPostSubscription, JobPostAlert, jobpost_alert_table | ||
from hasjob.views.index import fetch_jobposts | ||
|
||
|
||
def get_unseen_posts(subscription): | ||
posts = fetch_jobposts(filters=subscription.filterset.to_filters(), posts_only=True) | ||
seen_jobpostids = JobPost.query.join(jobpost_alert_table).join(JobPostAlert).filter( | ||
JobPostAlert.jobpost_subscription == subscription).options(db.load_only('id')).all() | ||
return [post for post in posts if post.id not in seen_jobpostids] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All of this could be a single server-side query. |
||
|
||
|
||
@job('hasjob') | ||
def send_email_alerts(): | ||
for subscription in JobPostSubscription.get_active_subscriptions(): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Active+right time must be a single query. You will be skipping more than sending, so this is going to pull in a lot of irrelevant subscriptions. |
||
if subscription.has_recent_alert(): | ||
# Alert was sent recently, break out of loop | ||
break | ||
|
||
if not subscription.is_right_time_to_send_alert(): | ||
break | ||
|
||
unseen_posts = get_unseen_posts(subscription) | ||
if not unseen_posts: | ||
# Nothing new to see, break out of loop | ||
break | ||
|
||
jobpost_alert = JobPostAlert(jobpost_subscription=subscription) | ||
jobpost_alert.jobposts = unseen_posts | ||
|
||
msg = Message(subject=u"New jobs on Hasjob", recipients=[subscription.email]) | ||
html = email_transform(render_template('job_alert_mailer.html.jinja2', posts=jobpost_alert.jobposts)) | ||
msg.html = html | ||
msg.body = html2text(html) | ||
try: | ||
mail.send(msg) | ||
jobpost_alert.register_delivery() | ||
except Exception as exc: | ||
jobpost_alert.register_failure(unicode(exc)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since exceptions should not happen, don't do a catch-all. Let this trigger our exception reporting. |
||
db.session.add(jobpost_alert) | ||
db.session.commit() |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ | |
from sqlalchemy.ext.associationproxy import association_proxy | ||
from . import db, BaseScopedNameMixin, JobType, JobCategory, Tag, Domain, Board | ||
from ..extapi import location_geodata | ||
from coaster.utils import buid, getbool | ||
|
||
|
||
__all__ = ['Filterset'] | ||
|
@@ -52,6 +53,8 @@ class Filterset(BaseScopedNameMixin, db.Model): | |
|
||
#: Welcome text | ||
description = db.Column(db.UnicodeText, nullable=False, default=u'') | ||
#: Display on sitemap | ||
sitemap = db.Column(db.Boolean, default=False, nullable=True, index=True) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You'll need to allow nullable names and titles as well, and maybe just use that as the sitemap filter. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It just occurred to me that Filterset should use UUID keys, and should use |
||
|
||
#: Associated job types | ||
types = db.relationship(JobType, secondary=filterset_jobtype_table) | ||
|
@@ -71,6 +74,39 @@ class Filterset(BaseScopedNameMixin, db.Model): | |
def __repr__(self): | ||
return '<Filterset %s "%s">' % (self.board.title, self.title) | ||
|
||
def __init__(self, **kwargs): | ||
filters = kwargs.pop('filters') if kwargs.get('filters') else {} | ||
super(Filterset, self).__init__(**kwargs) | ||
|
||
if not self.title: | ||
self.title = buid() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't do this. We don't want nonsense titles that will be hard to fix later. Set |
||
|
||
if filters: | ||
if filters.get('t'): | ||
self.types = JobType.query.filter(JobType.name.in_(filters['t'])).all() | ||
|
||
if filters.get('c'): | ||
self.categories = JobCategory.query.filter(JobCategory.name.in_(filters['c'])).all() | ||
|
||
if filters.get('l'): | ||
geonameids = [] | ||
for loc in filters.get('l'): | ||
geonameids.append(location_geodata(loc)['geonameid']) | ||
self.geonameids = geonameids | ||
|
||
if getbool(filters.get('anywhere')): | ||
self.remote_location = True | ||
|
||
if getbool(filters.get('equity')): | ||
self.equity = True | ||
|
||
if filters.get('currency') and filters.get('pay'): | ||
self.pay_currency = filters.get('currency') | ||
self.pay_cash = filters.get('pay') | ||
|
||
if filters.get('q'): | ||
self.keywords = filters.get('q') | ||
|
||
@classmethod | ||
def get(cls, board, name): | ||
return cls.query.filter(cls.board == board, cls.name == name).one_or_none() | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,99 @@ | ||
# -*- coding: utf-8 -*- | ||
|
||
from datetime import datetime, timedelta | ||
from coaster.sqlalchemy import StateManager | ||
from ..utils import random_long_key | ||
from . import db, BaseMixin, LabeledEnum, User, AnonUser | ||
|
||
__all__ = ['JobPostSubscription', 'JobPostAlert', 'jobpost_alert_table'] | ||
|
||
|
||
class EMAIL_FREQUENCY(LabeledEnum): | ||
DAILY = (1, 'Daily') | ||
WEEKLY = (7, 'Weekly') | ||
|
||
|
||
class JobPostSubscription(BaseMixin, db.Model): | ||
__tablename__ = 'jobpost_subscription' | ||
|
||
filterset_id = db.Column(None, db.ForeignKey('filterset.id'), nullable=False) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add |
||
filterset = db.relationship('Filterset', backref=db.backref('subscriptions', lazy='dynamic')) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
email = db.Column(db.Unicode(254), nullable=True) | ||
user_id = db.Column(None, db.ForeignKey('user.id'), nullable=True, index=True) | ||
user = db.relationship(User) | ||
anon_user_id = db.Column(None, db.ForeignKey('anon_user.id'), nullable=True, index=True) | ||
anon_user = db.relationship(AnonUser) | ||
|
||
active = db.Column(db.Boolean, nullable=False, default=False, index=True) | ||
email_verify_key = db.Column(db.String(40), nullable=True, default=random_long_key, unique=True) | ||
unsubscribe_key = db.Column(db.String(40), nullable=True, default=random_long_key, unique=True) | ||
subscribed_at = db.Column(db.DateTime, nullable=False, default=db.func.utcnow()) | ||
email_verified_at = db.Column(db.DateTime, nullable=True, index=True) | ||
unsubscribed_at = db.Column(db.DateTime, nullable=True) | ||
|
||
_email_frequency = db.Column('email_frequency', | ||
db.Integer, StateManager.check_constraint('email_frequency', EMAIL_FREQUENCY), | ||
default=EMAIL_FREQUENCY.DAILY, nullable=True) | ||
email_frequency = StateManager('_email_frequency', EMAIL_FREQUENCY, doc="Email frequency") | ||
|
||
__table_args__ = (db.UniqueConstraint('filterset_id', 'email'), | ||
db.CheckConstraint( | ||
db.case([(user_id != None, 1)], else_=0) + db.case([(anon_user_id != None, 1)], else_=0) <= 1, # NOQA | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I used There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's fix anon user handling then. This has to be |
||
name='jobpost_subscription_user_id_or_anon_user_id')) | ||
|
||
@classmethod | ||
def get(cls, filterset, email): | ||
return cls.query.filter(cls.filterset == filterset, cls.email == email).one_or_none() | ||
|
||
def verify_email(self): | ||
self.active = True | ||
self.email_verified_at = db.func.utcnow() | ||
|
||
def unsubscribe(self): | ||
self.active = False | ||
self.unsubscribed_at = db.func.utcnow() | ||
|
||
@classmethod | ||
def get_active_subscriptions(cls): | ||
return cls.query.filter(cls.active == True, cls.email_verified_at != None) | ||
|
||
def has_recent_alert(self): | ||
return JobPostAlert.query.filter( | ||
JobPostAlert.jobpost_subscription == self, | ||
JobPostAlert.sent_at >= datetime.utcnow() - timedelta(days=self.email_frequency.value) | ||
).order_by('created_at desc').notempty() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How does this handle preferred day of the week for weekly subscriptions? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It doesn't really, but it uses the same hack as deciding the time to send an alert. So, a weekly subscriber will essentially receive the alert on the day they subscribed. Should we ask the user for their preferred day? |
||
|
||
def is_right_time_to_send_alert(self): | ||
""" | ||
Checks if it's the right time to send this subscriber an alert. | ||
For now, the time at which the subscription was initiated is taken as the "preferred time" and | ||
uses a tolerance of 30 minutes | ||
""" | ||
return ((datetime.utcnow() - self.subscribed_at.time()).total_seconds()/60) <= 30 | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Extra newline required |
||
jobpost_alert_table = db.Table('jobpost_jobpost_alert', db.Model.metadata, | ||
db.Column('jobpost_id', None, db.ForeignKey('jobpost.id'), primary_key=True), | ||
db.Column('jobpost_alert_id', None, db.ForeignKey('jobpost_alert.id'), primary_key=True), | ||
db.Column('created_at', db.DateTime, nullable=False, default=db.func.utcnow()) | ||
) | ||
|
||
|
||
class JobPostAlert(BaseMixin, db.Model): | ||
__tablename__ = 'jobpost_alert' | ||
|
||
jobpost_subscription_id = db.Column(None, db.ForeignKey('jobpost_subscription.id'), | ||
index=True) | ||
jobpost_subscription = db.relationship(JobPostSubscription, backref=db.backref('alerts', | ||
lazy='dynamic', cascade='all, delete-orphan')) | ||
jobposts = db.relationship('JobPost', lazy='dynamic', secondary=jobpost_alert_table, | ||
backref=db.backref('alerts', lazy='dynamic')) | ||
sent_at = db.Column(db.DateTime, nullable=True) | ||
failed_at = db.Column(db.DateTime, nullable=True) | ||
fail_reason = db.Column(db.Unicode(255), nullable=True) | ||
|
||
def register_delivery(self): | ||
self.sent_at = db.func.utcnow() | ||
|
||
def register_failure(self, fail_reason): | ||
self.failed_at = db.func.utcnow() | ||
self.fail_reason = fail_reason |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
{% extends "inc/email_layout_lite.html.jinja2" %} | ||
|
||
{% block content %} | ||
<div itemscope itemtype="http://schema.org/EmailMessage"> | ||
<div itemprop="action" itemscope itemtype="http://schema.org/ViewAction"> | ||
<meta itemprop="name" content="Confirm your email for job alerts"/> | ||
</div> | ||
<meta itemprop="description" content="Job alerts"/> | ||
<div itemprop="publisher" itemscope itemtype="http://schema.org/Organization"> | ||
<meta itemprop="name" content="Hasjob"/> | ||
<link itemprop="url" href="{{ url_for('index', subdomain=none, _external=true) }}"/> | ||
</div> | ||
</div> | ||
Please click <a href="{{url_for('confirm_subscription_to_job_alerts', token=token, _external=True)}}">here</a> to confirm your subscription. | ||
<hr> | ||
<p><a href="{{ url_for('index', subdomain=none, _external=true) }}">Hasjob</a> is a service of <a href="https://hasgeek.com/">HasGeek</a>. Write to us at <a href="mailto:{{ config['SUPPORT_EMAIL'] }}">{{ config['SUPPORT_EMAIL'] }}</a> if you have suggestions or questions on this service.</p> | ||
{% endblock %} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
{% extends "inc/email_layout_lite.html.jinja2" %} | ||
|
||
{% block content %} | ||
<div itemscope itemtype="http://schema.org/EmailMessage"> | ||
<div itemprop="action" itemscope itemtype="http://schema.org/ViewAction"> | ||
<meta itemprop="name" content="Job Posts"/> | ||
</div> | ||
<meta itemprop="description" content="Job alerts"/> | ||
<div itemprop="publisher" itemscope itemtype="http://schema.org/Organization"> | ||
<meta itemprop="name" content="Hasjob"/> | ||
<link itemprop="url" href="{{ url_for('index', subdomain=none, _external=true) }}"/> | ||
</div> | ||
</div> | ||
<ul> | ||
{%- for post in posts %} | ||
<li><a href={{post.url_for(_external=True)}}>{{post.headline}}</a></li> | ||
{%- endfor %} | ||
</ul> | ||
<hr> | ||
<p><a href="{{ url_for('index', subdomain=none, _external=true) }}">Hasjob</a> is a service of <a href="https://hasgeek.com/">HasGeek</a>. Write to us at <a href="mailto:{{ config['SUPPORT_EMAIL'] }}">{{ config['SUPPORT_EMAIL'] }}</a> if you have suggestions or questions on this service.</p> | ||
{% endblock %} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,7 +33,7 @@ def remove_subdomain_parameter(endpoint, values): | |
def add_subdomain_parameter(endpoint, values): | ||
if app.url_map.is_endpoint_expecting(endpoint, 'subdomain'): | ||
if 'subdomain' not in values: | ||
values['subdomain'] = g.board.name if g.board and g.board.not_root else None | ||
values['subdomain'] = g.board.name if 'board' in g and g.board.not_root else None | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How did this blow up? |
||
|
||
|
||
@app.route('/board', methods=['GET', 'POST']) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -398,7 +398,7 @@ def load_viewcounts(posts): | |
g.viewcounts = dict(zip(viewcounts_keys, viewcounts_values)) | ||
|
||
|
||
def getposts(basequery=None, pinned=False, showall=False, statusfilter=None, ageless=False, limit=2000, order=True): | ||
def getposts(basequery=None, pinned=False, showall=False, statusfilter=None, ageless=False, limit=2000, board=None, order=True): | ||
if ageless: | ||
pinned = False # No pinning when browsing archives | ||
|
||
|
@@ -410,15 +410,17 @@ def getposts(basequery=None, pinned=False, showall=False, statusfilter=None, age | |
|
||
query = basequery.filter(statusfilter).options(*JobPost._defercols).options(db.joinedload('domain')) | ||
|
||
if g.board: | ||
if 'board' in g: | ||
board = g.board | ||
if board: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should not be required either. |
||
query = query.join(JobPost.postboards).filter(BoardJobPost.board == g.board) | ||
|
||
if not ageless: | ||
if showall: | ||
query = query.filter(JobPost.state.LISTED) | ||
else: | ||
if pinned: | ||
if g.board: | ||
if board: | ||
query = query.filter( | ||
db.or_( | ||
db.and_(BoardJobPost.pinned == True, JobPost.state.LISTED), | ||
|
@@ -432,7 +434,7 @@ def getposts(basequery=None, pinned=False, showall=False, statusfilter=None, age | |
query = query.filter(JobPost.state.NEW) | ||
|
||
if pinned: | ||
if g.board: | ||
if board: | ||
query = query.order_by(db.desc(BoardJobPost.pinned)) | ||
else: | ||
query = query.order_by(db.desc(JobPost.pinned)) | ||
|
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.
I'm uncomfortable with
jobs.*
importing fromviews.*
. Jobs are supposed to be independent of views. Can we isolate this (and other associated) functions into a separate helper module? We have many RQ jobs inviews/helper.py
that need to move as well.