From 53ba4765166fe75c9df1d85a95bd6c65b56fbd38 Mon Sep 17 00:00:00 2001
From: Sarah Gillespie <73313222+gillespi314@users.noreply.github.com>
Date: Mon, 29 Jul 2024 15:46:27 -0500
Subject: [PATCH] Fix unreleased bug: API error message for duplicate VPP apps
 (#20808)

---
 .../SoftwarePage/components/AppStoreVpp/helpers.tsx  | 12 +++++++++++-
 frontend/services/entities/mdm_apple.ts              |  5 ++++-
 server/datastore/mysql/vpp.go                        |  7 +++++++
 server/datastore/mysql/vpp_test.go                   | 11 +++++++++++
 4 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/frontend/pages/SoftwarePage/components/AppStoreVpp/helpers.tsx b/frontend/pages/SoftwarePage/components/AppStoreVpp/helpers.tsx
index b32495cbb8..f359a6441d 100644
--- a/frontend/pages/SoftwarePage/components/AppStoreVpp/helpers.tsx
+++ b/frontend/pages/SoftwarePage/components/AppStoreVpp/helpers.tsx
@@ -5,12 +5,22 @@ const ADD_SOFTWARE_ERROR_PREFIX = "Couldn’t add software.";
 const DEFAULT_ERROR_MESSAGE = `${ADD_SOFTWARE_ERROR_PREFIX} Please try again.`;
 
 const generateAlreadyAvailableMessage = (msg: string) => {
+  // This regex matches the API message where the title already has a software package (non-VPP) available for install.
   const regex = new RegExp(
     `${ADD_SOFTWARE_ERROR_PREFIX} (.+) already.+on the (.+) team.`
   );
 
   const match = msg.match(regex);
-  if (!match) return DEFAULT_ERROR_MESSAGE;
+  if (!match) {
+    if (msg.includes("VPPApp")) {
+      // This is the case where someone already added this VPP app. This should almost never happen
+      // because we omit apps that are already available from the list in the UI, but just in case of
+      // shenanigans with concurrent requests or something, we'll handle it with a generic message.
+      // The list should clear itself up on the next page load.
+      return `${ADD_SOFTWARE_ERROR_PREFIX} The software is already available to install on this team.`;
+    }
+    return DEFAULT_ERROR_MESSAGE;
+  }
 
   return (
     <>
diff --git a/frontend/services/entities/mdm_apple.ts b/frontend/services/entities/mdm_apple.ts
index 0c21341ade..8d770fd0b4 100644
--- a/frontend/services/entities/mdm_apple.ts
+++ b/frontend/services/entities/mdm_apple.ts
@@ -11,7 +11,7 @@ export interface IVppApp {
   name: string;
   icon_url: string;
   latest_version: string;
-  app_store_id: number;
+  app_store_id: number; // FIXME: This seems to be coming back as a string but we're not seeing TS errors because there's an implicit any cast
   added: boolean;
   platform: string; // "darwin" | "ios" | "ipados"
 }
@@ -68,6 +68,9 @@ export default {
   },
 
   addVppApp: (teamId: number, appStoreId: number) => {
+    // FIXME: API seems to expect app_store_id to be a string but we're not seeing TS errors because
+    // there's an implicit any cast on getVppApps response that is being fed into the appStoreId
+    // param (so it's being treated as a number even though it is really a string being passed in here)
     const { MDM_APPLE_VPP_APPS } = endpoints;
     return sendRequest("POST", MDM_APPLE_VPP_APPS, {
       app_store_id: appStoreId,
diff --git a/server/datastore/mysql/vpp.go b/server/datastore/mysql/vpp.go
index 85925e57bc..140f66ade4 100644
--- a/server/datastore/mysql/vpp.go
+++ b/server/datastore/mysql/vpp.go
@@ -288,6 +288,13 @@ VALUES
 	}
 
 	_, err := tx.ExecContext(ctx, stmt, adamID, globalOrTmID, teamID)
+	if IsDuplicate(err) {
+		err = &existsError{
+			Identifier:   adamID,
+			TeamID:       teamID,
+			ResourceType: "VPPAppAdamID",
+		}
+	}
 
 	return ctxerr.Wrap(ctx, err, "writing vpp app team mapping to db")
 }
diff --git a/server/datastore/mysql/vpp_test.go b/server/datastore/mysql/vpp_test.go
index dd2deaba1b..1414c26f61 100644
--- a/server/datastore/mysql/vpp_test.go
+++ b/server/datastore/mysql/vpp_test.go
@@ -62,6 +62,12 @@ func testVPPAppMetadata(t *testing.T, ds *Datastore) {
 	require.NoError(t, err)
 	require.Equal(t, &fleet.VPPAppStoreApp{Name: "vpp1", AppStoreID: vpp1}, meta)
 
+	// try to add the same app again, fails
+	var existsErr *existsError
+	_, err = ds.InsertVPPAppWithTeam(ctx, &fleet.VPPApp{Name: "vpp1", BundleIdentifier: "com.app.vpp1", AdamID: "adam_vpp_app_1"}, nil)
+	require.Error(t, err)
+	require.ErrorAs(t, err, &existsErr)
+
 	// create team1 app
 	va2, err := ds.InsertVPPAppWithTeam(ctx, &fleet.VPPApp{Name: "vpp2", BundleIdentifier: "com.app.vpp2", AdamID: "adam_vpp_app_2"}, &team1.ID)
 	require.NoError(t, err)
@@ -72,6 +78,11 @@ func testVPPAppMetadata(t *testing.T, ds *Datastore) {
 	require.NoError(t, err)
 	require.Equal(t, &fleet.VPPAppStoreApp{Name: "vpp2", AppStoreID: vpp2}, meta)
 
+	// try to add the same app again, fails
+	_, err = ds.InsertVPPAppWithTeam(ctx, &fleet.VPPApp{Name: "vpp2", BundleIdentifier: "com.app.vpp2", AdamID: "adam_vpp_app_2"}, &team1.ID)
+	require.Error(t, err)
+	require.ErrorAs(t, err, &existsErr)
+
 	// get it for team 2, does not exist
 	meta, err = ds.GetVPPAppMetadataByTeamAndTitleID(ctx, &team2.ID, titleID2)
 	require.Error(t, err)