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] Feat/notification gw handler and webhook #1080

Conversation

ixuz
Copy link
Member

@ixuz ixuz commented Nov 30, 2021

Related issues

#1074
Depends on #1075

Description

This is a draft implementation to support WebHookSubscription2021.

We opened this pull request to document and serve as subject for discussion during the upcoming architectural discussion for Solid Notifications held 9th of December.

TamSzaGot and others added 3 commits November 29, 2021 16:02
Preparaton for implementation of Solid Notifications according to https://solid.github.io/notifications/protocol
The proposal refers to https://www.w3.org/TR/activitystreams-core/ for the activity types.
In order to provide meaningful notifications to subscribers we have differentiated between the types of resource changes, such as CREATED, CHANGED and DELETED.
This required an introduction of a ModificationResult interface that wraps the changed ResourceIdentifiers.
…ModificationType to be an enum, and removed unnecessary factory functions 'createdResource', 'changedResource' & 'deletedResource' from 'ResourceStore'
…ificationGateway, NotificationHandler and a WebHookSubscription2021Handler.
@jaxoncreed
Copy link
Contributor

Thanks for posting @ixuz ! Do you have a components.js config file that works with these classes yet?

@ixuz
Copy link
Member Author

ixuz commented Nov 30, 2021

@jaxoncreed I don't have a components.js config file at hand, but let's see if my colleague @TamSzaGot might have one.

@TamSzaGot
Copy link

I have to test the configurations before I submit them. I'll do it today.

@TamSzaGot
Copy link

TamSzaGot commented Dec 1, 2021

Sorry I didn't have enough time to push the configuration files today. Since the implementation moved a bit since I had the server up and running with notifications I had to adjust the config files a bit and when I eventually had the server up and running with the notifications configured it didn't behave as expected. I'd like to debug and fix the issues before I push something. Will do as soon as possible.

Another thing to note is that the files pushed by ixuz are based on an early version of https://solid.github.io/notifications/protocol#webhooksubscription2021 and nothing from the https://github.com/solid/notifications-panel/blob/main/proposals/solid-webhook-notifications.md has been implemented. (Since we just recently came over that proposal).

@jaxoncreed
Copy link
Contributor

@TamSzaGot That's fine. I can help implement the new notifications stuff. I can get started on that once you push your configuration. Looking forward to it!

@jaxoncreed
Copy link
Contributor

Hey @ixuz @TamSzaGot. Is there any progress on getting the configuration?

@ixuz
Copy link
Member Author

ixuz commented Dec 3, 2021

@jaxoncreed The configuration files will be pushed in an hour or so to this branch.

@jaxoncreed
Copy link
Contributor

I'm making a PR to this branch here (individdata#1). I'll continue to work on this.

@jaxoncreed jaxoncreed mentioned this pull request Dec 9, 2021
10 tasks
@ixuz
Copy link
Member Author

ixuz commented Dec 9, 2021

I'm making a PR to this branch here (individdata#1). I'll continue to work on this.

Thank you @jaxoncreed! We'll closely collaborate together on this PR you've opened.

@TamSzaGot
Copy link

@jaxoncreed @ixuz I did some refactoring and added unit tests for 100% coverage. Have also made it up to date with the main branch.

@ixuz
Copy link
Member Author

ixuz commented Jan 29, 2022

Odd, looks like one of the checks failed with a socket timeout. Re-running the CI workflow might be necessary.

@ixuz
Copy link
Member Author

ixuz commented May 24, 2022

Brief update: This PR is still in progress and we'll push new code to the branch soonish... 😉

@ixuz
Copy link
Member Author

ixuz commented Jul 26, 2022

This PR is superseded by PR #1388

@ixuz ixuz closed this Jul 26, 2022
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.

None yet

3 participants