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

Alert template clean up "\r\n\n" #10541

Merged
merged 2 commits into from
Sep 5, 2019

Conversation

bestlong
Copy link
Contributor

@bestlong bestlong commented Aug 21, 2019

DO NOT DELETE THIS TEXT

Please note

Please read this information carefully. You can run ./scripts/pre-commit.php to check your code before submitting.

Testers

If you would like to test this pull request then please run: ./scripts/github-apply <pr_id>, i.e ./scripts/github-apply 5926
After you are done testing, you can remove the changes with ./scripts/github-remove. If there are schema changes, you can ask on discord how to revert.

@murrant
Copy link
Member

murrant commented Aug 22, 2019

These sql files aren't used anymore.

@bestlong
Copy link
Contributor Author

Ref #10495

Copy link
Contributor

@garysteers garysteers left a comment

Choose a reason for hiding this comment

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

This will fix any new installation, can a database/migration script be written to update the Template (but only IF it has the original content changed by these commits)

@murrant
Copy link
Member

murrant commented Aug 23, 2019

Can we remove the \r? It used to be needed for windows email clients I am pretty sure.

@bestlong
Copy link
Contributor Author

@murrant use web ui create new alert template input two line:

A
B

then use mysqldump get sql:

INSERT INTO `alert_templates` VALUES (50,'Default Alert Template','{{ $alert->title }}\nSeverity: {{ $alert->severity }}\n@if ($alert->state == 0) Time elapsed: {{ $alert->elapsed }} @endif\nTimestamp: {{ $alert->timestamp }}\nUnique-ID: {{ $alert->uid }}\nRule: @if ($alert->name) {{ $alert->name }} @else {{ $alert->rule }} @endif\n@if ($alert->faults) Faults:\n@foreach ($alert->faults as $key => $value)\n  #{{ $key }}: {{ $value[\'string\'] }}\n@endforeach\n@endif\nAlert sent to:\n@foreach ($alert->contacts as $key => $value)\n  {{ $value }} <{{ $key }}>\n@endforeach',NULL,NULL),(51,'Test CRLF','A\nB\n','','');

only see \n in (51,'Test CRLF','A\nB\n','',''), so i cleanup "\r".

@bestlong
Copy link
Contributor Author

Hi @garysteers database/migration script will affect all user, and difficult to identify data status.

Only four pieces of data can be solved by copying:

Default Alert Template

{{ $alert->title }}
Severity: {{ $alert->severity }}
@if ($alert->state == 0) Time elapsed: {{ $alert->elapsed }} @endif
Timestamp: {{ $alert->timestamp }}
Unique-ID: {{ $alert->uid }}
Rule: @if ($alert->name) {{ $alert->name }} @else {{ $alert->rule }} @endif
@if ($alert->faults) Faults:
@foreach ($alert->faults as $key => $value)
  #{{ $key }}: {{ $value['string'] }}
@endforeach
@endif
Alert sent to:
@foreach ($alert->contacts as $key => $value)
  {{ $value }} <{{ $key }}>
@endforeach

BGP Sessions.

{{ $alert->title }}
Severity: {{ $alert->severity }}
@if ($alert->state == 0) Time elapsed: {{ $alert->elapsed }} @endif
Timestamp: {{ $alert->timestamp }}
Unique-ID: {{ $alert->uid }}
Rule: @if ($alert->name) {{ $alert->name }} @else {{ $alert->rule }} @endif
@if ($alert->faults) Faults:
@foreach ($alert->faults as $key => $value)
  #{{ $key }}: {{ $value['string'] }}
  Peer: {{ $value['astext'] }}
  Peer IP: {{ $value['bgpPeerIdentifier'] }}
  Peer AS: {{ $value['bgpPeerRemoteAs'] }}
  Peer EstTime: {{ $value['bgpPeerFsmEstablishedTime'] }}
  Peer State: {{ $value['bgpPeerState'] }}
@endforeach
@endif

Ports

{{ $alert->title }}
Severity: {{ $alert->severity }}
@if ($alert->state == 0) Time elapsed: {{ $alert->elapsed }} @endif
Timestamp: {{ $alert->timestamp }}
Unique-ID: {{ $alert->uid }}
Rule: @if ($alert->name) {{ $alert->name }} @else {{ $alert->rule }} @endif
@if ($alert->faults) Faults:
@foreach ($alert->faults as $key => $value)
  #{{ $key }}: {{ $value['string'] }}
  Port: {{ $value['ifName'] }}
  Port Name: {{ $value['ifAlias'] }}
  Port Status: {{ $value['message'] }}
@endforeach
@endif

Temperature

{{ $alert->title }}
Severity: {{ $alert->severity }}
@if ($alert->state == 0) Time elapsed: {{ $alert->elapsed }} @endif
Timestamp: {{ $alert->timestamp }}
Unique-ID: {{ $alert->uid }}
Rule: @if ($alert->name) {{ $alert->name }} @else {{ $alert->rule }} @endif
@if ($alert->faults) Faults:
@foreach ($alert->faults as $key => $value)
  #{{ $key }}: {{ $value['string'] }}
  Temperature: {{ $value['sensor_current'] }}
  Previous Measurement: {{ $value['sensor_prev'] }}
@endforeach
@endif

or U can clean all alert template then run seed

root@ubuntu-bionic:~# cd /opt/librenms/
root@ubuntu-bionic:/opt/librenms# php artisan db:seed --class=DefaultAlertTemplateSeeder
**************************************
*     Application In Production!     *
**************************************

 Do you really wish to run this command? (yes/no) [no]:
 > yes

Database seeding completed successfully.
root@ubuntu-bionic:/opt/librenms#

@bestlong
Copy link
Contributor Author

Hi @murrant

Do i need to fix codeclimate issues ?

@murrant
Copy link
Member

murrant commented Aug 27, 2019

Treat code climate errors as warnings.

I think you need something like this to go with this:

Index: LibreNMS/Alert/Transport/Mail.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- LibreNMS/Alert/Transport/Mail.php	(date 1566502950000)
+++ LibreNMS/Alert/Transport/Mail.php	(date 1566916478486)
@@ -35,12 +35,10 @@
 
     public function contactMail($obj)
     {
-        if (empty($this->config['email'])) {
-            $email = $obj['contacts'];
-        } else {
-            $email = $this->config['email'];
-        }
-        return send_mail($email, $obj['title'], $obj['msg'], (Config::get('email_html') == 'true') ? true : false);
+        $email = empty($this->config['email']) ? $obj['contacts'] : $this->config['email'];
+        $msg = preg_replace("/(?<!\r)\n/", "\r\n", $obj['msg']); // fix line returns for windows mail clients
+
+        return send_mail($email, $obj['title'], $msg, (Config::get('email_html') == 'true') ? true : false);
     }
 
     public static function configTemplate()

@murrant
Copy link
Member

murrant commented Aug 28, 2019

@garysteers you can delete all existing templates (not just included ones) with TRUNCATE TABLE alert_templates; and then run ./lnms db:seed

@murrant
Copy link
Member

murrant commented Aug 28, 2019

I think this was a mistake when it was converted to the seeder only where where it wasn't inserting the line returns properly.

@murrant murrant merged commit a823a58 into librenms:master Sep 5, 2019
@murrant
Copy link
Member

murrant commented Sep 13, 2019

This pull request has been mentioned on LibreNMS Community. There might be relevant details there:

https://community.librenms.org/t/message-alert-mail/9550/3

@Akimace
Copy link

Akimace commented Sep 19, 2019

hello

@garysteers you can delete all existing templates (not just included ones) with TRUNCATE TABLE alert_templates; and then run ./lnms db:seed

I did TRUNCATE TABLE but when I do this comand: ./lnms db:seed
I have this error:

There are no commands defined in the "db" namespace.

Do you have an idea ?
My mail alert doesn't work since I did the TRUNCATE TABLE alert_templates;

Thank you

@murrant
Copy link
Member

murrant commented Sep 21, 2019

php artisan db:seed

@murrant
Copy link
Member

murrant commented Sep 30, 2019

This pull request has been mentioned on LibreNMS Community. There might be relevant details there:

https://community.librenms.org/t/v1-56-release-changelog-september-2019/9671/1

@lock lock bot locked as resolved and limited conversation to collaborators Nov 29, 2019
@bestlong bestlong deleted the fix-alert-templates-crlf branch September 21, 2020 16:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants