basic notify_push usage with session handling

[alangecker]

• Touches: #256
• Target version: master

Summary

keeps track of current active sessions on a board and shows them in the UI, including real time updates via notify_push.

it is not very useful in itself, but acts as a base and is probably most of the work towards getting live updates for changes #256 finally, because the server needs to know to whom it sends these updates.

If this step goes through, I'm also happy to work on the actual live updates :)

Part of the code is thankfully taken from @juliushaertl sessions in text

Requirement

it needs notify_push installed and running.

TODO

• [ ] get some confirmation that this way is somehow sane (also first time for me developing with nextcloud 🙂 )
• [x] Fix 500 errors when notify_push isn't enabled.
• [x] consider case of multiple open windows by the same user
• [x] fix existing tests and linting
• [ ] write some tests
• [ ] add API documentation

Checklist

• [x] Code is properly formatted
• [x] Sign-off message is added to all commits
• [ ] Tests (unit, integration, api and/or acceptance) are included
• [ ] Documentation (manuals or wiki) has been updated or is not required

[alangecker]

tests and analysis currently still fail with errors, which seem totally unrelated to my changes. Is master broken or do I oversee somthing?

• ERROR: UndefinedMethod - lib/Service/CirclesService.php:61:21 - Method OCA\Circles\CirclesManager::startSuperSession does not exist

  • Other PR's like also fail, so probably not my fault :)

• .

  1) OCA\Deck\Db\AssignmentMapperTest::testFind
  OCA\Deck\NoPermissionException: Creating boards has been disabled for your account.
  
  /home/runner/work/deck/deck/apps/deck/lib/Service/BoardService.php:316
  /home/runner/work/deck/deck/apps/deck/tests/integration/database/AssignmentMapperTest.php:98
  /home/runner/work/deck/deck/apps/deck/tests/integration/database/AssignmentMapperTest.php:93
  

• Error: Your lock file does not contain a compatible set of packages. Please run composer update. (didn't modify the compose files)

[juliusknorr]

Maybe to also kick of some discussion for the next steps, I'm not sure if you have thought about the live update process already, but there is probably a need for an improved approach from the refreshBoard that is currently happening. We already track and expose last modified dates for different elements so that might be an option so that the notify_push queue could advertise the last change date and the vuex store could refresh the content of just the affected items then.

[alangecker]

@juliushaertl thanks for all the feedback 🙂 It is now rebased (hopefully fixing the CI issues) and I resolved some of the smaller points, will continue on it but maybe won't find much time in the next 2 weeks

[juliusknorr]

Thanks for the update, looking forward to that :)

[matbgn]

@juliushaertl any plan for the review/merge of this highly awaited MR?

[alangecker]

I've added now some integration tests and would say that it is now ready to be reviewed from my side 🙂

[juliusknorr]

Thanks, I'll have another look next week.

[matbgn]

@juliushaertl kindly reminder 😸 ❤️

[juliusknorr]

Sorry for the delay, has been busy times 🙈 I'll schedule it next week again

[matbgn]

What an impressive job @alangecker ! 😍

I would be glad to buy you a coffee (yes maybe a big one)... Do you have any sponsorship link? Or any PayPal account?

[alangecker]

@matbgn thanks a lot ❤️

you've (hopefully) got a mail 🙂

(I've used the only address I could easily find now: mat.gh (at) bqn.**)

[matbgn]

@matbgn thanks a lot ❤️

you've (hopefully) got a mail 🙂

(If used the only address I could easily find now: mat.gh (at) bqn.**)

You've done it right! The order is gone, as I said it's not a big amount but I hope you will enjoy your next coffee and again: THANK YOU!

[alangecker]

@matbgn thank you very much!

as I already said in the mail, I'm always really happy that people donate for free software, even without asking - like you ❤️

[matbgn]

but for this PR I do not intend to implement actual updates for the stacks/cards, thought to limit it to the active session avatar list as the base and work on the live updates separately :)

And what I can promise you is that when you will release the live updates part I will just double it! 😉

Happy coding!

[alangecker]

so, from my side I don't see anything to do anymore. Would be great if someone (@juliushaertl ?) could do another review (especially the recent changes with passing on the session token)

I have also started working on the actual live updates, which seems already fully working 🙂 https://github.com/alangecker/nextcloud-deck/compare/sessions...live-updates ~...PR incoming!~ -> PR here https://github.com/nextcloud/deck/pull/4273

[shoetten]

Sorry for the spam, but.. hell yeah! Really excited for this feature! 🎉

[matbgn]

I can only agree, really excited to see @juliushaertl's feedback

[marcelklehr]

From looking through the code, it looks really good, apart from the two nitpicks above! I'll also have to setup a test env to properly test it, though.

[marcelklehr]

I was able to successfully test this. Functionality works when notify_push is enabled and nothing breaks when it's not installed. Great stuff!

[nimishavijay]

Design-wise this is great first step towards achieving live updating boards! It looks great :) Really nice work! 🚀

[marcelklehr]

@nimishavijay

closing the session when the tab is not active might be a bit confusing, even if it's technically a cool feature. Google Docs also doesn't do this afaik. @juliushaertl / @jancborchardt Do we want this?

[nimishavijay]

Ah, missed that, sorry!

I would say closing the session immediately after the tab is unfocused would be confusing. If there was a timeout of say, 15 mins or something that would make more sense :)

[shoetten]

@juliushaertl @marcelklehr @nimishavijay Can we merge this sooner than later and work out any feature wishes you might have from a company perspective later? As far as i understand there are no technical blockers anymore and this PR is now almost half a year in review.

[juliusknorr]

@alangecker Could you maybe rebase to latest master to resolve the conflict? Would be fine to get this in then from my side.

I'd generally agree that the session close after the tab is in the background should rather be deferred for some time. This may also help to avoid additional load for creating and removing the extra sessions. That can also be a follow up of course ;)

[juliusknorr]

Pushed a commit to make the static analysis happy. Good to go from my side once CI is green 👍

[alangecker]

I've just created an issue for further discussions about the session closing/creation topic https://github.com/nextcloud/deck/issues/4354 maybe someone is motivated to put some thinking effort into it? 🙃

@juliushaertl nice, big thanks for the commit, reviewing and everything else :)

[juliusknorr]

Thanks a lot for your awesome contribution. So great to see the first part being merged. Keep up the great work. ❤️