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

Subscribe Toggle in Notification Dropdown #41

Merged
merged 22 commits into from
Jun 1, 2024
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions config/services.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ services:
- '@phpbb.wpn.form_helper'
- '@language'
- '@template'
- '@notification_manager'
tags:
- { name: event.listener }

Expand Down
35 changes: 16 additions & 19 deletions event/listener.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
use phpbb\controller\helper as controller_helper;
use phpbb\webpushnotifications\form\form_helper;
use phpbb\language\language;
use phpbb\notification\manager;
use phpbb\template\template;

/**
Expand All @@ -27,10 +28,9 @@ class listener implements EventSubscriberInterface
public static function getSubscribedEvents()
{
return [
'core.ucp_notifications_output_notification_types_modify_template_vars' => 'load_template_data',
'core.ucp_display_module_before' => 'load_language',
'core.acp_main_notice' => 'compatibility_notice',
'core.page_header_after' => 'service_worker_url',
'core.page_header_after' => 'load_template_data',
];
}

Expand All @@ -46,32 +46,40 @@ public static function getSubscribedEvents()
/* @var template */
protected $template;

/* @var manager */
protected $phpbb_notifications;

/**
* Constructor
*
* @param controller_helper $controller_helper Controller helper object
* @param form_helper $form_helper Form helper object
* @param language $language Language object
* @param template $template Template object
* @param manager $phpbb_notifications Notifications manager object
*/
public function __construct(controller_helper $controller_helper, form_helper $form_helper, language $language, template $template)
public function __construct(controller_helper $controller_helper, form_helper $form_helper, language $language, template $template, manager $phpbb_notifications)
{
$this->controller_helper = $controller_helper;
$this->form_helper = $form_helper;
$this->language = $language;
$this->template = $template;
$this->phpbb_notifications = $phpbb_notifications;
}

/**
* Load template data
*
* @param \phpbb\event\data $event
*/
public function load_template_data($event)
public function load_template_data()
{
if ($event['method_data']['id'] === 'notification.method.phpbb.wpn.webpush')
$methods = $this->phpbb_notifications->get_subscription_methods();
if (array_key_exists('notification.method.phpbb.wpn.webpush', $methods))
{
$template_ary = $event['method_data']['method']->get_ucp_template_data($this->controller_helper, $this->form_helper);
if (!$this->language->is_set('NOTIFICATION_METHOD_PHPBB_WPN_WEBPUSH'))
{
$this->language->add_lang('webpushnotifications_module_ucp', 'phpbb/webpushnotifications');
}
$template_ary = $methods['notification.method.phpbb.wpn.webpush']['method']->get_ucp_template_data($this->controller_helper, $this->form_helper);
$this->template->assign_vars($template_ary);
}
}
Expand All @@ -91,15 +99,4 @@ public function compatibility_notice()
{
$this->template->assign_var('S_WPN_COMPATIBILITY_NOTICE', phpbb_version_compare(PHPBB_VERSION, '4.0.0-dev', '>='));
}

/**
* Generate service worker URL globally for update
*/
public function service_worker_url()
{
if (!$this->template->retrieve_var('U_WEBPUSH_WORKER_URL'))
{
$this->template->assign_var('U_WEBPUSH_WORKER_URL', $this->controller_helper->route('phpbb_webpushnotifications_ucp_push_worker_controller'));
}
}
}
2 changes: 2 additions & 0 deletions language/en/webpushnotifications_module_ucp.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,10 @@

$lang = array_merge($lang, [
'NOTIFICATION_METHOD_PHPBB_WPN_WEBPUSH' => 'Web Push',
'NOTIFY_WEBPUSH_ENABLE_SHORT' => 'Enable Web Push notifications',
'NOTIFY_WEBPUSH_ENABLE' => 'Enable receiving Web Push notifications',
'NOTIFY_WEBPUSH_ENABLE_EXPLAIN' => 'Enable receiving browser-based push notifications.<br>The notifications can be turned off at any time in your browser settings, by unsubscribing, or by disabling the push notifications below.',
'NOTIFY_WEBPUSH_SUBSCRIBE' => 'Subscribe',
'NOTIFY_WEBPUSH_UNSUBSCRIBE' => 'Unsubscribe',
'NOTIFY_WEBPUSH_SUBSCRIBED' => 'Subscribed',
]);
93 changes: 93 additions & 0 deletions migrations/update_user_notifications.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
<?php
/**
*
* phpBB Browser Push Notifications. An extension for the phpBB Forum Software package.
*
* @copyright (c) 2024, phpBB Limited <https://www.phpbb.com>
* @license GNU General Public License, version 2 (GPL-2.0)
*
*/

namespace phpbb\webpushnotifications\migrations;

use phpbb\db\migration\migration;

class update_user_notifications extends migration
{
/**
* @inheritDoc
*/
public static function depends_on()
{
return ['\phpbb\webpushnotifications\migrations\add_webpush'];
}

/**
* @inheritDoc
*/
public function effectively_installed()
{
$sql = 'SELECT method
FROM ' . $this->table_prefix . "user_notifications
WHERE method = '" . $this->db->sql_escape('notification.method.phpbb.wpn.webpush') . "'";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add AND notify = 1 as far as we're looking for enabled notification types.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this I'm just looking to see if it exists yet, because if it does it means it's already been installed and people have started making settings already.

$result = $this->db->sql_query($sql);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we need at least 1 positive result so probably $this->db->sql_query_limit($sql, 1).

$row = $this->db->sql_fetchrow($result);
$this->db->sql_freeresult($result);

return $row !== false;
}

/**
* @inheritDoc
*/
public function update_data()
{
return [
['custom', [[$this, 'update_notifications']]],
];
}

/**
* Add default push notifications for users in chunks
*
* @param $start int Start value for the update
* @return int|true Next start value or true if complete
*/
public function update_notifications($start)
{
$start = (int) $start;
$limit = 500;
$updated = 0;

$sql_ary = [];

$sql = 'SELECT user_id
FROM ' . $this->table_prefix . 'users
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably limit user types to USER_NORMAL and USER_FOUNDER to skip unused accounts.

ORDER BY user_id ASC';
$result = $this->db->sql_query_limit($sql, $limit, $start);

while ($row = $this->db->sql_fetchrow($result))
{
$sql_ary[] = [
'item_type' => 'notification.type.pm',
'item_id' => 0,
'user_id' => (int) $row['user_id'],
'notify' => 1,
'method' => 'notification.method.phpbb.wpn.webpush',
];
$sql_ary[] = [
'item_type' => 'notification.type.quote',
'item_id' => 0,
'user_id' => (int) $row['user_id'],
'notify' => 1,
'method' => 'notification.method.phpbb.wpn.webpush',
];
$updated++;
}
$this->db->sql_freeresult($result);

$this->db->sql_multi_insert($this->table_prefix . 'user_notifications', $sql_ary);

return ($updated === $limit) ? $start + $limit : true;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{% if NOTIFICATIONS_WEBPUSH_ENABLE and not notification_types|default %}
<div class="wpn-notification-dropdown-footer">
<span>{{ lang('NOTIFY_WEBPUSH_ENABLE_SHORT') ~ lang('COLON') }}</span>
<button id="subscribe_webpush" name="subscribe_webpush"><i class="icon fa-toggle-off fa-fw icon-lightgray"></i><span>{{ lang('NOTIFY_WEBPUSH_SUBSCRIBE') }}</span></button>
<button id="unsubscribe_webpush" name="unsubscribe_webpush" class="hidden"><i class="icon fa-toggle-on fa-fw icon-blue"></i><span>{{ lang('NOTIFY_WEBPUSH_SUBSCRIBED') }}</span></button>
</div>
{% endif %}
3 changes: 3 additions & 0 deletions styles/all/template/event/overall_header_head_append.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{% if NOTIFICATIONS_WEBPUSH_ENABLE %}
{% include '@phpbb_webpushnotifications/ucp_notifications_webpush.html' %}
{% endif %}
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,5 @@
</script>

{% INCLUDEJS '@phpbb_webpushnotifications/webpush.js' %}
{% INCLUDEJS '@phpbb_webpushnotifications/update_worker.js' %}
{% INCLUDECSS '@phpbb_webpushnotifications/phpbb_wpn.css' %}
5 changes: 2 additions & 3 deletions styles/all/template/update_worker.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
'use strict';

/* global phpbbWebpushOptions, domReady */
function webpushWorkerUpdate() {
if ('serviceWorker' in navigator) {
navigator.serviceWorker.getRegistration(serviceWorkerUrl)
navigator.serviceWorker.getRegistration(phpbbWebpushOptions.serviceWorkerUrl)
.then((registration) => {
registration.update();
})
Expand All @@ -13,7 +14,6 @@ function webpushWorkerUpdate() {
}
}
// Do not redeclare function if exist
/* global domReady */
if (typeof domReady === 'undefined') {
window.domReady = function(callBack) {
if (document.readyState === 'loading') {
Expand All @@ -25,6 +25,5 @@ if (typeof domReady === 'undefined') {
}

domReady(() => {
/* global serviceWorkerUrl */
webpushWorkerUpdate();
});
24 changes: 24 additions & 0 deletions styles/all/theme/phpbb_wpn.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
.button[disabled],
.button[disabled]:hover,
.button.disabled,
.button.disabled:hover {
background: #e0e0e0;
border-color: #9e9e9e;
color: #9e9e9e;
}

.button.hidden {
display: none;
}

.wpn-notification-dropdown-footer {
font-size: 12px;
border-top: solid 1px #b9b9b9;
display: flex;
justify-content: space-between;
padding: 5px 25px;
}

.wpn-notification-dropdown-footer button:disabled {
opacity: 0.7;
}
6 changes: 0 additions & 6 deletions styles/prosilver/template/event/overall_footer_after.html

This file was deleted.

This file was deleted.

12 changes: 0 additions & 12 deletions styles/prosilver/theme/phpbb_wpn.css

This file was deleted.

20 changes: 15 additions & 5 deletions tests/event/listener_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ class listener_test extends \phpbb_database_test_case
/** @var \phpbb\webpushnotifications\notification\method\webpush */
protected $notification_method_webpush;

/* @var \PHPUnit\Framework\MockObject\MockObject|\phpbb\notification\manager */
protected $phpbb_notifications;

protected static function setup_extensions()
{
return ['phpbb/webpushnotifications'];
Expand Down Expand Up @@ -81,6 +84,10 @@ protected function setUp(): void
->setMethods(array())
->getMock();

$this->notifications = $this->getMockBuilder('\phpbb\notification\manager')
->disableOriginalConstructor()
->getMock();

$this->notification_method_webpush = new \phpbb\webpushnotifications\notification\method\webpush(
$this->config,
$db,
Expand All @@ -101,7 +108,8 @@ protected function set_listener()
$this->controller_helper,
$this->form_helper,
$this->language,
$this->template
$this->template,
$this->notifications
);
}

Expand All @@ -114,7 +122,6 @@ public function test_construct()
public function test_getSubscribedEvents()
{
self::assertEquals([
'core.ucp_notifications_output_notification_types_modify_template_vars',
'core.ucp_display_module_before',
'core.acp_main_notice',
'core.page_header_after',
Expand Down Expand Up @@ -215,10 +222,13 @@ public function test_get_ucp_template_data($user_id, $method_data, $subscription
$this->set_listener();

$method_data['method'] = $this->notification_method_webpush;
$event_data = ['method_data'];

$this->notifications->expects(self::once())
->method('get_subscription_methods')
->willReturn([$method_data['id'] => $method_data]);

$dispatcher = new \phpbb\event\dispatcher();
$dispatcher->addListener('core.ucp_notifications_output_notification_types_modify_template_vars', [$this->listener, 'load_template_data']);
$dispatcher->trigger_event('core.ucp_notifications_output_notification_types_modify_template_vars', compact($event_data));
$dispatcher->addListener('core.page_header_after', [$this->listener, 'load_template_data']);
$dispatcher->trigger_event('core.page_header_after');
}
}
15 changes: 15 additions & 0 deletions tests/functional/functional_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -73,4 +73,19 @@ public function test_ucp_module()
$this->assertContainsLang('NOTIFY_WEBPUSH_ENABLE', $crawler->filter('label[for="subscribe_webpush"]')->text());
$this->assertContainsLang('NOTIFICATION_METHOD_PHPBB_WPN_WEBPUSH', $crawler->filter('th.mark')->eq(2)->text());
}

public function test_dropdown_subscribe_button()
{
$this->login();

$this->add_lang_ext('phpbb/webpushnotifications', 'webpushnotifications_module_ucp');

$crawler = self::request('GET', 'index.php');
$this->assertCount(1, $crawler->filter('.wpn-notification-dropdown-footer'));
$this->assertContainsLang('NOTIFY_WEBPUSH_SUBSCRIBE', $crawler->filter('.wpn-notification-dropdown-footer #subscribe_webpush')->text());
$this->assertContainsLang('NOTIFY_WEBPUSH_SUBSCRIBED', $crawler->filter('.wpn-notification-dropdown-footer #unsubscribe_webpush')->text());

$crawler = self::request('GET', 'ucp.php?i=ucp_notifications&mode=notification_options');
$this->assertCount(0, $crawler->filter('.wpn-notification-dropdown-footer'));
}
}
Loading