address PR feedback

CHANGELOG.md

@@ -2,10 +2,11 @@
 
 ## Unreleased
 
-## 1.0.0
+## 0.6.0
 
-- Resolve all link fields found within "items" in an API response, and replace the links with the actual entities if available from the "includes" section of API response. Inspired by https://github.com/contentful/contentful.js/blob/master/ADVANCED.md#link-resolution
-- Breaking change: 'assets' field on Entry struct is no longer populated. All Assets for an Entry are embedded directly within 'fields' now instead
+- [#181](https://github.com/contentful-userland/contentful.ex/pull/181) Resolve all link fields found within "items" in an API response, and replace the links with actual entities, if available from the "includes" section of API response. Inspired by https://github.com/contentful/contentful.js/blob/master/ADVANCED.md#link-resolution
+
+Note: BREAKING CHANGE, 'assets' field on Entry struct is no longer populated. All Assets for an Entry are now embedded directly within 'fields' instead
 
 ## 0.5.0
 

README.md

@@ -66,11 +66,13 @@ then open [`http://localhost:8000`](http://localhost:8000).
 
 ## Contributing & Development
 
-1. Fork it
-2. Create your feature branch (`git checkout -b my-new-feature`)
-3. Commit your changes (`git commit -am 'Add some feature'`)
-4. Push to the branch (`git push origin my-new-feature`)
-5. Create a new Pull Request
+1. Fork the repository
+2. Create your feature branch on your fork (`git checkout -b my-new-feature`)
+3. Make your changes
+4. Run tests (`mix test`) and static analysis (`mix credo --strict`)
+5. Commit your changes (`git commit -am 'Add some feature'`)
+6. Push to your branch (`git push origin my-new-feature`)
+7. Create a Pull Request on this original repository
 
 ### Notes on setup
 

lib/contentful/entry/link_resolver.ex

@@ -4,8 +4,8 @@ defmodule Contentful.Entry.LinkResolver do
   """
 
   alias Contentful.Delivery.ContentTypes
+  alias Contentful.Delivery.{Assets, ContentTypes, Entries, Locales, Spaces}
   alias Contentful.Entry
-  alias Contentful.Delivery.{Assets, Entries, Spaces, ContentTypes, Locales}
 
   @doc """
   In Contentful, you can create content that references other content. These are called "links".
@@ -18,27 +18,30 @@ defmodule Contentful.Entry.LinkResolver do
 
   Inspired by https://github.com/contentful/contentful.js/blob/master/ADVANCED.md#link-resolution
   """
-  @spec replace_in_situ(Entry.t(), map()) :: Entry.t()
-  def replace_in_situ(%Entry{fields: fields} = entry, %{} = includes) do
+  @spec replace_links_with_entities(Entry.t(), map()) :: Entry.t()
+  def replace_links_with_entities(%Entry{fields: fields} = entry, %{} = includes) do
     updated_fields =
       fields
       |> Enum.reduce(%{}, fn {name, value}, fields_with_links_resolved ->
-        new_value =
-          case resolved_field = resolve_links_in_field(value, includes) do
-            %Entry{} ->
-              replace_in_situ(resolved_field, includes)
-
-            _ ->
-              resolved_field
-          end
+        new_value = resolve_links_in_field_with_nesting(value, includes)
 
         Map.put(fields_with_links_resolved, name, new_value)
       end)
 
     struct(entry, fields: updated_fields)
   end
 
-  def replace_in_situ(entity, _includes), do: entity
+  def replace_links_with_entities(entity, _includes), do: entity
+
+  defp resolve_links_in_field_with_nesting(field_value, includes) do
+    case resolved = resolve_links_in_field(field_value, includes) do
+      %Entry{} ->
+        replace_links_with_entities(resolved, includes)
+
+      _ ->
+        resolved
+    end
+  end
 
   defp resolve_links_in_field(
          %{"sys" => %{"id" => id, "linkType" => link_type, "type" => "Link"}} = field_value,
@@ -53,9 +56,11 @@ defmodule Contentful.Entry.LinkResolver do
   end
 
   # matches structs like %Asset{}, which can't be iterated through using the Enum module
+  # and mean links in this field have already been fully resolved anyway
   defp resolve_links_in_field(%_{} = field_value, _includes), do: field_value
 
   # matches any other map that isn't a struct, maps can be iterated through using Enum
+  # map fields may still have nested links
   defp resolve_links_in_field(
          %{} = field_value,
          %{} = includes
@@ -64,24 +69,7 @@ defmodule Contentful.Entry.LinkResolver do
     field_value
     |> Enum.reduce(%{}, fn {nested_field_name, nested_field_value},
                            field_with_nested_links_resolved ->
-      updated_nested_field_value =
-        case resolved = resolve_links_in_field(nested_field_value, includes) do
-          %Entry{} ->
-            replace_in_situ(resolved, includes)
-
-          %_{} ->
-            resolved
-
-          %{} ->
-            resolve_links_in_field(resolved, includes)
-
-          [] ->
-            resolve_links_in_field(resolved, includes)
-
-          _ ->
-            resolved
-        end
-
+      updated_nested_field_value = resolve_links_in_field_with_nesting(nested_field_value, includes)
       Map.put(field_with_nested_links_resolved, nested_field_name, updated_nested_field_value)
     end)
   end
@@ -90,13 +78,7 @@ defmodule Contentful.Entry.LinkResolver do
        when is_list(field_value) and length(field_value) > 0 do
     field_value
     |> Enum.map(fn field ->
-      case resolved = resolve_links_in_field(field, includes) do
-        %Entry{} ->
-          replace_in_situ(resolved, includes)
-
-        _ ->
-          resolved
-      end
+      resolve_links_in_field_with_nesting(field, includes)
     end)
   end
 

lib/contentful_delivery/entries.ex

@@ -82,7 +82,9 @@ defmodule Contentful.Delivery.Entries do
     {:ok, entries, total: total} =
       resolve_collection_response(%{"total" => total, "items" => items})
 
-    {:ok, entries |> Enum.map(fn entry -> entry |> LinkResolver.replace_in_situ(includes) end),
+    {:ok,
+     entries
+     |> Enum.map(fn entry -> entry |> LinkResolver.replace_links_with_entities(includes) end),
      total: total}
   end
 

mix.exs

@@ -15,7 +15,7 @@ defmodule Contentful.Mixfile do
 
   alias Contentful.Delivery.{Assets, ContentTypes, Entries, Locales, Spaces}
 
-  @version "1.0.0"
+  @version "0.6.0"
 
   def project do
     [

test/contentful/entry/link_resolver_test.exs

@@ -2,10 +2,10 @@ defmodule Contentful.Entry.LinkResolverTest do
   use ExUnit.Case
 
   alias Contentful.Asset
+  alias Contentful.{ContentType, Entry, SysData}
   alias Contentful.Entry.LinkResolver
-  alias Contentful.{Entry, SysData, ContentType}
 
-  describe "replace_in_situ/2" do
+  describe "replace_links_with_entities/2" do
     test "Entry with no links returns unchanged Entry" do
       includes = %{
         "Entry" => [
@@ -31,7 +31,7 @@ defmodule Contentful.Entry.LinkResolverTest do
         ]
       }
 
-      %Entry{} = %Entry{} |> LinkResolver.replace_in_situ(includes)
+      %Entry{} = %Entry{} |> LinkResolver.replace_links_with_entities(includes)
     end
 
     test "empty includes returns unchanged Entry" do
@@ -55,7 +55,7 @@ defmodule Contentful.Entry.LinkResolverTest do
         }
       }
 
-      ^entry = entry |> LinkResolver.replace_in_situ(%{})
+      ^entry = entry |> LinkResolver.replace_links_with_entities(%{})
     end
 
     test "links found in 'includes' are resolved in entry, others not found are left unchanged" do
@@ -146,7 +146,7 @@ defmodule Contentful.Entry.LinkResolverTest do
             }
           }
         }
-      } = entry |> LinkResolver.replace_in_situ(includes)
+      } = entry |> LinkResolver.replace_links_with_entities(includes)
     end
 
     test "resolves links nested in complex fields" do
@@ -399,7 +399,7 @@ defmodule Contentful.Entry.LinkResolverTest do
             "nodeType" => "document"
           }
         }
-      } = entry |> LinkResolver.replace_in_situ(includes)
+      } = entry |> LinkResolver.replace_links_with_entities(includes)
     end
 
     test "ignores unknown LinkTypes we don't know how to parse even if matching entity exists in includes" do
@@ -447,7 +447,7 @@ defmodule Contentful.Entry.LinkResolverTest do
         ]
       }
 
-      ^entry = entry |> LinkResolver.replace_in_situ(includes)
+      ^entry = entry |> LinkResolver.replace_links_with_entities(includes)
     end
 
     test "resolves nested links within lists of Entries" do
@@ -591,7 +591,7 @@ defmodule Contentful.Entry.LinkResolverTest do
           created_at: "2019-03-22T08:33:45.069Z",
           content_type: %ContentType{id: "page"}
         }
-      } = LinkResolver.replace_in_situ(entry, includes)
+      } = LinkResolver.replace_links_with_entities(entry, includes)
     end
   end
 end