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

Variants dialog crashes when publishing (segment + no culture) #10274

Closed
PerplexDaniel opened this issue May 17, 2021 · 2 comments · Fixed by #10284
Closed

Variants dialog crashes when publishing (segment + no culture) #10274

PerplexDaniel opened this issue May 17, 2021 · 2 comments · Fixed by #10284

Comments

@PerplexDaniel
Copy link
Contributor

PerplexDaniel commented May 17, 2021

Umbraco version

I am seeing this issue on Umbraco version: 8.13.0

Bug summary

If a page has multiple variants that are varied only by segment (not by culture), the save/publish/unpublish dialog will crash when the user tries to perform a Save/Publish/Unpublish action.

The dialog will not render any variants and the console shows an error, see images below.

This was caused by the changes of PR #9797 which improves the sorting of variants in the specified dialog, specially in the sortVariants and getSortedVariantsAndSegments functions.
These functions directly access variant.language.{culture,isDefault,isMandatory} but do not account for the case where variant.language is simply null. That happens when culture variation is not enabled. So this will crash every time when only segment variations are enabled and some segment variants are created.

So the fix for this is to account for language being null and handle accordingly. That is, something like
a.language.isDefault
should probably just be
a.language != null && a.language.isDefault

image
image

Language is NULL:
image

Steps to reproduce

Expected result

The overlay shows all variants and they can be saved/published/unpublished without issues.

Actual result

The overlay is empty because the JavaScript crashes, we cannot save/publish/unpublish anything.


This item has been added to our backlog AB#11985

@nul800sebastiaan
Copy link
Member

Thanks for the report @PerplexDaniel - we'll have a look at fixing that for 8.13.1! 👍

@nielslyngsoe nielslyngsoe added the state/sprint-candidate We're trying to get this in a sprint at HQ in the next few weeks label May 17, 2021
madsrasmussen added a commit that referenced this issue May 18, 2021
…be any language (#10278)

* Variant sorting should take into account that there might not be any language available

* fix languages

Co-authored-by: Niels Lyngsø <[email protected]>
Co-authored-by: Mads Rasmussen <[email protected]>
nul800sebastiaan pushed a commit that referenced this issue May 19, 2021
…be any language (#10278) (#10284)

Co-authored-by: Niels Lyngsø <[email protected]>
Co-authored-by: Mads Rasmussen <[email protected]>
@nul800sebastiaan nul800sebastiaan added release/8.13.1 state/sprint-candidate We're trying to get this in a sprint at HQ in the next few weeks and removed state/sprint-candidate We're trying to get this in a sprint at HQ in the next few weeks labels May 19, 2021
@nul800sebastiaan
Copy link
Member

Fixed in #10284

nul800sebastiaan pushed a commit that referenced this issue May 19, 2021
…be any language (#10278)

* Variant sorting should take into account that there might not be any language available

* fix languages

Co-authored-by: Niels Lyngsø <[email protected]>
Co-authored-by: Mads Rasmussen <[email protected]>
(cherry picked from commit e19a598)
nul800sebastiaan pushed a commit that referenced this issue May 19, 2021
…be any language (#10278) (#10284)

Co-authored-by: Niels Lyngsø <[email protected]>
Co-authored-by: Mads Rasmussen <[email protected]>
(cherry picked from commit 017b56e)
@umbrabot umbrabot removed the state/sprint-candidate We're trying to get this in a sprint at HQ in the next few weeks label May 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants