Fix org adding (#821)

.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"
+}

.sqlx/query-ee6b35a83723e0b753bd30819bad9bb356455b6c0b87cdf1b43fa576ccc5bef5.json

@@ -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"
-}

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
+        sqlx::query!(
-        database::models::TeamMember::delete(
+            "
-            project_item.inner.team_id,
+            DELETE FROM team_members
-            organization_owner_user_id,
+            WHERE team_id = $1 AND (is_owner = TRUE OR user_id = $2)
-            &mut transaction,
+            ",
+            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,

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
+        //      -> Friend was never an owner of the alpha project
-        //              then friend was removed as owner when it was added to the organization
+        let members = test_env
-        // -> In both cases, user was removed entirely as a team_member as it is now the owner of the organization
+            .api
-        for team_id in [alpha_team_id, beta_team_id] {
+            .get_team_members_deserialized(alpha_team_id, USER_USER_PAT)
-            let members = test_env
+            .await;
-                .api
+        assert_eq!(members.len(), 1);
-                .get_team_members_deserialized(team_id, USER_USER_PAT)
+        assert_eq!(members[0].user.id.to_string(), FRIEND_USER_ID);
-                .await;
+        let user_member = members.iter().filter(|m| m.is_owner).collect::<Vec<_>>();
-            assert_eq!(members.len(), 1);
+        assert_eq!(user_member.len(), 0);
-            assert_eq!(members[0].user.id.to_string(), FRIEND_USER_ID);
+
-            let user_member = members.iter().filter(|m| m.is_owner).collect::<Vec<_>>();
+        // Beta project should have:
-            assert_eq!(user_member.len(), 0);
+        // - 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