Slice 2: Use ModInstance type in mod deployment code (2/3)
#9190
Conversation
Playwright test results
Details
Failed testschrome-setup › setup/affiliated.setup.ts › authenticate with affiliated user Flaky testschrome-setup › setup/unaffiliated.setup.ts › authenticate with unaffiliated user Skipped testschrome › tests/bricks/sidebarEffects.spec.ts › sidebar effect bricks › toggle sidebar brick |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feature/devex-mod-instance #9190 +/- ##
==============================================================
+ Coverage 74.77% 74.83% +0.05%
==============================================================
Files 1363 1364 +1
Lines 41977 41985 +8
Branches 7835 7833 -2
==============================================================
+ Hits 31390 31419 +29
+ Misses 10587 10566 -21 ☔ View full report in Codecov by Sentry. |
Fix broken tests Fix rename/bugs
twschiller
force-pushed
the
feature/devex-mod-instance-deployments
branch
from
d9f410a
to
240e111
Compare
| /** | ||
| * Deployment installed on the client. A deployment may be installed but not active (see DeploymentContext.active) | ||
| */ | ||
| export type InstalledDeployment = { |
There was a problem hiding this comment.
Move to contracts.ts because it's a backend type
twschiller
changed the title
ModInstance type in mod deployment code (2/2)ModInstance type in mod deployment code (2/3)
There was a problem hiding this comment.
This is a new file (the other file was deleted) — GitHub getting provenance wrong
| @@ -258,3 +258,18 @@ export type PackageVersionUpdates = { | |||
|
|
|||
| export type OrganizationAuthUrlPattern = | |||
| components["schemas"]["OrganizationAuthUrlPattern"]; | |||
|
|
|||
| /** | |||
There was a problem hiding this comment.
Moved to contract because the fields are defined by the backend. (Our Swagger is missing the exact shape of this part of the POST request 🤷 )
| * | ||
| * @see mapModComponentDefinitionToActivatedModComponent | ||
| */ | ||
| export function mapModInstanceToActivatedModComponents( |
There was a problem hiding this comment.
Adapter going in the reverse direction
twschiller
changed the title
ModInstance type in mod deployment code (2/3)ModInstance type in mod deployment code (2/3)
| deployments: modComponentsToDeactivate | ||
| .map((x) => x._deployment?.id) | ||
| .filter((x) => x != null), | ||
| deployments: uniq( |
There was a problem hiding this comment.
Why would there be duplicate mods?
There was a problem hiding this comment.
I think you mean deployment ids? There shouldn't be ever the same deployment id across mod instances. This is just slightly more defensive. But I agree we could drop the uniq
| .map((x) => x._deployment?.id) | ||
| .filter((x) => x != null), | ||
| deployments: uniq( | ||
| unassignedModInstances |
There was a problem hiding this comment.
See discussion here: https://github.com/pixiebrix/pixiebrix-extension/pull/9190/files#r1778725821
What does this PR do?
ModInstancetype and adapter applied to re-/activation code (1/3) #9182ModInstanceinstead of mod componentsFuture Work
ModInstancetype and adapter applied to re-/activation code (1/3) #9182For more information on our expectations for the PR process, see the
code review principles doc