luyadev / luya-module-cms

The LUYA CMS module provides a full functional CMS for adding contents based on blocks.
https://luya.io
MIT License
33 stars 46 forks source link

Multiple website support #274

Closed boehsermoe closed 3 years ago

boehsermoe commented 4 years ago

What are you changing/introducing

Handle multiple websites/domain in one luya cms module. Nav container will be assign to a website and all navs filter by website.

Screenshot 2020-07-15 at 20 51 18Screenshot 2020-07-15 at 20 51 57

Todo's

QA

Q A
New feature? yes
Breaks BC? yes
Tests pass? yes
Fixed issues #252
nadar commented 4 years ago

As its not ready for review i just want to ask conceptional questions: do i need to create a website first? is this backwards compatible?

Looking forward! thats a great feature, thank you!

JohnnyMcWeed commented 4 years ago

What does this mean in terms of custom modules?

boehsermoe commented 4 years ago

do i need to create a website first? is this backwards compatible?

yes a one website is always needed but in the migration a default website will be created. But I can try to work without any website, then it should be backwards compatible but I have to take a closer at that.

@JohnnyMcWeed

What does this mean in terms of custom modules?

What exactly do u mean? Every module and controller would available on each website. In the module you can ask for the current website. e.g. for news module it would mean that can can share the same news and categories for all websites. The news module could need also an update to assign news and categories to single websites.

JohnnyMcWeed commented 4 years ago

What exactly do u mean? Every module and controller would available on each website. In the module you can ask for the current website. e.g. for news module it would mean that can can share the same news and categories for all websites. The news module could need also an update to assign news and categories to single websites.

Correct :) So the docs should then take this into account aswell. Maybe it's possible to make some kind of interface for module blocks to use it directly? E. g. add one column to a table, use the model interface and it's connected by default... (just a quick thought about it...)

boehsermoe commented 4 years ago

So the docs should then take this into account aswell.

Yes, of course the docu still has to be adapted.

Maybe it's possible to make some kind of interface for module blocks to use it directly? E. g. add one column to a table, use the model interface and it's connected by default... (just a quick thought about it...)

That a good idea! Maybe doing it with a behaviour or a trait. And also a trait for model filtering like SoftDeleteTrait can be useful.

I think I will start also a new branch in the news module and take a closer look.

nadar commented 4 years ago

@boehsermoe i think full backwards compatibility is a must. A website in the migration is good for me, but if only one (1) website is defined we should hide "change navigation for domain" button. Because 99% of the cases is what we have now and not the opposite i would say.

Also when i was thinking of how the solve this issue with multiple domains, keep in mind we do have already definitions for this information: https://github.com/luyadev/luya/blob/master/core/web/Composition.php#L88-L123

Thanks for the work.

boehsermoe commented 4 years ago

@nadar I think for first it is good enough for a review and test of you. The docu I will do as next.

There are still some todo where the website scope should check:

But all this too much for one PR and we should make smaller steps for those improvement.

nadar commented 4 years ago

Alright, i will do a review asap. What i can say for sure: we need near 100% code coverage, otherwise the system gets unstable. Because since we follow "100%" coverage the system is so much more stable.

image

nadar commented 4 years ago

When creating a new site, the context of the website is lost, therefore all containers and pages display even though my new "localhost website" does not have any site yet:

Bildschirmfoto 2020-10-24 um 18 27 05
boehsermoe commented 3 years ago

When i switch to new container (which does not have sites yet, as i just have created that website)

I could not reproduce the js error. Where exactly it happens?

nadar commented 3 years ago

When i switch to new container (which does not have sites yet, as i just have created that website)

I could not reproduce the js error. Where exactly it happens?

Indeed the problem has nothing to do with your changes. i will fix that after merge. BUT what i saw is that the create page sub container system does not work:

2021-05-27_09-50

The subpage system uses the data from the the menu, so if you are on another "website" the sub pages array is wrong. Therefore we could either see that when creating a new page, the context from the menu panel is required. Which means:

if you like to create a PAGE for domain foobar.com -> you have to select that domain on the left first. This would remove the option to select all containers, and just show the containers from that domain/website.

What do you think?

boehsermoe commented 3 years ago

Now both fields will use from the left menu.

codeclimate[bot] commented 3 years ago

Code Climate has analyzed commit 93640654 and detected 18 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 14
Duplication 4

The test coverage on the diff in this pull request is 67.5% (50% is the threshold).

This pull request will bring the total coverage in the repository to 50.2%.

View more on Code Climate.

bastelix commented 11 months ago

seems this cann be added here. There is an generell issue when adding a new container for a website. e.g:

I have some different websites: SNAG-0688

and when I add a new container for theming: SNAG-0689

it results alltimes in using first created website: SNAG-0690

seems an issue in cms module for multi website support

nadar commented 11 months ago

@bastelix please create a fresh issue. I have tested to save and update the website for containers on the latest version and it works as expected.

hbugdoll commented 1 month ago

seems an issue in cms module for multi website support

@bastelix This issue was fixed by #396 resp. in cms module 4.5.2.

hbugdoll commented 1 month ago

Todo's

  • [x] Backward compatible
  • [ ] Docu
  • [x] CMS Pages
  • [x] Nav

3 years later and this topic is still missing in the guide...

@nadar has collected some related QA at https://github.com/orgs/luyadev/discussions/2138. I will continue there. @boehsermoe It would be great if you could help with some questions.