Merge pull request #106 from ComplexData-MILA/94-bug-add-update-member

bug fix

Author: ekmpa

src/python/__init__.py

@@ -1,6 +1,7 @@
 import os
 from pathlib import Path
 import re
+from urllib.error import HTTPError, URLError
 from urllib.request import urlopen, urlretrieve
 from collections import OrderedDict
 
@@ -50,15 +51,24 @@ def parse_issue_body(body):
 
 def find_urls(text):
     """
-    1. Find all urls using regex: https://stackoverflow.com/a/840110/13837091
-    2. Remove the trailing ")"
-    3. Get unique URLs only (set comprehension -> list)
+    Find URLs in markdown links/images, HTML tags, and plain text.
+    Return unique URLs while preserving order.
     """
-    return list(
-        OrderedDict.fromkeys(
-            [s for s in re.findall("(?P<url>https?://[^\s)]+)", text)]
-        )
-    )
+    if not text:
+        return []
+
+    patterns = [
+        r"\((https?://[^\s)]+)\)",
+        r"(?:src|href)=[\"'](https?://[^\"']+)[\"']",
+        r"https?://[^\s<>)\"']+",
+    ]
+
+    urls = []
+    for pattern in patterns:
+        urls.extend(re.findall(pattern, text))
+
+    cleaned_urls = [url.rstrip('.,;:!?"\'') for url in urls]
+    return list(OrderedDict.fromkeys(cleaned_urls))
 
 def get_non_alpha(text):
     for ch in text:
@@ -125,7 +135,11 @@ def save_url_image(
             file_path = image_dir / f"{fname}"
 
         image_dir.mkdir(parents=True, exist_ok=True)
-        urlretrieve(url, str(file_path))
+        try:
+            urlretrieve(url, str(file_path))
+        except (HTTPError, URLError, ValueError) as e:
+            print(f"Could not download {url} due to error: {e}")
+            continue
 
         if ext in ["svg", "gif"]:
             return "/" + str(file_path)

tests/test_add_update_member.py

@@ -3,9 +3,12 @@
 import os
 import shutil
 from pathlib import Path
+from urllib.error import HTTPError
+from unittest.mock import patch
 from ruamel.yaml import YAML
 
 import src.python.add_update_member as mod
+import src.python as core
 
 
 class TestAddUpdateMember(unittest.TestCase):
@@ -73,5 +76,58 @@ def test_update_member(self):
 
         self.assertEqual(output, expected, error_message)
 
+    def test_find_urls_handles_markdown_html_and_plain_text(self):
+        text = (
+            '![avatar](https://example.com/image.png) '
+            '<img src="https://bucket.example.com/photos/profile.jpg?download=1"> '
+            'https://plain.example.org/pic.webp'
+        )
+
+        urls = core.find_urls(text)
+
+        self.assertEqual(
+            urls,
+            [
+                "https://example.com/image.png",
+                "https://bucket.example.com/photos/profile.jpg?download=1",
+                "https://plain.example.org/pic.webp",
+            ],
+        )
+
+    @patch("src.python.Image.open")
+    @patch("src.python.urlretrieve")
+    def test_save_url_image_skips_failed_downloads(self, mock_urlretrieve, mock_image_open):
+        class DummyImage:
+            def thumbnail(self, *_):
+                return None
+
+            def save(self, *_args, **_kwargs):
+                return None
+
+        mock_image_open.return_value = DummyImage()
+        mock_urlretrieve.side_effect = [
+            HTTPError("https://example.com/missing.jpg", 404, "Not Found", None, None),
+            None,
+        ]
+
+        profile = {
+            "avatar": (
+                "![bad](https://example.com/missing.jpg) "
+                "![ok](https://example.com/found.png)"
+            )
+        }
+
+        saved_path = core.save_url_image(
+            fname="john-doe",
+            profile=profile,
+            key="avatar",
+            image_dir="tests/scratch/assets/images/bio/",
+            crop_center=False,
+            size=(300, 300),
+        )
+
+        self.assertEqual(saved_path, "/tests/scratch/assets/images/bio/john-doe.png")
+        self.assertEqual(mock_urlretrieve.call_count, 2)
+