Subscription proxying

@sylvain asked two questions on this:

J'ai un doute sur le fait qu'il faille lier le conteneur relais à un conteneur réel. Il me semble que ce sont 2 choses différentes : pas besoin d'avoir un conteneur de job offers pour relayer des notifications si ?

We're in need of an attached container to provide an ./inbox/

Par ailleurs, si on veut implémenter ta suggestion de relais "communauté", ça fait un relais universel, pas seulement relié à un type d'objet.

No, because a community provide a distinct container for each resource type it serve, like /communities/startinblox/circles/ which will have its own ./inbox/.

Yes but my point is /communities/startinblox/circles/ seems to only relay notifications related to circles. I guess it would make sense to relay all notifications to a community, whatever the object type.

That doesn't actually really change your spec, except maybe we could create explicit relay containers, instead of implicit ones like you suggest.

Following LDN/AS, any container or resource should have an inbox and an outbox. If we want this behavior too, we could also implement it on resources, this way /community/startinblox/ will have its own ./inbox/ too.

ok. Makes sense.

Is it implemented in djangoldp, the implicit inbox creation thing?

No, it's a to-do part

perfect

Hubl's use case

We have a job board, on which job offers are published. A job offer is associated with a Community.

We'd like a given user to subscribe to its community's job offers. Her community would in turn subscribe to other communities.

When one of those communities published a job offer, say Community A, we'd like to notify every user whose Community subscribed to Community A.

Who is financing that ? Who's implementing ?

To be estimated and implemented in the next weeks. @calummackervoy could you check that ?

FNK is financing.

Obviously, it depends how much, we we were ready to put several hundreds of tokens down on this one.

@balessan yes I can provide an estimation for this but I'll need to synchronise with someone who understands this issue well (@jbpasquier ?) because some of the second part of the issue isn't totally clear to me... is there a time that's good for you later this week for a call ?

@calummackervoy : let's talk about that on tomorrow's standup and we can reach @jbpasquier in the afternoon if needed.

OK sounds good

Perfect!

Had a call with @balessan about this.. to verify:

What we have

djangoldp currently provides a Follower model for receiving ActivityStreams messages about updates on a specified resource urlid (not container). The updates are modelled by Activity and we refer to them as "backlinks"
djangoldp_notification currently provides a Subscriber model for notifications. It allows users to manually sign up to notifications on a local model or container via the admin panel, to a distant inbox (in practice, Prosody). The notifications are sent using the ActivityQueueService, but the format is Linked Data Notifications (Prosody doesn't like ActivityStreams)

What we want, scope for Hubl

expose a view which allows users (via the front-end) application to subscribe to a community on the model djangoldp_community.models.CommunityJobOffer. I.e. when a JobOffer is created, linked to the Community that I asked for, I receive an email and an in-app notification on the bell icon
users only want to know when a CommunityJobOffer is created; they don't care if it's been updated, deleted, or referenced in a list etc
am I missing requirements ? @alexbourlier

The original specification

have one relay service per Community
each relay service exposes the endpoint /communities/x/job-offers/subscribe/. This allows the user to create a Subscription on the container, using djangoldp_notification (@jbpasquier ?)
the relay endpoints should be exposed automatically on Community.post_save

notes:

this creates a dependency on djangoldp_notification by djangoldp_community
the assumption is that CommunityJobOffer is created already when a user POSTs a new job offer in the app (it may be that they post to JobOffer?)
(spec) will the user need to pass their inbox/ or can we assume it will always be /users/x/inbox/?
if we want to subscribe to the entire community in one request /communities/x/subscribe/, we could expose a custom view to achieve it (but we should open a separate issue for this I think)

Alternative solution

we expose a generic /subscribe/ endpoint on djangoldp_notification, which allows users to POST a subscription to their inbox
the user passes the container which they want to subscribe to (/communities/x/job-offers/)

additional notes on alternative solution:

no dependency created in djangoldp_community
easier to extend for other containers
we are not considering here the possibility of a "virtual container", i.e. a container which exists as a view but not as a model (djangoldp-notification#34)
/communities/x/job-offers/subscribe/ is a cleaner URL. If we were to support this functionality via DjangoLDP-Core, we could expose each container like this via an auto-registered LDPViewSet (or subclass)

Lots of questions ! Mostly directed at @jbpasquier I suppose.. Let me know if you would prefer a call :)

What we want, scope for Hubl [...]

We need this, but we want a generic solution and not a Community-specific one.

have one relay service per Community

Not really, it's more:

Add ./inbox/ to every resources/containers.
Allow those inbox to be public (they are by default)
Allow any resource/container to create subscription (Not only users)

Everything else works like the actual implementation.

each relay service exposes the endpoint /communities/x/job-offers/subscribe/. This allows the user to create a Subscription on the container, using djangoldp_notification (@jbpasquier ?)

This could be a global /subscribe/ endpoint, like it is actually, it's fine.

the relay endpoints should be exposed automatically on Community.post_save

Every model's inbox should subscribe to their related model.

this creates a dependency on djangoldp_notification by djangoldp_community

The Community package is already some kind of glue between everything we need for a complete Hubl experience, having the notification package as an optional dependency is not an issue.

the assumption is that CommunityJobOffer is created already when a user POSTs a new job offer in the app (it may be that they post to JobOffer?)

Fact is that /communities/x/circles/inbox/ can subscribe to /potatoes/ if it wants to. It's the responsibility to users (or delegated to packages) to chose their subscriptions wisely.

(spec) will the user need to pass their inbox/ or can we assume it will always be /users/x/inbox/?

They'll need to pass their ./inbox/. Resources & containers will have too.

if we want to subscribe to the entire community in one request /communities/x/subscribe/, we could expose a custom view to achieve it (but we should open a separate issue for this I think)

We don't want. /communities/x/inbox/ is able to subscribe to /communities/x/whatever/inbox/ and so get used as a proxy.

the user passes the container which they want to subscribe to (/communities/x/job-offers/)

This is not a solution, this is what we already have. :-)

We need to allow resources/containers to subscribe to another container.

OK I see, so the /inbox/ is acting as the relay server, because the user then in turn subscribes to the inbox

Add ./inbox/ to every resources/containers.

I had originally started writing the backlinks system like this but we ran into a conflict on the user/x/inbox defined by djangoldp_notification (djangoldp-notification#23 (closed) & djangoldp-notification#26). Are we aiming to replace (at least) the inbox view from djangoldp_notification, and the ForeignKey with user ?

We need to allow resources/containers to subscribe to another container.

Why do we need this ? Playing Devil's Advocate because we may get the same end feature by using what we have

Currently we don't send notifications for external objects. This would need to change, but I can't see why we have this check. Do you remember ?

Are we aiming to replace (at least) the inbox view from djangoldp_notification, and the ForeignKey with user ?

Yes

Why do we need this ? Playing Devil's Advocate because we may get the same end feature by using what we have

We need to allow https://serverA/job-offers/inbox/ to subscribe to https://serverB/job-offers/inbox/. This way, if https://serverA/users/andy/inbox/ subscribe to the 1st one, they'll also receive notification for serverB's job offers.

This would need to change, but I can't see why we have this check. Do you remember ?

It used to avoid duplicate notification to Prosody like that: serverA's circle is updated, serverA notify Prosody + notify serverB, serverB update its backlink + notify (the wrong) Prosody again.

they'll also receive notification for serverB's job offers

OK this makes sense, I'd been under the impression it was to subscribe to job offers from a specific community

If we just plug this system into the backlink Follower, notifications will be sent for every action (create, update, add, remove, delete).. this is in line with the specification

Following is defined in the sense typically used within Social systems in which the actor is interested in any activity performed by or on the object

but I think that users might only want to receive emails for Create (and maybe Delete).. is this correct ? (@alexbourlier )

...as I'm planning it, my instinct is to add booleans to the Follower object for each type of activity

@calummackervoy That's a very good question. I'd say only Create for now.

@Cyrilthiriet @sophie: What do you think? As a user, should I receive a notification only when a job offer is created AND matches my skills, or also when it is updated or deleted?

I'd only notify when the job offer is created for now. In the future, will have a conversation module associated with each offer, and when someone interacts with the offer, I'll receive a notification then as well.

Yeah I 'd say as well notifications only when created first. As a wish list for later, user could define each settings by himself / by offer.

added P1 label

mentioned in issue management/startinblox-feature-requests#15

mentioned in issue djangoldp-notification#33 (closed)

changed the description

UPDATED with output of discussion below

Scope

Adding an ../inbox/ view to every container/resource in LDPViewSet
inbox view POST should store an activity with the local_id of this inbox
Activity post_save should relay the notification to all followers on the inbox
~~inbox view GET should return a set of activities associated with the inbox in the path~~. (EDIT: requires ActivitySerializer implementation, preferably using following proposal: #277 )
Inbox view PATCH, PUT and DELETE will return 405 for now, except on /users/x/inbox/ (see djangoldp_notification below)
Similarly ../inbox/x/ will return 404

Estimate: 3h

We need a hook which allows the subscribing code to inject custom logic on when the inbox fires a notification (like a spam filter). Moved to separate issue, out of scope.. #383 (closed)

Refactoring Backlinks

We currently have a global /inbox/ view which would be better replaced so that it behaves like the other views mentioned here, meaning:

POSTing to the global /inbox/ would relay the message to any Followers on the inbox
POSTing an activity to any /inbox/ will update the local database graph with the changes specified in the activity

Estimate: 1h

refactor
testing

We can use the existing backlinks listeners to detect and send the activities

Refactoring djangoldp_notification

~~Activity will be extended to link to an Actor. actor_urlid will replace the author field~~ (EDIT: moved to scope of #372 )
Activity will be extended to have a new field summary
Activity will be extended to have a new field published, which will replace date in Notification
~~Activity will be extended to have new optional fields object and target_or_origin, URL fields~~
Notification will continue to exist, but it will subclass Activity and have only the fields: user and unread
migration script for existing Notifications
(during release) backup of existing data and then perform the migration
The backlinks should auto-save these fields where possible
the Admin panel should be extended so that fields are displayed and searchable

^ Estimate: 5h

We will need to serve a custom viewset for users/x/inbox/ so that the subclass is used and the response is paginated
~~The custom viewset will also extend the POST, so that the received activity is saved with the custom model and unread = True, and PUT/PATCH so that users can mark it as unread~~. EDIT: we didn't need to extend the viewsets to resolve the conflict on inbox, just the precedence so that the nested field inbox, is prioritised over the default InboxView

^ Estimate: 3h

I think that the faster thing is to keep Subscriber as a Prosody thing for now and track this issue (djangoldp-notification#26) for refactoring it later. The logic for sending a message to Prosody can then stay more or less the same

^ Estimate: some time for testing Prosody

The logic for sending an email to the user should be able to stay more or less the same
~~Notification permissions will need to be adapted~~. Wasn't needed
~~NotificationSetting.user should be changed to a URL inbox~~

^ Estimate: 4h

front-end changes will be needed to handle the model changes

^ Estimate needed

Hubl JobOffers

I receive a notification when a job offer ... matches my skills

users will need to subscribe to skills/x/job-offers/inbox/
write the JobOffer model to send a custom Activity to this virtual endpoint when a Skill is added

Estimate: 2h

front-end implementation needed

Testing

We will need extensive unit tests for this I think. Since we're dependent on the backlinks for federation in so many live environments I think that it would be wise to budget 3 days for this alone

Experiment with JB's idea to use LiveServerTestCase to upgrade our tests to also simulate live federations
finish unit testing
finish staging testing
testing Prosody
testing front-end
merge and release

Some of this time is also for the tests on each individual feature and as padding on the estimation for unexpected complexities

~~If I had a magic wand I'd really love to have the ability to test a federation using unit tests (I posted a question about it this morning)~~

inbox view PATCH, PUT and DELETE will return 405 for now. Similarly ../inbox/x/ will return 404

We may want to keep the PATCH on /users/x/inbox/y/ to allow users to set their notification as read.

configure this filter to match the skill-checking logic

Could you enter on details here? How will you deal with skills as you'll not be aware of user's skills and even not be aware of skills by themselves as all are hosted on different servers?

I'd love to see if we can reach a generic solution here too.

although such a solution consciously ignores the possibility of virtual containers

We may need this at some point with communities.

We currently have a global /inbox/ view which would be better consolidated so that it behaves like the other views mentioned here, meaning:

POSTing to the global /inbox/ would relay the message to any Followers on the inbox

POSTing an activity to any /inbox/ will update the local database graph with the changes specified in the activity

Do we need a server inbox?

We will need to serve a custom viewset for users/x/inbox/ so that the subclass is used and the response is paginated. There is an issue here that we don't know how the user's inbox is served (e.g. that the 'x' here is a slug). We'll need to plug djangoldp_account.djangoldp_urls.py and document what to do if you're using a custom user model

Maybe we can use the Notification model to this purpose, only for users with the unread field + the foreignkey to an user?

About the djangoldp-account part, we can assume to use the USER_NESTED_FIELDS parameter as it's already documented for replacements ?

I think that the faster thing is to keep Subscriber as a Prosody thing for now and open an issue on refactoring it later. The logic for sending a message to Prosody can then stay more or less the same

It's even better if we don't change the notification format for now.

front-end changes will be needed to handle the model changes

@balessan How do we handle the solid-notification's upgrade on this new behavior?

@calummackervoy Maybe we can upgrade the actual one to have the same ontology as the one that'll have after shipping this new feature? This way it would be pretty much seamless for applications?

If I had a magic wand I'd really love to have the ability to test a federation using unit tests (I posted a question about it this morning)

I don't know how we're dealing with tests right now, but with Django we can use LiveServerTestCase to reach this purpose. Notice that the official doc stands on the usage of Selenium but you don't need it, you can use the same test case as the one you had, reaching self.live_server_url as a distant one.

keep the PATCH on /users/x/inbox/y/ to allow users to set their notification as read.

OK makes sense !

you'll not be aware of user's skills and even not be aware of skills by themselves as all are hosted on different servers

This is a good point, I was thinking that as the "hook" will be running in the inbox receiver I'll have access to the urlid of the local object (and access to the backlinked skills attached to it), but in that case it will be one or the other - either I have access to the JobOffer.skills or the LDPUser.skills, depending on which inbox I'm in

I think that in which case it may be necessary to do the filtering at the level of the federation - i.e. in JavaScript on the front-end ? Is this possible ? EDIT: if it is, email will still be a pain

Backend I would need to pull in the resource from the filter inbox, at which point we're effectively doing backend federation (and would need authentication with it!)

I'd love to see if we can reach a generic solution here too.

I think the hook is the cleanest generic solution we can achieve - I define my filter logic and plug it into my model

Do we need a server inbox?

I don't think so. For clarity by "we should consolidate it" I mean "we should replace it by including the behaviour in the resource/container inboxes

Maybe we can use the Notification model to this purpose, only for users with the unread field + the foreignkey to an user?

Yes good idea

upgrade the actual one to have the same ontology as the one that'll have after shipping this new feature?

The ontology for Notification? If we wrote (in turtle) sib:Notification a as:Activity this should cover it :)

EDIT: we will also need to define the properties sib:Unread and user on sib:Notification, and then we could remove any others that exist

but with Django we can use LiveServerTestCase to reach this purpose ... you can use the same test case as the one you had, reaching self.live_server_url as a distant one

Really ?! So with this I would have one test server and a duplicate live server alive at the same time during my test ?

Backend I would need to pull in the resource from the filter inbox, at which point we're effectively doing backend federation (and would need authentication with it!)

Exact, this is my concern.

I think the hook is the cleanest generic solution we can achieve - I define my filter logic and plug it into my model

The ontology for Notification? If we wrote (in turtle) sib:Notification a as:Activity this should cover it :) EDIT: we will also need to define the properties sib:Unread and user on sib:Notification, and then we could remove any others that exist

Exact

Really ?! So with this I would have one test server and a duplicate live server alive at the same time during my test ?

Sure :-)

May need some cicd configuration though, not a big deal.

I think that in which case it may be necessary to do the filtering at the level of the federation - i.e. in JavaScript on the front-end ? Is this possible ? EDIT: if it is, email will still be a pain

Backend I would need to pull in the resource from the filter inbox, at which point we're effectively doing backend federation (and would need authentication with it!)

How about if the users subscribe to the relationship between the skill and job-offer, i.e. skills/x/job-offers/ ?

We would then write the JobOffer model to send a custom Activity to this virtual endpoint when a Skill is added

EDIT: each time the user changes their skills, their subscriptions would need to be changed... but it might be a price worth paying ?

Skills may have no reverse relation in some case, but only a ldpurl field

Oof this is already making my head spin. I was working with Apache Jena over the weekend and they support the fetching of remote resources server-side by url, I think long-term we should too

Which relationship might be only the urlid ? Do you mean that it will be an object, but may be an external reference (a backlink) ?

Anything which is using an IdURLField, it'll be a simple text as reference which renders as {"@id":"myFieldContent"}

I think that's fine, all we would need is the skill's urlid and we can handle that it's a IdURLField in the JobOffer.post_save listener

This is the bit I'm concerned about:

each time the user changes their skills, their subscriptions would need to be changed

But if it's okay for you then I can provide an estimation I think ?

I guess that we have no choice here.

The good thing is that in practice, the user may be subscribed only to one relay which is at its turn subscribed to everything else.

mentioned in issue management/core-team#15

Hi guys,

How are we doing here? Can we have an update of where we are and when we think we'll be able to have a clean spec and an estimation?

Yeah sorry about the random pause, I just updated my original comment with a new spec and estimation for each thing

There are some front-end changes which I can't estimate, so the estimate doesn't include those.. will need @jbpasquier or someone else to estimate I guess ?

OK super, thanks @calummackervoy!

Is this the comment you're referring to? #332 (comment 49020)

@balessan Have you reviewed it? Is it OK with you?

@calummackervoy Can you see with @matthieu as sib-notification's maintainer for the front-end changes?

@alexbourlier yes sorry hid my link in the text!

@jbpasquier okay sounds good :)

@alexbourlier : not yet, still on my todo.

@balessan OK thanks. Let me know when you think it is settled and we see how we integrate this back to the Hubl's roadmap.

changed time estimate to 1w 2h

changed time estimate to 1w 7h 20m

added 5h 20m of time spent at 2021-01-20

hi @balessan do we have some news here ? thx much !

Status is that the estimate from @calummackervoy is of 6d, and this is only the backend side. This estimate is missing the client side needed adaptations which we cannot foresee in details already.

Current estimate is 6d, 300 tokens.

The specification itself as is in current up-to-date comment from @calummackervoy I do approve it.

Thanks.

@jbpasquier How many more tokens do we need on top of that to have working job board notifications based on matching skills?

I will be able to put a token estimation on this one after having implemented the proxy on communities as it may makes things easier or harder depending on the status of server to server authentication. #236

@balessan GO GO GO here, on myself and @Berengere's behalf

@jbpasquier OK. Can this be done in parallel with the current issue, or not really? Do we have an issue about it? Do we know how much it costs?

@Cyrilthiriet I said GO here then

Can this be done in parallel with the current issue, or not really? Do we have an issue about it? Do we know how much it costs?

Not really, I would be more confident to deal with it in a second time.

OK, I tried...

@calummackervoy I am not sure I would put this one here: management/core-team#15 ?

Ah no definitely not, removed it just now parts of the plan could definitely be divided though

Whats the ETA here guys?

I hadn't started yet, the first time I read the comments above I thought you were saying "go - wait don't go" - but re-reading it, the budget is available for this now, not waiting on front-end scoping and not waiting on #236 ?

@calummackervoy @jbpasquier It seems I also overlooked #236 then. Can you guys explain to me what it is and why we need it?

Just had a call with @balessan and I'm now understanding that #236 isn't blocking for this issue, it's blocking for the Hubl job notifications board, and this issue is blocking that as well

#236 is already financed by Trust, it's to do with exchanging server keys during the backlinks process to authenticate the sender is who they say they are. It's a massive security hole that we don't do that at the moment

We're contacting Autonomic today about them tackling this issue (with my help) and taking on #236. Either way we'll start on this later this week

Hmmm.... OK

It is 11 days since we gave the GO ahead. I'm having the impression it is often very painful to get the framework team to start taking on any given task. Whenever a ball lands in your garden, we know we won't get it back for weeks.

Anything I can do to reduce that to days, or ideally hours, next time a ball falls in your garden? It is a massive pain for us. I'm scared to have to trust you guys with anything because it always takes 3 to 10 times as long as when we do it outside of the framework team.

I'm not blaming anyone, and I'm all for finding solutions in good intelligence to this, but we do have a reactivity problem guys, and it is costing us users everyday.

@Cyrilthiriet It should not happen that you let something like this not moving for 11 days. After 2 days maximum you should be on everybody's back. What's the point of having a fancy roadmap tab if we don't make us of it?

I sent my response in an email

Well @alexbourlier I take it on me.

Otherwise I hope all the stuffs we are doing organizing teams, PO, maintainers, etc, by the last times could bring us on a better way to work.

@calummackervoy if you need, if I can help with something that make moving us, don't hesitate.

Hi @calummackervoy sorry I didn't see the email. Could we have an estimation of deliverability here ? Thx !

He is getting better

EDIT: I think you realise that I've given an estimation, and you mean can you have a deadline ? Do we have budget released for this issue regardless of any front-end issues which are missing estimations ?

In terms of where this issue was last at I gave my summary here: #332 (comment 54187)

We were waiting on Autonomic saying that they can do it, they got back to us but are going to work on #236 first. I said I'd start it later this week anyway but that didn't happen so it'll have to be Monday I guess. I've been working on the Trust/DPoP issues so I need to sync with @balessan if I'm working on this or someone else. If it's me I'll set a date on Monday

The email I sent is titled "Synchronisation between Core team and clients". In my opinion it's worth a read if you can find it or want me to forward it to you, but it also has a resolution already I believe

Do we have budget released for this issue regardless of any front-end issues which are missing estimations ?

Ah ah, you're trying us, right? The answer is yes, this issue is funded, and we're hoping it to start

Your overall answer is clear though: we have to decide if it is you or Autonomic who takes this one on. How do we move forward?

No I got confused by the conversations about other issues and other estimations and how that fitted in with your funding. I thought it's better to ask to be sure

I spoke with @balessan this morning about our roadmap. We're hitting a deadline on Trust at the end of this month and so we want to assign me to those tasks. I've been working on DPoP which is around 70% finished, today I'm starting on some DjangoLDP-Account issues and I need to look into using shapes to generate models as well - evidently it would be a lot to juggle this task as well

So du coup we want to go ahead with assigning Autonomic this issue - which they will want to start after they've finished with #236 (236 is also a pre-requisite for your end-feature). We can ask them for a due date but since we're onboarding them with these issues I think it would be a lot better to ask them to do one thing at a time. To give a rough timescale, I think we'd be looking to have everything finished with this in April

@alexbourlier can you see with Benoît if you want to do something differently please

thought it's better to ask to be sure

I agree!

@alexbourlier can you see with Benoît if you want to do something differently please

No I'm cool. Somewhere in April, Autonimic, I believe that answers our questions.

@Cyrilthiriet Are you a happy man?

@alexbourlier No doubt I am :) Thx @calummackervoy for explanations

@decentral1se @calummackervoy we looked at the conclusion of those comments yesterday and it actually was that nothing is blocking us from going forward. The link with the #236 is only indirect/an improvement to consider but it's not a strong requirement.

So is that something you could start quickly on ? @decentral1se do you have available resources to check this one too ?

@Cyrilthiriet sadly we are late, we stay stucked waiting for investigations from @plup which are actually not needed. Management fail here. We'll solve that quickly.

Thanks @balessan keep us updated, do you have an idea when this should be done ?

mentioned in issue management/startinblox-feature-requests#172

mentioned in issue #362 (closed)

mentioned in issue djangoldp-notification#36 (closed)

mentioned in issue infra/prosody-modules#19 (closed)

mentioned in issue #236

mentioned in issue #254

changed the description

Subscription proxying

Designs

Child items ...

Activity

What we have

What we want, scope for Hubl

The original specification

Alternative solution

Scope

Refactoring Backlinks

Refactoring djangoldp_notification

Hubl JobOffers

Testing