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

fix: bitbucket server edit url resolves correctly #20047

Closed

Conversation

fedy97
Copy link
Contributor

@fedy97 fedy97 commented Sep 20, 2023

Hey, I just made a Pull Request!

from this:
image

to this:
image

✔️ Checklist

  • A changeset describing the change and affected packages. (more info)
  • Added or updated documentation
  • Tests for new functionality and regression tests for bug fixes
  • Screenshots attached (for UI changes)
  • All your commits have a Signed-off-by line in the message. (more info)

Closes #20042

@fedy97 fedy97 requested review from a team as code owners September 20, 2023 14:21
@backstage-goalie
Copy link
Contributor

backstage-goalie bot commented Sep 20, 2023

Changed Packages

Package Name Package Path Changeset Bump Current Version
@backstage/integration packages/integration patch v1.7.0
@backstage/plugin-techdocs-node plugins/techdocs-node patch v1.8.0

@backstage-goalie
Copy link
Contributor

Thanks for the contribution!
All commits need to be DCO signed before they are reviewed. Please refer to the the DCO section in CONTRIBUTING.md or the DCO status for more info.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 20, 2023

Uffizzi Ephemeral Environment deployment-36427

☁️ https://app.uffizzi.com/github.com/backstage/backstage/pull/20047

📄 View Application Logs etc.

What is Uffizzi? Learn more!

Signed-off-by: Federico Morreale <frc.morreale@gmail.com>
@fedy97 fedy97 force-pushed the hotfix/edit-url-bitbucket-server branch from 72ddf2b to f67cfc3 Compare September 20, 2023 14:40
Copy link
Member

@freben freben left a comment

Choose a reason for hiding this comment

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

This needs a changeset to get shipped.

The comment above states that there is no edit mode to link to. Is that not true?

Also, hm. The comment that got removed is interesting. I wonder if that impact is still relevant. Does perhaps @backstage/techdocs-maintainers know? Either way, that sounds like something that should be fixed in the TechDocs end rather than here.

@fedy97
Copy link
Contributor Author

fedy97 commented Sep 21, 2023

This needs a changeset to get shipped.

The comment above states that there is no edit mode to link to. Is that not true?

Also, hm. The comment that got removed is interesting. I wonder if that impact is still relevant. Does perhaps @backstage/techdocs-maintainers know? Either way, that sounds like something that should be fixed in the TechDocs end rather than here.

Thanks @freben for your reply. The edit button in techdocs redirects you always to the master branch and not to the correct branch (the query params are cut also there), so it is another bug to be fixed.

Signed-off-by: Federico Morreale <frc.morreale@gmail.com>
@awanlin
Copy link
Collaborator

awanlin commented Sep 21, 2023

Some what related: #12892

@pjungermann any feedback or thoughts on this one?

@fedy97
Copy link
Contributor Author

fedy97 commented Sep 21, 2023

We could maybe keep only the at query param and remove the others (like mode)?

@pjungermann
Copy link
Contributor

pjungermann commented Sep 21, 2023

@fedy97 This change was done consciously by another Bitbucket Server user in order to make "edit URL" work for Tech Docs. By not removing the query parameters, you will break the "edit" button feature at Tech Docs.

The reason for this is mkdocs itself and how it supports edit_uri there.

I worked on a change at PR #16699 which will make it work without removing the query parameters, also for other systems like Bitbucket Cloud. However, it still requires some changes to mkdocs-techdocs-core and mkdocs-monorepo-plugin before it could be merged.

@fedy97
Copy link
Contributor Author

fedy97 commented Sep 21, 2023

@fedy97 This change was done consciously by another Bitbucket Server user in order to make "edit URL" work for Tech Docs. By not removing the query parameters, you will break the "edit" button feature at Tech Docs.

The reason for this is mkdocs itself and how it supports edit_uri there.

I worked on a change at PR #16699 which will make it work without removing the query parameters, also for other systems like Bitbucket Cloud. However, it still requires some changes to mkdocs-techdocs-core and mkdocs-monorepo-plugin before it could be merged.

I am not able to reproduce the breaking of the edit button, it always redirect me to the index.md on master branch (not in edit mode). It seems there is no difference if query params are set or not. Maybe a specific query params breaks it?

@pjungermann
Copy link
Contributor

I just found the original PR #12857 for context.

@pjungermann
Copy link
Contributor

@omerfarukdogan as the original author of the aforementioned PR, maybe you can have a look at this change.

@omerfarukdogan
Copy link
Contributor

