Cleans up and removes TODOs, adds tests (#844)

* removes version ordering from v2; simplifies now-unecessary three-level faceting

* resolved some todos

* test for game version updating

* merge fixes; display_categories fix

src/database/models/legacy_loader_fields.rs

@@ -41,15 +41,16 @@ impl MinecraftGameVersion {
         redis: &RedisPool,
     ) -> Result<Vec<MinecraftGameVersion>, DatabaseError>
     where
-        E: sqlx::Executor<'a, Database = sqlx::Postgres> + Copy,
+        E: sqlx::Acquire<'a, Database = sqlx::Postgres>,
     {
-        let game_version_enum = LoaderFieldEnum::get(Self::FIELD_NAME, exec, redis)
+        let mut exec = exec.acquire().await?;
+        let game_version_enum = LoaderFieldEnum::get(Self::FIELD_NAME, &mut *exec, redis)
             .await?
             .ok_or_else(|| {
                 DatabaseError::SchemaError("Could not find game version enum.".to_string())
             })?;
         let game_version_enum_values =
-            LoaderFieldEnumValue::list(game_version_enum.id, exec, redis).await?;
+            LoaderFieldEnumValue::list(game_version_enum.id, &mut *exec, redis).await?;
 
         let game_versions = game_version_enum_values
             .into_iter()
@@ -71,24 +72,6 @@ impl MinecraftGameVersion {
         Ok(game_versions)
     }
 
-    // TODO: remove this
-    pub async fn list_transaction(
-        transaction: &mut sqlx::Transaction<'_, sqlx::Postgres>,
-        redis: &RedisPool,
-    ) -> Result<Vec<MinecraftGameVersion>, DatabaseError> {
-        let game_version_enum = LoaderFieldEnum::get(Self::FIELD_NAME, &mut **transaction, redis)
-            .await?
-            .ok_or_else(|| {
-                DatabaseError::SchemaError("Could not find game version enum.".to_string())
-            })?;
-        let game_version_enum_values =
-            LoaderFieldEnumValue::list(game_version_enum.id, &mut **transaction, redis).await?;
-        Ok(game_version_enum_values
-            .into_iter()
-            .map(MinecraftGameVersion::from_enum_value)
-            .collect())
-    }
-
     // Tries to create a MinecraftGameVersion from a VersionField
     // Clones on success
     pub fn try_from_version_field(

src/models/v2/projects.rs

@@ -101,7 +101,6 @@ impl LegacyProject {
     // Requires any queried versions to be passed in, to get access to certain version fields contained within.
     // - This can be any version, because the fields are ones that used to be on the project itself.
     // - Its conceivable that certain V3 projects that have many different ones may not have the same fields on all of them.
-    // TODO: Should this return an error instead for v2 users?
     // It's safe to use a db version_item for this as the only info is side types, game versions, and loader fields (for loaders), which used to be public on project anyway.
     pub fn from(data: Project, versions_item: Option<version_item::QueryVersion>) -> Self {
         let mut client_side = LegacySideType::Unknown;
@@ -289,10 +288,6 @@ pub struct LegacyVersion {
     /// A list of loaders this project supports (has a newtype struct)
     pub loaders: Vec<Loader>,
 
-    // TODO: should we remove this? as this is a v3 field and tests for it should be isolated to v3
-    // it allows us to keep tests that use this struct in common
-    pub ordering: Option<i32>,
-
     pub id: VersionId,
     pub project_id: ProjectId,
     pub author_id: UserId,
@@ -354,7 +349,6 @@ impl From<Version> for LegacyVersion {
             files: data.files,
             dependencies: data.dependencies,
             game_versions,
-            ordering: data.ordering,
             loaders,
         }
     }

src/models/v2/search.rs

@@ -42,7 +42,7 @@ pub struct LegacyResultSearchProject {
 impl LegacyResultSearchProject {
     pub fn from(result_search_project: ResultSearchProject) -> Self {
         let mut categories = result_search_project.categories;
-        categories.extend(result_search_project.loaders);
+        categories.extend(result_search_project.loaders.clone());
         if categories.contains(&"mrpack".to_string()) {
             if let Some(mrpack_loaders) = result_search_project
                 .project_loader_fields
@@ -58,6 +58,7 @@ impl LegacyResultSearchProject {
             }
         }
         let mut display_categories = result_search_project.display_categories;
+        display_categories.extend(result_search_project.loaders);
         if display_categories.contains(&"mrpack".to_string()) {
             if let Some(mrpack_loaders) = result_search_project
                 .project_loader_fields

src/routes/v2/projects.rs

@@ -12,7 +12,6 @@ use crate::routes::v3::projects::ProjectIds;
 use crate::routes::{v2_reroute, v3, ApiError};
 use crate::search::{search_for_project, SearchConfig, SearchError};
 use actix_web::{delete, get, patch, post, web, HttpRequest, HttpResponse};
-use itertools::Itertools;
 use serde::{Deserialize, Serialize};
 use sqlx::PgPool;
 use std::collections::HashMap;
@@ -53,44 +52,22 @@ pub async fn project_search(
     web::Query(info): web::Query<SearchRequest>,
     config: web::Data<SearchConfig>,
 ) -> Result<HttpResponse, SearchError> {
-    // TODO: make this nicer
     // Search now uses loader_fields instead of explicit 'client_side' and 'server_side' fields
     // While the backend for this has changed, it doesnt affect much
     // in the API calls except that 'versions:x' is now 'game_versions:x'
-    let facets: Option<Vec<Vec<Vec<String>>>> = if let Some(facets) = info.facets {
+    let facets: Option<Vec<Vec<String>>> = if let Some(facets) = info.facets {
-        let facets = serde_json::from_str::<Vec<Vec<serde_json::Value>>>(&facets)?;
+        let facets = serde_json::from_str::<Vec<Vec<String>>>(&facets)?;
-        // Search can now *optionally* have a third inner array: So Vec(AND)<Vec(OR)<Vec(AND)< _ >>>
-        // For every inner facet, we will check if it can be deserialized into a Vec<&str>, and do so.
-        // If not, we will assume it is a single facet and wrap it in a Vec.
-        let facets: Vec<Vec<Vec<String>>> = facets
-            .into_iter()
-            .map(|facets| {
-                facets
-                    .into_iter()
-                    .map(|facet| {
-                        if facet.is_array() {
-                            serde_json::from_value::<Vec<String>>(facet).unwrap_or_default()
-                        } else {
-                            vec![serde_json::from_value::<String>(facet).unwrap_or_default()]
-                        }
-                    })
-                    .collect_vec()
-            })
-            .collect_vec();
 
         // These loaders specifically used to be combined with 'mod' to be a plugin, but now
         // they are their own loader type. We will convert 'mod' to 'mod' OR 'plugin'
         // as it essentially was before.
-        let facets = v2_reroute::convert_plugin_loaders_v3(facets);
+        let facets = v2_reroute::convert_plugin_loader_facets_v3(facets);
 
         Some(
             facets
                 .into_iter()
                 .map(|facet| {
                     facet
-                        .into_iter()
-                        .map(|facets| {
-                            facets
                         .into_iter()
                         .map(|facet| {
                             if let Some((key, operator, val)) = parse_facet(&facet) {
@@ -111,8 +88,6 @@ pub async fn project_search(
                         })
                         .collect::<Vec<_>>()
                 })
-                        .collect::<Vec<_>>()
-                })
                 .collect(),
         )
     } else {

src/routes/v2/versions.rs

@@ -215,7 +215,6 @@ pub struct EditVersion {
     pub downloads: Option<u32>,
     pub status: Option<VersionStatus>,
     pub file_types: Option<Vec<EditVersionFileType>>,
-    pub ordering: Option<Option<i32>>, //TODO: How do you actually pass this in json?
 }
 
 #[derive(Serialize, Deserialize)]
@@ -291,7 +290,7 @@ pub async fn version_edit(
                 })
                 .collect::<Vec<_>>()
         }),
-        ordering: new_version.ordering,
+        ordering: None,
         fields,
     };
 

src/routes/v2_reroute.rs

@@ -178,18 +178,18 @@ pub fn convert_side_types_v3(
     fields
 }
 
-// Converts plugin loaders from v2 to v3
+// Converts plugin loaders from v2 to v3, for search facets
 // Within every 1st and 2nd level (the ones allowed in v2), we convert every instance of:
 // "project_type:mod" to "project_type:plugin" OR "project_type:mod"
-pub fn convert_plugin_loaders_v3(facets: Vec<Vec<Vec<String>>>) -> Vec<Vec<Vec<String>>> {
+pub fn convert_plugin_loader_facets_v3(facets: Vec<Vec<String>>) -> Vec<Vec<String>> {
     facets
         .into_iter()
         .map(|inner_facets| {
-            if inner_facets == [["project_type:mod"]] {
+            if inner_facets == ["project_type:mod"] {
                 vec![
-                    vec!["project_type:plugin".to_string()],
+                    "project_type:plugin".to_string(),
-                    vec!["project_type:datapack".to_string()],
+                    "project_type:datapack".to_string(),
-                    vec!["project_type:mod".to_string()],
+                    "project_type:mod".to_string(),
                 ]
             } else {
                 inner_facets

src/routes/v3/version_creation.rs

@@ -583,8 +583,6 @@ async fn upload_file_to_version_inner(
     };
 
     let all_loaders = models::loader_fields::Loader::list(&mut **transaction, &redis).await?;
-
-    // TODO: this coded is reused a lot, it should be refactored into a function
     let selected_loaders = version
         .loaders
         .iter()

src/routes/v3/version_file.rs

@@ -132,7 +132,6 @@ pub async fn get_update_from_hash(
     .map(|x| x.1)
     .ok();
     let hash = info.into_inner().0.to_lowercase();
-
     if let Some(file) = database::models::Version::get_file_from_hash(
         hash_query
             .algorithm
@@ -185,7 +184,6 @@ pub async fn get_update_from_hash(
             }
         }
     }
-
     Err(ApiError::NotFound)
 }
 

src/routes/v3/versions.rs

@@ -202,8 +202,12 @@ pub struct EditVersion {
     pub downloads: Option<u32>,
     pub status: Option<VersionStatus>,
     pub file_types: Option<Vec<EditVersionFileType>>,
-
+    #[serde(
-    pub ordering: Option<Option<i32>>, //TODO: How do you actually pass this in json?
+        default,
+        skip_serializing_if = "Option::is_none",
+        with = "::serde_with::rust::double_option"
+    )]
+    pub ordering: Option<Option<i32>>,
 
     // Flattened loader fields
     // All other fields are loader-specific VersionFields
@@ -220,8 +224,6 @@ pub struct EditVersionFileType {
     pub file_type: Option<FileType>,
 }
 
-// TODO: Avoid this 'helper' pattern here and similar fnunctoins- a macro might be the best bet here to ensure it's callable from both v2 and v3
-// (web::Path can't be recreated naturally)
 pub async fn version_edit(
     req: HttpRequest,
     info: web::Path<(VersionId,)>,

src/search/mod.rs

@@ -131,8 +131,8 @@ pub struct UploadSearchProject {
     pub loaders: Vec<String>, // Search uses loaders as categories- this is purely for the Project model.
     pub links: Vec<LinkUrl>,
     pub gallery_items: Vec<GalleryItem>, // Gallery *only* urls are stored in gallery, but the gallery items are stored here- required for the Project model.
-    pub games: Vec<String>,              // Todo: in future, could be a searchable field.
+    pub games: Vec<String>,
-    pub organization_id: Option<String>, // Todo: in future, could be a searchable field.
+    pub organization_id: Option<String>,
     pub project_loader_fields: HashMap<String, Vec<serde_json::Value>>, // Aggregation of loader_fields from all versions of the project, allowing for reconstruction of the Project model.
 
     #[serde(flatten)]
@@ -183,8 +183,8 @@ pub struct ResultSearchProject {
     pub loaders: Vec<String>, // Search uses loaders as categories- this is purely for the Project model.
     pub links: Vec<LinkUrl>,
     pub gallery_items: Vec<GalleryItem>, // Gallery *only* urls are stored in gallery, but the gallery items are stored here- required for the Project model.
-    pub games: Vec<String>,              // Todo: in future, could be a searchable field.
+    pub games: Vec<String>,
-    pub organization_id: Option<String>, // Todo: in future, could be a searchable field.
+    pub organization_id: Option<String>,
     pub project_loader_fields: HashMap<String, Vec<serde_json::Value>>, // Aggregation of loader_fields from all versions of the project, allowing for reconstruction of the Project model.
 
     #[serde(flatten)]

src/validate/mod.rs

@@ -132,7 +132,7 @@ pub async fn validate_file(
                 .find_map(|v| MinecraftGameVersion::try_from_version_field(&v).ok())
                 .unwrap_or_default();
             let all_game_versions =
-                MinecraftGameVersion::list_transaction(&mut *transaction, redis).await?;
+                MinecraftGameVersion::list(None, None, &mut *transaction, redis).await?;
             validate_minecraft_file(
                 data,
                 file_extension,

tests/common/api_common/models.rs

@@ -77,9 +77,6 @@ pub struct CommonVersion {
     pub requested_status: Option<VersionStatus>,
     pub files: Vec<VersionFile>,
     pub dependencies: Vec<Dependency>,
-
-    // TODO: should ordering be in v2?
-    pub ordering: Option<i32>,
 }
 
 #[derive(Deserialize)]

tests/common/api_v2/tags.rs

@@ -21,8 +21,6 @@ use crate::{
 
 use super::ApiV2;
 
-// TODO: Tag gets do not include PAT, as they are public.
-
 impl ApiV2 {
     async fn get_side_types(&self) -> ServiceResponse {
         let req = TestRequest::get()

tests/common/api_v2/team.rs

@@ -66,10 +66,11 @@ impl ApiTeams for ApiV2 {
     ) -> Vec<CommonTeamMember> {
         let resp = self.get_team_members(id_or_title, pat).await;
         assert_status!(&resp, StatusCode::OK);
-        // TODO: Note, this does NOT deserialize to any other struct first, as currently TeamMember is the same in v2 and v3.
+        // First, deserialize to the non-common format (to test the response is valid for this api version)
-        // CommonTeamMember = TeamMember (v3)
+        let v: Vec<LegacyTeamMember> = test::read_body_json(resp).await;
-        // This may yet change, so we should keep common struct.
+        // Then, deserialize to the common format
-        test::read_body_json(resp).await
+        let value = serde_json::to_value(v).unwrap();
+        serde_json::from_value(value).unwrap()
     }
 
     async fn get_teams_members(
@@ -103,10 +104,11 @@ impl ApiTeams for ApiV2 {
     ) -> Vec<CommonTeamMember> {
         let resp = self.get_project_members(id_or_title, pat).await;
         assert_status!(&resp, StatusCode::OK);
-        // TODO: Note, this does NOT deserialize to any other struct first, as currently TeamMember is the same in v2 and v3.
+        // First, deserialize to the non-common format (to test the response is valid for this api version)
-        // CommonTeamMember = TeamMember (v3)
+        let v: Vec<LegacyTeamMember> = test::read_body_json(resp).await;
-        // This may yet change, so we should keep common struct.
+        // Then, deserialize to the common format
-        test::read_body_json(resp).await
+        let value = serde_json::to_value(v).unwrap();
+        serde_json::from_value(value).unwrap()
     }
 
     async fn get_organization_members(
@@ -128,10 +130,11 @@ impl ApiTeams for ApiV2 {
     ) -> Vec<CommonTeamMember> {
         let resp = self.get_organization_members(id_or_title, pat).await;
         assert_status!(&resp, StatusCode::OK);
-        // TODO: Note, this does NOT deserialize to any other struct first, as currently TeamMember is the same in v2 and v3.
+        // First, deserialize to the non-common format (to test the response is valid for this api version)
-        // CommonTeamMember = TeamMember (v3)
+        let v: Vec<LegacyTeamMember> = test::read_body_json(resp).await;
-        // This may yet change, so we should keep common struct.
+        // Then, deserialize to the common format
-        test::read_body_json(resp).await
+        let value = serde_json::to_value(v).unwrap();
+        serde_json::from_value(value).unwrap()
     }
 
     async fn join_team(&self, team_id: &str, pat: Option<&str>) -> ServiceResponse {

tests/common/api_v3/tags.rs

@@ -85,7 +85,6 @@ impl ApiV3 {
         test::read_body_json(resp).await
     }
 
-    // TODO: fold this into v3 API of other v3 testing PR
     async fn get_games(&self) -> ServiceResponse {
         let req = TestRequest::get()
             .uri("/v3/games")

tests/common/api_v3/version.rs

@@ -66,6 +66,16 @@ impl ApiV3 {
         test::read_body_json(resp).await
     }
 
+    pub async fn get_versions_deserialized(
+        &self,
+        version_ids: Vec<String>,
+        pat: Option<&str>,
+    ) -> Vec<Version> {
+        let resp = self.get_versions(version_ids, pat).await;
+        assert_status!(&resp, StatusCode::OK);
+        test::read_body_json(resp).await
+    }
+
     pub async fn update_individual_files(
         &self,
         algorithm: &str,
@@ -454,7 +464,6 @@ impl ApiVersion for ApiV3 {
         serde_json::from_value(value).unwrap()
     }
 
-    // TODO: remove redundancy in these functions
     async fn edit_version_ordering(
         &self,
         version_id: &str,

tests/common/dummy_data.rs

@@ -15,9 +15,8 @@ use zip::{write::FileOptions, CompressionMethod, ZipWriter};
 
 use crate::{
     assert_status,
-    common::{api_common::Api, database::USER_USER_PAT},
+    common::{api_common::Api, api_v3, database::USER_USER_PAT},
 };
-use labrinth::util::actix::{AppendsMultipart, MultipartSegment, MultipartSegmentData};
 
 use super::{
     api_common::{request_data::ImageData, ApiProject, AppendsOptionalPat},
@@ -342,58 +341,22 @@ pub async fn add_project_beta(api: &ApiV3) -> (Project, Version) {
     // Generate test project data.
     let jar = TestFile::DummyProjectBeta;
     // TODO: this shouldnt be hardcoded (nor should other similar ones be)
-    let json_data = json!(
+
-        {
+    let modify_json = serde_json::from_value(json!([
-            "name": "Test Project Beta",
+        { "op": "add", "path": "/summary", "value": "A dummy project for testing with." },
-            "slug": "beta",
+        { "op": "add", "path": "/description", "value": "This project is not-yet-approved, and versions are draft." },
-            "summary": "A dummy project for testing with.",
+        { "op": "add", "path": "/initial_versions/0/status", "value": "unlisted" },
-            "description": "This project is not-yet-approved, and versions are draft.",
+        { "op": "add", "path": "/status", "value": "private" },
-            "initial_versions": [{
+        { "op": "add", "path": "/requested_status", "value": "private" },
-                "file_parts": [jar.filename()],
+    ]))
-                "version_number": "1.2.3",
+    .unwrap();
-                "version_title": "start",
+
-                "status": "unlisted",
+    let creation_data = api_v3::request_data::get_public_project_creation_data(
-                "dependencies": [],
+        "beta",
-                "singleplayer": true,
+        Some(jar),
-                "client_and_server": true,
+        Some(modify_json),
-                "client_only": true,
-                "server_only": false,
-                "game_versions": ["1.20.1"] ,
-                "release_channel": "release",
-                "loaders": ["fabric"],
-                "featured": true
-            }],
-            "status": "private",
-            "requested_status": "private",
-            "categories": [],
-            "license_id": "MIT"
-        }
     );
-
+    api.create_project(creation_data, USER_USER_PAT).await;
-    // Basic json
-    let json_segment = MultipartSegment {
-        name: "data".to_string(),
-        filename: None,
-        content_type: Some("application/json".to_string()),
-        data: MultipartSegmentData::Text(serde_json::to_string(&json_data).unwrap()),
-    };
-
-    // Basic file
-    let file_segment = MultipartSegment {
-        name: jar.filename(),
-        filename: Some(jar.filename()),
-        content_type: Some("application/java-archive".to_string()),
-        data: MultipartSegmentData::Binary(jar.bytes()),
-    };
-
-    // Add a project.
-    let req = TestRequest::post()
-        .uri("/v3/project")
-        .append_pat(USER_USER_PAT)
-        .set_multipart(vec![json_segment.clone(), file_segment.clone()])
-        .to_request();
-    let resp = api.call(req).await;
-    assert_status!(&resp, StatusCode::OK);
 
     get_project_beta(api).await
 }

tests/common/environment.rs

@@ -25,9 +25,6 @@ pub async fn with_test_environment<Fut, A>(
     db.cleanup().await;
 }
 
-// TODO: This needs to be slightly redesigned in order to do both V2 and v3 tests.
-// TODO: Most tests, since they use API functions, can be applied to both. The ones that weren't are in v2/, but
-// all tests that can be applied to both should use both v2 and v3 (extract api to a trait  with all the API functions and call both).
 pub async fn with_test_environment_all<Fut, F>(max_connections: Option<u32>, f: F)
 where
     Fut: Future<Output = ()>,

tests/loader_fields.rs

@@ -5,6 +5,7 @@ use actix_web::test;
 use common::api_v3::ApiV3;
 use common::environment::{with_test_environment, TestEnvironment};
 use itertools::Itertools;
+use labrinth::database::models::legacy_loader_fields::MinecraftGameVersion;
 use labrinth::models::v3;
 use serde_json::json;
 
@@ -556,3 +557,79 @@ async fn test_multi_get_redis_cache() {
     })
     .await;
 }
+
+#[actix_rt::test]
+async fn minecraft_game_version_update() {
+    // We simulate adding a Minecraft game version, to ensure other data doesn't get overwritten
+    // This is basically a test for the insertion/concatenation query
+    // This doesn't use a route (as this behaviour isn't exposed via a route, but a scheduled URL call)
+    // We just interact with the labrinth functions directly
+    with_test_environment(None, |test_env: TestEnvironment<ApiV3>| async move {
+        let api = &test_env.api;
+
+        // First, get a list of all gameversions
+        let game_versions = api
+            .get_loader_field_variants_deserialized("game_versions")
+            .await;
+
+        // A couple specific checks- in the dummy data, all game versions are marked as major=false except 1.20.5
+        let name_to_major = game_versions
+            .iter()
+            .map(|x| {
+                (
+                    x.value.clone(),
+                    x.metadata.get("major").unwrap().as_bool().unwrap(),
+                )
+            })
+            .collect::<std::collections::HashMap<_, _>>();
+        for (name, major) in name_to_major {
+            if name == "1.20.5" {
+                assert!(major);
+            } else {
+                assert!(!major);
+            }
+        }
+
+        // Now, we add a new game version, directly to the db
+        let pool = test_env.db.pool.clone();
+        let redis = test_env.db.redis_pool.clone();
+        MinecraftGameVersion::builder()
+            .version("1.20.6")
+            .unwrap()
+            .version_type("release")
+            .unwrap()
+            .created(
+                // now
+                &chrono::Utc::now(),
+            )
+            .insert(&pool, &redis)
+            .await
+            .unwrap();
+
+        // Check again
+        let game_versions = api
+            .get_loader_field_variants_deserialized("game_versions")
+            .await;
+
+        let name_to_major = game_versions
+            .iter()
+            .map(|x| {
+                (
+                    x.value.clone(),
+                    x.metadata.get("major").unwrap().as_bool().unwrap(),
+                )
+            })
+            .collect::<std::collections::HashMap<_, _>>();
+        // Confirm that the new version is there
+        assert!(name_to_major.contains_key("1.20.6"));
+        // Confirm metadata is unaltered
+        for (name, major) in name_to_major {
+            if name == "1.20.5" {
+                assert!(major);
+            } else {
+                assert!(!major);
+            }
+        }
+    })
+    .await
+}

tests/scopes.rs

@@ -218,7 +218,6 @@ pub async fn notifications_scopes() {
 #[actix_rt::test]
 pub async fn project_version_create_scopes_v3() {
     with_test_environment(None, |test_env: TestEnvironment<ApiV3>| async move {
-        // TODO: If possible, find a way to use generic api functions with the Permissions/Scopes test, then this can be recombined with the V2 version of this test
         let api = &test_env.api;
 
         // Create project
@@ -409,16 +408,14 @@ pub async fn project_version_reads_scopes() {
             .await
             .unwrap();
 
-        // TODO: Should this be /POST? Looks like /GET
+        // TODO: This scope currently fails still as the 'version' field of QueryProject only allows public versions.
-        // TODO: this scope doesn't actually affect anything, because the Project::get_id contained within disallows hidden versions, which is the point of this scope
+        // TODO: This will be fixed when the 'extracts_versions' PR is merged.
-        // let req_gen = || {
+        // let req_gen = |pat: Option<String>| async move {
-        //     test::TestRequest::post()
+        //     api.get_update_from_hash(beta_file_hash, "sha1", None, None, None, pat.as_deref())
-        //     .uri(&format!("/v3/version_file/{beta_file_hash}/update"))
+        //         .await
-        //     .set_json(json!({}))
         // };
         // ScopeTest::new(&test_env).with_failure_code(404).test(req_gen, read_version).await.unwrap();
 
-        // TODO: Should this be /POST? Looks like /GET
         let req_gen = |pat: Option<String>| async move {
             api.get_versions_from_hashes(&[beta_file_hash], "sha1", pat.as_deref())
                 .await
@@ -432,30 +429,10 @@ pub async fn project_version_reads_scopes() {
         assert!(success.as_object().unwrap().contains_key(beta_file_hash));
 
         // Update version file
-        // TODO: Should this be /POST? Looks like /GET
+        // TODO: This scope currently fails still as the 'version' field of QueryProject only allows public versions.
-        // TODO: this scope doesn't actually affect anything, because the Project::get_id contained within disallows hidden versions, which is the point of this scope
+        // TODO: This will be fixed when the 'extracts_versions' PR is merged.
-
+        // let req_gen = |pat : Option<String>| async move {
-        // let req_gen = || {
+        //     api.update_files("sha1", vec![beta_file_hash.clone()], None, None, None, pat.as_deref()).await
-        //     test::TestRequest::post()
-        //     .uri(&format!("/v3/version_files/update_individual"))
-        //     .set_json(json!({
-        //         "hashes": [{
-        //             "hash": beta_file_hash,
-        //         }]
-        //     }))
-        // };
-        // let (failure, success) = ScopeTest::new(&test_env).with_failure_code(200).test(req_gen, read_version).await.unwrap();
-        // assert!(!failure.as_object().unwrap().contains_key(beta_file_hash));
-        // assert!(success.as_object().unwrap().contains_key(beta_file_hash));
-
-        // Update version file
-        // TODO: this scope doesn't actually affect anything, because the Project::get_id contained within disallows hidden versions, which is the point of this scope
-        // let req_gen = || {
-        //     test::TestRequest::post()
-        //     .uri(&format!("/v3/version_files/update"))
-        //     .set_json(json!({
-        //         "hashes": [beta_file_hash]
-        //     }))
         // };
         // let (failure, success) = ScopeTest::new(&test_env).with_failure_code(200).test(req_gen, read_version).await.unwrap();
         // assert!(!failure.as_object().unwrap().contains_key(beta_file_hash));
@@ -712,8 +689,7 @@ pub async fn version_write_scopes() {
             .await
             .unwrap();
 
-        //  Delete version file
+        //  Delete version file. Notably, this uses 'VERSION_WRITE' instead of 'VERSION_DELETE' as it is writing to the version
-        // TODO: Should this scope be VERSION_DELETE?
         let req_gen = |pat: Option<String>| async move {
             api.remove_version_file(alpha_file_hash, pat.as_deref())
                 .await

tests/v2/project.rs

@@ -90,7 +90,7 @@ async fn test_project_type_sanity() {
             );
         }
 
-        // TODO: as we get more complicated strucures with v3 testing, and alpha/beta get more complicated, we should add more tests here,
+        // As we get more complicated strucures with as v3 continues to expand, and alpha/beta get more complicated, we should add more tests here,
         // to ensure that projects created with v3 routes are still valid and work with v3 routes.
     })
     .await;
@@ -397,7 +397,6 @@ async fn permissions_patch_project_v2() {
     with_test_environment(Some(8), |test_env: TestEnvironment<ApiV2>| async move {
         let api = &test_env.api;
 
-        // TODO: This only includes v2 ones (as it should. See v3)
         // For each permission covered by EDIT_DETAILS, ensure the permission is required
         let edit_details = ProjectPermissions::EDIT_DETAILS;
         let test_pairs = [

tests/v2/scopes.rs

@@ -50,3 +50,32 @@ pub async fn project_version_create_scopes() {
     })
     .await;
 }
+
+#[actix_rt::test]
+pub async fn project_version_reads_scopes() {
+    with_test_environment(None, |_test_env: TestEnvironment<ApiV2>| async move {
+        // let api = &test_env.api;
+        // let beta_file_hash = &test_env.dummy.project_beta.file_hash;
+
+        // let read_version = Scopes::VERSION_READ;
+
+        // Update individual version file
+        // TODO: This scope currently fails still as the 'version' field of QueryProject only allows public versions.
+        // TODO: This will be fixed when the 'extracts_versions' PR is merged.
+        // let req_gen = |pat : Option<String>| async move {
+        //     api.update_individual_files("sha1", vec![
+        //         FileUpdateData {
+        //             hash: beta_file_hash.clone(),
+        //             loaders: None,
+        //             game_versions: None,
+        //             version_types: None
+        //         }
+        //     ], pat.as_deref())
+        //         .await
+        // };
+        // let (failure, success) = ScopeTest::new(&test_env).with_failure_code(200).test(req_gen, read_version).await.unwrap();
+        // assert!(!failure.as_object().unwrap().contains_key(beta_file_hash));
+        // assert!(success.as_object().unwrap().contains_key(beta_file_hash));
+    })
+    .await;
+}

tests/v2/search.rs

@@ -18,9 +18,6 @@ use std::sync::Arc;
 
 #[actix_rt::test]
 async fn search_projects() {
-    // TODO: ("Match changes in the 2 version of thee add_public_version_creation_data to those made in v3
-    // It should drastically simplify this function
-
     // Test setup and dummy data
     with_test_environment(Some(10), |test_env: TestEnvironment<ApiV2>| async move {
         let api = &test_env.api;
@@ -399,6 +396,8 @@ async fn search_projects() {
             );
             assert!(hit.categories.contains(&"forge".to_string()));
             assert!(hit.categories.contains(&"fabric".to_string()));
+            assert!(hit.display_categories.contains(&"forge".to_string()));
+            assert!(hit.display_categories.contains(&"fabric".to_string()));
 
             // Also, ensure author is correctly capitalized
             assert_eq!(hit.author, "User".to_string());

tests/version.rs

@@ -536,7 +536,9 @@ pub async fn test_project_versions() {
 
 #[actix_rt::test]
 async fn can_create_version_with_ordering() {
-    with_test_environment_all(None, |env| async move {
+    with_test_environment(
+        None,
+        |env: common::environment::TestEnvironment<ApiV3>| async move {
             let alpha_project_id_parsed = env.dummy.project_alpha.project_id_parsed;
 
             let new_version_id = get_json_val_str(
@@ -555,16 +557,19 @@ async fn can_create_version_with_ordering() {
 
             let versions = env
                 .api
-            .get_versions_deserialized_common(vec![new_version_id.clone()], USER_USER_PAT)
+                .get_versions_deserialized(vec![new_version_id.clone()], USER_USER_PAT)
                 .await;
             assert_eq!(versions[0].ordering, Some(1));
-    })
+        },
+    )
     .await;
 }
 
 #[actix_rt::test]
 async fn edit_version_ordering_works() {
-    with_test_environment_all(None, |env| async move {
+    with_test_environment(
+        None,
+        |env: common::environment::TestEnvironment<ApiV3>| async move {
             let alpha_version_id = env.dummy.project_alpha.version_id.clone();
 
             let resp = env
@@ -575,10 +580,11 @@ async fn edit_version_ordering_works() {
 
             let versions = env
                 .api
-            .get_versions_deserialized_common(vec![alpha_version_id.clone()], USER_USER_PAT)
+                .get_versions_deserialized(vec![alpha_version_id.clone()], USER_USER_PAT)
                 .await;
             assert_eq!(versions[0].ordering, Some(10));
-    })
+        },
+    )
     .await;
 }