Hotfix missing discussion items url

[Yago]

I'm aware it's not super clean, but it does the trick until Github fix its REST API (see #424). If so, the first test will pass and this code will never reach out again and we will be able to remove it without second thought 😉

Again, it's just an idea for the mean time, feel free to say that it's a bad idea. I didn't update the test due to the very “temporary” aspect of this code, but I can do so if needed.

[codebytere]

Thanks! Looking good, just a cleanup comment.

[codebytere]

We know the first and third conditionals will always be true at this point of code execution, so we probably can clean this up a little bit:

    // Some Notification types from GitHub are missing urls in their subjects.
    if (notification.subject.url) {
      const url = generateGitHubWebUrl(notification.subject.url);
      shell.openExternal(url);
    // For discussions, we can at least send users to the main discussions page.
    } else if (notification.subject.type === SubjectType.Discussion) {
      const url = generateGitHubWebUrl(`${notification.repository.url}/discussions`);
      shell.openExternal(url);
    }

[Yago]

Done 😉

[Luflosi]

What's the status of this? I'd really like to be able to click notifications to open discussions.

[Yago]

I'll do the cleanup asap 😉

[setchy]

Looking forward to this enhancement. Thanks @Yago for contributing this PR

[setchy]

@manosim - do you think this enhancement is ready to merge and release?

[Araxeus]

we can get the Discussion thread url using GraphQL search

I have created a repo to easily test this: https://github.com/Araxeus/github_discussion_notification_url

Any thoughts? (the graphQL query logic is exclusively in https://github.com/Araxeus/github_discussion_notification_url/blob/main/graphql.js)

EDIT: I've opened a PR #538 which uses that logic and in case it fails, defaults to the the method here

[afonsojramos]

Will merge #538 over this one. Thank you to everyone involved!!!