Discharge7443/fleet
1
0
Fork 0
Code Issues Pull Requests Actions 5 Packages Projects Releases Wiki Activity

Fleet UI: VPP Token All teams option bug fix (#31587)

Browse Source
This commit is contained in:
RachelElysia
2025-08-07 09:00:51 -04:00
committed by GitHub
parent aa02ec6f2b
commit aae6147487
5 changed files with 235 additions and 54 deletions
Download Patch File Download Diff File Expand all files Collapse all files
changes/30357-all-teams-dd-bug
+1
View File
@@ -0,0 +1 @@
- Fleet UI: Fixed VPP token dropdown to allow user to choose "All teams" selection
frontend/pages/admin/IntegrationsPage/cards/MdmSettings/VppPage/components/EditTeamsVppModal/EditTeamsVppModal.tests.tsx
+104 -9
View File
@@ -2,6 +2,7 @@ import {
APP_CONTEXT_ALL_TEAMS_ID,
APP_CONTEXT_NO_TEAM_ID,
} from "interfaces/team";
import { IMdmVppToken } from "interfaces/mdm";
import {
getOptions,
selectedValueFromToken,
@@ -68,10 +69,24 @@ describe("EditTeamsVppModal", () => {
}));
describe("getOptions", () => {
// Helper for getting a pendingTeamIds array from a token
const asArr = (token: IMdmVppToken) =>
selectedValueFromToken(token)
? selectedValueFromToken(token)
.split(",")
.map((v) => v.trim())
.filter(Boolean)
: [];
it("returns no options when another token is all teams", () => {
const tokens = [allTeamsToken, piratesAndNinjasToken];
const currentToken = piratesAndNinjasToken;
const options = getOptions(availableTeams, tokens, currentToken);
const options = getOptions(
availableTeams,
tokens,
currentToken,
asArr(currentToken)
);
expect(options).toEqual([]);
});
@@ -82,7 +97,12 @@ describe("EditTeamsVppModal", () => {
{ ...unassignedToken, id: 1338 },
];
const currentToken = unassignedToken;
const options = getOptions(availableTeams, tokens, currentToken);
const options = getOptions(
availableTeams,
tokens,
currentToken,
asArr(currentToken)
);
expect(options).toEqual(allOptions);
});
@@ -94,14 +114,24 @@ describe("EditTeamsVppModal", () => {
{ ...unassignedToken, id: 1338 },
];
const currentToken = allTeamsToken;
const options = getOptions(availableTeams, tokens, currentToken);
const options = getOptions(
availableTeams,
tokens,
currentToken,
asArr(currentToken)
);
expect(options).toEqual(allOptions);
});
it("excludes all teams option when any token is assigned", () => {
const tokens = [unassignedToken, piratesAndNinjasToken];
const currentToken = unassignedToken;
const options = getOptions(availableTeams, tokens, currentToken);
const options = getOptions(
availableTeams,
tokens,
currentToken,
asArr(currentToken)
);
expect(options).toEqual(
options.filter((o) => o.value !== APP_CONTEXT_ALL_TEAMS_ID)
);
@@ -116,7 +146,14 @@ describe("EditTeamsVppModal", () => {
];
// test with unassignedToken
expect(getOptions(availableTeams, tokens, unassignedToken)).toEqual([
expect(
getOptions(
availableTeams,
tokens,
unassignedToken,
asArr(unassignedToken)
)
).toEqual([
{ label: "Penguins", value: 4 }, // only penguins is available
]);
@@ -126,7 +163,14 @@ describe("EditTeamsVppModal", () => {
APP_CONTEXT_NO_TEAM_ID, // already assigned to noTeamToken
3, // already assigned to pandasToken
];
expect(getOptions(availableTeams, tokens, piratesAndNinjasToken)).toEqual(
expect(
getOptions(
availableTeams,
tokens,
piratesAndNinjasToken,
asArr(piratesAndNinjasToken)
)
).toEqual(
allOptions.filter((o) => !unavailableTeamIds.includes(o.value))
);
@@ -137,7 +181,9 @@ describe("EditTeamsVppModal", () => {
1, // already assigned to piratesAndNinjasToken
2, // already assigned to piratesAndNinjasToken
];
expect(getOptions(availableTeams, tokens, pandasToken)).toEqual(
expect(
getOptions(availableTeams, tokens, pandasToken, asArr(pandasToken))
).toEqual(
allOptions.filter((o) => !unavailableTeamIds.includes(o.value))
);
@@ -148,7 +194,9 @@ describe("EditTeamsVppModal", () => {
2, // already assigned to piratesAndNinjasToken
3, // already assigned to pandasToken
];
expect(getOptions(availableTeams, tokens, noTeamToken)).toEqual(
expect(
getOptions(availableTeams, tokens, noTeamToken, asArr(noTeamToken))
).toEqual(
allOptions.filter((o) => !unavailableTeamIds.includes(o.value))
);
@@ -160,7 +208,12 @@ describe("EditTeamsVppModal", () => {
3, // already assigned to pandasToken
];
expect(
getOptions(availableTeams, [...tokens, allTeamsToken], allTeamsToken)
getOptions(
availableTeams,
[...tokens, allTeamsToken],
allTeamsToken,
asArr(allTeamsToken)
)
).toEqual(
allOptions.filter((o) => !unavailableTeamIds.includes(o.value))
);
@@ -221,4 +274,46 @@ describe("EditTeamsVppModal", () => {
expect(teamIdsFromSelectedValue("2,1")).toEqual([2, 1]);
});
});
describe("pending edit scenarios", () => {
it("shows all teams option when user removes all teams in edit UI (pendingTeamIds = [])", () => {
const tokens = [piratesAndNinjasToken];
// simulating clearing everything in the modal before saving
const options = getOptions(
availableTeams,
tokens,
piratesAndNinjasToken,
[]
);
expect(options.some((o) => o.value === APP_CONTEXT_ALL_TEAMS_ID)).toBe(
true
);
});
it("shows only 'all teams' option when user selects all teams pending", () => {
const tokens = [unassignedToken, pandasToken];
const options = getOptions(
availableTeams,
tokens,
unassignedToken,
[APP_CONTEXT_ALL_TEAMS_ID.toString()] // user picks 'all teams'
);
expect(options.some((o) => o.value === APP_CONTEXT_ALL_TEAMS_ID)).toBe(
true
);
});
it("hides teams already assigned to other tokens when editing assignment", () => {
const tokens = [pandasToken];
// Simulate user picks a team not yet assigned
const options = getOptions(
availableTeams,
tokens,
unassignedToken,
["4"] // 'Penguins'
);
expect(options.find((o) => o.value === 3)).toBeUndefined(); // Pandas not selectable
expect(options.find((o) => o.value === 4)).not.toBeUndefined(); // Penguins selectable
});
});
});
frontend/pages/admin/IntegrationsPage/cards/MdmSettings/VppPage/components/EditTeamsVppModal/EditTeamsVppModal.tsx
+84 -36
View File
@@ -24,7 +24,8 @@ interface IEditTeamsVppModalProps {
}
/**
* Returns an array of team ids from a token. It includes special handling for "All teams".
* Returns a string of comma-separated team ids from a token.
* Special handling for "All teams".
*/
export const selectedValueFromToken = (token: IMdmVppToken) => {
if (!token.teams) {
@@ -47,60 +48,54 @@ export const teamIdsFromSelectedValue = (selectedValue: string) => {
if (selectedValue === APP_CONTEXT_ALL_TEAMS_ID.toString()) {
return [];
}
const ids = selectedValue.split(",").map((str) => parseInt(str, 10));
// NOTE: We could do some extra frontend validation here like filtering out -1 (to ensure that
// we're not trying to send all teams and other teams at the same time) and checking for isNaN,
// but instead we're relying on the API to return an error if the request is invalid.
const ids = selectedValue
.split(",")
.map((str) => parseInt(str, 10))
.filter((id) => !isNaN(id));
return ids;
};
/**
* Compare two comma-separated strings of team names and returns an updated value. It includes
* special handling for "All teams".
* Compare two comma-separated strings of team ids and returns an updated value.
* Includes special handling for "All teams".
*/
export const updateSelectedValue = (prev: string, next: string) => {
// react-select uses a string of comma-separated values for multi-select so we split it
// fo get an array of selected team ids
const nextParts = next.split(",").map((p) => p.trim());
if (nextParts.length === 1) {
// if only one team is selected, no need for other checks
return next;
}
// we need to do some special handling for "All teams"
const allTeamsId = APP_CONTEXT_ALL_TEAMS_ID.toString();
// split the previous value to get an array of team ids
const prevParts = prev.split(",").map((p) => p.trim());
if (prevParts.includes(allTeamsId)) {
// if "All teams" was previously selected, we need to remove it from the next selections
return nextParts.filter((p) => p !== allTeamsId).join(", ");
return nextParts.filter((p) => p !== allTeamsId).join(",");
}
// if "All teams" is newly selected, we need to remove any other selections
// If "All teams" is newly selected, remove other selections
if (nextParts.includes(allTeamsId)) {
return allTeamsId;
}
// otherwise, just return the next selections
// Otherwise, just return the next selections
return next;
};
const isTokenAllTeams = (token: IMdmVppToken) => token.teams?.length === 0;
const isTokenUnassigned = (token: IMdmVppToken) => token.teams === null;
/**
* Returns a dictionary of team ids that are already assigned tokens other than the current token.
* Returns a dictionary of team ids already assigned (other than current token).
*/
const getUnavailableTeamIds = (
currentTokenId: number,
tokens: IMdmVppToken[]
) => {
const unavailableTeamIds = {} as Record<string, boolean>;
const unavailableTeamIds: Record<string, boolean> = {};
tokens.forEach((token) => {
if (token.id === currentTokenId) {
return;
}
if (token.id === currentTokenId) return;
token.teams?.forEach((team) => {
unavailableTeamIds[team.team_id.toString()] = true;
});
@@ -114,32 +109,67 @@ const getUnavailableTeamIds = (
export const getOptions = (
availableTeams: ITeamSummary[],
tokens: IMdmVppToken[],
currentToken: IMdmVppToken
currentToken: IMdmVppToken,
pendingTeamIds: string[]
) => {
const allOptions =
availableTeams?.map((t) => ({
label: t.name,
value: t.id,
})) || [];
const allTeamsOption = {
label: "All teams",
value: APP_CONTEXT_ALL_TEAMS_ID,
};
// Filter for actual team options, add "All teams" to the front
const allOptions = [
allTeamsOption,
...availableTeams
.filter((t) => t.id !== APP_CONTEXT_ALL_TEAMS_ID)
.map((t) => ({
label: t.name,
value: t.id,
})),
];
// Determine state of pending assignment
const isPendingAllTeams = pendingTeamIds?.includes(
APP_CONTEXT_ALL_TEAMS_ID.toString()
);
// Case 1: All tokens are unassigned → show all options, including "All teams"
if (tokens.every(isTokenUnassigned)) {
// if all tokens are unassigned, we can include all options
return allOptions;
}
if (tokens.some(isTokenAllTeams) && !isTokenAllTeams(currentToken)) {
// if another token is assigned to all teams, we can't assign this token to any team
// Case 2: If another token (not current) is assigned "All teams", restrict everything unless current/pending choosing "all teams"
if (
tokens.some(
(token) => isTokenAllTeams(token) && token.id !== currentToken.id
) &&
!isPendingAllTeams
) {
return [];
}
// if other tokens are assigned to specific teams, we'll filter out those team options
const unavailableTeamIds = getUnavailableTeamIds(currentToken.id, tokens);
if (!isTokenAllTeams(currentToken)) {
// if current token isn't already assigned to all teams, we'll exclude that option too
unavailableTeamIds[APP_CONTEXT_ALL_TEAMS_ID] = true;
// Case 3: If ANY other token is assigned real teams (not all teams/not unassigned)...
const anotherAssigned = tokens
.filter((t) => t.id !== currentToken.id)
.some((t) => !isTokenAllTeams(t) && !isTokenUnassigned(t));
// If so, and we're not actively changing this token to "All teams", REMOVE "All teams" option
let filteredOptions = allOptions;
if (anotherAssigned && !isPendingAllTeams) {
filteredOptions = allOptions.filter(
(o) => o.value !== APP_CONTEXT_ALL_TEAMS_ID
);
}
return allOptions.filter((o) => !unavailableTeamIds[o.value]);
// Get teams unavailable due to assignment to other tokens
const unavailableTeamIds = getUnavailableTeamIds(currentToken.id, tokens);
// Return options not assigned, or that are in the pending selection
return filteredOptions.filter(
(o) =>
!unavailableTeamIds[o.value.toString()] ||
pendingTeamIds.includes(o.value.toString())
);
};
const EditTeamsVppModal = ({
@@ -158,9 +188,25 @@ const EditTeamsVppModal = ({
);
const [isSaving, setIsSaving] = useState(false);
const selectedValueArr = useMemo(
() =>
selectedValue
? selectedValue
.split(",")
.map((v) => v.trim())
.filter(Boolean)
: [],
[selectedValue]
);
const options = useMemo(() => {
return getOptions(availableTeams || [], tokens, currentToken);
}, [availableTeams, tokens, currentToken]);
return getOptions(
availableTeams || [],
tokens,
currentToken,
selectedValueArr
);
}, [availableTeams, tokens, currentToken, selectedValueArr]);
const isAnyTokenAllTeams = useMemo(() => tokens.some(isTokenAllTeams), [
tokens,
@@ -183,6 +229,8 @@ const EditTeamsVppModal = ({
onSuccess();
} catch (e) {
renderFlash("error", "Couldn’t edit. Please try again.");
} finally {
setIsSaving(false);
}
},
[currentToken.id, selectedValue, renderFlash, onSuccess]
server/datastore/mysql/vpp.go
+5 -8
View File
@@ -1646,21 +1646,18 @@ TEAMLOOP:
func checkVPPNullTeam(ctx context.Context, tx sqlx.ExtContext, currentID *uint, nullTeam fleet.NullTeamType) error {
nullTeamStmt := `SELECT vpp_token_id FROM vpp_token_teams WHERE null_team_type = ?`
anyTeamStmt := `SELECT vpp_token_id FROM vpp_token_teams WHERE null_team_type = 'allteams' OR null_team_type = 'noteam' OR team_id IS NOT NULL`
anyTeamStmt := `SELECT vpp_token_id FROM vpp_token_teams WHERE (null_team_type = 'allteams' OR null_team_type = 'noteam' OR team_id IS NOT NULL) AND vpp_token_id != ?`
if nullTeam == fleet.NullTeamAllTeams {
var ids []uint
if err := sqlx.SelectContext(ctx, tx, &ids, anyTeamStmt); err != nil {
if err := sqlx.SelectContext(ctx, tx, &ids, anyTeamStmt, *currentID); err != nil {
return ctxerr.Wrap(ctx, err, "scanning row in check vpp token null team")
}
// Only blocks assignment if another token is already assigned to one or more teams.
// Allows the current token to switch from teams to "all teams" freely
if len(ids) > 0 {
if len(ids) > 1 {
return ctxerr.Wrap(ctx, errors.New("Cannot assign token to All teams, other teams have tokens"))
}
if currentID == nil || ids[0] != *currentID {
return ctxerr.Wrap(ctx, errors.New("Cannot assign token to All teams, other teams have tokens"))
}
return ctxerr.Wrap(ctx, errors.New("Cannot assign token to All teams, other teams have tokens"))
}
}
server/datastore/mysql/vpp_test.go
+41 -1
View File
@@ -33,6 +33,7 @@ func TestVPP(t *testing.T) {
{"GetVPPAppByTeamAndTitleID", testGetVPPAppByTeamAndTitleID},
{"VPPTokensCRUD", testVPPTokensCRUD},
{"VPPTokenAppTeamAssociations", testVPPTokenAppTeamAssociations},
{"VPPTokenReassignTeamsToAllTeams", testVPPTokenReassignTeamsToAllTeams},
{"GetOrInsertSoftwareTitleForVPPApp", testGetOrInsertSoftwareTitleForVPPApp},
{"DeleteVPPAssignedToPolicy", testDeleteVPPAssignedToPolicy},
{"TestVPPTokenTeamAssignment", testVPPTokenTeamAssignment},
@@ -1540,6 +1541,46 @@ func testVPPTokenAppTeamAssociations(t *testing.T, ds *Datastore) {
assert.Error(t, err)
}
func testVPPTokenReassignTeamsToAllTeams(t *testing.T, ds *Datastore) {
ctx := context.Background()
// Set up two teams
team1, err := ds.NewTeam(ctx, &fleet.Team{Name: "All Teams Reassign Team 1"})
require.NoError(t, err)
team2, err := ds.NewTeam(ctx, &fleet.Team{Name: "All Teams Reassign Team 2"})
require.NoError(t, err)
// Insert token
tokenData, err := test.CreateVPPTokenData(time.Now().Add(24*time.Hour), "Org For Reassign", "Loc For Reassign")
require.NoError(t, err)
tok, err := ds.InsertVPPToken(ctx, tokenData)
require.NoError(t, err)
tokenID := tok.ID
// Assign token to team1 and team2
upTok, err := ds.UpdateVPPTokenTeams(ctx, tokenID, []uint{team1.ID, team2.ID})
require.NoError(t, err)
require.Len(t, upTok.Teams, 2)
// Now, reassign to ALL TEAMS (teams = [])
upTok, err = ds.UpdateVPPTokenTeams(ctx, tokenID, []uint{})
require.NoError(t, err)
require.NotNil(t, upTok.Teams)
require.Len(t, upTok.Teams, 0, "After reassigning to all teams, Teams should be zero-length (all teams)")
// Confirm that the assignment is present as "All teams"
gotTok, err := ds.GetVPPToken(ctx, tokenID)
require.NoError(t, err)
require.NotNil(t, gotTok.Teams)
require.Len(t, gotTok.Teams, 0, "After reassigning to all teams, Teams should be zero-length (all teams)")
// Now, assign back to just team1
upTok, err = ds.UpdateVPPTokenTeams(ctx, tokenID, []uint{team1.ID})
require.NoError(t, err)
require.Len(t, upTok.Teams, 1)
require.Equal(t, team1.ID, upTok.Teams[0].ID)
}
func testGetOrInsertSoftwareTitleForVPPApp(t *testing.T, ds *Datastore) {
ctx := context.Background()
@@ -2028,5 +2069,4 @@ func testGetUnverifiedVPPInstallsForHost(t *testing.T, ds *Datastore) {
require.NoError(t, err)
assert.Len(t, x, step.after)
}
}