Skip to content

Commit

Permalink
chore(Notifications): Properly subscribe and add graphql queries
Browse files Browse the repository at this point in the history
  • Loading branch information
Betree committed Mar 15, 2019
1 parent faf8501 commit 881fbbb
Showing 12 changed files with 159 additions and 25 deletions.
44 changes: 35 additions & 9 deletions apps/cf/lib/comments/comments.ex
Original file line number Diff line number Diff line change
@@ -5,6 +5,7 @@ defmodule CF.Comments do
import CF.Actions.ActionCreator

alias Ecto.Multi
alias Kaur.Result

alias DB.Repo
alias DB.Schema.Source
@@ -14,6 +15,7 @@ defmodule CF.Comments do
alias DB.Schema.UserAction

alias CF.Accounts.UserPermissions
alias CF.Notifications.Subscriptions
alias CF.Sources

# ---- Public API ----
@@ -28,6 +30,12 @@ defmodule CF.Comments do
|> Repo.all()
end

@doc """
Add a new comment.
[!] This function is very bad and should be refactored, especially the async
source fetcher should be moved to a job.
"""
def add_comment(user, video_id, params, source_url \\ nil, source_fetch_callback \\ nil) do
# TODO [Security] What if reply_to_id refer to a comment that is on a different statement ?
UserPermissions.check!(user, :create, :comment)
@@ -38,26 +46,44 @@ defmodule CF.Comments do
source_url &&
(Sources.get_by_url(source_url) || Source.changeset(%Source{}, %{url: source_url}))

# Insert comment in DB
full_comment =
comment_changeset =
user
|> Ecto.build_assoc(:comments)
|> Ecto.Changeset.change()
|> Ecto.Changeset.put_assoc(:source, source)
|> Comment.changeset(params)

Multi.new()
|> Multi.run(:comment, fn _ ->
comment_changeset
|> Repo.insert!()
|> Map.put(:user, user)
|> Repo.preload(:source)
|> Repo.preload([:source, :statement])
|> Map.put(:score, 0)
|> Result.ok()
end)
|> Multi.run(:action, fn %{comment: comment} ->
Repo.insert(action_create(user.id, video_id, comment, source_url))
end)
|> Multi.run(:suscription, fn %{comment: comment} ->
Subscriptions.subscribe(user, comment, :is_author)
end)
|> Repo.transaction()
|> case do
{:error, _operation, reason, _changes} ->
{:error, reason}

# Record action
Repo.insert(action_create(user.id, video_id, full_comment, source_url))
{:ok, %{comment: comment}} ->
# Set default on comment
full_comment = comment

# If new source, fetch metadata
unless is_nil(source) || !is_nil(Map.get(source, :id)),
do: fetch_source_metadata_and_update_comment(full_comment, source_fetch_callback)
# If new source, fetch metadata
if source && is_nil(Map.get(source, :id)),
do: fetch_source_metadata_and_update_comment(comment, source_fetch_callback)

full_comment
# Return comment
full_comment
end
end

# Delete
9 changes: 7 additions & 2 deletions apps/cf/lib/notifications/notifications.ex
Original file line number Diff line number Diff line change
@@ -14,10 +14,11 @@ defmodule CF.Notifications do
Get all notifications for user, last inserted first.
Paginated with `page` + `limit`.
"""
@spec all(User.t()) :: Scrivener.Page.t()
def all(%User{id: user_id}, page \\ 1, page_size \\ 10) do
@spec all(User.t(), integer(), integer(), :all | :seen | :unseen) :: Scrivener.Page.t()
def all(%User{id: user_id}, page \\ 1, page_size \\ 10, filter \\ :all) do
Notification
|> where([n], n.user_id == ^user_id)
|> add_filter(filter)
|> order_by(desc: :inserted_at)
|> Repo.paginate(page: page, page_size: page_size)
end
@@ -53,4 +54,8 @@ defmodule CF.Notifications do

def mark_as_seen(notification, _),
do: {:ok, notification}

defp add_filter(query, :seen), do: where(query, [n], not is_nil(n.seen_at))
defp add_filter(query, :unseen), do: where(query, [n], is_nil(n.seen_at))
defp add_filter(query, :all), do: query
end
20 changes: 16 additions & 4 deletions apps/cf/lib/notifications/subscriptions.ex
Original file line number Diff line number Diff line change
@@ -32,12 +32,18 @@ defmodule CF.Notifications.Subscriptions do
"""
def is_subscribed(%User{id: user_id}, %Video{id: video_id}) do
Subscription
|> select([:id])
|> select([:id, :is_subscribed])
|> where([s], s.scope == ^:video)
|> where([s], s.user_id == ^user_id)
|> where([s], s.video_id == ^video_id)
|> DB.Repo.one()
|> Kernel.!=(nil)
|> case do
nil ->
false

