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

feat(github): support #discussioncomment- urls #266

Conversation

shiftinv
Copy link
Contributor

@shiftinv shiftinv commented May 8, 2024

Adds support for embedding <owner>/<repo>/discussions/<id>#discussioncomment-<id> urls.

This is... rather difficult via the GraphQL api (which you are required to use for this :/), since you would need the global ID (DC_kwDOF-MrcM4AT8ii), which you can technically only get from prior API requests.

The other "intended" way of fetching a discussion comment based on the integer ID would be to query discussion(123) { comments(after: ...) {}} and paginate through all comments until we find a matching ID. no thanks.

Instead, we rely on the undocumented format of GraphQL global IDs, which are msgpack'ed arrays containing the repo ID and resource ID, with a type prefix. This works with essentially any resource, e.g. I_{b64(msgpack([0, repo_id, issue_id]))} to fetch an issue based on its ID, and so on; the prefixes are pretty trivial to figure out.

It's worth noting that these IDs also include an owner ID (i.e. repo_id above), which we don't get from the URL. This ID isn't required atm, but may be in the future. As per GHE source:

      # Currently this is defaulting to the same behavior as the legacy
      # id lookup.  In the future, this method needs to be modified
      # to  look up objects based on the ownership information in
      # the passed in GlobalID.

It works fine for now, and this ID format hasn't changed for the 4 years it has been used so far. If it does end up being required in the future, we'll have to make one extra API request to turn the <owner>/<repo> part from the URL into an ID as well.


image
(the timestamp was wrong, see #240.)

@shiftinv shiftinv force-pushed the feat/github-discussioncomment branch 3 times, most recently from d5c772d to f062eb4 Compare May 9, 2024 11:55
@shiftinv shiftinv force-pushed the feat/github-discussioncomment branch 3 times, most recently from 4897630 to 117b619 Compare September 19, 2024 21:36
as per GHE source:
```
- The string prefix is converted to an _index_ of possible templates for this object type
- The variable _values_ are put into an array
- Then, those values are MessagePacked and base64-encoded
```
@onerandomusername onerandomusername merged commit 96c1db7 into onerandomusername:main Sep 19, 2024
3 checks passed
@shiftinv shiftinv deleted the feat/github-discussioncomment branch September 19, 2024 22:38
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