backport(2.6): ensure embed fields are copied properly (#811)

Co-authored-by: shiftinv <8530778+shiftinv@users.noreply.github.com>

changelog/792.bugfix.rst

@@ -0,0 +1 @@
+Ensure that embed fields are copied properly by :func:`Embed.copy` and that the copied embed is completely separate from the original one.

disnake/embeds.py

@@ -279,10 +279,15 @@ def from_dict(cls, data: EmbedData) -> Self:
     def copy(self) -> Self:
         """Returns a shallow copy of the embed."""
         embed = type(self).from_dict(self.to_dict())
+
         # assign manually to keep behavior of default colors
         embed._colour = self._colour
-        # shallow copy of files
+
+        # copy files and fields collections
         embed._files = self._files.copy()
+        if self._fields is not None:
+            embed._fields = self._fields.copy()
+
         return embed
 
     def __len__(self) -> int:
@@ -305,8 +310,7 @@ def __bool__(self) -> bool:
                 self.title,
                 self.url,
                 self.description,
-                # not checking for falsy value as `0` is a valid color
-                self._colour not in (MISSING, None),
+                self._colour,
                 self._fields,
                 self._timestamp,
                 self._author,
@@ -717,13 +721,16 @@ def set_field_at(self, index: int, name: Any, value: Any, *, inline: bool = True
         if not self._fields:
             raise IndexError("field index out of range")
         try:
-            field = self._fields[index]
+            self._fields[index]
         except IndexError:
             raise IndexError("field index out of range")
 
-        field["name"] = str(name)
-        field["value"] = str(value)
-        field["inline"] = inline
+        field: EmbedFieldPayload = {
+            "inline": inline,
+            "name": str(name),
+            "value": str(value),
+        }
+        self._fields[index] = field
         return self
 
     def to_dict(self) -> EmbedData:

tests/test_embeds.py

@@ -83,31 +83,41 @@ def test_len(embed: Embed) -> None:
 
 
 def test_eq() -> None:
+    # basic test
     embed_1, embed_2 = Embed(), Embed()
     assert embed_1 == embed_2
 
-    # color=MISSING should not affect __eq__
+    embed_1.title, embed_2.title = None, ""
+    assert embed_1 == embed_2
+
+    # color tests
+    embed_1, embed_2 = Embed(), Embed()
     embed_1.color = Color(123456)
     assert not embed_1 == embed_2
 
     embed_1.color = MISSING
     assert embed_1 == embed_2
 
+    embed_1, embed_2 = Embed(color=None), Embed()
+    assert embed_1 == embed_2
+
+    try:
+        Embed.set_default_color(123456)
+        assert not embed_1 == embed_2
+    finally:
+        Embed.set_default_color(None)
+
+    # test fields
+    embed_1, embed_2 = Embed(), Embed()
     embed_1.add_field(name="This is a test field", value="69 test 69")
     embed_2.add_field(name="This is a test field", value="69 test 69", inline=False)
     assert not embed_1 == embed_2
 
     embed_1, embed_2 = Embed(), Embed()
-
-    embed_1.title, embed_2.title = None, ""
+    embed_1._fields = []
+    embed_2._fields = None
     assert embed_1 == embed_2
 
-    embed_1, embed_2 = Embed(color=None), Embed()
-    assert embed_1 == embed_2
-
-    embed_1.set_default_color(123456)
-    assert not embed_1 == embed_2
-
 
 def test_embed_proxy_eq() -> None:
     embed_1, embed_2 = Embed(), Embed()
@@ -124,10 +134,11 @@ def test_color_zero() -> None:
     e = Embed()
     assert not e
 
-    # color=0 should be applied to bool and dict
+    # color=0 should be applied to to_dict, __bool__, and __eq__
     e.color = 0
     assert e
     assert e.to_dict() == {"color": 0, **_BASE}
+    assert e != Embed(color=None)
 
 
 def test_default_color() -> None:
@@ -414,16 +425,48 @@ def test_copy(embed: Embed, file: File) -> None:
     copy = embed.copy()
     assert embed.to_dict() == copy.to_dict()
 
-    # shallow copy, but `_files` should be copied
+    # shallow copy, but `_files` and `_fields` should be copied
     assert embed._files == copy._files
     assert embed._files is not copy._files
+    assert embed._fields == copy._fields
+    assert embed._fields is not copy._fields
 
 
 def test_copy_empty() -> None:
     e = Embed.from_dict({})
     copy = e.copy()
     assert e.to_dict() == copy.to_dict() == {}
 
+    # shallow copy, but `_files` and `_fields` should be copied
+    assert e._files == copy._files
+    assert e._files is not copy._files
+    assert e._fields is None
+    assert copy._fields is None
+
+
+def test_copy_fields(embed: Embed) -> None:
+    embed.add_field("things", "stuff")
+    copy = embed.copy()
+    embed.clear_fields()
+    assert copy._fields
+
+    embed.insert_field_at(0, "w", "x")
+    copy = embed.copy()
+    embed.remove_field(0)
+    assert embed._fields == []
+    assert copy._fields == [{"name": "w", "value": "x", "inline": True}]
+
+    embed.insert_field_at(0, "w", "x")
+    copy = embed.copy()
+    embed.insert_field_at(1, "y", "z")
+    embed.set_field_at(0, "abc", "def", inline=False)
+
+    assert embed._fields == [
+        {"name": "abc", "value": "def", "inline": False},
+        {"name": "y", "value": "z", "inline": True},
+    ]
+    assert copy._fields == [{"name": "w", "value": "x", "inline": True}]
+
 
 # backwards compatibility
 def test_emptyembed() -> None: