promote: cleanup.

Consolidate duplicated logic and make methods more direct. There were
many monolithic methods that did many things and were called all over
the place when the desired action was just one piece of the method.

r2/r2/controllers/listingcontroller.py

@@ -248,7 +248,7 @@ class HotController(FixListing, ListingController):
                  c.user_is_loggedin and link.author_id == c.user._id)):
             self.abort403()
 
-        if not promote.is_live_on_sr(link, c.site.name):
+        if not promote.is_live_on_sr(link, c.site):
             self.abort403()
 
         res = wrap_links([link._fullname], wrapper=self.builder_wrapper,

r2/r2/lib/promote.py

@@ -64,6 +64,7 @@ from r2.models import (
     IDBuilder,
     Link,
     MultiReddit,
+    NO_TRANSACTION,
     NotFound,
     PromoCampaign,
     PROMOTE_STATUS,
@@ -158,26 +159,8 @@ def is_rejected(link):
 def is_promoted(link):
     return is_promo(link) and link.promote_status == PROMOTE_STATUS.promoted
 
-def is_live_on_sr(link, srname):
-    if not is_promoted(link):
-        return False
-    live = scheduled_campaigns_by_link(link)
-    srname = srname.lower()
-    srname = srname if srname != DefaultSR.name.lower() else ''
-
-    campaigns = PromoCampaign._byID(live, return_dict=True)
-    for campaign_id in live:
-        campaign = campaigns.get(campaign_id)
-        if campaign and campaign.sr_name.lower() == srname:
-            return True
-    return False
-
-
-def campaign_is_live(link, campaign_index):
-    if not is_promoted(link):
-        return False
-    live = scheduled_campaigns_by_link(link)
-    return campaign_index in live
+def is_live_on_sr(link, sr):
+    return bool(live_campaigns_by_link(link, sr=sr))
 
 
 # control functions
@@ -207,7 +190,7 @@ class RenderableCampaign():
     @classmethod
     def create(cls, link, campaigns):
         transactions = get_transactions(link, campaigns)
-        live_campaigns = scheduled_campaigns_by_link(link)
+        live_campaigns = live_campaigns_by_link(link)
         user_is_sponsor = c.user_is_sponsor
         today = promo_datetime_now().date()
         r = []
@@ -223,7 +206,7 @@ class RenderableCampaign():
             spent = get_spent_amount(camp)
             cpm = getattr(camp, 'cpm', g.cpm_selfserve.pennies)
             sr = camp.sr_name
-            live = camp._id in live_campaigns
+            live = camp in live_campaigns
             pending = today < to_date(camp.start_date)
             complete = (transaction and (transaction.is_charged() or
                                          transaction.is_refund()) and
@@ -293,6 +276,12 @@ def traffic_viewers(thing):
     return sorted(getattr(thing, "promo_traffic_viewers", set()))
 
 
+
+def update_promote_status(link, status):
+    set_promote_status(link, status)
+    hooks.get_hook('promote.edit_promotion').call(link=link)
+
+
 def new_promotion(title, url, selftext, user, ip):
     """
     Creates a new promotion with the provided title, etc, and sets it
@@ -313,9 +302,9 @@ def new_promotion(title, url, selftext, user, ip):
 
     # set the status of the link, populating the query queue
     if c.user_is_sponsor or user.trusted_sponsor:
-        set_promote_status(l, PROMOTE_STATUS.accepted)
+        update_promote_status(l, PROMOTE_STATUS.accepted)
     else:
-        set_promote_status(l, PROMOTE_STATUS.unpaid)
+        update_promote_status(l, PROMOTE_STATUS.unpaid)
 
     # the user has posted a promotion, so enable the promote menu unless
     # they have already opted out
@@ -365,9 +354,8 @@ def new_campaign(link, dates, bid, cpm, sr, priority):
         author = Account._byID(link.author_id, data=True)
         if getattr(author, "complimentary_promos", False):
             free_campaign(link, campaign, c.user)
-    else:
-        # non-cpm campaigns are never charged, so we need to fire the hook now
-        hooks.get_hook('promote.new_charge').call(link=link, campaign=campaign)
+
+    hooks.get_hook('promote.new_campaign').call(link=link, campaign=campaign)
     return campaign
 
 
@@ -399,7 +387,7 @@ def edit_campaign(link, campaign, dates, bid, cpm, sr, priority):
         if getattr(author, "complimentary_promos", False):
             free_campaign(link, campaign, c.user)
 
-    hooks.get_hook('campaign.edit').call(link=link, campaign=campaign)
+    hooks.get_hook('promote.edit_campaign').call(link=link, campaign=campaign)
 
 
 def delete_campaign(link, campaign):
@@ -407,7 +395,7 @@ def delete_campaign(link, campaign):
     void_campaign(link, campaign)
     campaign.delete()
     PromotionLog.add(link, 'deleted campaign %s' % campaign._id)
-    hooks.get_hook('campaign.void').call(link=link, campaign=campaign)
+    hooks.get_hook('promote.delete_campaign').call(link=link, campaign=campaign)
 
 def void_campaign(link, campaign):
     transactions = get_transactions(link, [campaign])
@@ -447,9 +435,8 @@ def auth_campaign(link, campaign, user, pay_id):
             new_status = max(PROMOTE_STATUS.unseen, link.promote_status)
         else:
             new_status = max(PROMOTE_STATUS.unpaid, link.promote_status)
-        set_promote_status(link, new_status)
-        # notify of campaign creation
-        # update the query queue
+        update_promote_status(link, new_status)
+
         if user and (user._id == link.author_id) and trans_id > 0:
             emailer.promo_bid(link, campaign.bid, campaign.start_date)
 
@@ -478,92 +465,42 @@ def promo_datetime_now(offset=None):
         now += timedelta(offset)
     return now
 
+
 def accept_promotion(link):
-    """
-    Accepting is campaign agnostic.  Accepting the ad just means that
-    it is allowed to run if payment has been processed.
-
-    If a campagn is able to run, this also requeues it.
-    """
-    # update the query queue
-    set_promote_status(link, PROMOTE_STATUS.accepted)
-
-    # campaigns that should be live now must be updated
-    now = promo_datetime_now(0)
-    promotion_weights = PromotionWeights.get_campaigns(now)
-    live_campaigns = {pw.promo_idx for pw in promotion_weights
-                                   if pw.thing_name == link._fullname}
-    if live_campaigns:
-        campaigns = PromoCampaign._byID(live_campaigns, data=True,
-                                        return_dict=False)
-        PromotionLog.add(link, 'has live campaigns, forcing live')
-        charge_pending(0) # campaign must be charged before it will go live
-        for campaign in campaigns:
-            hooks.get_hook('campaign.edit').call(link=link, campaign=campaign)
-        queue_changed_promo(link, "accepted")
-
-    # campaigns that were charged and will go live in the future must be updated
-    future_campaigns = [camp for camp in PromoCampaign._by_link(link._id)
-                        if camp.start_date > now]
-    transactions = get_transactions(link, future_campaigns)
-    charged_campaigns = [camp for camp in future_campaigns
-                         if (transactions.get(camp._id) and
-                             transactions.get(camp._id).is_charged())]
-    for campaign in charged_campaigns:
-        hooks.get_hook('campaign.edit').call(link=link, campaign=campaign)
+    update_promote_status(link, PROMOTE_STATUS.accepted)
 
     if link._spam:
         link._spam = False
         link._commit()
+
     emailer.accept_promo(link)
 
-def reject_promotion(link, reason=None):
-    # update the query queue
-    # Since status is updated first,
-    # if make_daily_promotions happens to run
-    # while we're doing work here, it will correctly exclude it
-    set_promote_status(link, PROMOTE_STATUS.rejected)
+    # if the link has campaigns running now charge them and promote the link
+    now = promo_datetime_now()
+    campaigns = list(PromoCampaign._by_link(link._id))
+    for camp in campaigns:
+        if is_accepted_promo(now, link, camp):
+            charge_campaign(link, camp)
+            if charged_or_not_needed(camp):
+                promote_link(link, camp)
 
-    if is_promoted(link):
-        PromotionLog.add(link, 'has live campaigns, terminating')
-        queue_changed_promo(link, "rejected")
+
+def reject_promotion(link, reason=None):
+    update_promote_status(link, PROMOTE_STATUS.rejected)
 
     # Send a rejection email (unless the advertiser requested the reject)
     if not c.user or c.user._id != link.author_id:
         emailer.reject_promo(link, reason=reason)
 
-    hooks.get_hook('promotion.void').call(link=link)
-
 
 def unapprove_promotion(link):
-    # update the query queue
-    set_promote_status(link, PROMOTE_STATUS.unseen)
-    hooks.get_hook('promotion.void').call(link=link)
+    update_promote_status(link, PROMOTE_STATUS.unseen)
 
 
-def accepted_campaigns(offset=0):
-    now = promo_datetime_now(offset=offset)
-    promo_weights = PromotionWeights.get_campaigns(now)
-    all_links = Link._by_fullname(set(x.thing_name for x in promo_weights),
-                                  data=True, return_dict=True)
-    accepted_links = {}
-    for link_fullname, link in all_links.iteritems():
-        if is_accepted(link):
-            accepted_links[link._id] = link
-
-    accepted_link_ids = accepted_links.keys()
-    campaign_query = PromoCampaign._query(PromoCampaign.c.link_id == accepted_link_ids,
-                                          data=True)
-    campaigns = dict((camp._id, camp) for camp in campaign_query)
-    for pw in promo_weights:
-        campaign = campaigns.get(pw.promo_idx)
-        if not campaign or (not campaign.trans_id and campaign.priority.cpm):
-            continue
-        link = accepted_links.get(campaign.link_id)
-        if not link:
-            continue
-
-        yield (link, campaign, pw.weight)
+def authed_or_not_needed(campaign):
+    authed = campaign.trans_id != NO_TRANSACTION
+    needs_auth = campaign.priority.cpm
+    return authed or not needs_auth
 
 
 def charged_or_not_needed(campaign):
@@ -573,92 +510,117 @@ def charged_or_not_needed(campaign):
     return charged or not needs_charge
 
 
-def get_scheduled(offset=0):
-    campaigns = []
-    for l, campaign, weight in accepted_campaigns(offset=offset):
-        if charged_or_not_needed(campaign):
-            campaigns.append(campaign)
-    return campaigns
+def is_accepted_promo(date, link, campaign):
+    return (campaign.start_date <= date < campaign.end_date and
+            is_accepted(link) and
+            authed_or_not_needed(campaign))
+
+
+def is_scheduled_promo(date, link, campaign):
+    return (is_accepted_promo(date, link, campaign) and 
+            charged_or_not_needed(campaign))
+
+
+def is_live_promo(link, campaign):
+    now = promo_datetime_now()
+    return is_promoted(link) and is_scheduled_promo(now, link, campaign)
+
+
+def get_promos(date, sr_names=None, link=None):
+    pws = PromotionWeights.get_campaigns(date, sr_names=sr_names, link=link)
+    campaign_ids = {pw.promo_idx for pw in pws}
+    campaigns = PromoCampaign._byID(campaign_ids, data=True, return_dict=False)
+    link_ids = {camp.link_id for camp in campaigns}
+    links = Link._byID(link_ids, data=True)
+    for camp in campaigns:
+        yield camp, links[camp.link_id]
+
+
+def get_accepted_promos(offset=0):
+    date = promo_datetime_now(offset=offset)
+    for camp, link in get_promos(date):
+        if is_accepted_promo(date, link, camp):
+            yield camp, link
+
+
+def get_scheduled_promos(offset=0):
+    date = promo_datetime_now(offset=offset)
+    for camp, link in get_promos(date):
+        if is_scheduled_promo(date, link, camp):
+            yield camp, link
+
+
+def charge_campaign(link, campaign):
+    if charged_or_not_needed(campaign):
+        return
+
+    user = Account._byID(link.author_id)
+    charge_succeeded = authorize.charge_transaction(user, campaign.trans_id,
+                                                    campaign._id)
+
+    if not charge_succeeded:
+        return
+
+    hooks.get_hook('promote.edit_campaign').call(link=link, campaign=campaign)
+
+    if not is_promoted(link):
+        update_promote_status(link, PROMOTE_STATUS.pending)
+
+    emailer.queue_promo(link, campaign.bid, campaign.trans_id)
+    text = ('auth charge for campaign %s, trans_id: %d' %
+            (campaign._id, campaign.trans_id))
+    PromotionLog.add(link, text)
 
 
 def charge_pending(offset=1):
-    for l, camp, weight in accepted_campaigns(offset=offset):
-        user = Account._byID(l.author_id)
-
-        if charged_or_not_needed(camp):
-            continue
-
-        charge_succeeded = authorize.charge_transaction(user, camp.trans_id,
-                                                        camp._id)
-
-        if not charge_succeeded:
-            continue
-
-        hooks.get_hook('promote.new_charge').call(link=l, campaign=camp)
-
-        if is_promoted(l):
-            emailer.queue_promo(l, camp.bid, camp.trans_id)
-        else:
-            set_promote_status(l, PROMOTE_STATUS.pending)
-            emailer.queue_promo(l, camp.bid, camp.trans_id)
-        text = ('auth charge for campaign %s, trans_id: %d' %
-                (camp._id, camp.trans_id))
-        PromotionLog.add(l, text)
+    for camp, link in get_accepted_promos(offset=offset):
+        charge_campaign(link, camp)
 
 
-def scheduled_campaigns_by_link(l, date=None):
-    # A promotion/campaign is scheduled/live if it's in
-    # PromotionWeights.get_campaigns(now) and
-    # charged_or_not_needed
-
-    date = date or promo_datetime_now()
-
-    if not is_accepted(l):
+def live_campaigns_by_link(link, sr=None):
+    if not is_promoted(link):
         return []
 
-    scheduled = PromotionWeights.get_campaigns(date)
-    campaigns = [c.promo_idx for c in scheduled if c.thing_name == l._fullname]
+    if sr:
+        sr_names = [''] if isinstance(sr, DefaultSR) else [sr.name]
+    else:
+        sr_names = None
 
-    # Check authorize
-    accepted = []
-    for campaign_id in campaigns:
-        campaign = PromoCampaign._byID(campaign_id, data=True)
-        if charged_or_not_needed(campaign):
-            accepted.append(campaign_id)
-
-    return accepted
+    now = promo_datetime_now()
+    return [camp for camp, link in get_promos(now, sr_names=sr_names,
+                                              link=link)
+            if is_live_promo(link, camp)]
+
+
+def promote_link(link, campaign):
+    if (not link.over_18 and
+        campaign.sr_name and Subreddit._by_name(campaign.sr_name).over_18):
+        link.over_18 = True
+        link._commit()
+
+    if not is_promoted(link):
+        update_promote_status(link, PROMOTE_STATUS.promoted)
+        emailer.live_promo(link)
 
 
-# Gotcha: even if links are scheduled and authorized, they won't be added to 
-# current promotions until they're actually charged, so make sure to call
-# charge_pending() before make_daily_promotions()
 def make_daily_promotions(offset=0):
-    campaigns = get_scheduled(offset)
-    link_ids = {camp.link_id for camp in campaigns}
-    links = Link._byID(link_ids, data=True)
-    srs = Subreddit._by_name([camp.sr_name for camp in campaigns.itervalues()
-                              if camp.sr_name])
+    # charge campaigns so they can go live
+    charge_pending(offset=0)
+    charge_pending(offset=1)
+
+    # promote links and record ids of promoted links
+    link_ids = set()
+    for campaign, link in get_scheduled_promos(offset):
+        link_ids.add(link._id)
+        promote_link(link, campaign)
 
     # expire finished links
     q = Link._query(Link.c.promote_status == PROMOTE_STATUS.promoted, data=True)
     q = q._filter(not_(Link.c._id.in_(link_ids)))
     for link in q:
-        set_promote_status(link, PROMOTE_STATUS.finished)
+        update_promote_status(link, PROMOTE_STATUS.finished)
         emailer.finished_promo(link)
 
-    for camp in campaigns:
-        link = links[camp.link_id]
-
-        # check for over_18 targets
-        if camp.sr_name and srs[camp.sr_name].over_18:
-            link.over_18 = True
-            link._commit()
-
-        # promote new links
-        if is_accepted(link) and not is_promoted(link):
-            set_promote_status(link, PROMOTE_STATUS.promoted)
-            emailer.live_promo(link)
-
     _mark_promos_updated()
     finalize_completed_campaigns(daysago=offset+1)
     hooks.get_hook('promote.make_daily_promotions').call(offset=offset)
@@ -757,17 +719,8 @@ PromoTuple = namedtuple('PromoTuple', ['link', 'weight', 'campaign'])
 @memoize('all_live_promo_srnames', time=600)
 def all_live_promo_srnames():
     now = promo_datetime_now()
-    pws = PromotionWeights.get_campaigns(now)
-    campaign_ids = {pw.promo_idx for pw in pws}
-    campaigns = PromoCampaign._byID(campaign_ids, data=True,
-                                    return_dict=False)
-    paid_campaigns = [camp for camp in campaigns
-                      if charged_or_not_needed(camp)]
-    link_ids = {camp.link_id for camp in paid_campaigns}
-    links = Link._byID(link_ids, data=True, return_dict=True)
-    live_campaigns = [camp for camp in paid_campaigns
-                      if is_promoted(links[camp.link_id])]
-    return {camp.sr_name for camp in live_campaigns}
+    return {camp.sr_name for camp, link in get_promos(now)
+            if is_live_promo(link, camp)}
 
 
 def srnames_from_site(user, site):
@@ -792,24 +745,13 @@ def srnames_with_live_promos(user, site):
 
 def _get_live_promotions(sr_names):
     now = promo_datetime_now()
-    pws = PromotionWeights.get_campaigns(now, sr_names=sr_names)
-    campaign_ids = {pw.promo_idx for pw in pws}
-    campaigns = PromoCampaign._byID(campaign_ids, data=True,
-                                    return_dict=False)
-    paid_campaigns = [camp for camp in campaigns
-                      if (charged_or_not_needed(camp) and
-                          camp.sr_name in sr_names)]
-    link_ids = {camp.link_id for camp in paid_campaigns}
-    links = Link._byID(link_ids, data=True, return_dict=True)
-    live_campaigns = [camp for camp in paid_campaigns
-                      if is_promoted(links[camp.link_id])]
-
     ret = {sr_name: [] for sr_name in sr_names}
-    for camp in live_campaigns:
-        link_name = links[camp.link_id]._fullname
-        weight=(camp.bid / camp.ndays)
-        pt = PromoTuple(link=link_name, weight=weight, campaign=camp._fullname)
-        ret[camp.sr_name].append(pt)
+    for camp, link in get_promos(now, sr_names=sr_names):
+        if is_live_promo(link, camp):
+            weight = (camp.bid / camp.ndays)
+            pt = PromoTuple(link=link._fullname, weight=weight,
+                            campaign=camp._fullname)
+            ret[camp.sr_name].append(pt)
     return ret
 
 
@@ -919,10 +861,7 @@ def Run(offset=0, verbose=True):
     scheduled changes to ads
     
     """
-    if verbose:
-        print "promote.py:Run() - charge_pending()"
-    charge_pending(offset=offset + 1)
-    charge_pending(offset=offset)
+
     if verbose:
         print "promote.py:Run() - amqp.add_item()"
     amqp.add_item(UPDATE_QUEUE, json.dumps(QUEUE_ALL),
@@ -947,17 +886,7 @@ def run_changed(drain=False, limit=100, sleep_time=10, verbose=True):
             # There's no promotion log to update in this case.
             print "Received %s QUEUE_ALL message(s)" % items.count(QUEUE_ALL)
             items = [i for i in items if i != QUEUE_ALL]
-        make_daily_promotions()
-        links = Link._by_fullname([i["link"] for i in items])
-        for item in items:
-            PromotionLog.add(links[item['link']],
-                             "Finished remaking current promotions (this link "
-                             "was: %(message)s" % item)
+            make_daily_promotions()
+
     amqp.handle_items(UPDATE_QUEUE, _run, limit=limit, drain=drain,
                       sleep_time=sleep_time, verbose=verbose)
-
-
-def queue_changed_promo(link, message):
-    msg = {"link": link._fullname, "message": message}
-    amqp.add_item(UPDATE_QUEUE, json.dumps(msg),
-                  delivery_mode=amqp.DELIVERY_TRANSIENT)