-
Notifications
You must be signed in to change notification settings - Fork 197
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
add-initial-element-template-link-check-file #5090
add-initial-element-template-link-check-file #5090
Conversation
d50f0a3
to
8198532
Compare
https://docs.camunda.io/docs/next/apis-tools/camunda-api-rest/specifications/create-document-link/ | ||
https://docs.camunda.io/docs/next/components/connectors/out-of-the-box-connectors/amazon-comprehend | ||
https://docs.camunda.io/docs/next/components/connectors/out-of-the-box-connectors/amazon-sagemaker | ||
https://docs.camunda.io/docs/next/components/connectors/out-of-the-box-connectors/amazon-textract | ||
https://docs.camunda.io/docs/next/components/connectors/out-of-the-box-connectors/aws-dynamodb/ | ||
https://docs.camunda.io/docs/next/components/connectors/out-of-the-box-connectors/aws-eventbridge/ | ||
https://docs.camunda.io/docs/next/components/connectors/out-of-the-box-connectors/sql | ||
https://docs.camunda.io/docs/next/components/connectors/out-of-the-box-connectors/sql/#connection | ||
https://docs.camunda.io/docs/next/components/connectors/out-of-the-box-connectors/sql/#uri-connection | ||
https://docs.camunda.io/docs/next/components/connectors/out-of-the-box-connectors/sql/#variables | ||
https://docs.camunda.io/docs/next/guides/setup-client-connection-credentials |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are all links to the /next
version. A few questions:
- Are they linking to
/next
intentionally? - If they aren't intentionally linking to
/next
, can they be changed to the current version? - If they're intentionally linking to
/next
(presumably because they don't exist in any version except next), could they link to a specific version number instead of/next
? We don't currently have a rule to redirect/8.8
URLs to/next
, but I think I would prefer adding one over preserving/next
links via redirect rules.
Some background:
We don't usually worry about preserving redirects for /next
docs, because we consider that area of the docs to be very fluid and undergoing change. To preserve these links would be an unusual thing for us to do, and I want to make sure they are the correct links before we commit to doing that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for reviewing and this input. I am not super sure how to handle this.
To answer your questions:
- Some are linking to
next
intentionally, but some are not. - We can change the latest versions of our element templates, but not really older ones, as they have be downloaded and are on different systems, e.g. on customers system where they have been downloaded to.
- For the latest versions of the element templates I can adjust this and replace
next
with8.8
So what we can do is:
A) Add a check to a github action in our/connectors repo, that no links with /next/
are in future element templates.
B) Fix links in the latest element templates to not link to next
Both (A+B) will require on your side that links to 8.8
are working, not like now, where they return 404. If i understand you correctly, that is what you suggested.
C) Older templates cannot be fixed and need either a redirect on your side, or if you do not want to this, will stay broken, but with A and B, this will "fade out" in the future, when customers update their element templates, but I would prefer them to be redirected and be working right away, but that's up to you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good, makes sense, and I agree on all points!
- I will let you know when I've got
/8.8
links redirecting properly, so you can move forward on your end. - I will add redirects for the few
/next
links that need them when I get the CI workflow properly checking these links, which I'll do on this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I completed the tasks on our side in this PR (replacing next links in current element templates and adding a ci check for avoiding future linking to /next/). No links that linked to next
where only available on 8.8 so i changed them to link to 8.7 or where possible to 8.6.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops @ztefanie I forgot to let you know that the /8.8
redirect is now in place. Instead of something like https://docs.camunda.io/docs/next/components/connectors/out-of-the-box-connectors/sql/#uri-connection
, we can use https://docs.camunda.io/docs/8.8/components/connectors/out-of-the-box-connectors/sql/#uri-connection
and it will redirect to the correct /next
doc. Numbered versions are preferred to /next
versions, so if you're able to change any, that would be great!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, yes I already changed this, see the comment above yours. There were no links only available in 8.8., but all pages already existed in 8.7. But I cannot change the links of "older" versions, so unfortunately we will keep having next
links in this file.
A status update on my end: https://github.com/camunda/documentation-team/issues/67#issuecomment-2701782505 TLDR I learned that our current link-checking can't check anchors, but I found a tool that will. I will work on implementing it when I get back to this work, probably in a few days. |
👋 🤖 ✅ Looks like the changes were ported across versions, nice job! 🎉 You can read more about the versioning within our docs in our documentation guidelines. |
1054fb2
to
9d355da
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is getting close! I've fixed most broken URLs with redirects (or custom heading IDs for URLs that were broken due to a non-existent anchor).
I have a few more to clean up, for a variety of reasons. They're listed below.
Still-broken URLs
- I'm not sure what's up with these, I thought I'd taken care of them with a redirect from apis-clients to apis-tools. I'll keep poking.
- /docs/apis-clients/operate-api/#filter
- /docs/apis-clients/operate-api/#sort
- I think these are all connectors newer than 8.6, and don't have docs for version 8.6. @ztefanie do you know if any of these can be changed at the source, to point at docs that exist?
- /docs/components/connectors/out-of-the-box-connectors/amazon-s3
- /docs/components/connectors/out-of-the-box-connectors/box
- /docs/components/connectors/out-of-the-box-connectors/google-gemini
- This one is a doozy. It is for a retired connector. The page itself redirects to the connectors index; but the presence of an old anchor tag causes the validaton to fail. I need to do something tricky with this one, like add support for a list of "ignore" URLs, because we can't retroactively add this old anchor to the connectors index page.
- id #oauth-token-endpoint not found /docs/components/connectors/out-of-the-box-connectors/power-automate/#oauth-token-
https://docs.camunda.io/docs/components/modeler/bpmn/none-events | ||
https://docs.camunda.io/docs/components/modeler/desktop-modeler/element-templates/using-templates/#replacing-templates | ||
https://docs.camunda.io/docs/components/modeler/web-modeler/camunda-marketplace | ||
https://docs.camunda.io/docs/components/modeler/web-modeler/connectors/available-connectors/automation-anywhere/ | ||
https://docs.camunda.io/docs/components/modeler/web-modeler/connectors/available-connectors/rest | ||
https://docs.camunda.io/docs/components/modeler/web-modeler/connectors/available-connectors/sendgrid |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ztefanie these URLs are a little surprising. Were connectors ever nested under the modeler docs?
The answer to that question is probably not important, but adding redirects for these URLs made me wonder if these are URLs that can be changed at the source, or if there's anything else funny about them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. I just checked. These links where added to our codebase 3 years ago. I will adjust them for newer element templates, but cannot change them for older ones, therefore we sill need redirects for them.
Thanks for the transparency and good work 🚀 I made adjustments to connectors here: camunda/connectors#4353 I think we now can remove these links from this file, as this was not yet released and therefore cannot break anywhere.
|
a35dd77
to
e838fd4
Compare
e838fd4
to
0a5af93
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I cannot approve as I am the author of this PR, but really good work. 👍 It will really improve the user experience. 🚀 🔥
❤️ Great teamwork, thanks for the collaboration on this. |
https://docs.camunda.io/docs/guides | ||
https://docs.camunda.io/docs/guides/setup-client-connection-credentials | ||
https://docs.camunda.io/docs/next/apis-tools/camunda-api-rest/specifications/create-document-link/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm surprised to see so many links to other places besides the Connectors docs.
RewriteRule ^docs/guides/migrating-from-Camunda-Platform/?(.*)$ /docs/guides/migrating-from-camunda-7/$1 [R=301,L] | ||
RewriteRule ^docs/reference/bpmn-processes/?(.*)$ /docs/components/modeler/bpmn/$1 [R=301,L] | ||
|
||
# URLs that need to be preserved for connector element templates, in perpetuity. | ||
RewriteRule ^docs/?$ / [R=301,L] | ||
RewriteRule ^docs/apis-clients/operate-api/?$ /docs/apis-tools/operate-api/overview/ [R=301,L] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non-blocking for this PR, but what's the plan when the Operate API is deprecated/removed and we only have the C8 API? Do we redirect to the C8 API overview page?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No blocking feedback.
We could consider adding some docs-on-docs to https://github.com/camunda/camunda-docs/blob/main/howtos/moving-content.md, but the inline docs within the htaccess file make it pretty clear what's going on here.
🧹 Preview environment for this PR has been torn down. |
Description
This PR is a collaboration:
build-docs
workflow to utilize muffet, the only link-checking tool I've found that combines (a) non-crawling functionality with (b) anchor validation. As a bonus, this tool also appears to run more quickly than lychee (anecdotally, 4s vs 7s for the sitemap validation). This improved link validation should enable us to catch element-template breaking URL changes immediately (as well as modeler product-link and sitemap URL breaking changes).build-docs
, so that the steps most likely to fail occur earlier in the workflow, and steps least likely to fail occur later in the workflow. This should help us identify build issues more quickly.Proof of functionality
Preservation of production sitemap URLs is required for a successful build
I temporarily removed a file from the vCurrent docs. This execution shows the build failing due to the sitemap URL no longer existing.
Preservation of Modeler product-link URLs is required for a successful build
This execution demonstrates the build failing due to a Modeler product-link URL no longer existing.
It also demonstrates that anchors are now validated.
Preservation of element template URLs is required for a successful build
This execution demonstrates the build failing due to element template URLs not existing.

When should this change go live?
bug
orsupport
label)available & undocumented
label)hold
label)low prio
label)PR Checklist
/docs
directory (version 8.8)./versioned_docs/version-8.7/
directory (version 8.7)./versioned_docs
directory.@camunda/tech-writers
unless working with an embedded writer.