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

Feature: Published Document Pending Changes #17812

Merged
merged 83 commits into from
Dec 18, 2024

Conversation

madsrasmussen
Copy link
Contributor

@madsrasmussen madsrasmussen commented Dec 13, 2024

Sorry for the big PR 🙈

This PR reimplements the Published Pending Changes feature (changes that have been saved but not yet published) on the client side. We do this by comparing the published version with the saved version.

The PR introduces a Document Publishing Workspace Context that is now responsible for all logic/states around publishing. It has for a long time been a wish to decouple the publishing logic from the main document workspace, to keep a better separation of concerns.

This means that all methods related to publishing are moved to this context (save and publish, schedule, unpublish, etc.). This is all done in a backward-compatible manner where a package still can call the methods on the Document Workspace but will receive a deprecation warning in the console. The methods have also been deprecated in jsDocs.

As the new thing for this feature, the publishing context also holds a state of variants with changes that anyone can subscribe to. We currently use it in the Variant selector and the info view. Right now the state is squeezed in between the states the server returns. Moving forward we should look into the possibility of a document having multiple states instead of only one. Instead of a Published (pending changes) state, it should be two states "Published" and "Unpublished Changes". This will allow for better extendability for others.

The many changes in this PR are also caused by collecting most of the files related to the publishing features into one feature folder. You will now find a feature folder inside the document module called "Publishing". This folder includes subfeatures like "Publish", "Schedule Publish", etc. Each feature folder now includes entity actions, workspace actions, models, etc that are related to that feature.

How to test:
Please test across variant, invariant documents that we can detect when there are Published Pending Changes.

@iOvergaard
Copy link
Contributor

Exciting new things. I just checked with Copilot for potential breaking changes as I had one concern about the PublishableWorkspaceContext interface. It found that and pointed out a few more places, so I'm just going to copy the text here. Would you check them to see if you agree?

--

Here are some potential breaking changes on public APIs or interfaces introduced by this pull request:

  • File: src/Umbraco.Web.UI.Client/src/packages/core/content/repository/content-validation-repository.interface.ts

    • Added a new interface UmbContentValidationRepository, which defines methods for validating content creation and saving.
  • File: src/Umbraco.Web.UI.Client/src/packages/core/content/workspace/content-detail-workspace-base.ts

    • Multiple modifications including new methods and properties related to content validation and submission.
    • Deprecated and added new methods for handling content validation and saving.
  • File: src/Umbraco.Web.UI.Client/src/packages/core/workspace/contexts/tokens/publishable-workspace-context.interface.ts

    • Removed the publish method from the UmbPublishableWorkspaceContext interface.

@madsrasmussen
Copy link
Contributor Author

Ah, very interesting approach @iOvergaard

I can see two of them aren't part of this PR but something I temporarily included in this PR. These changes are now gone after I merged the latest dev into this. Let's not hope they were breaking changes 🙈

I guess the UmbPublishableWorkspaceContext is technically a breaking change even though it lowers the requirements for the interface and our implementation was just throwing an error. I have reverted the change again. Then we can do this cleanup at the right time. Thanks!

@iOvergaard iOvergaard enabled auto-merge (squash) December 16, 2024 09:48
Copy link
Contributor

@iOvergaard iOvergaard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found a discrepancy in the UI where I have the following setup:

en-US: Published (pending changes)
da-DK: Published (pending changes) but I only clicked the Save button

The variant selector informs me that da-DK is published and has pending changes, but I never clicked the publish button - I only ever clicked the save button on that variant.

The Links and General boxes inform me (correctly, I presume?) that the item is not yet published (unpublished).

Untitled

What is correct? Is the da-DK variant published when the default variant en-US is published? Or is the variant selector wrong?

@madsrasmussen
Copy link
Contributor Author

@iOvergaard Good catch.

I have pushed a fix for this! We had some states that we were not currently clearing when moving from one document to another. If you navigated from a published document to an unpublished document, we still carried the published document data. I have added a check so that we will never compare two documents that don't have the same ID and fixed the states.

Copy link
Contributor

@iOvergaard iOvergaard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@iOvergaard iOvergaard merged commit e8c4fb9 into v15/dev Dec 18, 2024
27 of 29 checks passed
@iOvergaard iOvergaard deleted the v15/feature/published-pending-changes branch December 18, 2024 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants