bean/gh-365_serve_ads_from_tags (#373)

Author: StareIntoTheBeard

lib/ad_service/query/for_display.ex

@@ -2,7 +2,7 @@ defmodule AdService.Query.ForDisplay do
   import Ecto.Query
   alias AdService.UnrenderedAdvertisement
   alias CodeFund.Campaigns
-  alias CodeFund.Schema.{Audience, Campaign, Creative, Impression, Property}
+  alias CodeFund.Schema.{Campaign, Creative, Impression, Property}
 
   def fallback_ad_by_property_id(property_id) do
     from(property in Property,
@@ -30,22 +30,52 @@ defmodule AdService.Query.ForDisplay do
     |> UnrenderedAdvertisement.one()
   end
 
-  def build(%Audience{} = audience, client_country, ip_address, excluded_advertisers \\ []) do
+  def build(%Property{} = property, client_country, ip_address, excluded_advertisers \\ []) do
     from(
       creative in Creative,
       join: campaign in Campaign,
       on: campaign.creative_id == creative.id,
       join: large_image_asset in assoc(creative, :large_image_asset),
       left_join: small_image_asset in assoc(creative, :small_image_asset),
       left_join: wide_image_asset in assoc(creative, :wide_image_asset),
-      join: audience in assoc(campaign, :audience),
       distinct: campaign.id
     )
     |> where_country_in(client_country)
     |> AdService.Query.TimeManagement.where_accepted_hours_for_ip_address(ip_address)
     |> AdService.Query.TimeManagement.where_not_allowed_on_weekends(ip_address)
     |> with_daily_budget()
-    |> where([_creative, campaign, ...], campaign.audience_id == ^audience.id)
+    |> where(
+      [_creative, campaign, ...],
+      fragment(
+        "? && ?::varchar[]",
+        campaign.included_programming_languages,
+        ^property.programming_languages
+      )
+    )
+    |> where(
+      [_creative, campaign, ...],
+      fragment(
+        "? && ?::varchar[]",
+        campaign.included_topic_categories,
+        ^property.topic_categories
+      )
+    )
+    |> where(
+      [_creative, campaign, ...],
+      fragment(
+        "not ? && ?::varchar[]",
+        campaign.excluded_programming_languages,
+        ^property.programming_languages
+      )
+    )
+    |> where(
+      [_creative, campaign, ...],
+      fragment(
+        "not ? && ?::varchar[]",
+        campaign.excluded_topic_categories,
+        ^property.topic_categories
+      )
+    )
     |> where(
       [_creative, campaign, ...],
       campaign.id not in ^Campaigns.list_of_ids_for_companies(excluded_advertisers)

lib/ad_service/server.ex

@@ -13,13 +13,12 @@ defmodule AdService.Server do
     with {:ok, :no_cache_found} <- AdService.Impression.Cache.lookup(conn.remote_ip, property_id),
          {:ok, %{country: client_country}} <-
            Framework.Geolocation.find_by_ip(conn.remote_ip, :city),
-         %Property{status: 1, audience: audience} = property
-         when not is_nil(audience) <-
-           Properties.get_property!(property_id) |> CodeFund.Repo.preload([:user, :audience]),
+         %Property{status: 1} = property <-
+           Properties.get_property!(property_id) |> CodeFund.Repo.preload([:user]),
          :ok <- Framework.Browser.certify_human(conn),
          {:ok, ad_tuple} <-
            AdService.Query.ForDisplay.build(
-             audience,
+             property,
              client_country,
              conn.remote_ip,
              property.excluded_advertisers

lib/code_fund/audiences/audiences.ex

@@ -133,12 +133,6 @@ defmodule CodeFund.Audiences do
     Audience.changeset(audience, %{})
   end
 
-  def get_all_display_rates(%Audience{} = audience) do
-    AdService.Query.ForDisplay.build(audience, nil, nil)
-    |> CodeFund.Repo.all()
-    |> AdService.Math.Basic.get_all_display_rates()
-  end
-
   defp filter_config(:audiences) do
     defconfig do
       text(:name)

lib/code_fund/properties/properties.ex

@@ -139,8 +139,8 @@ defmodule CodeFund.Properties do
   def get_property_by_name!(name),
     do: Repo.get_by!(Property, name: name) |> Repo.preload([:user, :audience, :template])
 
-  def get_all_display_rates(%Property{audience: audience}) when not is_nil(audience) do
-    AdService.Query.ForDisplay.build(audience, nil, nil)
+  def get_all_display_rates(%Property{} = property) do
+    AdService.Query.ForDisplay.build(property, nil, nil)
     |> CodeFund.Repo.all()
     |> AdService.Math.Basic.get_all_display_rates()
   end

lib/code_fund_web/templates/audience/index.html.eex

@@ -24,7 +24,6 @@
                 <th>Fallback Campaign</th>
                 <th><%= table_link(@conn, "Programming Languages", :programming_languages) %></th>
                 <th><%= table_link(@conn, "Topic Categories", :topic_categories) %></th>
-                <th>Advertisers</th>
                 <th></th>
               </tr>
             </thead>
@@ -35,13 +34,6 @@
                   <td><%= if audience.campaigns, do: audience.campaigns.name, else: "None"  %></td>
                   <td><%= truncate(audience.programming_languages |> Enum.join(", ")) %></td>
                   <td><%= truncate(audience.topic_categories |> Enum.join(", ")) %></td>
-                  <td>
-                    <ul class="list-unstyled">
-                    <%= for advertiser <- CodeFund.Audiences.get_all_display_rates(audience) do %>
-                      <li><strong><%= Number.Percentage.number_to_percentage(advertiser.display_rate, precision: 1) %></strong> <%= advertiser.campaign_name %></li>
-                    <% end %>
-                    </ul>
-                  </td>
                   <td class="text-right p-2">
                     <%= link 'Show', to: audience_path(@conn, :show, audience), class: "btn btn-outline-info btn-sm" %>
                     <%= link 'Edit', to: audience_path(@conn, :edit, audience), class: "btn btn-outline-primary btn-sm" %>

lib/code_fund_web/templates/property/index.html.eex

@@ -54,6 +54,9 @@
                   <%= if has_any_role? @conn, ["admin", "developer"] do %>
                     <th>Excluded Advertisers</th>
                   <% end %>
+                  <%= if has_any_role? @conn, ["admin"] do %>
+                    <th>Advertisers</th>
+                  <% end %>
                   <th></th>
                 </tr>
               </thead>
@@ -73,6 +76,15 @@
                     <%= if has_any_role? @conn, ["admin", "developer"] do %>
                       <td><%= property.excluded_advertisers |> Enum.join(", ") %></td>
                     <% end %>
+                    <%= if has_any_role? @conn, ["admin"] do %>
+                      <td>
+                        <ul class="list-unstyled">
+                        <%= for advertiser <- CodeFund.Properties.get_all_display_rates(property) do %>
+                          <li><strong><%= Number.Percentage.number_to_percentage(advertiser.display_rate, precision: 1) %></strong> <%= advertiser.campaign_name %></li>
+                        <% end %>
+                        </ul>
+                      </td>
+                    <% end %>
                     <td class="text-right p-2">
                       <%= link 'Show', to: property_path(@conn, :show, property), class: "btn btn-outline-info btn-sm" %>
                       <%= link 'Edit', to: property_path(@conn, :edit, property), class: "btn btn-outline-primary btn-sm" %>

test/ad_service/query/for_display_test.exs

@@ -29,6 +29,22 @@ defmodule AdService.Query.ForDisplayTest do
       topic_categories: ["Development"]
     })
 
+    property =
+      insert(:property, %{
+        programming_languages: ["Ruby", "C"],
+        topic_categories: ["Programming"]
+      })
+
+    insert(:property, %{
+      programming_languages: ["Ruby", "C"],
+      topic_categories: ["Development"]
+    })
+
+    insert(:property, %{
+      programming_languages: ["Ruby", "Rust"],
+      topic_categories: ["Programming"]
+    })
+
     campaign =
       insert(
         :campaign,
@@ -40,6 +56,10 @@ defmodule AdService.Query.ForDisplayTest do
         end_date: Timex.now() |> Timex.shift(days: 1) |> DateTime.to_naive(),
         creative: creative,
         audience: audience,
+        included_programming_languages: ["Ruby"],
+        included_topic_categories: ["Programming"],
+        excluded_programming_languages: ["Rust"],
+        excluded_topic_categories: ["Development"],
         included_countries: ["US"],
         user: insert(:user, company: "Acme")
       )
@@ -54,6 +74,10 @@ defmodule AdService.Query.ForDisplayTest do
       end_date: Timex.now() |> Timex.shift(days: -1) |> DateTime.to_naive(),
       creative: creative,
       audience: audience,
+      included_programming_languages: ["Rust"],
+      included_topic_categories: ["Development"],
+      excluded_programming_languages: ["Ruby"],
+      excluded_topic_categories: ["Programming"],
       included_countries: ["US"]
     )
 
@@ -66,6 +90,10 @@ defmodule AdService.Query.ForDisplayTest do
       end_date: Timex.now() |> Timex.shift(days: 1) |> DateTime.to_naive(),
       total_spend: Decimal.new(100),
       creative: creative,
+      included_programming_languages: ["Rust"],
+      included_topic_categories: ["Development"],
+      excluded_programming_languages: ["Ruby"],
+      excluded_topic_categories: ["Programming"],
       included_countries: ["IN"],
       audience: insert(:audience, %{programming_languages: ["Java", "Rust"]})
     )
