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

Usync complete can't synchronised not-published nodes correctly #221

Open
bielu opened this issue Jan 31, 2024 · 11 comments
Open

Usync complete can't synchronised not-published nodes correctly #221

bielu opened this issue Jan 31, 2024 · 11 comments

Comments

@bielu
Copy link

bielu commented Jan 31, 2024

Describe the bug
Bug is related to synchronization of unpublished content, which was never published.

Version (please complete the following information):

  • Umbraco Version: 13.0.2
  • uSync Version 13.0.0
  • uSync.Complete Version 13.0.0

To Reproduce
Steps to reproduce the behavior:
1.Create node, add block editor, add content, save but not publish
2. try to synchronised it to other server

Current result
Node was created in other server, but was missing all properties!

Expected behavior
Node is created in other server with all properties.

@KevinJump
Copy link
Member

Hi,

It is a bug, but the other wat around - uSync.Publisher only syncronised published content, so ideally the default behavior would be the item does not get published at all.

i think what is happening is that our PublishedContentHandler, does only deal in published values, but it is syncing the undering content item, and then not setting the properies, ideally it should not publish the item!

(you can in theory change the behavior of uSync.Publisher by swapping out the PublishedContentHandler for the normal 'contentHandler' in config - i can dig that out if you need it).

@bielu
Copy link
Author

bielu commented Jan 31, 2024

HI Kevin,
if you can dig out this, it would be great!

@bielu
Copy link
Author

bielu commented Feb 5, 2024

@KevinJump did you had chance to look and fit bit of configuration which you mentioned?

@KevinJump
Copy link
Member

Hi,

yeah sorry. settings below.

  "uSync": {
    "Publisher": {
      "Handlers": {
        "DisabledHandlers": [
          "PublishedContentHandler",
          "ContentTemplateHandler"
        ]
      },
      "Settings": {
        "IncomingEnabled": true,
        "AppId": "YOUR-APP-ID-HERE",
        "AppKey": "YOUR-APP-KEY-HERE"
      }
    }
  }

So the default (when this isn't set) is to disable the content Handler and ContentTemplateHandler, so the "PublishedContentHandler" manages the content, this swaps that back.

the diffrence between the two handlers is small. PublishedHandelr uses the PublishedValue Content handler uses the EditedValue

@marcemarc
Copy link

Suggestion for when using the PublishedContentHandler, and there are later changes saved for the node, that it would be good to report a warning:
"Content includes Unpublished Changes, which have not been transferred"

The editor confusion is in the message "Completed without any issues at all, nothing to worry about here"

Not sure if empty nodes should be created though.

@bielu
Copy link
Author

bielu commented Nov 26, 2024

@KevinJump i had look into this issue and i have idea how to resolve it, but it would require breaking change... You already have method GetPropertyValue(IPropertyValue value) which decide how it synchronizing content, but for published it would be good to have context of item, so we can dynamically check if item is published and than use either item.PublishedValue or item.EditedValue
in this way I would we would be able to have dedicated handler specifically for publisher which handle it in this way when normal usync would Published/Content handlers.
What you think?

@KevinJump
Copy link
Member

there probibly a non-breaking way of adding this, (new method, calls old method by default). I think passing the content item and the options along at the same time would give us the most flexibility. I will take a quick look.

@KevinJump
Copy link
Member

(v15 version of this commit KevinJump/uSync@0fa8ec0)

will also add it back to v13 (but not v14 that is in support only now 🙂)

@KevinJump
Copy link
Member

KevinJump commented Nov 26, 2024

I might well also add this functionality as an option in publisher (now we pass it over) - I just need to think of the implications (and whether or not its worth documenting them)

*Sometimes it's better to leave the option in code, and then anyone using it will have explicitly understood what they are doing, vs changing an option and getting unexpected results.

@bielu
Copy link
Author

bielu commented Nov 26, 2024

@KevinJump it might be worth do similar things as we done in Translation manager, advance settings where we can do Optional Setting -> Include properties for unpublished content, and than you can modify PublishedContentSerializer to

  protected virtual object GetPropertyValue(TObject content, IPropertyValue value, SyncSerializerOptions options)
        =>content.Published || !settingEnabled  ? value.publishedvalue : value.EditedValue;

so it would kept old behaviour as published value if not enabled, but do edited value for unpublished items when enabled?

@KevinJump
Copy link
Member

yeah that is probably what the published serializer inside complete will end up looking like :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants