octo.nvim
octo.nvim copied to clipboard
pwntester
Feature request - open issues and PRs via notifications
Starting to use the plugin more and more in my professional work I realize a gap between the WebUI and using Octo in NeoVim. I read notifications in the browser, click on them to do a requested review for example and then realize I could/want to actually do this with Octo. I need to switch to the repository, run NeoVim, find the PR again with Octo and then start to review. That's a cumbersome workflow which Octo does not benefit from. But I think we can easily start to narrow this gap. The first link in the chain are the notifications for me. Sorry, confusing terminology. The notification to the user that there is a new notification objects (new entry in a thread object). Personally I have a tool that sends me desktop notification for this. I also use the mobile app with push notifications. Other user have e-mail messages for that. After all I think this isn't a topic Octo should try to solve. Maybe this is a thing for another little plugin that uses the new NeoVim notification API function. The second link is to open the list of open notifications, and three, click on it to interact with the threads subject object (e.g. an issue). And I think Octo can perfectly do this. We can query the API to get a user's notification. And then as for all other objects, we offer the user the list in Telescope. And when he selects a notification we open the related subject object and BAM, he gap got closed. :laughing:
One technical challenge is that unfortunately the notifications are exposed via the REST API endpoint, but not the GraphQL one. While this is not a problem by itself as the cli tool can also just query the REST endpoint, it brings some chaos into the code. We should consider to improve the plugins architecture first to push the dependencies to the borders and establish the Single-Responsibility-Principle (often misinterpreted by the name, don't get it wrong). Another challenge would be the Telescope previewer as it would need to handle different objects instead of single specific one as it is done now. But again, I think this points us to great improvement in the plugins architecture. Btw: we should filter notifications only for those who refer to subject objects we support (i.e. issues, PRs, gists and commits, no releases or discussions). Finally in the long long perspective we could think about how to render the specific comment in an issue/PR/gist that the thread notification is referring to. Thereby I mean showing it in the Telescope previewer as well as jumping/scrolling to it when a notification get opened.
All in all I think this whole feature isn't that big and not so hard to implement. Ignoring possible proceeding refactorings. But the effect and usability gain of the plugin is enormous. From my point of view...
I experienced the same gap in the past. The first step was to enable Octo command to handle URLs so you can do Octo https://github.com/pwntester/octo.nvim/issues/119 to open this issue. Then you just need to copy the URL from the GH notification and open it in Octo.
But then I entirely opted-out of GH email notifications in favor of https://github.com/notifications so I wrote an Octo extension to do exactly what you described. Unfortunately in order to do so I needed to access the notifications GraphQL endpoint which is still not exposed publically (Im a GH employee) and thats why I did not plublish that extension. The REST API (its also used for few other things in Octo) can get you the notifications but does not allow to mark them as read/done/saved so it can ease the pain of opening things in Octo but you still need to access the WebUI or notifications client to mark them as done, etc.
Im a GH employee
Very interesting. :grimacing: Well, this thread that asked to open the endpoint is open since 2 years without a respond by the GH team, except the initial response they put in their list. I don't know what's the problem and reason behind it, but does not seem to happen soonish.
That's amazing that you had it already implemented. Do you still have in a branch? Would it work with all the changes to the codebase?
That you can't mark them as red is really a pita. That sucks. So I assume you can't even set it as read (I assume set last-seen-date or similar)?
That's amazing that you had it already implemented. Do you still have in a branch? Would it work with all the changes to the codebase?
I have the graphql version implemented, the REST one would be easy to build based on that, but with the limitations mentioned above 😞
I mean after all it would be still a big chunk of improvements for the plugin. As current workaround the user could then open the object in the browser via an Octo command right? And then set it to done there.
Because I'm a very curious person, but also because I have a easier time to accept things if I know their reasoning (I must not agree): do you have any internal information why this endpoint gets hold back for so long? Ofc only if you are allowed to share these information. But I might allow a better estimate if this is something to come soon, or if it is worth to think about long-term alternative approaches for the plugin.
I have no idea sorry. I work in a completely unrelated team (security lab)
Just adding my 2 cents, I think having this feature, even with only the ability to mark notifications as read would be awesome!
@pwntester It seems like the graphql API is available now. Any plans on revisiting this issue?
@Dgarc359 can you point me to the docs. I cant find it in the GraphQL API, just the old endpoints in the REST one that do not allow to mark notifications as done
@Dgarc359 can you point me to the docs. I cant find it in the GraphQL API, just the old endpoints in the REST one that do not allow to mark notifications as done
Sorry, I thought that at the time of responding to the original commenters, you meant that the GraphQL API was just not generally available for all endpoints. I didn't realize notifications itself was what was unavailable. I took a look through the API spec just now and didn't find a documented endpoint, apologies for my confusion.
@pwntester can you please elaborate on what's the blocker for implementing this feature?
https://github.com/meiji163/gh-notify exists for 2 years already, so there is an API available.
