Skip to content

Commit

Permalink
Merge pull request #138 from CaptainFact/chore/speakers-title-length
Browse files Browse the repository at this point in the history
Edit speaker enhancements
  • Loading branch information
Betree authored Apr 1, 2019
2 parents ea9333b + a843220 commit 52f371a
Show file tree
Hide file tree
Showing 6 changed files with 96 additions and 10 deletions.
64 changes: 64 additions & 0 deletions apps/cf/lib/speakers/speakers.ex
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ defmodule CF.Speakers do
Speakers utils
"""

require Logger
import Ecto.Query

alias DB.Repo
Expand All @@ -25,6 +26,24 @@ defmodule CF.Speakers do
end
end

@doc """
Calls `fetch_picture/2` with the picture retrieved from wikidata using
`retrieve_wikimedia_picture_url/1`
"""
def fetch_picture_from_wikimedia(speaker = %Speaker{wikidata_item_id: nil}) do
{:error, "Cannot fetch picture from wikimedia if wikidata_item_id is no set"}
end

def fetch_picture_from_wikimedia(speaker) do
case retrieve_wikimedia_picture_url(speaker) do
{:error, reason} ->
{:error, reason}

{:ok, url} ->
fetch_picture(speaker, url)
end
end

@doc """
Generate slug or update existing one for `speaker`
"""
Expand All @@ -43,4 +62,49 @@ defmodule CF.Speakers do
|> Repo.all()
|> Enum.map(&generate_slug/1)
end

def retrieve_wikimedia_picture_url(%Speaker{wikidata_item_id: nil}) do
nil
end

def retrieve_wikimedia_picture_url(speaker = %Speaker{wikidata_item_id: qid}) do
wikidata_url =
"https://www.wikidata.org/w/api.php?action=wbgetclaims&entity=#{qid}&property=P18&format=json"

with {:ok, %HTTPoison.Response{status_code: 200, body: body}} <- HTTPoison.get(wikidata_url),
{:ok, decoded_response} <- Poison.decode(body),
filename when not is_nil(filename) <- picture_filename_from_response(decoded_response) do
{:ok, wikimedia_url_from_filename(filename)}
else
{:error, error = %HTTPoison.Error{}} ->
Logger.warn("Wikidata query failed: #{error.reason}")
{:error, "Connection failed"}

e ->
Logger.info("No picture found for #{speaker.full_name}")
{:error, "No picture found"}
end
end

defp picture_filename_from_response(%{"claims" => %{"P18" => images}}) do
case images do
[%{"mainsnak" => %{"datavalue" => %{"value" => filename}}}] ->
filename

[%{"mainsnak" => %{"datavalue" => %{"value" => filename}}} | _] ->
Logger.debug("Multiple pictures available: #{inspect(images)}")
filename
end
end

defp picture_filename_from_response(_), do: nil

defp wikimedia_url_from_filename(filename) do
formatted_filename = String.replace(filename, " ", "_")
hash = Base.encode16(:crypto.hash(:md5, formatted_filename), case: :lower)
hash_1 = String.at(hash, 0)
hash_2 = String.at(hash, 1)
path = "#{hash_1}/#{hash_1}#{hash_2}/#{formatted_filename}"
"https://upload.wikimedia.org/wikipedia/commons/#{path}"
end
end
2 changes: 1 addition & 1 deletion apps/cf/priv/limitations.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ add:
speaker: [ 0 , 0 , 0 , 3 , 6 , 15 , 30 , 40 , 50 ]
update:
statement: [ 0 , 0 , 5 , 15 , 30 , 75 , 125 , 150 , 200 ]
speaker: [ 0 , 0 , 3 , 7 , 15 , 30 , 60 , 80 , 100 ]
speaker: [ 0 , 0 , 0 , 0 , 5 , 20 , 30 , 50 , 100 ]
video: [ 0 , 0 , 0 , 0 , 3 , 6 , 12 , 25 , 40 ]
user: [ 1 , 10 , 15 , 20 , 20 , 30 , 30 , 50 , 50 ]
delete:
Expand Down
18 changes: 18 additions & 0 deletions apps/cf_rest_api/lib/channels/video_debate_channel.ex
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ defmodule CF.RestApi.VideoDebateChannel do
alias DB.Schema.VideoSpeaker

alias CF.Videos
alias CF.Speakers
alias CF.Accounts.UserPermissions
alias CF.Notifications.Subscriptions
alias CF.RestApi.{VideoView, SpeakerView, ChangesetView}
Expand Down Expand Up @@ -170,6 +171,9 @@ defmodule CF.RestApi.VideoDebateChannel do
|> Repo.transaction()
|> case do
{:ok, %{speaker: speaker}} ->
unless is_nil(changeset.changes[:wikidata_item_id]),
do: fetch_speaker_picture_from_wiki_async(socket, speaker)

rendered_speaker = View.render_one(speaker, SpeakerView, "speaker.json")
broadcast!(socket, "speaker_updated", rendered_speaker)
{:reply, :ok, socket}
Expand Down Expand Up @@ -239,4 +243,18 @@ defmodule CF.RestApi.VideoDebateChannel do
defp is_subscribed(user_id, video) do
Subscriptions.is_subscribed(%User{id: user_id}, video)
end

defp fetch_speaker_picture_from_wiki_async(socket, speaker) do
Task.start(fn ->
case Speakers.fetch_picture_from_wikimedia(speaker) do
{:ok, speaker} ->
rendered_speaker = View.render_one(speaker, SpeakerView, "speaker.json")
broadcast!(socket, "speaker_updated", rendered_speaker)

_ ->
# We don't care about errors here
nil
end
end)
end
end
6 changes: 3 additions & 3 deletions apps/db/lib/db_schema/speaker.ex
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,10 @@ defmodule DB.Schema.Speaker do
|> cast_attachments(params, @optional_file_fields)
|> update_change(:full_name, &DB.Utils.String.trim_all_whitespaces/1)
|> update_change(:title, &DB.Utils.String.trim_all_whitespaces/1)
|> validate_length(:full_name, min: 3, max: 60)
|> validate_length(:title, min: 3, max: 60)
|> validate_length(:full_name, min: 3, max: 120)
|> validate_length(:title, min: 3, max: 240)
|> validate_required(:full_name)
|> update_change(:wikidata_item_id, &String.upcase/1)
|> update_change(:wikidata_item_id, &DB.Utils.String.upcase/1)
|> validate_format(:wikidata_item_id, ~r/Q[1-9]\d*/)
|> unique_constraint(:wikidata_item_id)
end
Expand Down
4 changes: 4 additions & 0 deletions apps/db/lib/db_utils/string.ex
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,8 @@ defmodule DB.Utils.String do
|> String.trim()
|> String.replace(~r/\s+/, " ")
end

def upcase(nil), do: nil

def upcase(str), do: String.upcase(str)
end
12 changes: 6 additions & 6 deletions apps/db/test/db_schema/speaker_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -19,24 +19,24 @@ defmodule DB.Schema.SpeakerTest do
refute changeset.valid?
end

test "full name must be between 3 and 60 characters" do
test "full name must be between 3 and 120 characters" do
# Too short
attrs = %{full_name: "x"}
assert {:full_name, "should be at least 3 character(s)"} in errors_on(%Speaker{}, attrs)

# Too long
attrs = %{full_name: String.duplicate("x", 61)}
assert {:full_name, "should be at most 60 character(s)"} in errors_on(%Speaker{}, attrs)
attrs = %{full_name: String.duplicate("x", 121)}
assert {:full_name, "should be at most 120 character(s)"} in errors_on(%Speaker{}, attrs)
end

test "title must be between 3 and 60 characters" do
test "title must be between 3 and 240 characters" do
# Too short
attrs = %{title: "x"}
assert {:title, "should be at least 3 character(s)"} in errors_on(%Speaker{}, attrs)

# Too long
attrs = %{title: String.duplicate("x", 61)}
assert {:title, "should be at most 60 character(s)"} in errors_on(%Speaker{}, attrs)
attrs = %{title: String.duplicate("x", 241)}
assert {:title, "should be at most 240 character(s)"} in errors_on(%Speaker{}, attrs)
end

test "name and title are trimmed" do
Expand Down

0 comments on commit 52f371a

Please sign in to comment.