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

Implementation of Email & Telegram notification #71

Closed

Conversation

thnilsen
Copy link
Contributor

Here is the PR requests for email notification. This implements the email notification together with @jhuesser Telegram notification.

I have chosen to keep token for the subscribers in the subscriber table rather than using the tokens table, since the token table is very much linked to the admin users as it is now. This can of course be changed if so desired but would require a way to avoid the overlapping table IDs.

In addition to the email notification the changes will also add Markdown support for incidents.

The code is working and can be tested on my test server as linked to in #65.

Design of dropdown menu, email and HTML template should be considered work in progress. Help on that part is very much appreciated!

Install.php has yet to be updated to render settings with the new config.php options.

Subscriber menu as of now:
dropdown_menu

Manage subscription: (with support for IDN domains!)
subscription

Email notification:
status_email

Telegram notification:
telegram_message

- $tg_user was no longer used for validating if a subscriber  was logged
  in or not, as this has been moved to a $_SESSION variable.
- Renamed get_service_details to populate_impacted_services
- Removed unused variables
@Chipx0
Copy link

Chipx0 commented Nov 9, 2019

All i get is a blank page when clicking "subscribe"

@jhuesser
Copy link
Contributor

jhuesser commented Nov 9, 2019 via email

@Chipx0
Copy link

Chipx0 commented Nov 9, 2019

Follow this guide: https://core.telegram.org/widgets/login

Im wanting to use the email subscribe, Not the telegram one.

@yigitkeremoktay
Copy link
Member

@jhuesser and @thnilsen I am starting to work on a new database class which will move some of the settings into database as @Pryx suggested thus allowing this PR to be deployed. Since I could not understand the problem with this PR I will ask you to clarify a few a things.

  1. Is this PR functional
  2. If it is not what is the issue with it.
    Thanks.

@thnilsen
Copy link
Contributor Author

@yigitkeremoktay The PR is functional with the exception of missing the database schema changes since there currently isn't any method to apply db changes automatically. I could also do with some design work on the HTML email template as it is far from good. I'll post a db schema change here later, then we can agree how to implement it.

@yigitkeremoktay
Copy link
Member

@thnilsen I am trying to implement this as I was creating the database class but I decided not to because it turns out that this not something that I can do without your help as it was impossible to reverse engineer

@yigitkeremoktay
Copy link
Member

@thnilsen I would be glad if you could help me though 👍

@thnilsen
Copy link
Contributor Author

@yigitkeremoktay Here is a quick diff. I don't have the time to double check it right now but it should hopefully be correct. Test it and see if it works for you. If not I'll check it later tonight.

subscription.sql.txt

@yigitkeremoktay
Copy link
Member

@thnilsen I've managed to make it stable enough but see https://skyfallenhosted.ml/ss-demo/ . Your menu is not showing up.

@thnilsen
Copy link
Contributor Author

thnilsen commented Aug 15, 2020

@yigitkeremoktay Are you still unable to see the subscriber menu? I can see it on all my devices. There's no services added to that instance of you server-status demo so I can't really test the full functionallity, but at least I was able to start the sign up process, get the subscription email and get to the logged in page where one would choose which services to subscribe to.

@yigitkeremoktay
Copy link
Member

yigitkeremoktay commented Aug 15, 2020

I reverted the CSS update.
See this for the new look update (https://status.theskyfallen.com)
It has the issue with the new css change

@yigitkeremoktay
Copy link
Member

I don't have time to work on it for now though :(

@thnilsen
Copy link
Contributor Author

Have you defined the config parameters? At least one of these two needs to be true, otherwise it will not render the menu.

define('SUBSCRIBE_EMAIL', true);
define('SUBSCRIBE_TELEGRAM', true);

The code to handle sending of notification was missing. This change
makes two different options available on how notifications will be
handled which will be controlled by CRON_SERVER_IP config option.

 - If CRON_SERVER_IP is set, the server with the given IP should
   call URL ../admin/?task=cron every x minutes. If the config
   is left empty, the notification will be called once the
   incident has  been saved. (The latter meothod might cause
   server timeout if there are large numbers of subscribers!)

Other minor changes:

 - Removed old commented code

 - Removed call to syslog used for debugging
@yigitkeremoktay
Copy link
Member

@thnilsen As we have merged these feature I think you may close this PR as it is now a duplicate

@thnilsen
Copy link
Contributor Author

Closing PR as it is being included via @yigitkeremoktay repo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants