search

wefork / wekan

The open-source Trello-like kanban (built with Meteor)
https://wekan.io
MIT License
61 stars 12 forks source link

Wefork sandstorm update #1

Closed dwrensha closed 7 years ago

dwrensha commented 7 years ago

This is a rebase of https://github.com/wekan/wekan/pull/704, which gets the Sandstorm package working again and adds integration with Sandstorm notifications and in-grain sharing.

pierreozoux commented 7 years ago

I'd be glad if a sandstorm user could test it. I quickly checked the code, and it looks good to me.

ocdtrekkie commented 7 years ago

As a useful note, there is a test link for this PR: https://oigimtlx81ukwop56zlo.oasis.sandstorm.io/

Discussion: https://groups.google.com/forum/#!topic/sandstorm-dev/26WWtYu0-kk

pierreozoux commented 7 years ago

Ok, I see this conversation related to (sorry, I'm also catching up with the project): https://github.com/wekan/wekan/issues/415 https://github.com/wekan/wekan/issues/543

@AlexanderS could you comment on this PR please also?

I'm actually a bit concerned of seeing sandstorm.js growing. Just a quick question, for sandstorm notification, do you reuse notification facility that was created already in this PR: https://github.com/wekan/wekan/pull/454

(Just to make sure code is not being duplicated, and that sandstorm functionalities match "traditional" functionality)

dwrensha commented 7 years ago

do you reuse notification facility that was created already in this PR...

No, we do not use Wekan's built-in notification facility. Instead, we integrate on the level of activity events. We push all of Wekan's activity events to Sandstorm and leave it to Sandstorm to generate notifications from them. There's a bit of documentation about how this works in activity.capnp. The idea is that Sandstorm can provide a uniform, configurable interface for notifications across all grains -- even for grains of apps that don't independently implement notifications. Moreover, with the activity stream, Sandstorm can provide a uniform way to keep audit logs.

Note that the Wekan Sandstorm app already hides all of the UI elements that non-Sandstorm Wekan uses for configuring notifications.

dwrensha commented 7 years ago

I'm actually a bit concerned of seeing sandstorm.js growing

The logic in sandstorm.js is only enabled when Wekan is running under Sandstorm. As with sandstorm-pkgdef.capnp, if you are not a Sandstorm user then it will not affect you.

We at Sandstorm have a vested interest in Wekan continuing to work well on our platform, and would be happy to take responsibility for maintaining these Sandstorm-specific files.