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

WIP: Implement comment notifications #4

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Mikescops
Copy link
Owner

No description provided.

@Mikescops Mikescops self-assigned this May 4, 2020
@NoahTheDuke
Copy link

I would love to see this finished.

@Mikescops
Copy link
Owner Author

Yeah me too, I'll take a look again when I have time

@vinnymac
Copy link

@Mikescops anything I can do to help move this forward? I would love to see this extension support notifications.

@Mikescops
Copy link
Owner Author

@vinnymac well, I have let down this feature for now.

I think we need to find an architecture of how it could work with the limitation of the API.
At the beginning, I was thinking of comparing 2 consecutive requests to the API but it's not that easy.
I'll think about it again soon.
If you have ideas let me know.

@vinnymac
Copy link

vinnymac commented Nov 3, 2022

@vinnymac well, I have let down this feature for now.

I think we need to find an architecture of how it could work with the limitation of the API. At the beginning, I was thinking of comparing 2 consecutive requests to the API but it's not that easy. I'll think about it again soon. If you have ideas let me know.

Perhaps I am missing some knowledge about how this works, but reading over the code: wouldn’t it be possible for us to pull the last response out of storage, comparing it to the latest api response, and send notifications based on what is new or updated?

Then we would update the local store with the latest api responses, and wait for the next poll, and so on.

If this wouldn’t work, even rudimental-y I wonder what is preventing it. I may spend some time investigating myself as well.

@Mikescops
Copy link
Owner Author

Yeah right, that's the plan, comparing is just not easy between what exists, what doesn't anymore, and what changed.
I would have preferred an event-based API but managing webhooks from the extension is not possible.

@vinnymac
Copy link

vinnymac commented Nov 4, 2022

@Mikescops this might not support all of the desired features, but what if instead we request (and store) the users events

https://docs.gitlab.com/ee/api/events.html

Filter the api response by only events that haven’t been seen before. Then send a notification to the user for each new event.

This would avoid having to diff too many types of API requests. And make the notifications very specific to changes that actually occurred.

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.

3 participants