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.

Sign up for GitHub

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

Jump to bottom

Variant ContentSavedState incorrectly updated for failed publish #11290

Closed
p-m-j opened this issue Oct 5, 2021 · 2 comments
Closed

Variant ContentSavedState incorrectly updated for failed publish #11290

p-m-j opened this issue Oct 5, 2021 · 2 comments
Labels
release/8.18.0 release/9.1.0 type/bug

Comments

@p-m-j
Copy link
Contributor

p-m-j commented Oct 5, 2021 •
edited

Which exact Umbraco version are you using? For example: 8.13.1 - don't just write v8

9.0.0

Bug summary

Attempting to save and publish a variant which has a scheduled release incorrectly updates the variants ContentSavedState.
Reproducible without VaryByCulture on single language site.

Specifics

No response

Steps to reproduce

  1. Find or create a 9.0.0 site with at least one document type
  2. Add a textstring property named heading (doesn't reproduce without a property)
  3. Create a new content node
  4. Save and publish (can't repro from draft)
  5. Make a small edit
  6. Click schedule (drop down on save and publish button)
  7. Select a date for "Publish At"
  8. Click Schedule
  9. Open info tab note status "published (pending changes)" with "publish at" date set
    image
  10. Click save and publish
  11. Note warning shows (could not publish, pending release) however ContentSavedState state is now "published" instead of "published (pending changes)"
    image

Expected result / actual result

ContentSavedState state shouldn't update, doesn't occur in v8

This item has been added to our backlog AB#14305
@p-m-j p-m-j added the type/bug label Oct 5, 2021
@p-m-j p-m-j changed the title v9 PublishedState updated for failed publish v9 PublishedState incorrectly updated for failed publish Oct 5, 2021
@p-m-j p-m-j changed the title v9 PublishedState incorrectly updated for failed publish v9 variant ContentSavedState incorrectly updated for failed publish Oct 6, 2021
@bergmania bergmania added the state/sprint-candidate We're trying to get this in a sprint at HQ in the next few weeks label Oct 6, 2021
@p-m-j
Copy link
Contributor Author

p-m-j commented Oct 8, 2021

This bug is also present in 8.15.

Just to add confusion, if the nodes name differs in the published vs draft content (dbo.umbracoContentVersion) then the feature works as intended which is how I ended up thinking it worked in v8.

Great investigation work from @Zeegaan to establish this.

@p-m-j
Copy link
Contributor Author

p-m-j commented Oct 8, 2021 •
edited

Reminder

This breaks in ContentService.SaveAndPublish before call to CommitDocumentChangesInternal.
content.PublishCulture(impact) copies the new value in content.Properties over the variant properties PublishedValue.

Later when comparing PublishedValue to EditedValue no change is detected.

Can we create a slimmed down StrategyCanPublish check that only considers trashed, Expired & AwaitingRelease.
When falsey save and bail without calling CommitDocumentChangesInternal

p-m-j pushed a commit that referenced this issue Oct 12, 2021
Resolve incorrect ContentSavedState for failed publish
f6f5723 
Closes #11290 (for v8)
@p-m-j p-m-j mentioned this issue Oct 12, 2021
Merged
@Zeegaan Zeegaan closed this as completed Oct 12, 2021
@Zeegaan Zeegaan mentioned this issue Oct 12, 2021
Merged
@umbrabot umbrabot removed the state/sprint-candidate We're trying to get this in a sprint at HQ in the next few weeks label Oct 12, 2021
@nul800sebastiaan nul800sebastiaan changed the title v9 variant ContentSavedState incorrectly updated for failed publish Variant ContentSavedState incorrectly updated for failed publish Oct 19, 2021
bergmania added a commit that referenced this issue Oct 20, 2021
@bergmania @bjarnef
@cstrijkert @soreng @nathanwoulfe @patrickdemooij9 @callumbwhyte @BatJan @skttl @Shazwazza @nul800sebastiaan @ronaldbarendse @OwainJ @nikolajlauridsen @lordscarlet @ZioTino @Zeegaan @madsrasmussen @elit0451 @abjerner
Merge v8/dev 20-10-2021 (#11426)
56595a7 
* Adjust icon in umb-checkbox and ensure icon is centered

* Missing nl translation for blockEditor_addBlock

* Implement icon parameter for doctype editor (#11008)

* fix: implement icon parameter for doctype editor

issue #10108

* fix: move color from icon to class attribute

* fix: removed defined colors, defaulting to the standard dark grey (ie "no color picked" in icon picker)

* cleaned up unused dependencies, double quotes to single, removed unused 'color' param from the create methods, and use shorthand object creation in createDocType (if the key has the same name as the variable passed as a prop, we only need to pass the key name)

* fix comment

Co-authored-by: Nathan Woulfe <nathan@nathanw.com.au>

* Align sortable handle vertically in multivalues prevalue editor

* 10341: Use different picker for content types (#10896)

* 10341: Use different picker for content types

* use es6 where possible (inc removing underscore for teeny tiny performance improvement)

Co-authored-by: Nathan Woulfe <nathan@nathanw.com.au>

* Falling back to contentTypeName when Block List label is empty (#10963)

* Falling back to contentTypeName when Block List label is empty

* Adding $contentTypeName variable for Block List labels

* Fix incorrect attribute

* Grid: Add button styling fix (#10978)

* Add missing focus styling

* Ensure add button is perfectly rounded and remove unused / uneeded CSS.

* Remove redundant border-color property

* Revert removal of unused css

Co-authored-by: BatJan <jaskov@gmail.com>
Co-authored-by: Jan Skovgaard Olsen <jso@co3.dk>

* Create content template localization (#10945)

* Don't use self-closing element for custom HTML elements

* Use button element for close/cancel in copy dialog

* Update localization of "createBlueprintFrom"

Co-authored-by: Nathan Woulfe <nathan@nathanw.com.au>

* Cleanup examine search results, and adds ability to toggle fields (#9141)

* Cleanup examine search results, and adds ability to toggle fields

* update table to use joinarray filter with one-time binding to avoid recalculating filter values, updated filter to not explode when array arg is null

* fix failing tests - improve filter to not fail on non-array params, update tests accordingly

Co-authored-by: Nathan Woulfe <nathan@nathanw.com.au>

* Add EntityController GetUrlsByUdis

Enables loading multiple URLs in a single request for Media & Documents

* Update content picker to use GetUrlsByUdis

* Allows replacing MainDom with alternate DB

There are some cases where there is a complex hosting strategy and folks want a readonly database and are hosting on Azure. In that case, it is not entirely possible to have a readonly Umbraco database because SqlMainDom is required and part of that requirement is to have read/write access to the umbraco key value table.
This PR allows for the default MainDom to be replaced and to allow for an SqlMainDomLock to use an alternate connection string so that a separate read/write database can be used.

* Remove inherited property group id/key when local properties are added (#11231)

* Remove inherited property group id/key when local properties are added

* Rebind saved content type values

* Remove inherited from save group

* Rename parameter for clarity

* Removes annoying wait text, which causes layout jank

* v8: Backoffice Welsh language translation updates (#11240)

* Updated the Welsh language file to include newly added keys (based on the en us language file)

* Updated the searchInputDescription key

* Updated the endTitle key

* Use medium sized overlay

* Use umb-icon component for icons in content type groups and tabs

* fixes wrong reference to enterSubmitFolder method in ng-keydown

* 11251: Don't add default dashboard to url

* Fix preview of SVG when height and width not are set

* If caching a published document, make sure you use the published Name… (#11313)

* If caching a published document, make sure you use the published Name. Closes #11074.

* Fix case of new node

Co-authored-by: Moore, Douglas S <Douglas.Moore@opm.gov>

* Added missing Italian translations (#11197)

* Resolve incorrect ContentSavedState for failed publish

Closes #11290 (for v8)

* add modelValue validation for server to correctly update validation errors

* 11048: Bugfix for groups and properties that get replaced (#11257)

(cherry picked from commit 1605dc1)

* Icon fallback to `icon-document` for existing document types (#11283)

* Align create buttons styling (#11352)

* Added button for cancelling dictionary create action

* Use hideMenu

* Align dictionary create with the other creates

* Align import documenttype

* Align for data type folder create

* Align document type create buttons

* Forgot small ng-show

* Align create media folder buttons

* Align create macro buttons

* Align create relation buttons

* Align create partial view macro folder buttons

* Align partial view folder create buttons

* Align create scripts folder buttons

* Align create scripts folder buttons

* Use primary instead of success

* V8: Duplicate MemberGroup names cause MemberGroup mixup (#11291)

* Prevented duplicate member group names

* Added English lang

* Updated 'Exist' typo

* add labels in FR and NL

* Adding property group aliases to ex.message

* Adding invalid prop group aliases as ModelState errors, so we don't introduce breaking changes

* Pointing the actual reason for invalidating composition

* Validate all content type dependencies and throw a single InvalidCompositionException

* Rename based on review comments

* Update composition validation error messages

* Update InvalidCompositionException message

* Allow switching property editor from numeric to slider (#11287)

* Make it possible to change from numeric/decimal property editor to slider without breaking editor

* Formatting

* Enables friendly pasting in multipletextbox

* UI API docs: Added reset rules for .close class

* UI API docs: Fixed incorrect method name

* 11331: Check property on instance if id is not set yet

* Fixed cypress tests

Co-authored-by: Bjarne Fyrstenborg <bjarne_fyrstenborg@hotmail.com>
Co-authored-by: Corné Strijkert <cornestrijkert@hotmail.com>
Co-authored-by: Søren Gregersen <soreng@gmail.com>
Co-authored-by: Nathan Woulfe <nathan@nathanw.com.au>
Co-authored-by: patrickdemooij9 <patrickdemooij98@hotmail.com>
Co-authored-by: Callum Whyte <hey@callumwhyte.com>
Co-authored-by: Jan Skovgaard <1932158+BatJan@users.noreply.github.com>
Co-authored-by: BatJan <jaskov@gmail.com>
Co-authored-by: Jan Skovgaard Olsen <jso@co3.dk>
Co-authored-by: Søren Kottal <sk@ecreo.dk>
Co-authored-by: Paul Johnson <pmj@umbraco.com>
Co-authored-by: Shannon <sdeminick@gmail.com>
Co-authored-by: Sebastiaan Janssen <sebastiaan@umbraco.com>
Co-authored-by: Ronald Barendse <ronald@barend.se>
Co-authored-by: Owain Jones <owain.jones@method4.co.uk>
Co-authored-by: Mole <nikolajlauridsen@protonmail.ch>
Co-authored-by: Doug Moore <lordscarlet@idledreams.net>
Co-authored-by: Moore, Douglas S <Douglas.Moore@opm.gov>
Co-authored-by: Martino Gabrielli <39855999+ZioTino@users.noreply.github.com>
Co-authored-by: Nikolaj Geisle <70372949+Zeegaan@users.noreply.github.com>
Co-authored-by: Mads Rasmussen <madsr@hey.com>
Co-authored-by: Jamie Townsend <jamie.townsend@unitstack.co.uk>
Co-authored-by: Elitsa Marinovska <elm@umbraco.dk>
Co-authored-by: Anders Bjerner <abjerner@skybrud.dk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
release/8.18.0 release/9.1.0 type/bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@nul800sebastiaan @bergmania @p-m-j @umbrabot @Zeegaan