Fix link generation for GitHub Enterprise by using `GITHUB_SERVER_URL`

kazuma0129 commented 1 week ago

Summary

This pull request fixes an issue where the todo-to-issue GitHub Action does not correctly generate links in GitHub Enterprise (GHE) environments due to incorrect base URLs. The action was previously using the API URL (GITHUB_URL) for generating web links, which is not appropriate for GHE, as the API URL and web interface URL differ.

Reason for Changes

Issue in GHE Environments

The action was using the API base URL (GITHUB_URL) to generate links, which is incorrect for GHE.
In GHE, the API URL (e.g., https://github.company.com/api/v3) is different from the web interface URL (e.g., https://github.company.com/).

Solution

By introducing GITHUB_SERVER_URL as an input, we can obtain the correct web interface URL. This ensures that issue links, code permalinks, and code snippets point to the correct location.

Impact

For GitHub Enterprise Users

Correctly generates links in issues.
Improves compatibility and usability of the action in GHE environments.

For GitHub.com Users

No impact on existing functionality.
The action continues to work as before, using the default github.server_url.

Documentation Updates

The README has not been updated at the time this Pull Request was created. This is because I think the content of this PR is controversial. (regarding naming of GITHUB_SEREVR_URL, etc.) So if the content of the implementation is OK, I will add the content to the README.

Thank you for considering this pull request. Please let me know if you have any questions or need further adjustments.

alstr commented 1 week ago

Thanks for the contribution.

As a non-enterprise user I've not noticed that, so thanks for flagging it up.

Just for my understanding, there is some existing support for GHES, so it would be good to figure out what is happening there before adding a new environment variable.

This was added in this PR: https://github.com/alstr/todo-to-issue-action/pull/139

The correct URL should be the GITHUB_URL environment variable.

self.github_url = os.getenv('INPUT_GITHUB_URL')  # This should be the correct URL
self.base_url = f'{self.github_url}/'
if self.base_url == 'https://api.github.com/':
    self.line_base_url = 'https://github.com/'
else:
    self.line_base_url = self.base_url

What do you think isn't working as it should?

kazuma0129 commented 2 days ago

@alstr Sorry for the late reply.

Reason for Changes

Issue in GHE Environments

The action was using the API base URL (GITHUB_URL) to generate links, which is incorrect for GHE. In GHE, the API URL (e.g., https://github.company.com/api/v3) is different from the web interface URL (e.g., https://github.company.com/).

The differences in values between the GitHub.com and GHE environments are organized in a table. I believe that this table will help you to see what the problem is now.

	github.api_url	github.server_url
GitHub.com	https://api.github.com	https://github.com
GitHub Enterprise	https://github.company.com/api/v3	https://github.company.com

Assuming that base_url is set to github.api_url by default, read the existing code.

if self.base_url == 'https://api.github.com/':

The above expression is always false in the GHE environment. The reason is shown in the table above.

else:
    self.line_base_url = self.base_url

As a result self.line_base_url is assigned self.base_url, which is incorrect.

There is an assumption in the code as if line_base_url and base_url play the same role, but this varies depending on the organization using GHE.

The following is a summary of current and expected behavior.

	GitHub.com	GHE (current impl)	GHE (expectation)
base_url	https://api.github.com	https://github.company.com/api/v3	https://github.company.com/api/v3
line_base_url	https://github.com/	https://github.company.com/api/v3	https://github.company.com/
permanent link on createed issue	https://github.com/{org}/{repo}/{path_to_code}	https://github.company.com/api/v3/{org}/{repo}/{path_to_code}	https://github.company.com/{org}/{repo}/{path_to_code}

As can be seen from the table, base_url and line_base_url expect different URLs. The #139 implementation does not cover this, so GITHUB_SERVER_URL is added in this PR.

alstr commented 15 hours ago

Thanks for the explanation. Will get this merged!

alstr / todo-to-issue-action