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

[docs] Set the right site_url #3060

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[docs] Set the right site_url #3060

wants to merge 1 commit into from

Conversation

daenney
Copy link
Member

@daenney daenney commented Jul 2, 2024

Description

Though the entry point is docs.gotosocial.org, that's redirected by RTD to docs.gotosocial.org/en/latest/ which is where the actual site is served from. However, other URLs like docs.gotosocial.org/admin aren't redirected to docs.gotosocial.org/en/latest/admin. They just 404.

Without us including the /en/latest/ all the generated og:img URLs as well as the link rel=canonical result in URLs that all 404.

This means that currently the social cards aren't working well, but indexing the docs site by search engines is probably also partially broken, since our sitemap.xml is also pointing at things that don't exist.

Checklist

Please put an x inside each checkbox to indicate that you've read and followed it: [ ] -> [x]

If this is a documentation change, only the first checkbox must be filled (you can delete the others if you want).

  • I/we have read the GoToSocial contribution guidelines.
  • I/we have discussed the proposed changes already, either in an issue on the repository, or in the Matrix chat.
  • I/we have not leveraged AI to create the proposed changes.
  • I/we have performed a self-review of added code.
  • I/we have written code that is legible and maintainable by others.
  • I/we have commented the added code, particularly in hard-to-understand areas.
  • I/we have made any necessary changes to documentation.
  • I/we have added tests that cover new code.
  • I/we have run tests and they pass locally with the changes.
  • I/we have run go fmt ./... and golangci-lint run.

Though the entry point is docs.gotosocial.org, that's redirected by RTD
to docs.gotosocial.org/en/latest/ which is where the actual site is
served from. However, other URLs like docs.gotosocial.org/admin aren't
redirected to docs.gotosocial.org/en/latest/admin. They just 404.

Without us including the /en/latest/ all the generated og:img URLs as
well as the link rel=canonical result in URLs that all 404.

This means that currently the social cards aren't working well, but
indexing the docs site by search engines is probably also partially
broken, since our sitemap.xml is also pointing at things that don't
exist.
@daenney
Copy link
Member Author

daenney commented Jul 2, 2024

You can see the problem currently, by going to https://docs.gotosocial.org/sitemap.xml first. That serves a simple document:

<urlset>
<url>
<loc>https://docs.gotosocial.org/en/latest/</loc>
<lastmod>2024-07-01T11:41:57.573026+00:00</lastmod>
<changefreq>weekly</changefreq>
<priority>1</priority>
</url>
</urlset>

But if you then navigate to https://docs.gotosocial.org/en/latest/sitemap.xml, you get:

<urlset>
<url>
<loc>https://docs.gotosocial.org/</loc>
<lastmod>2024-07-01</lastmod>
<changefreq>daily</changefreq>
</url>
<url>
<loc>https://docs.gotosocial.org/faq/</loc>
<lastmod>2024-07-01</lastmod>
<changefreq>daily</changefreq>
</url>
[..]

For one, that sitemap is illegal because it being served at /en/latest/sitemap.xml means it can't include URLs outside of /en/latest/. But all the URLs it includes also 404, because that's not where things are.

@daenney
Copy link
Member Author

daenney commented Jul 2, 2024

Unfortunately for the PR build, it's not able to pick up the "PR" environment. But if you look at https://gotosocial--3060.org.readthedocs.build/en/3060/sitemap.xml, it now correctly outputs:

<urlset>
<url>
<loc>https://docs.gotosocial.org/en/latest/</loc>
<lastmod>2024-07-02</lastmod>
<changefreq>daily</changefreq>
</url>
<url>
<loc>https://docs.gotosocial.org/en/latest/faq/</loc>
<lastmod>2024-07-02</lastmod>
<changefreq>daily</changefreq>
</url>
<url>
<loc>
https://docs.gotosocial.org/en/latest/admin/backup_and_restore/
</loc>
<lastmod>2024-07-02</lastmod>
<changefreq>daily</changefreq>
</url>
[..]

@daenney
Copy link
Member Author

daenney commented Jul 2, 2024

The other way to see it is for example opening a page (except for the home page): https://docs.gotosocial.org/en/latest/user_guide/posts/

That will show the following in the HTML:

<link href="https://docs.gotosocial.org/user_guide/posts/" rel="canonical">

That's incorrect, but if you open the PR build instead:

<link href="https://docs.gotosocial.org/en/latest/user_guide/posts/" rel="canonical">

That's what we want.

@daenney
Copy link
Member Author

daenney commented Jul 2, 2024

The alternative would be being able to configure RTD to rewrite all URLs without /en/latest/ to somehow include it. But I have no idea how to tell them to do that.

@daenney
Copy link
Member Author

daenney commented Jul 2, 2024

This feels like the wrong fix because somehow the PR build is partially overriding site_url in some contexts? Or maybe it's not used to build the URLs in all contexts. It seems a bit wobbly.

@tsmethurst
Copy link
Contributor

@daenney Should I review this? Or would you rather go with another solution? Sorry for waiting so long to take a proper look!

@daenney
Copy link
Member Author

daenney commented Nov 17, 2024

I think we should fix it, because:

  • Social cards are broken
  • Search engine indexing is broken because our sitemap.xml is incorrect
  • Lots of links don't redirect

I'm not sure if this is the best solution, so if you want to take a look and see if there's anything on the RTD side too that maybe we could do that would be helpful. It might be possible to redirect non-language qualified URLs on their side to always go to /en/latest which would potentially also solve this.

I haven't looked at how this works now that we have the zh docs site.

@tsmethurst
Copy link
Contributor

Ahh got you okay :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants