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

Add notifcation when publishing varying culture without domains configured #11328

Merged
merged 35 commits into from Oct 15, 2021

Conversation

nikolajlauridsen
Copy link
Contributor

@nikolajlauridsen nikolajlauridsen commented Oct 8, 2021

This PR adds a notification in the UI as well as a log warning when a content editor creates a piece of multilingual content that has no domain configured for it, this fixes: #10350.

The pattern for what is considered a valid and what is not is a bit weird:

Assume a content type that varies in language A , B, and C the following rules apply as far as I can tell

  • Root node - Published in language A
    • Child node - Published in language A

This is considered valid because only one language is published = we can route this

  • Root node - Published in language A
    • Child node - Published in language A and B

This is invalid because there's more than one culture published in child node, so we can't route child node

  • Root node - Published in language A, with domain for language A
    • Child node - Published in language A and B with, with domain for language B
      • Grandchild node - Published in language A and B

This is valid because somewhere in the ancestor path there's a domain for both languages A and B

  • Root node - Published in language A, with domain for language A
    • Child node - Published in language A and B with, with domain for language B
      • Grandchild node - Published in language A, B, and C

This is not valid because there's no domain for language C anywhere

Testing

  1. Add a couple of languages
  2. Create a document type that varies by language
  3. Start testing out some of the patterns above and ensure that the warning notifications show up when expected and that a warning is logged.

Please try some different combinations as well, there may have been some "rules" I've missed

Copy link
Member

@Zeegaan Zeegaan left a comment

Choose a reason for hiding this comment

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

Found a few minor things (thank you stylecop)
Rest LGTM 👍

nikolajlauridsen and others added 5 commits October 15, 2021 08:26
Co-authored-by: Nikolaj Geisle <70372949+Zeegaan@users.noreply.github.com>
Fixed issue where if you were 3 levels deep and published only 1 culture, warning wouldn't fire
@Zeegaan
Copy link
Member

Zeegaan commented Oct 15, 2021

With the changes (and the recent bugfix) i tested again and now everything works as it should 👍

Merging 💪

@Zeegaan Zeegaan merged commit ee6abde into v9/dev Oct 15, 2021
@Zeegaan Zeegaan deleted the v9/feature/add-notifcation-for-url-collision branch October 15, 2021 10:31
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.

None yet

2 participants