#2539 - Feature request: Pushover.net notifications #2540
Conversation
| @@ -660,3 +660,8 @@ div.fade { | |||
| } | |||
|
|
|||
| /* END EDIT */ | |||
|
|
|||
There was a problem hiding this comment.
May want to put this above the END EDIT line :-)
sql/000_base_structure.sql
Outdated
| @@ -248,6 +248,12 @@ CREATE TABLE `statistics_users` ( | |||
| KEY `account_id_timestamp` (`account_id`,`timestamp`) | |||
| ) ENGINE=InnoDB DEFAULT CHARSET=utf8; | |||
|
|
|||
| CREATE TABLE IF NOT EXISTS `push_notification_settings` ( | |||
There was a problem hiding this comment.
Maybe this could be a more generic user_settings table? I remember that I wanted to implement this in the past to allow for a extraction of per-user settings from the accounts table to a settings table. Just an idea, maybe this is sufficient.
There was a problem hiding this comment.
Follow up on this, it's entirely possible to use a more generic user_settings table that include account_id, key, value as the fields. Your key could be push_notification_setting, value whatever you need to store.
This will change how we access a user account (JOIN the tables and add it to user information?) but may be better?
|
|
||
| <div class="panel panel-info"> | ||
| <div class="panel-heading"> | ||
| <i class="fa fa-gear fa-fw"></i> Push Notification Settings |
There was a problem hiding this comment.
Is this required in it's own block or could this also be edited via Notifications below? Maybe adding this all the time can confuse users once they selected e-mail notifications (which means none of these push settings would apply).
There was a problem hiding this comment.
PushNotification "sliders" are hidden by CSS and visible only if pushnotifications is set
| @@ -142,7 +142,7 @@ CREATE TABLE IF NOT EXISTS `settings` ( | |||
| UNIQUE KEY `setting` (`name`) | |||
| ) ENGINE=InnoDB DEFAULT CHARSET=utf8; | |||
|
|
|||
| INSERT INTO `settings` (`name`, `value`) VALUES ('DB_VERSION', '1.0.1'); | |||
| INSERT INTO `settings` (`name`, `value`) VALUES ('DB_VERSION', '1.0.2'); | |||
There was a problem hiding this comment.
You'll also have to define the expected version via: https://github.com/MPOS/php-mpos/blob/master/include/version.inc.php#L5
I did see you supplied the upgrade script, so that's great - just also add the new version to the include file so the admin dashboard properly displays the version change :-)
|
Looks good so far, I'll give you a bit to iron out any issues you may encounter then merge into development for others to try it out. Don't forget to update the Wiki with a page or paragraph about push notifications :-) |
|
Merging into development so others can start trying it out on that branch. |
|
Hey @blondak - not sure if you still track this but users reporting issues with your implementation. If you get a chance maybe check out the new tickets opened up regarding notifications :-) |
Here is it,...