SAK-49287 Notifications. Improved push permissions workflow #11947

adrianfish · 2023-09-27T11:35:43Z

https://sakaiproject.atlassian.net/browse/SAK-49287

adrianfish · 2023-09-27T11:39:41Z

csev · 2023-09-27T11:57:58Z

Tasty - I was wondering how we were going to handle this. Folks say 'no' to the "accept notifications" for all sites - so for an LMS they need a second chance. We will need to see if browsers kind of beat us up - they probably have some trick to keep sites from asking over and over.

webcomponents/bundle/src/main/bundle/sui-notifications.properties

ern · 2023-09-27T17:28:48Z

sakai-root/pom.xml

+
+  <name>Sakai Root</name>
+  <groupId>org.sakaiproject</groupId>
+  <artifactId>ROOT</artifactId>


Adrian can you explain this artifact ROOT? do we have a better name?

So the war file is called ROOT.war and is deployed to the tomcat root, that's all.

Do you think I can change the artifact id while still creating ROOT.war to deploy to tomcat?

the reason we have this sakai-root thing is that we need to deploy a single service worker to the top level of our hierarchy so it can control all of Sakai's paths. It's crucial.

the artifact is now "serviceworker" and I've used deployId to deploy to tomcat as the ROOT war.

ern

Not particularly a fan of the path names maybe we can discuss that?

adrianfish · 2023-09-28T11:28:16Z

which path names in particular?

adrianfish · 2024-01-31T18:15:27Z

adrianfish · 2024-01-31T18:16:53Z

I've updated this to change the ROOT sakai maven module to just be called "root" and not "sakai-root". Also, I've added a button in the notifications window to make it explicit that the user is opting into Sakai notifications.

adrianfish · 2024-03-14T18:18:51Z

I've now updated this maven module to be called "serviceworker". No more root.

stetsche · 2024-03-15T09:46:39Z

@adrianfish I think I can take a look at the PR sometime next week, if it's still needed. But just to get some context, when the user does not accept the push notifications, there will be no notifications within the portal neither?

adrianfish · 2024-03-15T10:49:27Z

@stetsche Exactly that. Our challenge is inform the user that Sakai notifications are good. Push is so powerful and it is a shame that companies have abused it so heavily.

stetsche · 2024-03-15T11:00:10Z

@adrianfish, okay I understand what the PR is for. But when I enable push notifications, will notifications display only in the portal, or like other push notifications as OS notifications (What a lot of websites are abusing)?

adrianfish · 2024-03-15T11:01:56Z

Depending on the browser, both. Chrome and firefox will popup a OS notification as well as adding a notification to the portal area.

stetsche · 2024-03-15T11:04:45Z

Okay, and if you want to have only portal notifications you need to enable them in the browser and block browser notifications on the OS level?

adrianfish · 2024-03-15T11:21:28Z

I suppose so. Or we could just disable them in the js. They are explicitly triggered, the os notifications.

webcomponents/tool/src/main/frontend/packages/sakai-push-utils/test/sakai-push-utils.test.js

serviceworker/pom.xml

adrianfish · 2024-03-21T18:32:26Z

Done with this now. I've removed sakai-message-broker.js. Most of the logic moved nicely into sakai-push.utils.js and the remainder I just tacked onto portal.init.js.

ottenhoff · 2024-03-22T13:26:58Z

This seems to consistently break Cypress tests. I saw lots of errors in my console around theme switching. Maybe theme switcher was depending on something that got moved?

stetsche

@adrianfish I can take a closer look at next week

webcomponents/tool/src/main/frontend/packages/sakai-push-utils/package.json

webcomponents/tool/src/main/frontend/packages/sakai-push-utils/README.md

webcomponents/tool/src/main/frontend/packages/sakai-push-utils/test/sakai-push-utils.test.js

webcomponents/tool/src/main/frontend/packages/sakai-push-utils/web-test-runner.config.mjs

https://sakaiproject.atlassian.net/browse/SAK-49287

portal/portal-render-engine-impl/impl/src/webapp/vm/morpheus/includeAccountNav.vm

portal/portal-render-engine-impl/impl/src/webapp/vm/morpheus/includeMobileFooter.vm

portal/portal-render-engine-impl/impl/src/webapp/vm/morpheus/includeStandardHead.vm

webcomponents/bundle/src/main/bundle/sakai-notifications.properties

webcomponents/tool/src/main/frontend/packages/sakai-push-utils/LICENSE

webcomponents/tool/src/main/frontend/packages/sakai-push-utils/src/sakai-push-utils.js

kernel/api/src/main/java/org/sakaiproject/messaging/api/model/UserNotification.java

portal/portal-render-engine-impl/impl/src/webapp/vm/morpheus/includeAccountNav.vm

stetsche

The PR looks good to me now. Well done @adrianfish! I only found a console.log to remove, but I'll mark this as approved already, so I don't have to come back to the PR

webcomponents/tool/src/main/frontend/packages/sakai-push-utils/src/sakai-push-utils.js

…/src/sakai-push-utils.js Co-authored-by: stetsche <[email protected]>

adrianfish · 2024-03-28T15:32:32Z

Thanks a ton @stetsche, for the review. And anybody else who reviewed this PR. Much appreciated :)

Made changes to the artefact name and sakai code module name

bgarciaentornos · 2024-04-04T11:45:20Z

I think the husky command is failing for windows

Co-authored-by: stetsche <[email protected]> (cherry picked from commit 3cbd7d2) Conflicts: portal/portal-impl/impl/src/java/org/sakaiproject/portal/charon/SkinnableCharonPortal.java portal/portal-render-engine-impl/impl/src/webapp/vm/morpheus/includeStandardHead.vm webcomponents/tool/src/main/frontend/packages/sakai-notifications/src/SakaiNotifications.js

ottenhoff reviewed Sep 27, 2023

View reviewed changes

webcomponents/bundle/src/main/bundle/sui-notifications.properties Outdated Show resolved Hide resolved

ottenhoff reviewed Sep 27, 2023

View reviewed changes

webcomponents/bundle/src/main/bundle/sui-notifications.properties Outdated Show resolved Hide resolved

ern reviewed Sep 27, 2023

View reviewed changes

ern previously requested changes Sep 27, 2023

View reviewed changes

adrianfish force-pushed the SAK-49287 branch 2 times, most recently from f2f1c35 to 7706cfc Compare December 6, 2023 08:12

adrianfish force-pushed the SAK-49287 branch from 7706cfc to 2455bf8 Compare January 19, 2024 10:20

adrianfish requested a review from ern March 5, 2024 17:25

adrianfish force-pushed the SAK-49287 branch 3 times, most recently from 5d77766 to 1b2f73e Compare March 14, 2024 18:08

adrianfish requested review from ottenhoff and stetsche March 15, 2024 07:56

adrianfish force-pushed the SAK-49287 branch 2 times, most recently from 7cabc84 to 8a9d022 Compare March 19, 2024 12:54

jesusmmp reviewed Mar 20, 2024

View reviewed changes

webcomponents/tool/src/main/frontend/packages/sakai-push-utils/test/sakai-push-utils.test.js Outdated Show resolved Hide resolved

adrianfish force-pushed the SAK-49287 branch 2 times, most recently from 7d4b3b6 to 1fe1cd2 Compare March 21, 2024 10:13

ottenhoff reviewed Mar 21, 2024

View reviewed changes

serviceworker/pom.xml Outdated Show resolved Hide resolved

adrianfish force-pushed the SAK-49287 branch 2 times, most recently from 3e7136d to cdfb529 Compare March 21, 2024 14:04

stetsche requested changes Mar 22, 2024

View reviewed changes

adrianfish added 6 commits March 24, 2024 09:54

SAK-49287 Notifications. Improved push permissions workflow

d89bcaf

https://sakaiproject.atlassian.net/browse/SAK-49287

Prevent multiple push registrations

3c58c82

Moved push code to web component

b044069

Fixed the test and the lint-staged command

e1c242e

husky changes to package.json

6e0373f

Updated README and cleaned up deps

5df525e

stetsche requested changes Mar 25, 2024

View reviewed changes

review fixes

95c923d

adrianfish force-pushed the SAK-49287 branch from 8a126c7 to 95c923d Compare March 25, 2024 16:31

adrianfish added 2 commits March 25, 2024 16:44

i18n

f06c5db

Moved no permissions indicator to bootstrap icon

c2e90fa

ottenhoff reviewed Mar 26, 2024

View reviewed changes

portal/portal-render-engine-impl/impl/src/webapp/vm/morpheus/includeAccountNav.vm Show resolved Hide resolved

adrianfish added 2 commits March 27, 2024 16:13

Notification permission message uses ui.service

da397da

Added sh command back

dc60ad9

stetsche approved these changes Mar 28, 2024

View reviewed changes

webcomponents/tool/src/main/frontend/packages/sakai-push-utils/src/sakai-push-utils.js Outdated Show resolved Hide resolved

Update webcomponents/tool/src/main/frontend/packages/sakai-push-utils…

6510a7e

…/src/sakai-push-utils.js Co-authored-by: stetsche <[email protected]>

adrianfish merged commit 3cbd7d2 into sakaiproject:master Mar 29, 2024
5 checks passed

adrianfish deleted the SAK-49287 branch March 29, 2024 10:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SAK-49287 Notifications. Improved push permissions workflow #11947

SAK-49287 Notifications. Improved push permissions workflow #11947

adrianfish commented Sep 27, 2023

adrianfish commented Sep 27, 2023

csev commented Sep 27, 2023

ern Sep 27, 2023

adrianfish Sep 27, 2023

adrianfish Oct 4, 2023

adrianfish Oct 4, 2023

adrianfish Mar 14, 2024

ern left a comment

adrianfish commented Sep 28, 2023

adrianfish commented Jan 31, 2024

adrianfish commented Jan 31, 2024

adrianfish commented Mar 14, 2024

stetsche commented Mar 15, 2024

adrianfish commented Mar 15, 2024

stetsche commented Mar 15, 2024 •

edited

Loading

adrianfish commented Mar 15, 2024

stetsche commented Mar 15, 2024

adrianfish commented Mar 15, 2024

adrianfish commented Mar 21, 2024

ottenhoff commented Mar 22, 2024

stetsche left a comment

stetsche left a comment

adrianfish commented Mar 28, 2024

bgarciaentornos commented Apr 4, 2024

SAK-49287 Notifications. Improved push permissions workflow #11947

SAK-49287 Notifications. Improved push permissions workflow #11947

Conversation

adrianfish commented Sep 27, 2023

adrianfish commented Sep 27, 2023

csev commented Sep 27, 2023

ern Sep 27, 2023

Choose a reason for hiding this comment

adrianfish Sep 27, 2023

Choose a reason for hiding this comment

adrianfish Oct 4, 2023

Choose a reason for hiding this comment

adrianfish Oct 4, 2023

Choose a reason for hiding this comment

adrianfish Mar 14, 2024

Choose a reason for hiding this comment

ern left a comment

Choose a reason for hiding this comment

adrianfish commented Sep 28, 2023

adrianfish commented Jan 31, 2024

adrianfish commented Jan 31, 2024

adrianfish commented Mar 14, 2024

stetsche commented Mar 15, 2024

adrianfish commented Mar 15, 2024

stetsche commented Mar 15, 2024 • edited Loading

adrianfish commented Mar 15, 2024

stetsche commented Mar 15, 2024

adrianfish commented Mar 15, 2024

adrianfish commented Mar 21, 2024

ottenhoff commented Mar 22, 2024

stetsche left a comment

Choose a reason for hiding this comment

stetsche left a comment

Choose a reason for hiding this comment

adrianfish commented Mar 28, 2024

bgarciaentornos commented Apr 4, 2024

stetsche commented Mar 15, 2024 •

edited

Loading