-
Notifications
You must be signed in to change notification settings - Fork 22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
#8131 Auto-deactivate unassigned mods on Mods Page #8164
Changes from all commits
d7ad836
cf524da
9e9ef90
0b6c1a9
1048c30
6355e9f
743096d
240238b
df41426
bbb01f2
792b051
ec5cf41
b55f544
be64708
fb4d12f
651256a
e840dfc
5443136
f4b0a28
32dc724
7aca896
448b221
294c6d6
b186dda
85dc080
b3cfce7
fcfa90d
4d4f35e
bf1725e
1f332e5
b9564a4
edd5316
adf690c
e90ad50
d9d8137
565860b
7ec9f5a
099ef85
bb2de0d
827bfbf
9f3cd5e
2e01199
81954f0
df55f43
ac7dcae
806de9a
c56b111
6fb900d
59f8e5e
baa6a21
c3c16db
9866ee0
d5db378
f34313b
683f4a3
6cf04ab
ca06d21
671c4b5
de2b1d5
08e8ead
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -46,13 +46,14 @@ import useAutoDeploy from "@/extensionConsole/pages/deployments/useAutoDeploy"; | |
import { activateDeployments } from "@/extensionConsole/pages/deployments/activateDeployments"; | ||
import { useGetDeploymentsQuery } from "@/data/service/api"; | ||
import { fetchDeploymentModDefinitions } from "@/modDefinitions/modDefinitionRawApiCalls"; | ||
import { isEqual } from "lodash"; | ||
import { isEmpty, isEqual } from "lodash"; | ||
import useMemoCompare from "@/hooks/useMemoCompare"; | ||
import useDeriveAsyncState from "@/hooks/useDeriveAsyncState"; | ||
import type { Deployment } from "@/types/contract"; | ||
import useBrowserIdentifier from "@/hooks/useBrowserIdentifier"; | ||
import type { ActivatableDeployment } from "@/types/deploymentTypes"; | ||
import type { Permissions } from "webextension-polyfill"; | ||
import useDeactivateUnassignedDeploymentsEffect from "@/extensionConsole/pages/deployments/useDeactivateUnassignedDeploymentsEffect"; | ||
|
||
export type DeploymentsState = { | ||
/** | ||
|
@@ -94,10 +95,10 @@ export type DeploymentsState = { | |
function useDeployments(): DeploymentsState { | ||
const dispatch = useDispatch<Dispatch>(); | ||
const { data: browserIdentifier } = useBrowserIdentifier(); | ||
const activeExtensions = useSelector(selectActivatedModComponents); | ||
const activatedModComponents = useSelector(selectActivatedModComponents); | ||
const { state: flagsState } = useFlags(); | ||
const activeDeployments = useMemoCompare<InstalledDeployment[]>( | ||
selectInstalledDeployments(activeExtensions), | ||
selectInstalledDeployments(activatedModComponents), | ||
isEqual, | ||
); | ||
|
||
|
@@ -117,10 +118,20 @@ function useDeployments(): DeploymentsState { | |
deploymentsState, | ||
flagsState, | ||
async (deployments: Deployment[], { restrict }: Restrict) => { | ||
const isUpdated = makeUpdatedFilter(activeExtensions, { | ||
const isUpdated = makeUpdatedFilter(activatedModComponents, { | ||
restricted: restrict("uninstall"), | ||
}); | ||
|
||
const deployedModIds = new Set( | ||
deployments.map((deployment) => deployment.package.package_id), | ||
); | ||
|
||
const unassignedModComponents = activatedModComponents.filter( | ||
(activeModComponent) => | ||
!isEmpty(activeModComponent._deployment) && | ||
!deployedModIds.has(activeModComponent._recipe?.id), | ||
); | ||
|
||
const updatedDeployments = deployments.filter((x) => isUpdated(x)); | ||
|
||
const [activatableDeployments] = await Promise.all([ | ||
|
@@ -150,6 +161,7 @@ function useDeployments(): DeploymentsState { | |
|
||
return { | ||
activatableDeployments, | ||
unassignedModComponents, | ||
extensionUpdateRequired: checkExtensionUpdateRequired( | ||
activatableDeployments, | ||
), | ||
|
@@ -159,20 +171,27 @@ function useDeployments(): DeploymentsState { | |
); | ||
|
||
// Fallback values for loading/error states | ||
const { activatableDeployments, extensionUpdateRequired, permissions } = | ||
deploymentUpdateState.data ?? { | ||
// `useAutoDeploy` expects `null` to represent deployment loading state. It tries to activate once available | ||
activatableDeployments: null as ActivatableDeployment[] | null, | ||
extensionUpdateRequired: false as boolean, | ||
permissions: [] as Permissions.Permissions, | ||
}; | ||
const { | ||
activatableDeployments, | ||
unassignedModComponents, | ||
extensionUpdateRequired, | ||
permissions, | ||
} = deploymentUpdateState.data ?? { | ||
// `useAutoDeploy` expects `null` to represent deployment loading state. It tries to activate once available | ||
activatableDeployments: null as ActivatableDeployment[] | null, | ||
extensionUpdateRequired: false as boolean, | ||
unassignedModComponents: [], | ||
permissions: [] as Permissions.Permissions, | ||
}; | ||
|
||
const { isAutoDeploying } = useAutoDeploy({ | ||
activatableDeployments, | ||
installedExtensions: activeExtensions, | ||
activatedModComponents, | ||
extensionUpdateRequired, | ||
}); | ||
|
||
useDeactivateUnassignedDeploymentsEffect(unassignedModComponents); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the fix |
||
|
||
const handleUpdateFromUserGesture = useCallback(async () => { | ||
// IMPORTANT: can't do a fetch or any potentially stalling operation (including IDB calls) because the call to | ||
// request permissions must occur within 5 seconds of the user gesture. ensurePermissionsFromUserGesture check | ||
|
@@ -224,13 +243,13 @@ function useDeployments(): DeploymentsState { | |
await activateDeployments({ | ||
dispatch, | ||
activatableDeployments, | ||
activatedModComponents: activeExtensions, | ||
activatedModComponents, | ||
}); | ||
notify.success("Updated team deployments"); | ||
} catch (error) { | ||
notify.error({ message: "Error updating team deployments", error }); | ||
} | ||
}, [dispatch, activatableDeployments, permissions, activeExtensions]); | ||
}, [dispatch, activatableDeployments, permissions, activatedModComponents]); | ||
|
||
const updateExtension = useCallback(async () => { | ||
await reloadIfNewVersionIsReady(); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -106,3 +106,37 @@ export async function activateDeployments({ | |
throw errors[0]; | ||
} | ||
} | ||
|
||
export function deactivateUnassignedModComponents({ | ||
dispatch, | ||
unassignedModComponents, | ||
}: { | ||
dispatch: Dispatch; | ||
unassignedModComponents: ModComponentBase[]; | ||
}) { | ||
const deactivatedModComponents = []; | ||
|
||
for (const modComponent of unassignedModComponents) { | ||
try { | ||
dispatch( | ||
actions.removeExtension({ | ||
extensionId: modComponent.id, | ||
}), | ||
); | ||
deactivatedModComponents.push(modComponent); | ||
} catch (error) { | ||
reportError( | ||
new Error("Error deactivating unassigned mod component", { | ||
cause: error, | ||
}), | ||
); | ||
} | ||
} | ||
|
||
reportEvent(Events.DEPLOYMENT_DEACTIVATE_UNASSIGNED, { | ||
auto: true, | ||
deployments: deactivatedModComponents.map( | ||
(modComponent) => modComponent._deployment.id, | ||
), | ||
}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also mirrors logic in deploymentUpdater, see deactivateUnassignedDeployments. It's tricky to extract and share this logic with the current architecture because the deploymentUpdater can't dispatch redux actions. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Technically you can, you just have to manually call the reducer and use our set the resulting state using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Right, what I mean is that having to do that makes logic sharing tricky between the background and the react code |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
/* | ||
Check failure on line 1 in src/extensionConsole/pages/deployments/useDeactivateUnassignedDeploymentsEffect.ts GitHub Actions / strictNullChecksstrictNullChecks
|
||
* Copyright (C) 2024 PixieBrix, Inc. | ||
* | ||
* This program is free software: you can redistribute it and/or modify | ||
* it under the terms of the GNU Affero General Public License as published by | ||
* the Free Software Foundation, either version 3 of the License, or | ||
* (at your option) any later version. | ||
* | ||
* This program is distributed in the hope that it will be useful, | ||
* but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
* GNU Affero General Public License for more details. | ||
* | ||
* You should have received a copy of the GNU Affero General Public License | ||
* along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
*/ | ||
|
||
import { deactivateUnassignedModComponents } from "@/extensionConsole/pages/deployments/activateDeployments"; | ||
import { useEffect } from "react"; | ||
import { useDispatch } from "react-redux"; | ||
import type { Dispatch } from "@reduxjs/toolkit"; | ||
import { type ModComponentBase } from "@/types/modComponentTypes"; | ||
|
||
const useDeactivateUnassignedDeploymentsEffect = ( | ||
unassignedModComponents: ModComponentBase[], | ||
) => { | ||
const dispatch = useDispatch<Dispatch>(); | ||
useEffect(() => { | ||
if (unassignedModComponents.length === 0) return; | ||
|
||
deactivateUnassignedModComponents({ | ||
dispatch, | ||
unassignedModComponents, | ||
}); | ||
}, [unassignedModComponents]); | ||
}; | ||
|
||
export default useDeactivateUnassignedDeploymentsEffect; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mirrors logic in deploymentUpdater
pixiebrix-extension/src/background/deploymentUpdater.ts
Lines 185 to 193 in c3c16db