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

Internal link gives different URL in placeholder vs static placeholder #165

Closed
jasper-flux opened this issue Jan 18, 2019 · 1 comment
Closed

Comments

@jasper-flux
Copy link

The URL generated for internal links to the current site differs depending on whether the plugin is located in a static placeholder or a regular placeholder.

In a regular placeholder, a link in the format /<slug> is generated.
In a static placeholder, a link in the format //example.com/<slug> is generated.

I'd say that the behaviour should be the same for both cases.

My (probably naïve) suggestion would be to change the following code:

https://github.com/divio/djangocms-link/blob/fbb67410377c953ff1e153a91e5306080b2a46fa/djangocms_link/models.py#L150-L153

Instead of overriding ref_page with the current site, I think it should be retrieved from the self.internal_link property, and cms_page_site_id should be retrieved from the current site. That way a meaningful comparison can be made and it can be decided whether the domain prefix is necessary.

@FinalAngel
Copy link
Member

FinalAngel commented Jun 20, 2019

@jasper-flux I tried to understand the problems you are experiencing and created a PR with various tests:

#171

The issue originates from:

cms_page = self.placeholder.page if self.placeholder_id else None

Static placeholders act different than regular placeholder and thus self.placeholder.page is not available resulting in the reformatting of the link through:

if ref_page_site_id != cms_page_site_id:

by passing through:

else:
    ref_page = Site.objects.get_current()
    ref_page_site_id = ref_page.pk
    cms_page_site_id = None

This is a bit tricky to figure out as the else is designed to adapt links if there is no placeholder attached as a fallback. There'd need to be an additional conditional statement checking for the static_placeholder's page itself.

@czpython would you have any advice on how to get the page from a static placeholder?

FinalAngel added a commit that referenced this issue Jun 21, 2019
* add tests for 165

* only apply to newer versions

* make things more clear

* add teardown here too
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants