Skip to content

Conversation

@apata
Copy link
Contributor

@apata apata commented Nov 4, 2025

Changes

Adds back the baseurl to shared links. It's needed for the links to be one-click shareable.

Tests

  • Automated tests have been added

Changelog

  • Entry has been added to changelog

Documentation

  • This change does not need a documentation update

Dark mode

  • The UI has been tested both in dark and light mode

@apata apata changed the title Fix share link hostname Fix shared link hostname Nov 4, 2025
@zoldar zoldar added the preview label Nov 4, 2025
@github-actions
Copy link

github-actions bot commented Nov 4, 2025

Preview environment👷🏼‍♀️🏗️
PR-5870

end

defp shared_link_dest(site, link) do
Routes.stats_path(PlausibleWeb.Endpoint, :shared_link, site.domain, auth: link.slug)
Copy link
Contributor Author

@apata apata Nov 4, 2025

Choose a reason for hiding this comment

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

This has the benefit of ensuring proper encoding even for auth query parameter, which Plausible.Sites.shared_link_url/2 doesn't actually do. We could make use of this same logic in Plausible.Sites.shared_link_url/2, but it didn't make sense to me when I considered the relation between the modules Routes / Plausible.Sites.

Comment on lines +1038 to +1045
test "doesn't share the same domain formatting with public dashboard links" do
site = new_site(domain: "a-café.fr")
link = insert(:shared_link, site: site)

assert "http://localhost:8000/a-café.fr" = PlausibleWeb.StatsView.pretty_stats_url(site)

assert "http://localhost:8000/share/a-caf%C3%A9.fr?" <> _q =
Sites.shared_link_url(site, link)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This codifies current non-ideal behavior that I noticed. It's worth thinking about how to unify the two and all our other route utils as well.

@apata apata added this pull request to the merge queue Nov 5, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 5, 2025
@apata apata added this pull request to the merge queue Nov 5, 2025
github-merge-queue bot pushed a commit that referenced this pull request Nov 5, 2025
* Fix missing share link hostname

* Update changelog

* Tests
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 5, 2025
@apata apata added this pull request to the merge queue Nov 5, 2025
Merged via the queue into master with commit af7dd46 Nov 5, 2025
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants