get_link will always return absolute urls instead of relative urls, making it hard to author content/test out locally

[silkentrance]

To follow up on #165

I tend to believe that the placeholder cms_page is redundant as Site.objects.get_current() will already return the correct site. And, considering that any placeholder.page.site is always the current site, when available, the code could be reduced to just

    def get_link(self):
        if self.internal_link:
            ref_page = self.internal_link
            link = ref_page.get_absolute_url()
            ref_page_site_id = ref_page.node.site_id
            if ref_page_site_id != Site.objects.get_current().pk:
                ref_site = Site.objects._get_site_by_id(ref_page_site_id).domain
                link = '//{}{}'.format(ref_site, link)

  ...

I have implemented this locally and it works like a charm, unless I missed some very specific detail 😄

I can provide you with a PR for this if you want.

[marksweb]

@silkentrance Yes, I'd agree that going through the placeholder to get a site_id in order to find the correct Site object isn't the best way to go about things.

However I don't think you can simplify ref_page_site_id like that. The node attr needs checking as it is now.

[silkentrance]

@marksweb considering that pages are always created by going through the API

https://github.com/django-cms/django-cms/blob/a3110e1ff24085373898c7d2a85f628abeb8518d/cms/api.py#L100

then the existence of the node foreign key is ensured,

https://github.com/django-cms/django-cms/blob/a3110e1ff24085373898c7d2a85f628abeb8518d/cms/api.py#L187

even more so since the page model does not declare it to be nullable,

https://github.com/django-cms/django-cms/blob/a3110e1ff24085373898c7d2a85f628abeb8518d/cms/models/pagemodel.py#L230

so it is a mandatory property of the page.

Therefore I tend to believe that the presented code is correct and should work under all circumstances.

Yet, looking at

djangocms-link/djangocms_link/models.py

Line 162 in 1d0417e

# a plugin might not be attached to a page and thus has no site

I am not so sure anymore. I will check the multiple usage scenarios and see what I can find...

[marksweb]

@silkentrance Actually, I think you're right. Yesterday I was thinking back to times I've built things for menus and it's the node that you can't guarantee is attached to a page, but this is the other side of that 🤦

By the way, I'm not sure if you're a slack user, but if you are it might be worth joining our slack django-cms.org/slack

[silkentrance]

I have played around with links in static placeholders, links in modules, links linking to pages from a different site than the current one. As of now, everything seems to be working just fine.