From 917b89e44f20791e428300c3ade8f3ad5d14a955 Mon Sep 17 00:00:00 2001 From: Geometrically <18202329+Geometrically@users.noreply.github.com> Date: Sat, 6 Jan 2024 16:34:20 -0500 Subject: [PATCH] Fix org adding (#821) --- ...e84976023c0cefa777b4ad86636bf43aa6920.json | 15 ++++ ...d9bb356455b6c0b87cdf1b43fa576ccc5bef5.json | 14 ---- src/routes/v3/organizations.rs | 32 +++------ tests/organizations.rs | 68 +++++++++++++++---- 4 files changed, 79 insertions(+), 50 deletions(-) create mode 100644 .sqlx/query-2ae397b672260d1be8b54962e59e84976023c0cefa777b4ad86636bf43aa6920.json delete mode 100644 .sqlx/query-ee6b35a83723e0b753bd30819bad9bb356455b6c0b87cdf1b43fa576ccc5bef5.json diff --git a/.sqlx/query-2ae397b672260d1be8b54962e59e84976023c0cefa777b4ad86636bf43aa6920.json b/.sqlx/query-2ae397b672260d1be8b54962e59e84976023c0cefa777b4ad86636bf43aa6920.json new file mode 100644 index 00000000..cb05425a --- /dev/null +++ b/.sqlx/query-2ae397b672260d1be8b54962e59e84976023c0cefa777b4ad86636bf43aa6920.json @@ -0,0 +1,15 @@ +{ + "db_name": "PostgreSQL", + "query": "\n DELETE FROM team_members\n WHERE team_id = $1 AND (is_owner = TRUE OR user_id = $2)\n ", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Int8", + "Int8" + ] + }, + "nullable": [] + }, + "hash": "2ae397b672260d1be8b54962e59e84976023c0cefa777b4ad86636bf43aa6920" +} diff --git a/.sqlx/query-ee6b35a83723e0b753bd30819bad9bb356455b6c0b87cdf1b43fa576ccc5bef5.json b/.sqlx/query-ee6b35a83723e0b753bd30819bad9bb356455b6c0b87cdf1b43fa576ccc5bef5.json deleted file mode 100644 index d8024601..00000000 --- a/.sqlx/query-ee6b35a83723e0b753bd30819bad9bb356455b6c0b87cdf1b43fa576ccc5bef5.json +++ /dev/null @@ -1,14 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "\n UPDATE team_members\n SET is_owner = FALSE\n WHERE id = $1\n ", - "describe": { - "columns": [], - "parameters": { - "Left": [ - "Int8" - ] - }, - "nullable": [] - }, - "hash": "ee6b35a83723e0b753bd30819bad9bb356455b6c0b87cdf1b43fa576ccc5bef5" -} diff --git a/src/routes/v3/organizations.rs b/src/routes/v3/organizations.rs index 955233e3..7c07a644 100644 --- a/src/routes/v3/organizations.rs +++ b/src/routes/v3/organizations.rs @@ -664,7 +664,6 @@ pub async fn organization_projects_add( ) .await? .ok_or_else(|| ApiError::InvalidInput("You are not a member of this project!".to_string()))?; - let organization_team_member = database::models::TeamMember::get_from_user_id_organization( organization.id, current_user.id.into(), @@ -705,22 +704,7 @@ pub async fn organization_projects_add( // The former owner is no longer an owner (as it is now 'owned' by the organization, 'given' to them) // The former owner is still a member of the project, but not an owner // When later removed from the organization, the project will be owned by whoever is specified as the new owner there - if !current_user.role.is_admin() { - let team_member_id = project_team_member.id; - sqlx::query!( - " - UPDATE team_members - SET is_owner = FALSE - WHERE id = $1 - ", - team_member_id as database::models::ids::TeamMemberId - ) - .execute(&mut *transaction) - .await?; - } - // The owner of the organization, should be removed as a member of the project, if they are - // (As it is an organization project now, and they should not have more specific permissions) let organization_owner_user_id = sqlx::query!( " SELECT u.id @@ -735,16 +719,20 @@ pub async fn organization_projects_add( let organization_owner_user_id = database::models::ids::UserId(organization_owner_user_id.id); - // If the owner of the organization is a member of the project, remove them - database::models::TeamMember::delete( - project_item.inner.team_id, - organization_owner_user_id, - &mut transaction, + sqlx::query!( + " + DELETE FROM team_members + WHERE team_id = $1 AND (is_owner = TRUE OR user_id = $2) + ", + project_item.inner.team_id as database::models::ids::TeamId, + organization_owner_user_id as database::models::ids::UserId, ) + .execute(&mut *transaction) .await?; transaction.commit().await?; + database::models::User::clear_project_cache(&[current_user.id.into()], &redis).await?; database::models::TeamMember::clear_cache(project_item.inner.team_id, &redis).await?; database::models::Project::clear_cache( project_item.inner.id, @@ -905,7 +893,7 @@ pub async fn organization_projects_remove( .await?; transaction.commit().await?; - + database::models::User::clear_project_cache(&[current_user.id.into()], &redis).await?; database::models::TeamMember::clear_cache(project_item.inner.team_id, &redis).await?; database::models::Project::clear_cache( project_item.inner.id, diff --git a/tests/organizations.rs b/tests/organizations.rs index 54b6e513..360acfc0 100644 --- a/tests/organizations.rs +++ b/tests/organizations.rs @@ -293,6 +293,16 @@ async fn add_remove_organization_projects() { let alpha_project_slug: &str = &test_env.dummy.project_alpha.project_slug; let zeta_organization_id: &str = &test_env.dummy.organization_zeta.organization_id; + // user's page should show alpha project + // It may contain more than one project, depending on dummy data, but should contain the alpha project + let projects = test_env + .api + .get_user_projects_deserialized_common(USER_USER_ID, USER_USER_PAT) + .await; + assert!(projects + .iter() + .any(|p| p.id.to_string() == alpha_project_id)); + // Add/remove project to organization, first by ID, then by slug for alpha in [alpha_project_id, alpha_project_slug] { let resp = test_env @@ -309,6 +319,16 @@ async fn add_remove_organization_projects() { assert_eq!(projects[0].id.to_string(), alpha_project_id); assert_eq!(projects[0].slug, Some(alpha_project_slug.to_string())); + // Currently, intended behaviour is that user's page should NOT show organization projects. + // It may contain other projects, depending on dummy data, but should not contain the alpha project + let projects = test_env + .api + .get_user_projects_deserialized_common(USER_USER_ID, USER_USER_PAT) + .await; + assert!(!projects + .iter() + .any(|p| p.id.to_string() == alpha_project_id)); + // Remove project from organization let resp = test_env .api @@ -321,6 +341,17 @@ async fn add_remove_organization_projects() { .await; assert_status!(&resp, StatusCode::OK); + // Get user's projects as user - should be 1, the alpha project, + // as we gave back ownership to the user when we removed it from the organization + // So user's page should show the alpha project (and possibly others) + let projects = test_env + .api + .get_user_projects_deserialized_common(USER_USER_ID, USER_USER_PAT) + .await; + assert!(projects + .iter() + .any(|p| p.id.to_string() == alpha_project_id)); + // Get organization projects let projects = test_env .api @@ -428,23 +459,32 @@ async fn add_remove_organization_project_ownership_to_user() { ); } - // Both alpha and beta project should have: + // Alpha project should have: // - 1 member, FRIEND_USER_ID + // -> User was removed entirely as a team_member as it is now the owner of the organization // - No owner. // -> For alpha, user was removed as owner when it was added to the organization - // -> For beta, user was removed as owner when ownership was transferred to friend - // then friend was removed as owner when it was added to the organization - // -> In both cases, user was removed entirely as a team_member as it is now the owner of the organization - for team_id in [alpha_team_id, beta_team_id] { - let members = test_env - .api - .get_team_members_deserialized(team_id, USER_USER_PAT) - .await; - assert_eq!(members.len(), 1); - assert_eq!(members[0].user.id.to_string(), FRIEND_USER_ID); - let user_member = members.iter().filter(|m| m.is_owner).collect::>(); - assert_eq!(user_member.len(), 0); - } + // -> Friend was never an owner of the alpha project + let members = test_env + .api + .get_team_members_deserialized(alpha_team_id, USER_USER_PAT) + .await; + assert_eq!(members.len(), 1); + assert_eq!(members[0].user.id.to_string(), FRIEND_USER_ID); + let user_member = members.iter().filter(|m| m.is_owner).collect::>(); + assert_eq!(user_member.len(), 0); + + // Beta project should have: + // - No members + // -> User was removed entirely as a team_member as it is now the owner of the organization + // -> Friend was made owner of the beta project, but was removed as a member when it was added to the organization + // If you are owner of a projeect, you are removed from the team when it is added to an organization, + // so that your former permissions are not overriding the organization permissions by default. + let members = test_env + .api + .get_team_members_deserialized(beta_team_id, USER_USER_PAT) + .await; + assert!(members.is_empty()); // Transfer ownership of zeta organization to FRIEND let resp = test_env