@@ -80,12 +108,23 @@ defmodule AdService.Query.ForDisplayTest do
       end_date: Timex.now() |> Timex.shift(days: 1) |> DateTime.to_naive(),
       creative: creative,
       audience: audience,
+      included_programming_languages: ["Rust"],
+      included_topic_categories: ["Development"],
+      excluded_programming_languages: ["Ruby"],
+      excluded_topic_categories: ["Programming"],
       included_countries: ["CN"]
     )
 
     [cdn_host: cdn_host] = Application.get_env(:code_fund, Framework.FileStorage)
 
-    {:ok, %{audience: audience, creative: creative, campaign: campaign, cdn_host: cdn_host}}
+    {:ok,
+     %{
+       audience: audience,
+       creative: creative,
+       campaign: campaign,
+       cdn_host: cdn_host,
+       property: property
+     }}
   end
 
   describe "fallback_ad_by_property_id/1" do
@@ -122,13 +161,13 @@ defmodule AdService.Query.ForDisplayTest do
   end
 
   describe "build/1" do
-    test "get_by_property_filters excludes indicated countries", %{audience: audience} do
-      refute AdService.Query.ForDisplay.build(audience, "CN", nil, ["Foobar"])
+    test "get_by_property_filters excludes indicated countries", %{property: property} do
+      refute AdService.Query.ForDisplay.build(property, "CN", nil, ["Foobar"])
              |> CodeFund.Repo.one()
     end
 
     test "it returns advertisements by audience, included country and excluded advertisers", %{
-      audience: audience,
+      property: property,
       campaign: campaign,
       creative: creative
     } do
