test: add github sync importer/webhook/etc tests

* fixed issue with re-imported releases not updating the release package
  info (downloadurl, tag name, etc.)
* refactor metadata extractor for easier priority changes

Author: sgfost

django/library/github_integration.py

@@ -421,6 +421,14 @@ def reimport_release(self, release) -> bool:
             ):
                 return False
 
+        package = release.imported_release_package
+        package.name = self.github_release.get("tag_name")
+        package.display_name = self.github_release.get("name", "")
+        package.html_url = self.github_release.get("html_url", "")
+        package.download_url = self.github_release.get("zipball_url", "")
+        package.extra_data = self.github_release
+        package.save()
+
         return self._import_package_and_metadata(release)
 
     def _resolve_tags(self, tag_names: list[str]) -> list:

django/library/jinja2/library/review/email/model_revisions_requested_imported.jinja

@@ -16,6 +16,7 @@ Since this release is imported from GitHub, you will need to update it there and
 3. **Update the release tag:**
   - In the release edit form, you can choose which tag or branch the release should be associated with.
   - To include your latest changes, you should create a new tag. You can do this by typing a new version number in the "Tag version" box and selecting "Create new tag: [new_version] on publish". Make sure this new tag points to the branch with your recent commits.
+  - Keep in mind that the original version number will be kept, so its recommended to append a suffix to the tag instead of incrementing, e.g. "v1.0.0-rev1"
 4. **Confirm the release has been updated:** Follow [this link]({{ build_absolute_uri(release.get_edit_url()) }}) to confirm that the release has been updated, making sure files are categorized and metadata is complete.
 5. **Notify reviewer:** Once changes have been made, use the "Notify Reviewer of Changes" button to send an alert to the reviewer.
 

django/library/metadata.py

@@ -415,34 +415,36 @@ def _ensure_list(self, value):
             return []
         return [value]
 
-    def extract_license_spdx_id(self) -> str | None:
-        # priority: github_repository.license.spdx_id > cff.license
+    def _extract_license_from_github(self) -> str | None:
         gh_license = self._get_field(self.github_repository, "license")
         if isinstance(gh_license, dict):
             spdx_id = gh_license.get("spdx_id", None)
             if spdx_id:
                 return spdx_id
-        # cff.license should be an Enum with a valid spdx id (or a list of them)
+        return None
+
+    def _extract_license_from_cff(self) -> str | None:
         cff_licenses = self._ensure_list(self._get_field(self.cff, "license"))
         for cff_license in cff_licenses:
-            if cff_license and cff_license.value:
+            if cff_license and hasattr(cff_license, "value"):
                 return cff_license.value
+        return None
 
-    def extract_release_notes(self) -> str | None:
-        # priority: codemeta.releaseNotes > github_release.body
-        # codemeta.releaseNotes can be a list, make sure its a str, take the first one
+    def _extract_release_notes_from_codemeta(self) -> str | None:
         release_notes = self._get_field(self.codemeta, "releaseNotes")
         if release_notes:
             if isinstance(release_notes, list):
                 release_notes = release_notes[0]
                 return release_notes if isinstance(release_notes, str) else None
             elif isinstance(release_notes, str):
                 return release_notes
+        return None
+
+    def _extract_release_notes_from_github(self) -> str | None:
         body = self._get_field(self.github_release, "body")
         return body if isinstance(body, str) else None
 
-    def extract_os(self) -> str:
-        # priority: codemeta.operatingSystem
+    def _extract_os_from_codemeta(self) -> str:
         codemeta_os = self._get_field(self.codemeta, "operatingSystem")
         if not codemeta_os:
             return ""
@@ -467,8 +469,7 @@ def extract_os(self) -> str:
                 return "other"
         return ""
 
-    def extract_programming_languages(self) -> list[str] | None:
-        # priority: codemeta.programmingLanguage > [github_repository.language]
+    def _extract_programming_languages_from_codemeta(self) -> list[str] | None:
         codemeta_langs = self._ensure_list(
             self._get_field(self.codemeta, "programmingLanguage")
         )
@@ -480,15 +481,15 @@ def extract_programming_languages(self) -> list[str] | None:
                 lang_name = self._get_field(codemeta_lang, "name")
                 if lang_name:
                     langs.append(lang_name)
-        if langs:
-            return langs
-        else:
-            gh_lang = self._get_field(self.github_repository, "language")
-            if gh_lang and isinstance(gh_lang, str):
-                return [gh_lang]
+        return langs if langs else None
+
+    def _extract_programming_languages_from_github(self) -> list[str] | None:
+        gh_lang = self._get_field(self.github_repository, "language")
+        if gh_lang and isinstance(gh_lang, str):
+            return [gh_lang]
+        return None
 
-    def extract_platforms(self) -> list[str] | None:
-        # priority: codemeta.runtimePlatform
+    def _extract_platforms_from_codemeta(self) -> list[str] | None:
         runtime_platforms = self._ensure_list(
             self._get_field(self.codemeta, "runtimePlatform")
         )
@@ -501,12 +502,25 @@ def extract_platforms(self) -> list[str] | None:
     def convert(self) -> dict:
         """return a dictionary with the metadata fields for a codebase release from
         given sources"""
+        # set priority
+        license_spdx_id = (
+            self._extract_license_from_github() or self._extract_license_from_cff()
+        )
+        release_notes = (
+            self._extract_release_notes_from_codemeta()
+            or self._extract_release_notes_from_github()
+        )
+        programming_languages = (
+            self._extract_programming_languages_from_codemeta()
+            or self._extract_programming_languages_from_github()
+        )
+
         return {
-            "license_spdx_id": self.extract_license_spdx_id(),
-            "release_notes": self.extract_release_notes(),
-            "os": self.extract_os(),
-            "programming_languages": self.extract_programming_languages(),
-            "platforms": self.extract_platforms(),
+            "license_spdx_id": license_spdx_id,
+            "release_notes": release_notes,
+            "os": self._extract_os_from_codemeta(),
+            "programming_languages": programming_languages,
+            "platforms": self._extract_platforms_from_codemeta(),
         }
 
 

django/library/tests/test_github_integration.py

@@ -0,0 +1,233 @@
+from unittest.mock import patch, MagicMock
+from django.test import TestCase
+
+from core.tests.base import UserFactory
+from library.github_integration import (
+    GitHubRepoValidator,
+    GitHubReleaseImporter,
+)
+from library.models import (
+    CodebaseGitRemote,
+    GithubIntegrationAppInstallation,
+    CodebaseRelease,
+)
+from library.tests.base import CodebaseFactory
+
+SAMPLE_PAYLOAD = {
+    "action": "released",
+    "release": {
+        "id": 12345,
+        "tag_name": "v1.0.0",
+        "name": "Version 1.0.0",
+        "body": "Initial release",
+        "draft": False,
+        "prerelease": False,
+        "zipball_url": "https://api.github.com/repos/testuser/test-repo/zipball/v1.0.0",
+        "html_url": "https://github.com/testuser/test-repo/releases/tag/v1.0.0",
+    },
+    "repository": {
+        "name": "test-repo",
+        "owner": {"login": "testuser"},
+        "private": False,
+    },
+    "installation": {"id": 54321},
+}
+
+
+class GitHubRepoValidatorTests(TestCase):
+    def setUp(self):
+        # set up a user and installation for testing validator methods
+        self.user = UserFactory().create()
+        self.installation = GithubIntegrationAppInstallation.objects.create(
+            user=self.user,
+            github_login="testuser",
+            installation_id=1,
+            github_user_id=123,
+        )
+
+    def test_validate_format(self):
+        # test valid repo names
+        for name in ["repo", "repo-1", "repo.1", "my_repo"]:
+            with self.subTest(name=name):
+                validator = GitHubRepoValidator(name)
+                self.assertIsNone(validator.validate_format())
+
+        # test invalid repo names
+        for name in [
+            "repo with space",
+            "repo!",
+            "a" * 101,
+            "repo.git",
+            "contains-github",
+        ]:
+            with self.subTest(name=name):
+                validator = GitHubRepoValidator(name)
+                with self.assertRaises(ValueError):
+                    validator.validate_format()
+
+
+class GitHubReleaseImporterTests(TestCase):
+    def setUp(self):
+        # set up a user, codebase, and remote for importing releases
+        self.user = UserFactory().create()
+        self.installation = GithubIntegrationAppInstallation.objects.create(
+            user=self.user,
+            github_login="testuser",
+            installation_id=54321,
+            github_user_id=123,
+        )
+        self.codebase = CodebaseFactory(submitter=self.user).create()
+        mirror = self.codebase.create_git_mirror()
+        self.remote = CodebaseGitRemote.objects.create(
+            mirror=mirror,
+            owner="testuser",
+            repo_name="test-repo",
+            is_user_repo=True,
+            should_import=True,
+        )
+        self.payload = SAMPLE_PAYLOAD.copy()
+
+    def test_init_success(self):
+        # should initialize successfully with a valid payload
+        importer = GitHubReleaseImporter(self.payload)
+        self.assertEqual(importer.github_release_id, "12345")
+        self.assertEqual(importer.codebase, self.codebase)
+        self.assertTrue(importer.is_new_github_release)
+
+    def test_init_failures(self):
+        # test various invalid payloads that should raise ValueError
+        test_cases = {
+            "draft": ("release", "draft", True),
+            "prerelease": ("release", "prerelease", True),
+            "private": ("repository", "private", True),
+            "wrong_action": ("action", None, "created"),
+            "no_remote": ("repository", "name", "non-existent-repo"),
+        }
+        for name, (key1, key2, value) in test_cases.items():
+            with self.subTest(name=name):
+                bad_payload = self.payload.copy()
+                if key2:
+                    bad_payload[key1] = bad_payload[key1].copy()
+                    bad_payload[key1][key2] = value
+                else:
+                    bad_payload[key1] = value
+                with self.assertRaises(ValueError):
+                    GitHubReleaseImporter(bad_payload)
+
+    def test_extract_semver(self):
+        # test semantic version extraction
+        importer = GitHubReleaseImporter(self.payload)
+        self.assertEqual(importer.extract_semver("v1.2.3"), "1.2.3")
+        self.assertEqual(importer.extract_semver("1.2.3"), "1.2.3")
+        self.assertEqual(importer.extract_semver("version 1.2.3-beta"), "1.2.3")
+        self.assertIsNone(importer.extract_semver("1.2"))
+        self.assertIsNone(importer.extract_semver("invalid-version"))
+
+    @patch("library.models.CodebaseRelease.get_fs_api")
+    @patch("library.github_integration.GitHubApi.get_user_installation_access_token")
+    def test_import_new_release(self, mock_get_token, mock_get_fs_api):
+        # mock token and fs_api calls
+        mock_get_token.return_value = "fake-token"
+        mock_fs_api = MagicMock()
+        mock_fs_api.import_release_package.return_value = ({}, {})  # codemeta, cff
+        mock_get_fs_api.return_value = mock_fs_api
+
+        # import a new release
+        importer = GitHubReleaseImporter(self.payload)
+        success = importer.import_new_release()
+
+        # check that it was successful and objects were created
+        self.assertTrue(success)
+        self.assertTrue(
+            CodebaseRelease.objects.filter(
+                codebase=self.codebase, version_number="1.0.0"
+            ).exists()
+        )
+        release = CodebaseRelease.objects.get(version_number="1.0.0")
+        self.assertEqual(release.imported_release_package.uid, "12345")
+        self.assertEqual(release.submitter, self.codebase.submitter)
+        mock_fs_api.import_release_package.assert_called_once()
+
+    @patch("library.models.CodebaseRelease.get_fs_api")
+    @patch("library.github_integration.GitHubApi.get_user_installation_access_token")
+    def test_reimport_release(self, mock_get_token, mock_get_fs_api):
+        # mock token and fs_api calls
+        mock_get_token.return_value = "fake-token"
+        mock_fs_api = MagicMock()
+        mock_fs_api.import_release_package.return_value = ({}, {})  # codemeta, cff
+        mock_get_fs_api.return_value = mock_fs_api
+
+        # first, import a new release
+        importer = GitHubReleaseImporter(self.payload)
+        importer.import_or_reimport()
+
+        self.assertEqual(CodebaseRelease.objects.count(), 1)
+        release = CodebaseRelease.objects.first()
+        self.assertEqual(
+            release.imported_release_package.download_url,
+            "https://api.github.com/repos/testuser/test-repo/zipball/v1.0.0",
+        )
+
+        # now, create a new importer with an updated payload for an "edited" event
+        reimport_payload = self.payload.copy()
+        reimport_payload["action"] = "edited"
+        reimport_payload["release"] = reimport_payload["release"].copy()
+        new_url = (
+            "https://api.github.com/repos/testuser/test-repo/zipball/v1.0.0-updated"
+        )
+        reimport_payload["release"]["zipball_url"] = new_url
+
+        importer2 = GitHubReleaseImporter(reimport_payload)
+        success = importer2.import_or_reimport()
+
+        # assert that the re-import was successful and the release was updated
+        self.assertTrue(success)
+        self.assertEqual(mock_fs_api.import_release_package.call_count, 2)
+
+        release.refresh_from_db()
+        self.assertEqual(release.imported_release_package.download_url, new_url)
+
+        self.remote.refresh_from_db()
+        self.assertIn("Successfully re-imported", self.remote.last_import_log)
+
+        # release version number should NOT have changed
+        self.assertEqual(release.version_number, "1.0.0")
+
+        # re-importing with no change does nothing
+        importer3 = GitHubReleaseImporter(reimport_payload)
+        success_no_change = importer3.import_or_reimport()
+        self.assertFalse(success_no_change)
+        # should still be 2, not called again
+        self.assertEqual(mock_fs_api.import_release_package.call_count, 2)
+
+        # re-importing a published release does nothing
+        release.status = CodebaseRelease.Status.PUBLISHED
+        release.save()
+        published_reimport_payload = reimport_payload.copy()
+        published_reimport_payload["release"][
+            "zipball_url"
+        ] = "https://another.url/zipball.zip"
+        importer4 = GitHubReleaseImporter(published_reimport_payload)
+        success_published = importer4.import_or_reimport()
+        self.assertFalse(success_published)
+        # fs_api should not be called again
+        self.assertEqual(mock_fs_api.import_release_package.call_count, 2)
+        self.remote.refresh_from_db()
+        self.assertIn("Release already exists", self.remote.last_import_log)
+
+        # re-importing an under_review release should work
+        release.status = CodebaseRelease.Status.UNDER_REVIEW
+        release.save()
+        review_reimport_payload = self.payload.copy()
+        review_reimport_payload["action"] = "edited"
+        review_reimport_payload["release"] = review_reimport_payload["release"].copy()
+        review_url = (
+            "https://api.github.com/repos/testuser/test-repo/zipball/v1.0.0-review"
+        )
+        review_reimport_payload["release"]["zipball_url"] = review_url
+        importer5 = GitHubReleaseImporter(review_reimport_payload)
+        success_review = importer5.import_or_reimport()
+        self.assertTrue(success_review)
+        self.assertEqual(mock_fs_api.import_release_package.call_count, 3)
+        release.refresh_from_db()
+        self.assertEqual(release.imported_release_package.download_url, review_url)