Skip to content
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

Remove package config from deployments response #7808

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
2bb9eb0
Remove package config from deployments response
johnnymetz Mar 4, 2024
e527ead
Merge branch 'main' into feature/remove-package-config-from-deploymen…
johnnymetz Mar 5, 2024
9c58518
PR feedback: Add files to strictNullCheck, add comments, remove unuse…
johnnymetz Mar 5, 2024
d017015
Create and use DeploymentModDefinitionPair type
johnnymetz Mar 5, 2024
16488a6
Add performance improvement comment
johnnymetz Mar 5, 2024
44655a7
Explicitly pass version in header
johnnymetz Mar 5, 2024
f1c41e0
Fix deploymentUtils tests
johnnymetz Mar 5, 2024
322a64c
Fix useAutoDeploy tests
johnnymetz Mar 5, 2024
1204ec3
Fix deploymentUpdater tests
johnnymetz Mar 5, 2024
7f00979
Merge branch 'main' into feature/remove-package-config-from-deploymen…
johnnymetz Mar 5, 2024
77be9a3
Merge branch 'main' into feature/remove-package-config-from-deploymen…
johnnymetz Mar 5, 2024
6c85f25
Extract types out
twschiller Mar 6, 2024
6d83282
Handle refresh error
twschiller Mar 6, 2024
a080770
Implement deploymentModDefinitionPairFactory
johnnymetz Mar 6, 2024
0ea0aaf
Fix deploymentUpdater tests by resetting axios mock
johnnymetz Mar 6, 2024
1fca5f1
Rename activateDeployments because it duplicates with an existing fun…
johnnymetz Mar 6, 2024
5549a29
Fix type error
johnnymetz Mar 6, 2024
b023200
Merge branch 'main' into feature/remove-package-config-from-deploymen…
johnnymetz Mar 6, 2024
fd66faa
Only fetch mod defs for deployments to update, not all deployments
johnnymetz Mar 6, 2024
b4a63ec
Fix most DeploymentsContext tests
johnnymetz Mar 6, 2024
269a477
Re-add .swcrc
johnnymetz Mar 6, 2024
4494a77
Add log for debugging
johnnymetz Mar 6, 2024
de74901
Remove unnecessary API status checks
johnnymetz Mar 6, 2024
ae047fa
Rename DeploymentModDefinitionPair to ActivatableDeployment
johnnymetz Mar 6, 2024
df823be
Fix test
johnnymetz Mar 6, 2024
9133cc0
Fix lint error
johnnymetz Mar 6, 2024
f5af219
Link ticket for future work
johnnymetz Mar 6, 2024
280051c
Merge branch 'main' into feature/remove-package-config-from-deploymen…
johnnymetz Mar 6, 2024
655413f
Fix dead code error
johnnymetz Mar 6, 2024
646ecef
Merge branch 'main' into feature/remove-package-config-from-deploymen…
johnnymetz Mar 6, 2024
85d4103
Pass path to axios mock calls
johnnymetz Mar 6, 2024
8ebb897
Remove console log
johnnymetz Mar 6, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
97 changes: 71 additions & 26 deletions src/background/deploymentUpdater.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,14 @@ import { type UUID } from "@/types/stringTypes";
import { type UnresolvedModComponent } from "@/types/modComponentTypes";
import { type RegistryId } from "@/types/registryTypes";
import { type OptionsArgs } from "@/types/runtimeTypes";
import { type ModDefinition } from "@/types/modDefinitionTypes";
import { checkDeploymentPermissions } from "@/permissions/deploymentPermissionsHelpers";
import { Events } from "@/telemetry/events";
import { allSettled } from "@/utils/promiseUtils";
import type { Manifest } from "webextension-polyfill";
import { getRequestHeadersByAPIVersion } from "@/data/service/apiVersioning";
import { fetchDeploymentModDefinitions } from "@/modDefinitions/modDefinitionRawApiCalls";
import { services } from "@/background/messenger/api";

