Repository navigation
Support in-place publishing page transformation - #1290
Merged
Merged
Conversation
1 of 3 tasks
TaoziZ03
marked this pull request as ready for review
October 12, 2026 02:05
Restore the ArgumentException that both PublishingPageTransformator constructors threw for a null targetClientContext. Aliasing null to the source context silently turned existing callers that relied on that guard into in-place transformations against their source Web. In-place is now opted into explicitly through the CreateInPlace factory overloads, which pass the source context as the target. Transform() already routes that to SameWeb through the site and Web identities, so no additional flag is needed. Signal a missing target page with PageDoesNotExistInWebException instead of matching LogStrings text. LogStrings are English-only constants that are documented as pending localization, so the previous Message.EndsWith check would have silently stopped routing in-place collisions. The new type derives from ArgumentException, so existing handlers that test for ArgumentException keep their current behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e36329ef-32f5-4a3e-945a-ecc5f1f68f9d
YifengMao
approved these changes
Oct 15, 2026
Saving a client side page appends the .aspx extension when it is missing, while the collision probe, the save call and the readback all used the caller supplied target page name verbatim. A name such as 'existing' therefore probed /SitePages/existing, reported the target as free, and then saved over the existing /SitePages/existing.aspx, because the save path has no overwrite concept of its own. The readback used the uncanonicalized name again, so the transformation reported a failure after the existing page had already been updated. Canonicalize TargetPageName once, directly after the source page default is applied, so the probe, the save and the readback all resolve to the same file. This also closes the same hole on the cross site path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e36329ef-32f5-4a3e-945a-ecc5f1f68f9d
The same uncanonicalized name that let a publishing transformation overwrite an existing target page also reaches PageTransformator. Its collision probe, its save call and its readback each use TargetPageName verbatim, so a cross site conversion with a name such as 'existing' probed /SitePages/existing, reported the target as free, then saved over /SitePages/existing.aspx and failed on the readback. Move EnsureAspxExtension to BasePageTransformator and apply it in PageTransformator directly after the target name default is resolved, so both transformators canonicalize once before the probe. Verified live against a publishing enabled tenant: before this change a cross site wiki conversion without -Overwrite replaced the target page content and then reported a failure; after it, the collision is rejected and the target page is byte identical. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e36329ef-32f5-4a3e-945a-ecc5f1f68f9d
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Publishing pages can be transformed to modern Site Pages in the source Web or in a supported target in another site collection. The transformation mode is determined from the actual Site and Web identities.
In-place transformation is an explicit opt-in through the new
PublishingPageTransformator.CreateInPlacefactory overloads, which supply the source context as the target. Both existing constructors still reject a nulltargetClientContext, so current callers keep their present behavior.Same-Web conversion requires an existing writable Site Pages library, retains the source publishing page, and rejects existing targets and inconclusive collision probes before the first target-page operation. Different Webs within the same site collection remain outside the supported target scope. Cross-site permission, naming, and overwrite behavior is preserved.
A missing target page is signaled with the new
PageDoesNotExistInWebExceptioninstead of matchingLogStringsmessage text. Those constants are English-only and the file documents them as pending localization, so a text match would have silently stopped in-place collision routing. The new type derives fromArgumentException, so existing handlers that test forArgumentExceptionare unaffected.Image mappings emit numeric
-1defaults when file dimensions are absent or the referenced file cannot be found. Missing files retain their original references and produce a warning instead of leaving unresolved numeric mapping tokens. This lets conversion continue; it does not restore a missing image.Modernization loads root-Web metadata through an independent CSOM object query. This avoids a service-side
target-null failure observed when RootWeb is expanded through Site.PowerShell consumer: pnp/powershell#5490. That PR still constructs the transformator with a null target when no target parameter is supplied, and needs a follow-up change to call
CreateInPlaceinstead. It must not merge before this change ships in a PnP.Framework release.No
CHANGELOG.mdentry is included:src/lib/CHANGELOG.mdstates that the file is maintained by the repo owner.Validation
netstandard2.0,net8.0,net9.0, andnet10.0: 0 errors; existing repository/compiler and NuGet warnings remain.PublishingPageInPlaceTransformationTestsandBuiltInImageLookupTestssuite passed: 27 passed, 0 failed, 0 skipped. Coverage includes theCreateInPlaceopt-in, null-target rejection by both constructors, the collision and rejected-target seams, and the missing-file case validating numeric JSON in both image mappings.ArgumentExceptioncall sites were re-checked against the new exception type:PageTransformatorstill matches it throughex is ArgumentException, and theCrossSiteTestsoverwrite assertions continue to observe a plainArgumentExceptioncarrying the existing message.CreateInPlaceproduces the same source-as-target state those runs exercised, so the observed in-place behavior is unchanged; the constructor change only affects callers that passed an explicit null.-TargetWebUrland-TargetConnection. Native editor save/exit passed.-PageLayoutMappingon this tenant. Its catalog item lacks PublishingAssociatedContentType, preventing automatic layout analysis. A locally validated mapping bound to PublishingPageContent and the actual Top/Bottom/Usage/Right zones converted both modes; all 16 checked body blocks persisted. The mapping is validation input, not a new built-in template.-Overwrite, missing Site Pages, and different-Web/same-site-collection negative cases were rejected without changing source/target files.git diff --checkpassed.Live acceptance is bounded to the tested tenant, existing writable Site Pages libraries, the stock Enterprise Wiki/ArticleLeft layouts, and the supplied IPKit_Home mapping. Draft creation and editor persistence do not establish complete asset, list, script, or visual fidelity.