@@ -141,13 +180,16 @@ defmodule AdService.Query.ForDisplayTest do
         start_date: Timex.now() |> Timex.shift(days: -1) |> DateTime.to_naive(),
         end_date: Timex.now() |> Timex.shift(days: 1) |> DateTime.to_naive(),
         creative: creative,
-        audience: audience,
+        included_programming_languages: ["Ruby"],
+        included_topic_categories: ["Programming"],
+        excluded_programming_languages: ["Rust"],
+        excluded_topic_categories: ["Development"],
         included_countries: ["US"],
         user: insert(:user, company: "Foobar")
       )
 
       advertisement =
-        AdService.Query.ForDisplay.build(audience, "US", nil, ["Foobar"])
+        AdService.Query.ForDisplay.build(property, "US", nil, ["Foobar"])
         |> CodeFund.Repo.one()
 
       small_image_asset = CodeFund.Schema.Asset |> Repo.get!(creative.small_image_asset.id)
@@ -178,7 +220,7 @@ defmodule AdService.Query.ForDisplayTest do
     end
 
     test "it excludes campaigns which are not allowed on the weekends when its a weekend", %{
-      audience: audience,
+      property: property,
       creative: creative
     } do
       CodeFund.Schema.Campaign
@@ -195,22 +237,25 @@ defmodule AdService.Query.ForDisplayTest do
         end_date: Timex.now() |> Timex.shift(days: 1) |> DateTime.to_naive(),
         creative: creative,
         weekdays_only: true,
-        audience: audience,
+        included_programming_languages: ["Ruby"],
+        included_topic_categories: ["Programming"],
+        excluded_programming_languages: ["Rust"],
+        excluded_topic_categories: ["Development"],
         included_countries: ["US"],
         user: insert(:user, company: "Acme")
       )
 
       TimeMachinex.ManagedClock.set(DateTime.from_naive!(~N[2018-09-22 11:00:00], "Etc/UTC"))
 
       advertisement =
-        AdService.Query.ForDisplay.build(audience, "US", {72, 229, 28, 185}, ["Foobar"])
+        AdService.Query.ForDisplay.build(property, "US", {72, 229, 28, 185}, ["Foobar"])
         |> CodeFund.Repo.one()
 
       refute advertisement
     end
 
     test "it will exclude campaigns that are over their daily budget", %{
-      audience: audience,
+      property: property,
       creative: creative
     } do
       CodeFund.Schema.Campaign
@@ -227,15 +272,18 @@ defmodule AdService.Query.ForDisplayTest do
           start_date: Timex.now() |> Timex.shift(days: -1) |> DateTime.to_naive(),
           end_date: Timex.now() |> Timex.shift(days: 1) |> DateTime.to_naive(),
           creative: creative,
-          audience: audience,
+          included_programming_languages: ["Ruby"],
+          included_topic_categories: ["Programming"],
+          excluded_programming_languages: ["Rust"],
+          excluded_topic_categories: ["Development"],
           included_countries: ["US"],
           user: insert(:user, company: "Acme")
         )
 
       insert(:impression, campaign: campaign, revenue_amount: Decimal.new(9.5))
 
       advertisement =
-        AdService.Query.ForDisplay.build(audience, "US", nil, ["Foobar"]) |> CodeFund.Repo.one()
+        AdService.Query.ForDisplay.build(property, "US", nil, ["Foobar"]) |> CodeFund.Repo.one()
 
       refute advertisement
     end