%{is_subscribed: is_subscribed} = a ->
is_subscribed
end
end

@doc """
@@ -56,8 +62,14 @@ defmodule CF.Notifications.Subscriptions do
"""
@spec unsubscribe(User.t(), subscribable_entities()) :: {:ok, Subscription.t()} | nil
def unsubscribe(user, entity) do
with subscription when not is_nil(subscription) <- load_subscription(user, entity) do
DB.Repo.delete(subscription)
user
|> load_subscription(entity)
|> case do
nil ->
{:error, "not_found"}

subscription ->
DB.Repo.update(Subscription.changeset_subscribed(subscription, false))
end
end

25 changes: 25 additions & 0 deletions apps/cf/lib/notifications/subscriptions_matcher.ex
Original file line number Diff line number Diff line change
@@ -24,6 +24,7 @@ defmodule CF.Notifications.SubscriptionsMatcher do
|> or_where([s], s.statement_id == ^action.statement_id)
|> or_where([s], s.video_id == ^action.video_id)
|> Repo.all()
|> uniq_subscriptions()
end

def match_action(action = %{entity: :statement, type: type, video_id: video_id})
@@ -32,13 +33,15 @@ defmodule CF.Notifications.SubscriptionsMatcher do
|> where([s], s.scope in ^[:video, :statement])
|> where([s], s.statement_id == ^action.statement_id or s.video_id == ^video_id)
|> Repo.all()
|> uniq_subscriptions()
end

def match_action(%{entity: :video, type: :update, video_id: video_id}) do
Subscription
|> where([s], s.scope == ^:video)
|> where([s], s.video_id == ^video_id)
|> Repo.all()
|> uniq_subscriptions()
end

def match_action(%{entity: :speaker, type: type, video_id: video_id})
@@ -47,7 +50,29 @@ defmodule CF.Notifications.SubscriptionsMatcher do
|> where([s], s.scope == ^:video)
|> where([s], s.video_id == ^video_id)
|> Repo.all()
|> uniq_subscriptions()
end

def match_action(_), do: []

# Ensure we only return one subscription per user/entity.
# Always returns the more precise subscription
defp uniq_subscriptions(subscriptions) do
subscriptions
|> Enum.group_by(& &1.user_id)
|> Enum.map(fn
# Only one subscription, return it
{_, [subscription | []]} ->
subscription

# Multiple subscriptions, let's pick the most precise one
{_, user_subscriptions} ->
Enum.max_by(user_subscriptions, &subscription_precision/1)
end)
end

defp subscription_precision(%{video_id: nil}), do: 0
defp subscription_precision(%{statement_id: nil}), do: 1
defp subscription_precision(%{comment_id: nil}), do: 2
defp subscription_precision(_), do: 3
end
44 changes: 42 additions & 2 deletions apps/cf_graphql/lib/resolvers/notifications.ex
Original file line number Diff line number Diff line change
@@ -14,11 +14,13 @@ defmodule CF.Graphql.Resolvers.Notifications do
@doc """
User notifications, only if authenticated
"""
def for_user(user, %{page: page, page_size: page_size}, %{context: %{user: loggedin_user}}) do
def for_user(user, %{page: page, page_size: page_size, filter: filter}, %{
context: %{user: loggedin_user}
}) do
if user.id !== loggedin_user.id do
{:error, "unauthorized"}
else
{:ok, Notifications.all(user, page, page_size)}
{:ok, Notifications.all(user, page, page_size, filter)}
end
end

@@ -46,4 +48,42 @@ defmodule CF.Graphql.Resolvers.Notifications do
|> elem(1)
|> Result.ok()
end

@doc """
Update the given subscription
"""
def update_subscription(
_,
%{
scope: scope,
entity_id: entity_id,
is_subscribed: is_subscribed
} = params,
%{context: %{user: loggedin_user}}
) do
entity = load_entity(scope, entity_id)

cond do
is_nil(entity) ->
{:error, "not_found"}

is_subscribed ->
Subscriptions.subscribe(loggedin_user, entity, params[:reason])

true ->
Subscriptions.unsubscribe(loggedin_user, entity)
end
end

defp load_entity("comment", entity_id) do
DB.Repo.get(DB.Schema.Comment, entity_id)
end

defp load_entity("statement", entity_id) do
DB.Repo.get(DB.Schema.Statement, entity_id)
end

defp load_entity("video", entity_id) do
DB.Repo.get(DB.Schema.Video, entity_id)
end
end
6 changes: 2 additions & 4 deletions apps/cf_graphql/lib/router.ex
Original file line number Diff line number Diff line change
@@ -27,8 +27,7 @@ defmodule CF.GraphQLWeb.Router do
Absinthe.Plug.GraphiQL,
schema: CF.Graphql.Schema,
analyze_complexity: true,
# (6 joins = 300) + 20 fields
max_complexity: 320
max_complexity: 400
)
end

@@ -37,8 +36,7 @@ defmodule CF.GraphQLWeb.Router do
Absinthe.Plug,
schema: CF.Graphql.Schema,
analyze_complexity: true,
# (6 joins = 300) + 20 fields
max_complexity: 320
max_complexity: 400
)
end
end
10 changes: 10 additions & 0 deletions apps/cf_graphql/lib/schema/schema.ex
Original file line number Diff line number Diff line change
@@ -68,5 +68,15 @@ defmodule CF.Graphql.Schema do

resolve(&Resolvers.Notifications.update/3)
end

@desc "Use this to (un)subscribe from an item notifications"
field :update_subscription, :notifications_subscription do
arg(:scope, non_null(:string))
arg(:entity_id, non_null(:id))
arg(:is_subscribed, non_null(:boolean))
arg(:reason, :string)

resolve(&Resolvers.Notifications.update_subscription/3)
end
end
end
3 changes: 3 additions & 0 deletions apps/cf_graphql/lib/schema/types/user.ex
Original file line number Diff line number Diff line change
@@ -13,6 +13,8 @@ defmodule CF.Graphql.Schema.Types.User do
import_types(CF.Graphql.Schema.Types.Paginated)
import_types(CF.Graphql.Schema.Types.Notification)

enum(:notifications_filter, values: [:all, :seen, :unseen])

@desc "A user registered on the website"
object :user do
@desc "Unique user ID"
@@ -53,6 +55,7 @@ defmodule CF.Graphql.Schema.Types.User do
complexity(join_complexity())
arg(:page, :integer, default_value: 1)
arg(:page_size, :integer, default_value: 10)
arg(:filter, :notifications_filter, default_value: :all)
resolve(&Resolvers.Notifications.for_user/3)
end

6 changes: 6 additions & 0 deletions apps/cf_graphql/lib/schema/types/user_action.ex
Original file line number Diff line number Diff line change
@@ -67,6 +67,12 @@ defmodule CF.Graphql.Schema.Types.UserAction do
resolve(assoc(:comment))
end

@desc "Associated speaker"
field :speaker, :speaker do
complexity(join_complexity())
resolve(assoc(:speaker))
end

@desc "A map with all the changes made by this action"
field(
:changes,
7 changes: 6 additions & 1 deletion apps/db/lib/db_schema/subscription.ex
Original file line number Diff line number Diff line change
@@ -63,10 +63,15 @@ defmodule DB.Schema.Subscription do
changeset(struct, %{
video_id: video.id,
scope: :video,
reason: reason
reason: reason,
is_subscribed: true
})
end

def changeset_subscribed(struct, is_subscribed) do
change(struct, is_subscribed: is_subscribed)
end

defp validate_required_by_scope(changeset) do
case get_field(changeset, :scope) do
nil ->
6 changes: 5 additions & 1 deletion apps/db/lib/db_type/subscription_reason.ex
Original file line number Diff line number Diff line change
@@ -3,5 +3,9 @@ import EctoEnum
defenum(
DB.Type.SubscriptionReason,
:subscription_reason,
[:is_author]
[
:is_author,
:manual,
:suggestion
]
)
Original file line number Diff line number Diff line change
@@ -13,11 +13,11 @@ defmodule DB.Repo.Migrations.CreateNotifications do
timestamps()
end

create(index(:notifications, :user_id))
create(index(:notifications, [:user_id, :action_id]))
end

def down do
drop(index(:notifications, :user_id))
drop(index(:notifications, [:user_id, :action_id]))
drop(table(:notifications))
DB.Type.NotificationType.drop_type()
end

0 comments on commit 881fbbb

Please sign in to comment.