@fedy97 Which version of Backstage are you using? I have just tested on ours (1.13.0) and the edit button still works correctly. It opens the source of the current page (https://<host>/projects/<project>/repos/<repo>/browse/docs/sub-page.md)

Are you setting up techdocs-url like this?

"backstage.io/techdocs-ref": "url:https://<host>/projects/<project>/repos/<repo>/browse/"

@fedy97
Copy link
Contributor Author

fedy97 commented Sep 22, 2023

@fedy97 Which version of Backstage are you using? I have just tested on ours (1.13.0) and the edit button still works correctly. It opens the source of the current page (https://<host>/projects/<project>/repos/<repo>/browse/docs/sub-page.md)

Are you setting up techdocs-url like this?

"backstage.io/techdocs-ref": "url:https://<host>/projects/<project>/repos/<repo>/browse/"

@omerfarukdogan the issue is this #20042 : I have this catalog info

localhost:7990/projects/WIT/repos/test_dp_bitbucket/browse/catalog-info.yaml?at=release_5

which has

backstage.io/techdocs-ref: dir:.

When I import it, the edit-url is wrong, it is missing the at query param that specifies the branch. The edit button works, but always redirects to master doc instead of release_5 in this case.

@omerfarukdogan
Copy link
Contributor

omerfarukdogan commented Sep 22, 2023

@fedy97 I don't think the function you are modifying is related to the annotation you have an issue with (backstage.io/edit-url). This is a function under techdocs plugin and it is executed during the document generation step. The annotation is not generated here and should not be affected by this change. The annotation is for editing the entity definition itself, not the document. Please correct me if I'm wrong @freben, @pjungermann.

On the other hand, the parameters on the document URL are indeed removed from the "edit document" button. Otherwise mkdocs generates an incorrect URL for editing (IIRC https://<host>/projects/<project>/repos/<repo>/browse/?at=release5/docs/sub-page.md instead of https://<host>/projects/<project>/repos/<repo>/browse/docs/sub-page.md?at=release5).

@fedy97
Copy link
Contributor Author

fedy97 commented Sep 22, 2023

@fedy97 I don't think the function you are modifying is related to the annotation you have an issue with (backstage.io/edit-url). This is a function under techdocs plugin and it is executed during the document generation step. The annotation is not generated here and should not be affected by this change. The annotation is for editing the entity definition itself, not the document. Please correct me if I'm wrong @freben, @pjungermann.

On the other hand, the parameters on the document URL are indeed removed from the "edit document" button. Otherwise mkdocs generates an incorrect URL for editing (IIRC https://<host>/projects/<project>/repos/<repo>/browse/?at=release5/docs/sub-page.md instead of https://<host>/projects/<project>/repos/<repo>/browse/docs/sub-page.md?at=release5).

@omerfarukdogan The change I made in this PR affects only the annotation backstage.io/edit-url and so if you go in the inspect entity you can see the entire url with the at query param. The problem to be fixed is the edit button that in this case will redirect me to http://localhost:7990/projects/WIT/repos/test_dp_bitbucket/browse/docs?at=release_5/index.md which is a wrong url, but with the current version of backstage it would redirect me to http://localhost:7990/projects/WIT/repos/test_dp_bitbucket/browse/docs/index.md which is wrong too because I do not want to go to master branch

@omerfarukdogan
Copy link
Contributor

@fedy97 I would imagine most people to use the main branch for documentation so it wouldn't cause a problem for most cases. Our options are:

  1. Generate a correct URL with a possibly incorrect branch
  2. Generate an incorrect URL
  3. Generate a correct URL (by making mkdocs handle the query parameters)

This PR is going with 2 (unless we also fix mkdocs), I would prefer 1 rather than 2, but it's up to the maintainers IMO.

Signed-off-by: Federico Morreale <frc.morreale@gmail.com>
Signed-off-by: Federico Morreale <frc.morreale@gmail.com>
@fedy97 fedy97 requested a review from a team as a code owner September 22, 2023 10:21
@fedy97
Copy link
Contributor Author

fedy97 commented Sep 22, 2023

@omerfarukdogan I fixed the edit button link, now it redirects always to master, but the annotation backstage.io/edit-url now points to the right url

image

@github-actions github-actions bot added the area:techdocs Related to the TechDocs Project Area label Sep 22, 2023
Signed-off-by: Federico Morreale <frc.morreale@gmail.com>
@pjungermann
Copy link
Contributor

@omerfarukdogan I've started to work on option 3 at the PR I've referenced above. There is a way on newer mkdocs versions. However, still a few things to adjust at backstage's own plugins.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 5, 2023

This PR has been automatically marked as stale because it has not had recent activity from the author. It will be closed if no further activity occurs. If the PR was closed and you want it re-opened, let us know and we'll re-open the PR so that you can continue the contribution!

@github-actions github-actions bot added the stale label Oct 5, 2023
@github-actions github-actions bot closed this Oct 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:techdocs Related to the TechDocs Project Area stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛 Bug Report: Bitbucket Server Integration does not resolve edit url correctly
5 participants