Refactor: Convert TODO Comments Into Issues (#1262)

* refactor: make sender configurable

* fix: throw an error if operating system is unkown

* refactor: move todo to #1260

* refactor: todo has been covered by #1229

* refactor: separate function due to bad abstraction

* docs: fix typo and error example

* test: remove failed test

docs/src/pages/guides/tls-checkers.md

@@ -7,7 +7,8 @@ You can check TLS validity and set the threshold to send notification before the
 
 ```yaml
 probes:
-  - requests:
+  - id: 'Example'
+    requests:
       - url: http://example.com
 certificate:
   domains:

docs/src/pages/quick-start.md

@@ -9,7 +9,7 @@ There are many ways to install Monika. However, currently only x64 architecture
 
 ### Windows
 
-The recommendeded approach is to use [Chocolatey](https://community.chocolatey.org/packages/monika), a popular package manager for Windows. Check [Monika page on Chocolatey](https://community.chocolatey.org/packages/monika) for more detailed information.
+The recommended approach is to use [Chocolatey](https://community.chocolatey.org/packages/monika), a popular package manager for Windows. Check [Monika page on Chocolatey](https://community.chocolatey.org/packages/monika) for more detailed information.
 
 If you have installed Chocolatey in your PC, then run the following command to install Monika:
 

packages/notification/channel/mailgun.ts

@@ -26,6 +26,8 @@ import FormData from 'form-data'
 import Joi from 'joi'
 import type { NotificationMessage } from '.'
 import Mailgen from 'mailgen'
+
+import { getSender } from '../utils/default-sender'
 import { sendHttpRequest } from '../utils/http'
 
 export type NotificationData = {
@@ -109,10 +111,8 @@ function getContent({
   subject: string
   domain: string
 }): Content {
-  // TODO: Read from ENV Variables
-  const DEFAULT_SENDER_NAME = 'Monika'
+  const DEFAULT_SENDER_NAME = getSender().name
   const from = `${DEFAULT_SENDER_NAME} <mailgun@${domain}>`
-
   const mailGenerator = new Mailgen({
     theme: 'default',
     product: {

packages/notification/channel/smtp.ts

@@ -26,6 +26,7 @@ import * as nodemailer from 'nodemailer'
 import Mailgen from 'mailgen'
 import Joi from 'joi'
 
+import { getSender } from '../utils/default-sender'
 import type { NotificationMessage } from '.'
 
 type NotificationData = {
@@ -51,8 +52,7 @@ export const send = async (
   { hostname, password, port, recipients, username }: NotificationData,
   { body, subject }: NotificationMessage
 ): Promise<void> => {
-  // TODO: Read from ENV Variables
-  const DEFAULT_EMAIL = '[email protected]'
+  const DEFAULT_EMAIL = getSender().email
   const DEFAULT_SENDER_NAME = 'Monika'
   const transporter = nodemailer.createTransport({
     host: hostname,

packages/notification/index.ts

@@ -22,13 +22,23 @@
  * SOFTWARE.                                                                      *
  **********************************************************************************/
 
-import { channels, Notification, NotificationMessage } from './channel'
+import {
+  channels,
+  type Notification,
+  type NotificationMessage,
+} from './channel'
 import { getErrorMessage } from './utils/catch-error-handler'
+import { type InputSender, updateSender } from './utils/default-sender'
 
 async function sendNotifications(
   notifications: Notification[],
-  message: NotificationMessage
+  message: NotificationMessage,
+  sender?: InputSender
 ): Promise<void> {
+  if (sender) {
+    updateSender(sender)
+  }
+
   await Promise.all(
     notifications.map(async ({ data, type }) => {
       const channel = channels[type]

packages/notification/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@hyperjumptech/monika-notification",
-  "version": "1.16.1",
+  "version": "1.17.0",
   "description": "notification package for monika",
   "main": "lib/index.js",
   "types": "lib/index.d.ts",

packages/notification/utils/default-sender.ts

@@ -0,0 +1,19 @@
+type Sender = {
+  name: string
+  email: string
+}
+
+export type InputSender = Partial<Sender>
+
+let defaultSender: Sender = {
+  name: 'Monika',
+  email: '[email protected]',
+}
+
+export function getSender(): Sender {
+  return defaultSender
+}
+
+export function updateSender(sender: InputSender) {
+  defaultSender = { ...defaultSender, ...sender }
+}

src/components/config/index.ts

@@ -232,7 +232,6 @@ function watchConfigFile({ flags, path }: WatchConfigFileParams) {
 }
 
 const getPathAndTypeFromFlag = (flags: MonikaFlags) => {
-  // TODO: Assuming the first index of config is the primary config
   let path = flags.config?.[0]
   let type = 'monika'
 

src/components/logger/flush.test.ts

@@ -23,10 +23,7 @@
  **********************************************************************************/
 
 import sinon from 'sinon'
-import { ux } from '@oclif/core'
-import { test } from '@oclif/test'
 import * as history from './history'
-import cmd from '../../commands/monika'
 import { flush } from './flush'
 
 let flushAllLogsStub: sinon.SinonStub
@@ -49,32 +46,4 @@ describe('Flush command', () => {
       sinon.assert.calledOnce(flushAllLogsStub)
     })
   })
-
-  describe('Not force', () => {
-    test
-      // TODO: Remove skip
-      .skip()
-      // arrange
-      .stub(ux.ux, 'prompt', (stub) => stub.resolves('n'))
-      .stdout()
-      // act
-      .do(() => cmd.run(['--flush']))
-      .it('should cancel flush', () => {
-        // assert
-        sinon.assert.notCalled(flushAllLogsStub)
-      })
-
-    test
-      // TODO: Remove skip
-      .skip()
-      // arrange
-      .stub(ux.ux, 'prompt', (stub) => stub.resolves('Y'))
-      .stdout()
-      // act
-      .do(() => cmd.run(['--flush']))
-      .it('should flush', () => {
-        // assert
-        sinon.assert.calledOnce(flushAllLogsStub)
-      })
-  })
 })

src/components/logger/history.ts

@@ -135,7 +135,6 @@ export function setDatabase(
 
 async function migrate() {
   await database().migrate({
-    // TODO: Current vercel/pkg is dependent with CommonJS
     // eslint-disable-next-line unicorn/prefer-module
     migrationsPath: path.join(__dirname, '../../../db/migrations'),
   })
@@ -448,7 +447,6 @@ export async function saveProbeRequestLog({
   const now = Math.round(Date.now() / 1000)
   const requestConfig = probe.requests?.[requestIndex]
 
-  // TODO: limit data stored.
   const responseBody = requestConfig?.saveBody
     ? typeof probeRes.data === 'string'
       ? probeRes.data

src/components/probe/prober/http/index.ts

@@ -260,7 +260,6 @@ function getProbeResultMessage({
   request,
   response,
 }: ProbeResultMessageParams): string {
-  // TODO: make this more generic not probe dependent
   if (request?.ping) {
     return response?.body as string
   }

src/jobs/tls-check.ts

@@ -22,20 +22,24 @@
  * SOFTWARE.                                                                      *
  **********************************************************************************/
 
+import {
+  sendNotifications,
+  type Notification,
+  type NotificationMessage,
+} from '@hyperjumptech/monika-notification'
+import { format } from 'date-fns'
+
 import { getConfig } from '../components/config'
 import { saveNotificationLog } from '../components/logger/history'
-import { sendAlerts } from '../components/notification'
+import {
+  getMonikaInstance,
+  getOSName,
+} from '../components/notification/alert-message'
 import { checkTLS, getHostname } from '../components/tls-checker'
-import type { Notification } from '@hyperjumptech/monika-notification'
-import type { ValidatedResponse } from '../plugins/validate-response'
+import { getContext } from '../context'
+import getIp from '../utils/ip'
 import { log } from '../utils/pino'
-import { probeRequestResult } from '../interfaces/request'
-
-type SendTLSErrorNotificationProps = {
-  hostname: string
-  notifications: Notification[]
-  errorMessage: string
-}
+import { publicIpAddress } from '../utils/public-ip'
 
 export function tlsChecker(): void {
   const config = getConfig()
@@ -64,62 +68,86 @@ export function tlsChecker(): void {
         return
       }
 
-      sendTLSErrorNotification({
+      sendErrorNotification({
+        errorMessage: error.message,
         hostname,
         notifications: notifications || [],
-        errorMessage: error.message,
-      })
+      }).catch(console.error)
     })
   }
 }
 
-function sendTLSErrorNotification({
+type SendErrorNotificationParams = {
+  errorMessage: string
+  hostname: string
+  notifications: Notification[]
+}
+
+async function sendErrorNotification({
+  errorMessage,
   hostname,
   notifications,
-  errorMessage,
-}: SendTLSErrorNotificationProps) {
-  // TODO: Remove probe below
-  // probe is used because probe detail is needed to save the notification log
+}: SendErrorNotificationParams) {
   const probe = {
     id: '',
+    alerts: [],
+    interval: 10,
     name: '',
     requests: [],
-    interval: 10,
-    alerts: [],
   }
 
-  for (const notification of notifications) {
-    // TODO: Remove validation below
-    // validation is used because it is needed to send alert
-    const validation: ValidatedResponse = {
-      alert: {
-        id: '',
-        assertion: '',
-        message: errorMessage,
-      },
-      isAlertTriggered: true,
-      response: {
-        status: 500,
-        responseTime: 0,
-        data: {},
-        body: {},
-        headers: {},
-        result: probeRequestResult.success,
-      },
-    }
-
-    saveNotificationLog(probe, notification, 'NOTIFY-TLS', errorMessage).catch(
-      (error) => log.error(error.message)
-    )
-
-    // TODO: invoke sendNotifications function instead
-    // looks like the sendAlerts function does not handle this
-    sendAlerts({
+  await Promise.allSettled(
+    notifications.map(async (notification) => {
+      await sendNotifications(
+        notifications,
+        await getNotificationMessage({ hostname, errorMessage })
+      )
+      await saveNotificationLog(probe, notification, 'NOTIFY-TLS', errorMessage)
+    })
+  )
+}
+
+type GetMessageParams = {
+  hostname: string
+  errorMessage: string
+}
+
+async function getNotificationMessage({
+  hostname,
+  errorMessage,
+}: GetMessageParams): Promise<NotificationMessage> {
+  const privateIpAddress = getIp()
+  const { userAgent } = getContext()
+  const [monikaInstance, osName] = await Promise.all([
+    getMonikaInstance(privateIpAddress),
+    getOSName(),
+  ])
+  const time = format(new Date(), 'yyyy-MM-dd HH:mm:ss XXX')
+  const body = `Message: ${errorMessage}
+
+  URL: ${hostname}
+
+  Time: ${time}
+
+  From: ${monikaInstance}
+
+  OS: ${osName}
+
+  Version: ${userAgent}`
+
+  return {
+    subject: 'New Incident from Monika',
+    body,
+    summary: errorMessage,
+    meta: {
+      hostname,
+      privateIpAddress,
       probeID: '',
+      publicIpAddress,
+      time,
+      type: 'incident',
       url: hostname,
-      probeState: 'invalid',
-      notifications: notifications ?? [],
-      validation,
-    }).catch((error) => log.error(error.message))
+      version: userAgent,
+    },
   }
 }

src/utils/open-website.ts

@@ -44,8 +44,7 @@ export const open = (url: string): void => {
     }
 
     default: {
-      // TODO: Handle new OS
-      break
+      throw new Error(`Unknown operating system: ${operatingSystem}`)
     }
   }
 }

src/utils/pino.ts

@@ -43,7 +43,6 @@ function getOptions() {
     hideObject: true,
   }
 
-  // TODO: Current vercel/pkg is dependent with CommonJS
   // eslint-disable-next-line unicorn/prefer-module
   const project = path.join(__dirname, '../../tsconfig.json')
   const dev = fs.existsSync(project)