From d51d6517be0e73a41365138716376eec866d5eea Mon Sep 17 00:00:00 2001
From: Wyatt Verchere <wverchere@gmail.com>
Date: Mon, 8 Jan 2024 07:28:33 -0800
Subject: [PATCH] fixes modpacks losing version data on modification (#840)

---
 src/routes/v2/versions.rs | 29 ++++++++++++++++++++++++++++-
 tests/search.rs           |  2 +-
 tests/v2/project.rs       | 35 +++++++++++++++++++++++++++++++++--
 3 files changed, 62 insertions(+), 4 deletions(-)

diff --git a/src/routes/v2/versions.rs b/src/routes/v2/versions.rs
index 4ec062c9f..f2ca356dd 100644
--- a/src/routes/v2/versions.rs
+++ b/src/routes/v2/versions.rs
@@ -244,13 +244,40 @@ pub async fn version_edit(
         );
     }
 
+    // Get the older version to get info from
+    let old_version = v3::versions::version_get_helper(
+        req.clone(),
+        info.clone().0,
+        pool.clone(),
+        redis.clone(),
+        session_queue.clone(),
+    )
+    .await
+    .or_else(v2_reroute::flatten_404_error)?;
+    let old_version = match v2_reroute::extract_ok_json::<Version>(old_version).await {
+        Ok(version) => version,
+        Err(response) => return Ok(response),
+    };
+
+    // If this has 'mrpack_loaders' as a loader field previously, this is a modpack.
+    // Therefore, if we are modifying the 'loader' field in this case,
+    // we are actually modifying the 'mrpack_loaders' loader field
+    let mut loaders = new_version.loaders.clone();
+    if old_version.fields.contains_key("mrpack_loaders") && new_version.loaders.is_some() {
+        fields.insert(
+            "mrpack_loaders".to_string(),
+            serde_json::json!(new_version.loaders),
+        );
+        loaders = None;
+    }
+
     let new_version = v3::versions::EditVersion {
         name: new_version.name,
         version_number: new_version.version_number,
         changelog: new_version.changelog,
         version_type: new_version.version_type,
         dependencies: new_version.dependencies,
-        loaders: new_version.loaders,
+        loaders,
         featured: new_version.featured,
         primary_file: new_version.primary_file,
         downloads: new_version.downloads,
diff --git a/tests/search.rs b/tests/search.rs
index d607404ec..59355ae8e 100644
--- a/tests/search.rs
+++ b/tests/search.rs
@@ -106,7 +106,7 @@ async fn search_projects() {
                     found_project_ids.sort();
                     println!("Facets: {:?}", facets);
                     assert_eq!(found_project_ids, expected_project_ids);
-                    assert_eq!(num_hits, expected_project_ids.len() as usize);
+                    assert_eq!(num_hits, { expected_project_ids.len() });
                 }
             })
             .await;
diff --git a/tests/v2/project.rs b/tests/v2/project.rs
index 20312aac3..57e8b7817 100644
--- a/tests/v2/project.rs
+++ b/tests/v2/project.rs
@@ -34,22 +34,25 @@ async fn test_project_type_sanity() {
             ("mod", "test-mod", TestFile::build_random_jar()),
             ("modpack", "test-modpack", TestFile::build_random_mrpack()),
         ] {
+            // Create a modpack or mod
+            // both are 'fabric' (but modpack is actually 'mrpack' behind the scenes, through v3,with fabric as a 'mrpack_loader')
             let (test_project, test_version) = api
                 .add_public_project(slug, Some(file), None, USER_USER_PAT)
                 .await;
             let test_project_slug = test_project.slug.as_ref().unwrap();
 
-            // TODO:
-            // assert_eq!(test_project.project_type, mod_or_modpack);
+            // Check that the loader displays correctly as fabric from the version creation
             assert_eq!(test_project.loaders, vec!["fabric"]);
             assert_eq!(test_version[0].loaders, vec!["fabric"]);
 
+            // Check that the project type is correct when getting the project
             let project = api
                 .get_project_deserialized(test_project_slug, USER_USER_PAT)
                 .await;
             assert_eq!(test_project.loaders, vec!["fabric"]);
             assert_eq!(project.project_type, mod_or_modpack);
 
+            // Check that the project type is correct when getting the version
             let version = api
                 .get_version_deserialized(&test_version[0].id.to_string(), USER_USER_PAT)
                 .await;
@@ -57,6 +60,34 @@ async fn test_project_type_sanity() {
                 version.loaders.iter().map(|x| &x.0).collect_vec(),
                 vec!["fabric"]
             );
+
+            // Edit the version loader to change it to 'forge'
+            let resp = api
+                .edit_version(
+                    &test_version[0].id.to_string(),
+                    json!({
+                        "loaders": ["forge"],
+                    }),
+                    USER_USER_PAT,
+                )
+                .await;
+            assert_status!(&resp, StatusCode::NO_CONTENT);
+
+            // Check that the project type is still correct when getting the project
+            let project = api
+                .get_project_deserialized(test_project_slug, USER_USER_PAT)
+                .await;
+            assert_eq!(project.project_type, mod_or_modpack);
+            assert_eq!(project.loaders, vec!["forge"]);
+
+            // Check that the project type is still correct when getting the version
+            let version = api
+                .get_version_deserialized(&test_version[0].id.to_string(), USER_USER_PAT)
+                .await;
+            assert_eq!(
+                version.loaders.iter().map(|x| &x.0).collect_vec(),
+                vec!["forge"]
+            );
         }
 
         // TODO: as we get more complicated strucures with v3 testing, and alpha/beta get more complicated, we should add more tests here,