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

Alerts for job posts based on filtersets [WIP] #435

Open
wants to merge 23 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
23519c0
subscriptions [wip]
shreyas-satish Apr 5, 2018
620d045
working backend for job alerts
shreyas-satish Apr 9, 2018
3c457f0
simplify and cleanup
shreyas-satish Apr 9, 2018
df699d7
Using StateManager for JobApplication model (#432)
Apr 11, 2018
7e87088
turn off autocomplete for the filters form
shreyas-satish Apr 11, 2018
3681656
changed background job to 10 minutes, custom __init__ for Filterset, …
shreyas-satish Apr 11, 2018
75c8976
added user association. added timing for alerts. cleanup
shreyas-satish Apr 11, 2018
3583eb3
Merge branch 'job-alerts' into jobalerts
shreyas-satish Apr 12, 2018
661d6c7
added time check for alert sending. cleanup. set sitemap
shreyas-satish Apr 12, 2018
9903e76
Merge branch 'master' into jobalerts
shreyas-satish Apr 16, 2018
c085782
removed fail logging. versioned api endpoints. added cascade for jobp…
shreyas-satish Apr 17, 2018
dfe052e
make title and name in filterset nullable
shreyas-satish Apr 17, 2018
ed3a605
changed function name
shreyas-satish Apr 17, 2018
0f9e3ac
Merge branch 'master' into jobalerts
shreyas-satish Apr 17, 2018
9d0432d
user or anon_user
shreyas-satish Apr 17, 2018
20512a2
added frequency to subscription init
shreyas-satish Apr 17, 2018
da775ca
Add form to subscribe to email alerts for job posts.
vidya-ram Apr 19, 2018
30993bd
Sync with latest.
vidya-ram Apr 19, 2018
27c2104
use form for email_frequency
shreyas-satish Apr 21, 2018
0623088
Update UI of job alerts form.
vidya-ram Apr 23, 2018
1219f55
Update text of subscribe form. Remove the border.
vidya-ram Apr 23, 2018
7a818d0
Update alert mailer template.
vidya-ram Apr 26, 2018
f82ee05
Merge branch 'master' into jobalerts
vidya-ram Apr 26, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion hasjob/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@

# Third, after config, import the models and views

from . import models, views # NOQA
from . import models, views, jobs # NOQA
from .models import db # NOQA

# Configure the app
Expand Down
3 changes: 3 additions & 0 deletions hasjob/jobs/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# -*- coding: utf-8 -*-

from . import job_alerts # NOQA
48 changes: 48 additions & 0 deletions hasjob/jobs/job_alerts.py
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
Copy link
Member

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 from views.*. 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 in views/helper.py that need to move as well.



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]
Copy link
Member

Choose a reason for hiding this comment

The 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():
Copy link
Member

Choose a reason for hiding this comment

The 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))
Copy link
Member

Choose a reason for hiding this comment

The 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()
1 change: 1 addition & 0 deletions hasjob/models/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,3 +93,4 @@ class CANDIDATE_FEEDBACK(LabeledEnum):
from .flags import *
from .campaign import *
from .filterset import *
from .jobpost_alert import *
36 changes: 36 additions & 0 deletions hasjob/models/filterset.py
Original file line number Diff line number Diff line change
Expand Up @@ -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']
Expand Down Expand Up @@ -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)
Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Member

Choose a reason for hiding this comment

The 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 url_name_suuid in URLs, since we'll keep tweaking name and title for SEO.


#: Associated job types
types = db.relationship(JobType, secondary=filterset_jobtype_table)
Expand All @@ -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()
Copy link
Member

Choose a reason for hiding this comment

The 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 __title_blank_allowed__ = True on the class instead.


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()
Expand Down
99 changes: 99 additions & 0 deletions hasjob/models/jobpost_alert.py
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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add ondelete='CASCADE'. We keep forgetting to add this to our foreign keys, so we have a lot to fix.

filterset = db.relationship('Filterset', backref=db.backref('subscriptions', lazy='dynamic'))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

backref=db.backref('subscriptions', lazy='dynamic', cascade='all, delete-orphan')

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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be == 1, not <= 1 since having both null is invalid.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used <=1 since anon user tracking isn't enabled right now, so there's a case where both g.user and g.anon_user will be None but we'll have the user's email id.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's fix anon user handling then. This has to be == 1.

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()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this handle preferred day of the week for weekly subscriptions?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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

Copy link
Member

Choose a reason for hiding this comment

The 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
17 changes: 17 additions & 0 deletions hasjob/templates/job_alert_email_confirmation.html.jinja2
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 %}
21 changes: 21 additions & 0 deletions hasjob/templates/job_alert_mailer.html.jinja2
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 %}
2 changes: 1 addition & 1 deletion hasjob/views/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,4 @@ def root_paths():
]

from . import (index, error_handling, helper, listing, location, static, login, board, kiosk, campaign, # NOQA
admindash, domain, api, admin_filterset)
admindash, domain, api, admin_filterset, job_alerts)
2 changes: 1 addition & 1 deletion hasjob/views/admin_filterset.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def new(self):

form = FiltersetForm(parent=g.board)
if form.validate_on_submit():
filterset = Filterset(board=g.board, title=form.title.data)
filterset = Filterset(board=g.board, title=form.title.data, sitemap=True)
form.populate_obj(filterset)
try:
db.session.add(filterset)
Expand Down
2 changes: 1 addition & 1 deletion hasjob/views/board.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How did this blow up? g.board should always be present.



@app.route('/board', methods=['GET', 'POST'])
Expand Down
10 changes: 6 additions & 4 deletions hasjob/views/helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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:
Copy link
Member

Choose a reason for hiding this comment

The 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),
Expand All @@ -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))
Expand Down
Loading