parse feedback better

facebook · Sep 30, 2024 · 6dee071 · 6dee071
1 parent a82dd7b
commit 6dee071
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 26 deletions.
diff --git a/python-threatexchange/threatexchange/exchanges/clients/ncmec/hash_api.py b/python-threatexchange/threatexchange/exchanges/clients/ncmec/hash_api.py
@@ -130,7 +130,7 @@ class NCMECFeedbackType(Enum):
     pdna = "PDNA"
     pdq = "PDQ"
     netclean = "NETCLEAN"
-    Videntifier = "VIDENTIFIER"
+    videntifier = "VIDENTIFIER"
     tmk_pdqf = "TMK_PDQF"
     ssvh_pdna = "SSVH_PDNA"
     ssvh_safer_hash = "SSVH_SAFER_HASH"
@@ -162,18 +162,36 @@ def from_xml(cls, xml: _XMLWrapper) -> "NCMECEntryUpdate":
             fingerprints={
                 x.tag: x.text for x in xml.maybe("fingerprints") if x.has_text
             },
-            feedback=[
-                {
-                    "type": x.tag,  # "affirmativeFeedback" or "negativeFeedback"
-                    "members": x.maybe(
-                        "members"  # "timestamp", "member.id", "member.name"
-                    ),
-                    "reasons": x.maybe(
-                        "reasons"
-                    ),  # "reason" with "guid", "name", "type" | "members"
-                }
-                for x in xml.maybe("feedback")
-            ],
+            feedback=(
+                [
+                    {
+                        "sentiment": x.tag,  # "affirmativeFeedback" or "negativeFeedback"
+                        "type": x.str("type"),
+                        "latest_feedback_time": x.str("lastUpdateTimestamp"),
+                        "members": [
+                            {"id": m.str("id"), "name": m.text}
+                            for m in x.maybe("members")
+                            if m.has_text
+                        ],
+                        "reasons": [
+                            {
+                                "guid": r.maybe("reason").str("guid"),
+                                "name": r.maybe("reason").str("name"),
+                                "type": r.maybe("reason").str("type"),
+                                "members": [
+                                    {"id": m.str("id"), "name": m.text}
+                                    for m in x.maybe("members")
+                                ],
+                            }
+                            for r in x.maybe("reasons")
+                            if r.maybe("reason")
+                        ],
+                    }
+                    for x in xml.maybe("feedback")
+                ]
+                if xml.maybe("feedback").has_text
+                else []
+            ),
         )
 
 
@@ -454,6 +472,7 @@ def members(self) -> t.List[StatusResult]:
         ]
 
     def feedback_reasons(self) -> GetFeedbackReasonsResponse:
+        """Get the possible negative feedback reasons for each feedback type"""
         for feedbackType in NCMECFeedbackType:
             resp = self._get(
                 NCMECEndpoint.feedback, path=f"{feedbackType.value}/reasons"
@@ -530,19 +549,19 @@ def submit_feedback(
         if not self.member_id:
             self.status()
 
-        # need valid reasons to submit feedback
-        if not self.reasons_map:
+        # need valid reasons to submit negative feedback
+        if not affirmative and not self.reasons_map:
             self.feedback_reasons()
 
         # Prepare the XML payload
         root = ET.Element("feedbackSubmission")
         root.set("xmlns", "https://hashsharing.ncmec.org/hashsharing/v2")
         vote = ET.SubElement(root, "affirmative" if affirmative else "negative")
 
-        valid_reason_ids = [
-            reason["guid"] for reason in self.reasons_map[feedback_type.value]
-        ]
         if not affirmative:
+            valid_reason_ids = [
+                reason["guid"] for reason in self.reasons_map[feedback_type.value]
+            ]
             if reason_id not in valid_reason_ids:
                 print(
                     "must choose from the following reasons: ",

diff --git a/python-threatexchange/threatexchange/exchanges/clients/ncmec/tests/data.py b/python-threatexchange/threatexchange/exchanges/clients/ncmec/tests/data.py
@@ -39,17 +39,17 @@
             </fingerprints>
             <feedback lastUpdateTimestamp="2023-06-01T12:48:14Z">
                 <affirmativeFeedback type="Md5" lastUpdateTimestamp="2023-06-01T12:48:14Z">
-                <members timestamp="2023-06-01T12:48:14Z">
-                    <member id="42">Example Member</member>
-                </members>
-                </affirmativeFeedback>
-                <negativeFeedback type="Sha1" lastUpdateTimestamp="2023-05-31T19:41:51Z">
-                <reasons>
-                    <reason guid="01234567-abcd-0123-4567-012345678900" name="Example Reason 1" type="Sha1"/>
                     <members timestamp="2023-06-01T12:48:14Z">
                         <member id="42">Example Member</member>
                     </members>
-                </reasons>
+                </affirmativeFeedback>
+                <negativeFeedback type="Sha1" lastUpdateTimestamp="2023-05-31T19:41:51Z">
+                    <reasons>
+                        <reason guid="01234567-abcd-0123-4567-012345678900" name="Example Reason 1" type="Sha1"/>
+                        <members timestamp="2023-06-01T12:48:14Z">
+                            <member id="42">Example Member</member>
+                        </members>
+                    </reasons>
                 </negativeFeedback>
             </feedback>
         </image>