refactor: make set_community_flags readable

Author: mydearxym

lib/groupher_server/cms/cms.ex

@@ -84,7 +84,7 @@ defmodule GroupherServer.CMS do
   defdelegate favorited_category(thread, content_id, user), to: FavoritedContents
   # ArticleOperation
   # >> set flag on article, like: pin / unpin article
-  defdelegate set_community_flags(queryable, community_id, attrs), to: ArticleOperation
+  defdelegate set_community_flags(community_info, queryable, attrs), to: ArticleOperation
   defdelegate pin_content(queryable, community_id, topic), to: ArticleOperation
   defdelegate undo_pin_content(queryable, community_id, topic), to: ArticleOperation
   defdelegate pin_content(queryable, community_id), to: ArticleOperation

lib/groupher_server/cms/delegates/article_curd.ex

@@ -129,25 +129,10 @@ defmodule GroupherServer.CMS.Delegate.ArticleCURD do
   defp exec_set_tag(_thread, _id, _attrs), do: {:ok, :pass}
 
   # TODO:  flag 逻辑似乎有问题
-  defp exec_set_community_flag(%Community{id: cid}, content, %{flag: _flag}) do
-    # TODO:  1. 参数改变一下顺序，把 community 放在前面
-    # TODO:  2. 直接传 community 下去，免去 set_community_flags 函数再在内部查 community
-    # TODO:  3. 该函数似乎逻辑似乎有点问题, 没有区分 action trash|..
-    # 要考虑第二条是否符合现实？
-    ArticleOperation.set_community_flags(content, cid, %{
+  defp exec_set_community_flag(%Community{} = community, content, %{flag: _flag}) do
+    ArticleOperation.set_community_flags(community, content, %{
       trash: false
     })
-
-    # TODO: remove this judge, as content should have a flag
-    # case action |> Map.has_key?(:flag) do
-    #   true ->
-    #     ArticleOperation.set_community_flags(content, community.id, %{
-    #       trash: false
-    #     })
-
-    #   false ->
-    #     {:ok, :pass}
-    # end
   end
 
   defp exec_set_community_flag(_community, _content, _action) do

lib/groupher_server/cms/delegates/article_operation.ex

@@ -97,7 +97,14 @@ defmodule GroupherServer.CMS.Delegate.ArticleOperation do
   @doc """
   trash / untrash articles
   """
-  def set_community_flags(content, community_id, attrs) do
+  def set_community_flags(%Community{id: cid}, content, attrs) do
+    with {:ok, content} <- ORM.find(content.__struct__, content.id),
+         {:ok, record} <- insert_flag_record(content, cid, attrs) do
+      {:ok, struct(content, %{trash: record.trash})}
+    end
+  end
+
+  def set_community_flags(community_id, content, attrs) do
     with {:ok, content} <- ORM.find(content.__struct__, content.id),
          {:ok, community} <- ORM.find(Community, community_id),
          {:ok, record} <- insert_flag_record(content, community.id, attrs) do

lib/groupher_server_web/resolvers/cms_resolver.ex

@@ -167,9 +167,9 @@ defmodule GroupherServerWeb.Resolvers.CMS do
 
   defp set_community_flags(community_id, thread, id, flag) do
     with {:ok, content} <- match_action(thread, :self) do
-      content.target
-      |> struct(%{id: id})
-      |> CMS.set_community_flags(community_id, flag)
+      queryable = content.target |> struct(%{id: id})
+
+      CMS.set_community_flags(community_id, queryable, flag)
     end
   end
 

test/groupher_server/cms/content_flag_test.exs

@@ -1,16 +1,14 @@
 defmodule GroupherServer.Test.CMS.ContentFlags do
+  @moduledoc false
+
   use GroupherServer.TestTools
 
   alias GroupherServer.CMS
 
   alias CMS.{
-    Post,
     PostCommunityFlag,
-    Repo,
     RepoCommunityFlag,
-    Job,
     JobCommunityFlag,
-    Video,
     VideoCommunityFlag
   }
 
@@ -35,7 +33,7 @@ defmodule GroupherServer.Test.CMS.ContentFlags do
       {:ok, found} = PostCommunityFlag |> ORM.find_by(~m(post_id community_id)a)
       assert found.trash == false
 
-      CMS.set_community_flags(%Post{id: post.id}, community.id, %{trash: true})
+      CMS.set_community_flags(community, post, %{trash: true})
 
       {:ok, found} = PostCommunityFlag |> ORM.find_by(~m(post_id community_id)a)
 
@@ -55,7 +53,7 @@ defmodule GroupherServer.Test.CMS.ContentFlags do
       {:ok, found} = JobCommunityFlag |> ORM.find_by(~m(job_id community_id)a)
       assert found.trash == false
 
-      CMS.set_community_flags(%Job{id: job.id}, community.id, %{trash: true})
+      CMS.set_community_flags(community, job, %{trash: true})
 
       {:ok, found} = JobCommunityFlag |> ORM.find_by(~m(job_id community_id)a)
 
@@ -73,7 +71,7 @@ defmodule GroupherServer.Test.CMS.ContentFlags do
       {:ok, found} = VideoCommunityFlag |> ORM.find_by(~m(video_id community_id)a)
       assert found.trash == false
 
-      CMS.set_community_flags(%Video{id: video.id}, community.id, %{trash: true})
+      CMS.set_community_flags(community, video, %{trash: true})
 
       {:ok, found} = VideoCommunityFlag |> ORM.find_by(~m(video_id community_id)a)
 
@@ -91,7 +89,7 @@ defmodule GroupherServer.Test.CMS.ContentFlags do
       {:ok, found} = RepoCommunityFlag |> ORM.find_by(~m(repo_id community_id)a)
       assert found.trash == false
 
-      CMS.set_community_flags(%Repo{id: repo.id}, community.id, %{trash: true})
+      CMS.set_community_flags(community, repo, %{trash: true})
 
       {:ok, found} = RepoCommunityFlag |> ORM.find_by(~m(repo_id community_id)a)
 

test/groupher_server_web/mutation/cms/cms_manager_test.exs

@@ -28,6 +28,7 @@ defmodule GroupherServer.Test.Mutation.CMS.Manager do
       }
     }
     """
+    @tag :wip2
     test "root can trash a post", ~m(community user)a do
       post_attrs = mock_attrs(:post, %{community_id: community.id})
       {:ok, post} = CMS.create_content(community, :post, post_attrs, user)

test/groupher_server_web/mutation/cms/job_flag_test.exs

@@ -57,7 +57,7 @@ defmodule GroupherServer.Test.Mutation.JobFlag do
     test "auth user can undo trash job", ~m(community job)a do
       variables = %{id: job.id, communityId: community.id}
 
-      {:ok, _} = CMS.set_community_flags(job, community.id, %{trash: true})
+      {:ok, _} = CMS.set_community_flags(community, job, %{trash: true})
 
       passport_rules = %{community.raw => %{"job.undo_trash" => true}}
       rule_conn = simu_conn(:user, cms: passport_rules)

test/groupher_server_web/mutation/cms/post_flag_test.exs

@@ -57,7 +57,7 @@ defmodule GroupherServer.Test.Mutation.PostFlag do
     test "auth user can undo trash post", ~m(community post)a do
       variables = %{id: post.id, communityId: community.id}
 
-      {:ok, _} = CMS.set_community_flags(post, community.id, %{trash: true})
+      {:ok, _} = CMS.set_community_flags(community, post, %{trash: true})
 
       passport_rules = %{community.raw => %{"post.undo_trash" => true}}
       rule_conn = simu_conn(:user, cms: passport_rules)

test/groupher_server_web/mutation/cms/repo_flag_test.exs

@@ -57,7 +57,7 @@ defmodule GroupherServer.Test.Mutation.RepoFlag do
     test "auth user can undo trash repo", ~m(community repo)a do
       variables = %{id: repo.id, communityId: community.id}
 
-      {:ok, _} = CMS.set_community_flags(repo, community.id, %{trash: true})
+      {:ok, _} = CMS.set_community_flags(community, repo, %{trash: true})
 
       passport_rules = %{community.raw => %{"repo.undo_trash" => true}}
       rule_conn = simu_conn(:user, cms: passport_rules)

test/groupher_server_web/mutation/cms/video_flag_test.exs

@@ -57,7 +57,7 @@ defmodule GroupherServer.Test.Mutation.VideoFlag do
     test "auth user can undo trash video", ~m(community video)a do
       variables = %{id: video.id, communityId: community.id}
 
-      {:ok, _} = CMS.set_community_flags(video, community.id, %{trash: true})
+      {:ok, _} = CMS.set_community_flags(community, video, %{trash: true})
 
       passport_rules = %{community.raw => %{"video.undo_trash" => true}}
       rule_conn = simu_conn(:user, cms: passport_rules)