Add Umbraco Deploy support for SeoToolkit - #557
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…eleton Implements core foundation types for SeoToolkit Deploy: - SeoToolkitDeployConstants with 8 GuidUdi entity types and RootKeyValuesGuid - SeoToolkitArtifactDependency helper for Exist/Ordering defaults - SeoToolkitDeploySettings bound from config section - Composer/Component skeleton with UdiParser registration Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Also fixes ScriptManagerService.Save so a script arriving with a preset Key that doesn't exist in the target is inserted (preserving the Key) rather than silently no-op updated — required for Deploy/uSync transfers to work. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Also fixes SeoDomainsRepository.Save to preserve a caller-supplied collection Id on insert (instead of generating a new Guid), so domain collections keep their identifier when transferred via Deploy. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ectors Adds IMetaFieldsValueRepository.GetAllValues(Guid) and GetAllNodeKeys() to enumerate per-node values across cultures for transfer. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Uses ArtifactExportingNotification (fires before serialization) rather than ArtifactExportedNotification — the export pipeline serializes the artifact between the two, so dependencies must be added on the -ing notification. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds ScriptSaved/Deleted, SeoDomainCollectionSaved/Deleted and SeoKeyValueSaved notifications published from the ScriptManager/Domains services and the key-value controller, plus disk-refresher handlers that (re)write .uda artifacts on change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
PR Summary by QodoAdd Umbraco Deploy support for SeoToolkit settings and per-node SEO data
AI Description
Diagram
High-Level Assessment
Files changed (93)
|
Code Review by Qodo
Context used✅ Compliance rules (platform):
8 rules 1. SeoToolkit.Umbraco.Deploy missing ManifestLoader
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
|
Hi @robertjf |
|
I'll be thoroughly testing the new code today, but it is largely based on the Commerce Deploy project and uses notifications to write the seo record as a dependency rather than try to inject it into the content - it also picked up a few bugs in the existing code. Anyway, I'll do a full review and end-to-end test today on an actual project |
Resolves qodo review findings on PR patrickdemooij9#557: - Add ManifestLoader to the Deploy package (convention parity with the other packages); registers a package manifest, no backoffice entry point. - Export domain-scoped scripts, not just global ones, by enumerating each domain collection in the script connector (deduped by key). - Optimise per-node value enumeration: GetAllNodeKeys uses SELECT DISTINCT NodeKey, and a new HasAnyValues(Guid) COUNT query backs the content export dependency check (no more loading/deserialising all values). - Add opt-in convergent restores via SeoToolkit:Deploy:PruneMissing (default off, preserving documented overwrite-only behaviour). When on, key/values and meta field values delete target-only data and meta field settings rebuild from a fresh DTO (dropping removed fields/inheritance). Adds test coverage for domain-script export and prune-on/off behaviour. README updated for the new setting. 89/89 tests pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Follow-up on the informational findings (#6–#8, 7687c29)#7 #8 Export handler deserialises to count — Fixed. Added #6 Tests under All 89 tests pass. |
Follow-up addressing the "custom dependencies on existing artifacts" limitation discussed in the Umbraco forum/Deploy issues: - Ride-along per-node dependencies now use ArtifactDependencyMode.Match instead of Exist, so Deploy compares the artifact checksum and re-transfers per-node SEO data whenever it changes (Exist only ensured presence and left stale values on the target after the first transfer). - Register MetaFieldsValue and SitemapContent as disk entity types so their .uda is written/read per node when the node is exported, rather than only resolving during server-to-server transfers. - Removing some of a node's MetaFields values now converges on the target under PruneMissing (an empty artifact clears all remaining values). Documented the full-removal boundary case in the README. 90/90 tests pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Per-node dependency reliability (fb56518)Reviewed two long-standing reports about attaching custom data to existing Deploy artifacts:
Both conclude it wasn't possible to attach third-party per-node data to a content artifact. That capability now exists (
Runtime verification note: the |
Closes the full-clear convergence gap for per-node MetaFields values: - Add MetaFieldsValueChangedNotification, published by MetaFieldsValueService whenever a node's values are added/updated (AddValues) or removed (Delete). - Add MetaFieldsValueDiskRefresherHandler: rewrites the node's .uda while it still has values and deletes it once the node is emptied, so removing all of a node's SEO values now propagates as a delete on restore instead of leaving stale data on the target. Tests cover both disk-refresher branches and the service publishing the notification on add and delete. README updated. 94/94 tests pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Per-node MetaFields value convergence — full fix (82e6566)Closed the full-clear boundary noted in the previous comment (no longer just documented):
This makes per-node values behave like the settings entities on the disk model, so convergence no longer depends on the ride-along dependency being present at export time. Notes:
Tests cover both disk-refresher branches (write when values remain, delete when emptied) and the service publishing on add/delete. 94/94 pass. The disk write/delete + |
|
New changes have been pushed pending an audit by Claude Fable - here are the findings: SeoToolkit.Umbraco.Deploy — Audit FindingsSource-code audit of the Deploy package (branch 1. MetaFields settings with an image field break on export (HIGH)
Where:
Problem: On import the connector runs Text and Fix:
For media this yields a 2. Target-side writes bypass cache invalidation and notifications (MEDIUM-HIGH)
Where:
Problem:
Fix: 3. Media referenced by SEO values gets no artifact dependency (MEDIUM)
Where:
Problem: Fix: 4. Artifact checksums built from unordered dictionaries (MEDIUM)
Where:
Problem: Fix: 5. Sitemap-content
|
Follow-up code review — 8 findings, 7 fixedRan a second-pass high-effort review over the deploy-connector diff. It found that two items from the earlier audit were only partially resolved, plus six issues the first audit missed. Fixes are in Correctness fixes
Findings 1, 4 and the culture-match issue (5) are code-traced; the rest are reproducible under specific data/config states (orphaned rows, legacy NULL cultures, disabled connectors). Notes
|
Follow-up fixes found while diagnosing why SeoToolkit schema failed to
import (and kept re-flagging as "changed") on a self-hosted restore.
* Session scope poisoning (the import blocker): repository read/delete
methods created scopes with CreateScope() but never completed them.
Nested inside Deploy's session scope this poisoned the ambient scope
("Failed to complete the session because its scope would not complete").
Read/delete scopes now use CreateScope(autoComplete: true), matching the
sibling repositories. Affects MetaFieldsSettingsDatabaseRepository,
MetaFieldsDatabaseRepository and SeoDomainsRepository (the metafields
Delete scopes also never completed, so deletes silently rolled back).
* Per-node types are content-coupled, not schema: MetaFieldsValue and
SitemapContent are no longer registered as disk entity types (that wrote
a schema .uda per content node). They travel with their document as Match
dependencies via SeoToolkitContentExportingHandler. Removed their disk
refresher handlers.
* Schema converges like core artifacts: removed the PruneMissing setting.
Collection-valued connectors (key/values, metafields settings/values) now
reconcile unconditionally, replacing the target to match the artifact --
otherwise target-only rows meant the checksum never matched the source.
* ListValueConverter (keywords): ConvertDatabaseToObject now always returns
a non-null array, so null and empty-string forms both export as "[]" and
converge instead of flipping between "[]" and an omitted field.
* Disk refreshers refresh signatures: SeoToolkitDiskRefresherHandlerBase now
calls ISignatureService.SetSignature/ClearSignature alongside the disk
write, matching EntitySavedDeployRefresherNotificationAsyncHandlerBase.
* Cache entity resolution via IContextCache in the base connector, per the
Deploy extending docs.
Adds MetaFieldsSettingConvergenceTests; updates existing tests for the
removed PruneMissing parameter, signature refresh, and PassThroughCache.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Pushed a set of follow-up fixes to the deploy connector (dd51a9e), found while testing a self-hosted restore where SeoToolkit schema wouldn't import. Root cause of the import failure was mundane but fatal: a few repository read/delete methods opened scopes with While tracking that down I also cleaned up a few related things that caused entities to re-flag as "changed" on every deploy:
New |
|
@patrickdemooij9 the latest patch has been tested on a real project and is working well for deploying schema changes (settings, etc.) using Deploy, but it currently doesn't transfer the actual metadata values etc. which I'm now working on. Because there is no way to hook into a content item transfer currently (I'll make a feature request), we can't just ride along with the content item transfer unless we add something like a property to the content item so that it gets pulled in as a dependency. So the current alternative is to add actions to the content action menu and the publish button (sub-action) to transfer/queue the seo meta values independently. |
Move the MetaFieldsValue / SitemapContent per-node types from the content-export coupling approach to first-class transfer/restore entities, and add the backoffice UI to drive transfer/queue/restore. - Register per-node types as transfer entities (SupportsImportExport false so they never write a schema .uda); refresh their Deploy signature on change via new signature-refresher handlers. - Remove the now-dead SeoToolkitContentExportingHandler and its tests. - Add SeoToolkitEntityServiceConnectorBase.AllowMissingEntity so a node with no SEO data of a given kind is a queue/transfer no-op, not a throw. - Add SeoToolkitDeployController (seoTransferItems) so the client can discover which per-node SEO entities exist for a node. - Add the Vite/TS backoffice frontend: Transfer SEO Now, Add SEO to Transfer Queue, and Restore SEO actions; register the entry point in ManifestLoader and switch the project to the Razor SDK. - Fix InvalidProgramException on transfer: MetaFieldsDatabaseRepository now uses the generic Delete<T>(Sql) overload instead of the non-generic Delete(object) that NPoco mis-binds. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Update: per-node SEO as transfer entities + backoffice actionsReworked how the per-node types ( Architecture change
Backoffice UI (new Vite/TS frontend)
Bug fix
|
Restore SEO: open Deploy's own partial-restore dialog for source-environment
selection and drop the forced ignoreDependencies flag. Forcing it returned a
500 ("Ignoring dependencies is not allowed") when the environment disallows
it; unnecessary because the SEO artifacts depend on the document in Exist mode,
so a normal restore never touches document content.
Transfer SEO Now: deploy to the target's deployUrl instead of umbracoUrl.
umbracoUrl is the backoffice URL, so opening the remote Deploy session against
it 404'd -> RemoteApiException ("The remote API was not found") in
SourceDeployWorkItem.
Add SEO To Transfer Queue: route through Deploy's DeployTransferQueueManager
context (add()) instead of a raw POST to /queue/add, so the transfer queue
widget refreshes (reload + broadcast + entity signs), matching the native
"Add to Transfer Queue" action. Removed the now-dead queueAdd client method.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Follow-up: fix SEO transfer/restore backoffice actions (
|
|
@patrickdemooij9 this is now ready for review - I've fully tested it on a real project and it's working well. Transferring / Restoring SEO metadata/robots etc. can't happen when triggered by a node transfer, so I've had to add to the Publish Button and the Content Action menu: 
The transfer mechanism currently transfers the SEO metadata / robots and the content node itself - an acceptable workaround, but we could change this to only queue the SEO metadata/robots. Ideally, we wouldn't need these additional actions, but there's currently no way to hook into that transfer that I've found so far! |
Code Review by Qodo
Context used✅ Compliance rules (platform):
8 rules 1.
|
|
Code review by qodo was updated up to the latest commit f646876 |
|
Code review by qodo was updated up to the latest commit 0ae23a6 |
Ah yeah, that was the same issue that I ran up against. But I never thought of working around it like that! I think that's perfect for now until they make a change where we can transfer it together with the content. |
…descendants Transfer and queue SEO entities without moving the content node itself (the SEO artifacts depend on the document in Exist mode only, so Deploy just requires the node to exist upstream). The tree "Add SEO to Transfer Queue" action now reuses Deploy's native queue dialog (Deploy.Modal.Queue) to capture the "include descendants" choice, and queuing runs server-side in a single request: a new POST seoQueueAdd endpoint enumerates the node (and every descendant when requested) and adds each SEO entity to the transfer queue via ITransferQueue — so the client makes one call and refreshes the queue widget once, instead of one queue/add per item. Adds SeoToolkitDeployControllerTests covering single-node queuing, descendants, and the unauthorized case. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Update: SEO transfer independent of the content node + optional descendants (server-side)Pushed 1. SEO transfers independently of the content node. Both "Transfer SEO Now" (instant) and "Add SEO to Transfer Queue" now move only the SEO entities, never the document node. This is safe because the SEO artifacts declare only an Exist-mode dependency on the document — Deploy requires the node to already exist upstream but never re-transfers its content. 2. Optional descendants when queuing — done server-side in one call. The tree "Add SEO to Transfer Queue" action reuses Deploy's own queue dialog (
Tests: new Notes:
|
|
@patrickdemooij9 any chance you might be able to merge this in at some stage? :) |
|
Hi @robertjf I'll see if I can fix it myself, but just giving you a headsup in case you know the answer ============ Managed to find the issue. I didn't have Umbraco deploy installed on the site. I would have thought the package would add that as a reference, but good to know that it doesn't |
| namespace SeoToolkit.Umbraco.Deploy.Connectors.ServiceConnectors | ||
| { | ||
| [UdiDefinition(SeoToolkitDeployConstants.UdiEntityType.Script, UdiType.GuidUdi)] | ||
| public class SeoToolkitScriptServiceConnector( |
There was a problem hiding this comment.
For the uSync implementation, I actually didn't add scriptmanager to the syncs as I think each environment will probably have different settings for their scripts anyway. Do you have a different view on that? We could still keep it here though as people are able to turn it off if they don't want it
| restoreNodes: udis.map((udi) => ({ udi, includeDescendants: false, selector: "this" })), | ||
| }), | ||
| }); | ||
| } |
There was a problem hiding this comment.
Do we need to use this seoDeployClient? Can't we use the hey-api like in the other projects so we have typesafety on the endpoints?
| // Order names/settings (ordinal) for a stable serialization/checksum. | ||
| var domainNames = entity.DomainIds | ||
| .Select(id => allDomains.FirstOrDefault(d => d.Id == id)?.DomainName) | ||
| .Where(name => name is not null) |
There was a problem hiding this comment.
I think you could use .WhereNotNull() here so you don't have to do .Where().Select()
| keyValueRepository.Set(key, value, domainId); | ||
| } | ||
|
|
||
| // The repository write skips the controller's notification, so publish it here — that |
There was a problem hiding this comment.
Should we perhaps move the notification handler to the repository then? I think that makes more sense than having it only trigger in the controller
| var json = value is null ? null : JsonConvert.SerializeObject(value); | ||
|
|
||
| // UDIs embedded in the value JSON (e.g. RTE-like content). | ||
| foreach (var referencedUdi in UdiJsonHelper.FindUdis(json)) |
There was a problem hiding this comment.
I am trying to think in which case we have Udis inside of the content. I can only think of the media one and you already have that covered below
| } | ||
| } | ||
|
|
||
| // We wrote straight to the repository (rather than through the service's culture-aware |
There was a problem hiding this comment.
Do you know why we can't go through the service class and have to go straight to the repository?
|
Hi @robertjf Thanks again for the great work you did! I am sorry it took a bit for me to go through it but I have finally done it (it did take some hours :P) I'll also let Claude do a review through the code to see if it spots anything that I might have missed, but I think the above points will be the main things to still look at. Do let me know if you need any help with and I am more than happy to help out! |
2 participants
Summary
Adds a new
SeoToolkit.Umbraco.Deploypackage that lets Umbraco Deploy transfer SeoToolkit settings and per-node SEO data between environments — via.udadisk artifacts, queue-for-transfer/restore, and content import/export.Implemented from the plan at
docs/superpowers/plans/2026-07-14-seotoolkit-deploy-connector.md(12 tasks, each committed separately, TDD throughout).Resolves #232
What's included
GuidUdiconnectors (allseotoolkit-prefixed): SEO enable toggle, MetaFields doc-type settings, sitemap page-type settings, scripts, domain collections, key/values, plus per-node MetaFields values and sitemap content overrides.ArtifactExportingNotification, so it travels automatically with content transfers..uda) integration: settings artifacts are (re)written on save/delete via new Core notifications and disk-refresher handlers.SeoToolkit:Deploy:DisabledEntityTypes.Core changes (additive, required for transfers to work)
ScriptManagerService.Savenow inserts (preserving Key) when a script arrives with a preset Key not present in the target, instead of a silent no-op update.SeoDomainsRepository.Savepreserves a caller-supplied collection Id on insert instead of generating a new Guid.IMetaFieldsValueRepositorygainsGetAllValues(Guid)andGetAllNodeKeys()to enumerate per-node values across cultures.IEventAggregator).Notable deviation from the plan
The ride-along handler uses
ArtifactExportingNotification, notArtifactExportedNotification. The export pipeline (ArtifactImportExportService) serializes the artifact between the two notifications, so dependencies added on the-ednotification would never reach the exported artifact. Recorded in the plan file.Verification
dotnet build src/SeoToolkit.Umbraco.sln— succeeds, 0 errors.dotnet test ... --filter "FullyQualifiedName~Deploy"— 23/23 pass.dotnet pack— producesSeoToolkit.Umbraco.Deploy.1.0.0-beta1.nupkg.Runtime end-to-end verification against a real Deploy environment (checksum/dependency-graph behaviour, pass-7 scheduling) remains a manual follow-up, as noted in the plan.
🤖 Generated with Claude Code