// eslint-disable-next-line local-rules/persistBackgroundData -- Static
const { reducer: optionsReducer, actions: optionsActions } = extensionsSlice;
Expand Down Expand Up @@ -219,11 +223,17 @@ async function uninstallRecipe(
return { options, editor };
}

async function installDeployment(
optionsState: ModComponentState,
editorState: EditorState | undefined,
deployment: Deployment,
): Promise<{
async function installDeployment({
optionsState,
editorState,
deployment,
johnnymetz marked this conversation as resolved.
Show resolved Hide resolved
deploymentModDefinition,
}: {
optionsState: ModComponentState;
editorState: EditorState | undefined;
deployment: Deployment;
deploymentModDefinition: ModDefinition;
}): Promise<{
options: ModComponentState;
editor: EditorState | undefined;
}> {
Expand All @@ -248,12 +258,13 @@ async function installDeployment(
options = optionsReducer(
options,
optionsActions.installMod({
modDefinition: deployment.package.config,
modDefinition: deploymentModDefinition,
deployment,
configuredDependencies: await mergeDeploymentIntegrationDependencies(
configuredDependencies: await mergeDeploymentIntegrationDependencies({
deployment,
locateAllForService,
),
deploymentModDefinition,
locate: services.locateAllForId,
}),
// Assume backend properly validates the options
optionsArgs: deployment.options_config as OptionsArgs,
screen: "background",
Expand All @@ -272,20 +283,30 @@ async function installDeployment(
/**
* Install all deployments
* @param deployments deployments that PixieBrix already has permission to run
* @param deploymentsModDefinitionMap map of deployment package UUIDs to mod definitions
*/
async function installDeployments(deployments: Deployment[]): Promise<void> {
async function installDeployments({
deployments,
deploymentsModDefinitionMap,
}: {
deployments: Deployment[];
deploymentsModDefinitionMap: Map<Deployment["package"]["id"], ModDefinition>;
}): Promise<void> {
let [optionsState, editorState] = await Promise.all([
getModComponentState(),
getEditorState(),
]);

for (const deployment of deployments) {
// eslint-disable-next-line no-await-in-loop -- running reducer, need to update states serially
const result = await installDeployment(
const result = await installDeployment({
optionsState,
editorState,
deployment,
);
deploymentModDefinition: deploymentsModDefinitionMap.get(
deployment.package.id,
),
});
optionsState = result.options;
editorState = result.editor;
}
Expand All @@ -296,6 +317,7 @@ async function installDeployments(deployments: Deployment[]): Promise<void> {

type DeploymentConstraint = {
deployment: Deployment;
deploymentModDefinition: ModDefinition;
hasPermissions: boolean;
extensionVersion: SemVer;
};
Expand All @@ -310,23 +332,25 @@ type DeploymentConstraint = {
*/
async function canAutomaticallyInstall({
deployment,
deploymentModDefinition,
hasPermissions,
extensionVersion,
}: DeploymentConstraint): Promise<boolean> {
if (!hasPermissions) {
return false;
}

const requiredRange = deployment.package.config.metadata.extensionVersion;
const requiredRange = deploymentModDefinition.metadata.extensionVersion;
if (requiredRange && !satisfies(extensionVersion, requiredRange)) {
return false;
}

const personalConfigs =
await findLocalDeploymentConfiguredIntegrationDependencies(
await findLocalDeploymentConfiguredIntegrationDependencies({
deployment,
locateAllForService,
);
deploymentModDefinition,
locate: locateAllForService,
});
return personalConfigs.every(({ configs }) => configs.length === 1);
}

Expand Down Expand Up @@ -482,15 +506,21 @@ export async function updateDeployments(): Promise<void> {
void updateUserData(selectUserDataUpdate(profile));

const { data: deployments, status: deploymentResponseStatus } =
await client.post<Deployment[]>("/api/deployments/", {
uid: await getUUID(),
version: getExtensionVersion(),
active: selectInstalledDeployments(extensions),
campaignIds,
});
await client.post<Deployment[]>(
"/api/deployments/",
{
uid: await getUUID(),
version: getExtensionVersion(),
active: selectInstalledDeployments(extensions),
campaignIds,
},
{
headers: getRequestHeadersByAPIVersion("1.1"),
johnnymetz marked this conversation as resolved.
Show resolved Hide resolved
},
);

if (deploymentResponseStatus >= 400) {
// Our server is active up, check again later
// Our server is acting up, check again later
console.debug(
"Skipping deployments update because /api/deployments/ request failed",
);
Expand Down Expand Up @@ -550,7 +580,11 @@ export async function updateDeployments(): Promise<void> {
return;
}

// `clipboardWrite` is no strictly required to use the clipboard brick, so allow it to auto-install.
const deploymentsModDefinitionMap =
await fetchDeploymentModDefinitions(deployments);
johnnymetz marked this conversation as resolved.
Show resolved Hide resolved
console.log(deploymentsModDefinitionMap);
johnnymetz marked this conversation as resolved.
Show resolved Hide resolved

// `clipboardWrite` is not strictly required to use the clipboard brick, so allow it to auto-install.
// Behind a feature flag in case it causes problems for enterprise customers.
// Could use browser.runtime.getManifest().optional_permissions here, but that also technically supports the Origin
// type so the types wouldn't match with checkDeploymentPermissions
Expand All @@ -562,7 +596,12 @@ export async function updateDeployments(): Promise<void> {
const deploymentRequirements = await Promise.all(
updatedDeployments.map(async (deployment) => ({
deployment,
...(await checkDeploymentPermissions(deployment, locateAllForService, {
...(await checkDeploymentPermissions({
deployment,
deploymentModDefinition: deploymentsModDefinitionMap.get(
deployment.package.id,
),
locate: locateAllForService,
optionalPermissions,
})),
})),
Expand All @@ -574,6 +613,9 @@ export async function updateDeployments(): Promise<void> {
isAutomatic: await canAutomaticallyInstall({
...requirement,
extensionVersion,
deploymentModDefinition: deploymentsModDefinitionMap.get(
requirement.deployment.package.id,
),
}),
})),
);
Expand All @@ -584,7 +626,10 @@ export async function updateDeployments(): Promise<void> {

if (automatic.length > 0) {
try {
await installDeployments(automatic.map((x) => x.deployment));
await installDeployments({
deployments: automatic.map((x) => x.deployment),
deploymentsModDefinitionMap,
});
} catch (error) {
reportError(error);
automaticError = true;
Expand Down
22 changes: 20 additions & 2 deletions src/data/service/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,9 @@ import {
type Milestone,
type Organization,
type Package,
type PackageConfigDetail,
type PackageUpsertResponse,
type PackageVersion,
type PackageVersionDeprecated,
type PendingInvitation,
type RecipeResponse,
type RemoteIntegrationConfig,
Expand Down Expand Up @@ -357,6 +358,19 @@ export const appApi = createApi({
query: ({ id }) => ({ url: `/api/bricks/${id}/`, method: "get" }),
providesTags: (result, error, { id }) => [{ type: "Package", id }],
}),
getPackageViaRegistryId: builder.query<
PackageConfigDetail,
{ registryId: RegistryId; version?: string }
>({
query: ({ registryId, version }) => ({
url: `/api/registry/bricks/${encodeURIComponent(registryId)}/`,
method: "get",
params: { version },
}),
providesTags: (result, error, { registryId }) => [
{ type: "Package", id: registryId },
],
}),
createPackage: builder.mutation<PackageUpsertResponse, UnknownObject>({
query(data) {
return {
Expand Down Expand Up @@ -396,7 +410,10 @@ export const appApi = createApi({
"EditablePackages",
],
}),
listPackageVersions: builder.query<PackageVersion[], { id: UUID }>({
listPackageVersions: builder.query<
PackageVersionDeprecated[],
{ id: UUID }
>({
query: ({ id }) => ({
url: `/api/bricks/${id}/versions/`,
method: "get",
Expand Down Expand Up @@ -462,6 +479,7 @@ export const {
useUpdateRecipeMutation,
useGetInvitationsQuery,
useGetPackageQuery,
useGetPackageViaRegistryIdQuery,
useCreatePackageMutation,
useUpdatePackageMutation,
useDeletePackageMutation,
Expand Down
45 changes: 45 additions & 0 deletions src/data/service/apiVersioning.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/*

Check failure on line 1 in src/data/service/apiVersioning.ts

View workflow job for this annotation

GitHub Actions / strictNullChecks

strictNullChecks

src/data/service/apiVersioning.ts was not found in tsconfig.strictNullChecks.json
* 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/>.
*/

// See similar file in the App codebase

// See REST_FRAMEWORK["ALLOWED_VERSIONS"] in the Django settings
const API_VERSIONS = ["1.0", "1.1", "2.0"] as const;
export type ApiVersion = (typeof API_VERSIONS)[number];

// See REST_FRAMEWORK["DEFAULT_VERSION"] in the Django settings
const DEFAULT_API_VERSION = "1.0";

export function getRequestHeadersByAPIVersion(
apiVersion: ApiVersion | undefined,
) {
if (!apiVersion || apiVersion === DEFAULT_API_VERSION) {
// No headers are required for the default version.
// But we set as a default axios header in baseQuery.ts anyway.
return {};
johnnymetz marked this conversation as resolved.
Show resolved Hide resolved
}

const nonDefaultApiVersions = API_VERSIONS.filter(
johnnymetz marked this conversation as resolved.
Show resolved Hide resolved
(version) => version !== DEFAULT_API_VERSION,
);

if (nonDefaultApiVersions.includes(apiVersion)) {
return { Accept: `application/json; version=${apiVersion}` };
}

throw new Error("Unknown API version");
}
8 changes: 7 additions & 1 deletion src/data/service/baseQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,11 @@ type QueryArgs = {
* Optional URL parameters to be sent with the request
*/
params?: AxiosRequestConfig["params"];

/**
* Optional headers to be sent with the request
*/
headers?: AxiosRequestConfig["headers"];
};

// https://redux-toolkit.js.org/rtk-query/usage/customizing-queries#axios-basequery
Expand All @@ -61,12 +66,13 @@ const baseQuery: BaseQueryFn<QueryArgs> = async ({
requireLinked = true,
meta,
params,
headers,
}) => {
try {
const client = await (requireLinked
? getLinkedApiClient()
: getApiClient());
const result = await client({ url, method, data, params });
const result = await client({ url, method, data, params, headers });

return { data: result.data, meta };
} catch (error) {
Expand Down
4 changes: 2 additions & 2 deletions src/extensionConsole/pages/brickEditor/BrickHistory.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import { uuidSequence } from "@/testUtils/factories/stringFactories";
import selectEvent from "react-select-event";
import { waitForEffect } from "@/testUtils/testHelpers";
import { render } from "@/extensionConsole/testHelpers";
import { type Package, type PackageVersion } from "@/types/contract";
import { type Package, type PackageVersionDeprecated } from "@/types/contract";
import { type Timestamp } from "@/types/stringTypes";

const axiosMock = new MockAdapter(axios);
Expand All @@ -45,7 +45,7 @@ describe("BrickHistory", () => {
};

it("renders select components for choosing versions and displays the diff", async () => {
const testVersions: PackageVersion[] = [
const testVersions: PackageVersionDeprecated[] = [
{
id: testBrickId,
version: "1.0.1",
Expand Down
2 changes: 1 addition & 1 deletion src/extensionConsole/pages/brickEditor/BrickHistory.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ const BrickHistory: React.FunctionComponent<{
// We look up the large rawConfig up separately instead of including it in the versionOptions array because
// it's a large string, and it causes the UI to hang.
// TODO: use a specific endpoint for fetching just version metadata without the entire mod config
// https://github.com/pixiebrix/pixiebrix-app/issues/4627
// https://github.com/pixiebrix/pixiebrix-extension/issues/7692
const versionARawConfig = useMemo(
() =>
packageVersions?.find((version) => version.version === versionA?.value)
Expand Down