ronantakizawa/codereview-bench huggingface dataset:
Repository: louislam/uptime-kuma
File: server/notification-providers/pushover.js (JavaScript)

Code before:
            "title": notification.pushovertitle,
            "retry": "30",
            "expire": "3600",
            "html": 1,
        };

        const baseURL = await setting("primaryBaseURL");
        if (baseURL && monitorJSON) {
            data["url"] = baseURL + getMonitorRelativeURL(monitorJSON.id);
            data["url_title"] = "Link to Monitor";
        }

        if (notification.pushoverdevice) {
            data.device = notification.pushoverdevice;
        }
        if (notification.pushoverttl) {
            data.ttl = notification.pushoverttl;
        }

        try {
            if (heartbeatJSON == null) {
                await axios.post(url, data);
                return okMsg;
            }

            if (heartbeatJSON.status === UP) {
                data.sound = notification.pushoversounds_up;
            }

            data.message += `\n<b>Time (${heartbeatJSON["timezone"]})</b>: ${heartbeatJSON["localDateTime"]}`;
            await axios.post(url, data);
            return okMsg;

        } catch (error) {
            this.throwGeneralAxiosError(error);
        }
    }
}

module.exports = Pushover;


Review comment (suggestion):
In the current implementation, previous pushover users might NOT get a sound for these notifications or cause an error, depending on how pushover handles `null`.
Let's check for its existance

```suggestion
            if (heartbeatJSON.status === UP && notification.pushoversounds_up) {
```

Code after:
            "title": notification.pushovertitle,
            "retry": "30",
            "expire": "3600",
            "html": 1,
        };

        const baseURL = await setting("primaryBaseURL");
        if (baseURL && monitorJSON) {
            data["url"] = baseURL + getMonitorRelativeURL(monitorJSON.id);
            data["url_title"] = "Link to Monitor";
        }

        if (notification.pushoverdevice) {
            data.device = notification.pushoverdevice;
        }
        if (notification.pushoverttl) {
            data.ttl = notification.pushoverttl;
        }

        try {
            if (heartbeatJSON == null) {
                await axios.post(url, data);
                return okMsg;
            }

            if (heartbeatJSON.status === UP && notification.pushoversounds_up) {
                // default = DOWN => DOWN-sound is also played for non-UP/DOWN notiifcations
                data.sound = notification.pushoversounds_up;
            }

            data.message += `\n<b>Time (${heartbeatJSON["timezone"]})</b>: ${heartbeatJSON["localDateTime"]}`;
            await axios.post(url, data);
            return okMsg;

        } catch (error) {
            this.throwGeneralAxiosError(error);
        }
    }
}

module.exports = Pushover;
Repository: louislam/uptime-kuma
File: db/knex_init_db.js (JavaScript)

Code before:
     */
    await knex.schema.table("monitor", function (table) {
        table.text("oauth_client_id").defaultTo(null);
        table.text("oauth_client_secret").defaultTo(null);
        table.text("oauth_token_url").defaultTo(null);
        table.text("oauth_scopes").defaultTo(null);
        table.text("oauth_auth_method").defaultTo(null);
    });

    /*
    patch-add-timeout-monitor.sql
    ALTER TABLE monitor
    ADD timeout DOUBLE default 0 not null;
     */
    await knex.schema.table("monitor", function (table) {
        table.double("timeout").defaultTo(0).notNullable();
    });

    /*
    patch-add-ping-advanced-options.sql
    ALTER TABLE monitor ADD ping_count INTEGER default 1 not null;
    ALTER TABLE monitor ADD ping_numeric BOOLEAN default true not null;
    ALTER TABLE monitor ADD ping_deadline INTEGER default 10 not null;
    ALTER TABLE monitor ADD ping_timeout INTEGER default 2 not null;
    */
    await knex.schema.table("monitor", function (table) {
        table.integer("ping_count").defaultTo(1).notNullable();
        table.boolean("ping_numeric").defaultTo(true).notNullable();
        table.integer("ping_deadline").defaultTo(10).notNullable();
        table.integer("ping_timeout").defaultTo(2).notNullable();
    });

    /*
    patch-add-gamedig-given-port.sql
    ALTER TABLE monitor
    ADD gamedig_given_port_only BOOLEAN default 1 not null;
     */
    await knex.schema.table("monitor", function (table) {
        table.boolean("gamedig_given_port_only").defaultTo(1).notNullable();
    });

    log.info("mariadb", "Created basic tables for MariaDB");
}

module.exports = {
    createTables,
};


Review comment (refactor):
please remove this part and add it to a [new migration](https://github.com/louislam/uptime-kuma/tree/003ffc2cc2a3b572c197658b08b34a6618f804fa/db/knex_migrations), and only there (the `.sql` files are a crutch which we did not have the time/energy to clean up yet)

I think this file will only be run for new users, but not existing users.


Code after:
     */
    await knex.schema.table("monitor", function (table) {
        table.text("oauth_client_id").defaultTo(null);
        table.text("oauth_client_secret").defaultTo(null);
        table.text("oauth_token_url").defaultTo(null);
        table.text("oauth_scopes").defaultTo(null);
        table.text("oauth_auth_method").defaultTo(null);
    });

    /*
    patch-add-timeout-monitor.sql
    ALTER TABLE monitor
    ADD timeout DOUBLE default 0 not null;
     */
    await knex.schema.table("monitor", function (table) {
        table.double("timeout").defaultTo(0).notNullable();
    });

    /*
    patch-add-gamedig-given-port.sql
    ALTER TABLE monitor
    ADD gamedig_given_port_only BOOLEAN default 1 not null;
     */
    await knex.schema.table("monitor", function (table) {
        table.boolean("gamedig_given_port_only").defaultTo(1).notNullable();
    });

    log.info("mariadb", "Created basic tables for MariaDB");
}

module.exports = {
    createTables,
};
Repository: louislam/uptime-kuma
File: db/knex_migrations/2025-03-04-0000-ping-advanced-options.js (JavaScript)

Code before:
/* SQL:
ALTER TABLE monitor ADD ping_count INTEGER default 1 not null;
ALTER TABLE monitor ADD ping_numeric BOOLEAN default true not null;
ALTER TABLE monitor ADD ping_deadline INTEGER default 10 not null;
*/
exports.up = function (knex) {
    // Add new columns to table monitor
    return knex.schema
        .alterTable("monitor", function (table) {
            table.integer("ping_count").defaultTo(1).notNullable();
            table.boolean("ping_numeric").defaultTo(true).notNullable();
            table.integer("ping_deadline").defaultTo(10).notNullable();
        });

};

exports.down = function (knex) {
    return knex.schema
        .alterTable("monitor", function (table) {
            table.dropColumn("ping_count");
            table.dropColumn("ping_numeric");
            table.dropColumn("ping_deadline");
        });
};


Review comment (suggestion):
To be consistent with other monitors, deadline is the current timeout field.

Let's not be special here and introduce a second timeout. 
Adding a `ping_per_sub_ping_timeout` field or similar is fair gain, as long as it defaults to the current timeout for existing monitors in the migration.

Otherwise this might be breaking => would need to wait for V3.0

Code after:
/* SQL:
ALTER TABLE monitor ADD ping_count INTEGER default 1 not null;
ALTER TABLE monitor ADD ping_numeric BOOLEAN default true not null;
ALTER TABLE monitor ADD ping_per_request_timeout INTEGER default 2 not null;
*/
exports.up = function (knex) {
    // Add new columns to table monitor
    return knex.schema
        .alterTable("monitor", function (table) {
            table.integer("ping_count").defaultTo(1).notNullable();
            table.boolean("ping_numeric").defaultTo(true).notNullable();
            table.integer("ping_per_request_timeout").defaultTo(2).notNullable();
        });

};

exports.down = function (knex) {
    return knex.schema
        .alterTable("monitor", function (table) {
            table.dropColumn("ping_count");
            table.dropColumn("ping_numeric");
            table.dropColumn("ping_per_request_timeout");
        });
};
Repository: louislam/uptime-kuma
File: server/analytics/umami-analytics.js (JavaScript)

Code before:
 * @param {string} domainUrl Domain name with tld to use with the Umami Analytics script.
 * @param {string} websiteId Website ID to use with the Umami Analytics script.
 * @returns {string} HTML script tags to inject into page
 */
function getUmamiAnalyticsScript(domainUrl, websiteId) {
    let escapedDomainUrlJS = jsesc(domainUrl, { isScriptContext: true });
    let escapedWebsiteIdJS = jsesc(websiteId, { isScriptContext: true });

    if (escapedDomainUrlJS) {
        escapedDomainUrlJS = escapedDomainUrlJS.trim();
    }

    if (escapedWebsiteIdJS) {
        escapedWebsiteIdJS = escapedWebsiteIdJS.trim();
    }

    // Escape the domain url for use in an HTML attribute.
    let escapedDomainUrlHTMLAttribute = escape(escapedDomainUrlJS);

    // Escape the website id for use in an HTML attribute.
    let escapedWebsiteIdHTMLAttribute = escape(escapedWebsiteIdJS);

    return `
        <script defer src="https://${escapedDomainUrlHTMLAttribute}/script.js" data-website-id="${escapedWebsiteIdHTMLAttribute}"></script>
    `;
}

module.exports = {
    getUmamiAnalyticsScript,
};


Review comment (suggestion):
I kinda wish this was not hardcoded to assume a domain and ID.  My umami is setup with proxy forwarders on each domain.  

For example, in my environments:
`<script defer src="/proxy-path/umi.js" data-website-id="WEB-ID"></script>` proxies to my umami instance.  This is so the path appears on the same domain of the app instead of a separate one. 

Code after:
 * @param {string} scriptUrl the Umami Analytics script url.
 * @param {string} websiteId Website ID to use with the Umami Analytics script.
 * @returns {string} HTML script tags to inject into page
 */
function getUmamiAnalyticsScript(scriptUrl, websiteId) {
    let escapedScriptUrlJS = jsesc(scriptUrl, { isScriptContext: true });
    let escapedWebsiteIdJS = jsesc(websiteId, { isScriptContext: true });

    if (escapedScriptUrlJS) {
        escapedScriptUrlJS = escapedScriptUrlJS.trim();
    }

    if (escapedWebsiteIdJS) {
        escapedWebsiteIdJS = escapedWebsiteIdJS.trim();
    }

    // Escape the Script url for use in an HTML attribute.
    let escapedScriptUrlHTMLAttribute = escape(escapedScriptUrlJS);

    // Escape the website id for use in an HTML attribute.
    let escapedWebsiteIdHTMLAttribute = escape(escapedWebsiteIdJS);

    return `
        <script defer src="${escapedScriptUrlHTMLAttribute}" data-website-id="${escapedWebsiteIdHTMLAttribute}"></script>
    `;
}

module.exports = {
    getUmamiAnalyticsScript,
};
Repository: louislam/uptime-kuma
File: src/components/notifications/YZJ.vue (Vue)

Code before:
<template>
    <div class="mb-3">
        <label for="yzjWebHookUrl" class="form-label">{{ $t("YZJ Webhook URL") }}<span style="color: red;"><sup>*</sup></span></label>
        <input id="yzjWebHookUrl" v-model="$parent.notification.yzjWebHookUrl" type="text" class="form-control" required />
        <label for="yzjType" class="form-label">{{ $t("YZJ Robot Type") }}<span style="color: red;"><sup>*</sup></span></label>
        <input id="yzjType" v-model="$parent.notification.yzjType" type="text" class="form-control" required />
        <label for="yzjToken" class="form-label">{{ $t("YZJ Robot Token") }}<span style="color: red;"><sup>*</sup></span></label>
        <hidden-input id="yzjToken" v-model="$parent.notification.yzjToken" type="text" class="form-control" required />
        <i18n-t class="form-text" keypath="wayToGetTeamsURL">
            <a href="https://www.yunzhijia.com/opendocs/docs.html#/tutorial/index/robot" target="_blank">{{ $t("here") }}</a>
        </i18n-t>
    </div>
</template>
<script setup lang="ts">
import HiddenInput from "../HiddenInput.vue";
</script>


Review comment (suggestion):
```suggestion
        <input id="yzjWebHookUrl" v-model="$parent.notification.yzjWebHookUrl" type="url" class="form-control" required />
```

Code after:
<template>
    <div class="mb-3">
        <label for="yzjWebHookUrl" class="form-label">{{ $t("YZJ Webhook URL") }}<span style="color: red;"><sup>*</sup></span></label>
        <input id="yzjWebHookUrl" v-model="$parent.notification.yzjWebHookUrl" type="url" class="form-control" required />
        <i18n-t class="form-text" keypath="wayToGetTeamsURL">
            <a href="https://www.yunzhijia.com/opendocs/docs.html#/tutorial/index/robot" target="_blank">{{ $t("here") }}</a>
        </i18n-t>
    </div>
    <div class="mb-3">
        <label for="yzjToken" class="form-label">{{ $t("YZJ Robot Token") }}<span style="color: red;"><sup>*</sup></span></label>
        <HiddenInput id="yzjToken" v-model="$parent.notification.yzjToken" :required="true" autocomplete="new-password"></HiddenInput>
        <i18n-t class="form-text" keypath="wayToGetLineNotifyToken">
            <a href="https://www.yunzhijia.com/opendocs/docs.html#/server-api/im/index?id=%e6%8e%a5%e5%8f%a3%e5%9c%b0%e5%9d%80%e5%92%8c%e6%8e%88%e6%9d%83%e7%a0%81" target="_blank">{{ $t("here") }}</a>
        </i18n-t>
    </div>
</template>
<script setup lang="ts">
import HiddenInput from "../HiddenInput.vue";
</script>
Repository: louislam/uptime-kuma
File: src/components/notifications/YZJ.vue (Vue)

Code before:
<template>
    <div class="mb-3">
        <label for="yzjWebHookUrl" class="form-label">{{ $t("YZJ Webhook URL") }}<span style="color: red;"><sup>*</sup></span></label>
        <input id="yzjWebHookUrl" v-model="$parent.notification.yzjWebHookUrl" type="text" class="form-control" required />
        <label for="yzjType" class="form-label">{{ $t("YZJ Robot Type") }}<span style="color: red;"><sup>*</sup></span></label>
        <input id="yzjType" v-model="$parent.notification.yzjType" type="text" class="form-control" required />
        <label for="yzjToken" class="form-label">{{ $t("YZJ Robot Token") }}<span style="color: red;"><sup>*</sup></span></label>
        <hidden-input id="yzjToken" v-model="$parent.notification.yzjToken" type="text" class="form-control" required />
        <i18n-t class="form-text" keypath="wayToGetTeamsURL">
            <a href="https://www.yunzhijia.com/opendocs/docs.html#/tutorial/index/robot" target="_blank">{{ $t("here") }}</a>
        </i18n-t>
    </div>
</template>
<script setup lang="ts">
import HiddenInput from "../HiddenInput.vue";
</script>


Review comment (suggestion):
with these dividers, we "usually" try to keep inputs separated.
Could add them as well?

```suggestion
    <div class="mb-3">
        <label for="yzjWebHookUrl" class="form-label">{{ $t("YZJ Webhook URL") }}<span style="color: red;"><sup>*</sup></span></label>
        <input id="yzjWebHookUrl" v-model="$parent.notification.yzjWebHookUrl" type="text" class="form-control" required />
    </div>
```

Code after:
<template>
    <div class="mb-3">
        <label for="yzjWebHookUrl" class="form-label">{{ $t("YZJ Webhook URL") }}<span style="color: red;"><sup>*</sup></span></label>
        <input id="yzjWebHookUrl" v-model="$parent.notification.yzjWebHookUrl" type="url" class="form-control" required />
        <i18n-t class="form-text" keypath="wayToGetTeamsURL">
            <a href="https://www.yunzhijia.com/opendocs/docs.html#/tutorial/index/robot" target="_blank">{{ $t("here") }}</a>
        </i18n-t>
    </div>
    <div class="mb-3">
        <label for="yzjToken" class="form-label">{{ $t("YZJ Robot Token") }}<span style="color: red;"><sup>*</sup></span></label>
        <HiddenInput id="yzjToken" v-model="$parent.notification.yzjToken" :required="true" autocomplete="new-password"></HiddenInput>
        <i18n-t class="form-text" keypath="wayToGetLineNotifyToken">
            <a href="https://www.yunzhijia.com/opendocs/docs.html#/server-api/im/index?id=%e6%8e%a5%e5%8f%a3%e5%9c%b0%e5%9d%80%e5%92%8c%e6%8e%88%e6%9d%83%e7%a0%81" target="_blank">{{ $t("here") }}</a>
        </i18n-t>
    </div>
</template>
<script setup lang="ts">
import HiddenInput from "../HiddenInput.vue";
</script>
Repository: louislam/uptime-kuma
File: src/components/notifications/YZJ.vue (Vue)

Code before:
<template>
    <div class="mb-3">
        <label for="yzjWebHookUrl" class="form-label">{{ $t("YZJ Webhook URL") }}<span style="color: red;"><sup>*</sup></span></label>
        <input id="yzjWebHookUrl" v-model="$parent.notification.yzjWebHookUrl" type="text" class="form-control" required />
        <label for="yzjType" class="form-label">{{ $t("YZJ Robot Type") }}<span style="color: red;"><sup>*</sup></span></label>
        <input id="yzjType" v-model="$parent.notification.yzjType" type="text" class="form-control" required />
        <label for="yzjToken" class="form-label">{{ $t("YZJ Robot Token") }}<span style="color: red;"><sup>*</sup></span></label>
        <hidden-input id="yzjToken" v-model="$parent.notification.yzjToken" type="text" class="form-control" required />
        <i18n-t class="form-text" keypath="wayToGetTeamsURL">
            <a href="https://www.yunzhijia.com/opendocs/docs.html#/tutorial/index/robot" target="_blank">{{ $t("here") }}</a>
        </i18n-t>
    </div>
</template>
<script setup lang="ts">
import HiddenInput from "../HiddenInput.vue";
</script>


Review comment (refactor):
> You can learn how to create a webhook URL [here](https://www.yunzhijia.com/opendocs/docs.html#/tutorial/index/robot)

Please move this up to the webhook url and add a short helptext here where in the UI `yzjToken`s can be found.

Code after:
<template>
    <div class="mb-3">
        <label for="yzjWebHookUrl" class="form-label">{{ $t("YZJ Webhook URL") }}<span style="color: red;"><sup>*</sup></span></label>
        <input id="yzjWebHookUrl" v-model="$parent.notification.yzjWebHookUrl" type="url" class="form-control" required />
        <i18n-t class="form-text" keypath="wayToGetTeamsURL">
            <a href="https://www.yunzhijia.com/opendocs/docs.html#/tutorial/index/robot" target="_blank">{{ $t("here") }}</a>
        </i18n-t>
    </div>
    <div class="mb-3">
        <label for="yzjToken" class="form-label">{{ $t("YZJ Robot Token") }}<span style="color: red;"><sup>*</sup></span></label>
        <HiddenInput id="yzjToken" v-model="$parent.notification.yzjToken" :required="true" autocomplete="new-password"></HiddenInput>
        <i18n-t class="form-text" keypath="wayToGetLineNotifyToken">
            <a href="https://www.yunzhijia.com/opendocs/docs.html#/server-api/im/index?id=%e6%8e%a5%e5%8f%a3%e5%9c%b0%e5%9d%80%e5%92%8c%e6%8e%88%e6%9d%83%e7%a0%81" target="_blank">{{ $t("here") }}</a>
        </i18n-t>
    </div>
</template>
<script setup lang="ts">
import HiddenInput from "../HiddenInput.vue";
</script>
Repository: louislam/uptime-kuma
File: src/components/notifications/YZJ.vue (Vue)

Code before:
<template>
    <div class="mb-3">
        <label for="yzjWebHookUrl" class="form-label">{{ $t("YZJ Webhook URL") }}<span style="color: red;"><sup>*</sup></span></label>
        <input id="yzjWebHookUrl" v-model="$parent.notification.yzjWebHookUrl" type="url" class="form-control" required />
        <i18n-t class="form-text" keypath="wayToGetTeamsURL">
            <a href="https://www.yunzhijia.com/opendocs/docs.html#/tutorial/index/robot" target="_blank">{{ $t("here") }}</a>
        </i18n-t>
    </div>
    <div class="mb-3">
        <label for="yzjType" class="form-label">{{ $t("YZJ Robot Type") }}<span style="color: red;"><sup>*</sup></span></label>
        <select id="yzjType" v-model="$parent.notification.yzjType" class="form-control" required>
            <option :value="0">{{ $t("YZJ General Robot") }}</option>
        </select>
    </div>
    <div class="mb-3">
        <label for="yzjToken" class="form-label">{{ $t("YZJ Robot Token") }}<span style="color: red;"><sup>*</sup></span></label>
        <hidden-input id="yzjToken" v-model="$parent.notification.yzjToken" type="text" class="form-control" required />
        <i18n-t class="form-text" keypath="wayToGetLineNotifyToken">
            <a href="https://www.yunzhijia.com/opendocs/docs.html#/server-api/im/index?id=%e6%8e%a5%e5%8f%a3%e5%9c%b0%e5%9d%80%e5%92%8c%e6%8e%88%e6%9d%83%e7%a0%81" target="_blank">{{ $t("here") }}</a>
        </i18n-t>
    </div>
</template>
<script setup lang="ts">
import HiddenInput from "../HiddenInput.vue";
</script>


Review comment (suggestion):
Lets not be special here..
Currently, the `form-control` would be adding the weird halo..

```suggestion
        <HiddenInput id="yzjToken" v-model="$parent.notification.yzjToken" :required="true" autocomplete="new-password"></HiddenInput>
```

Code after:
<template>
    <div class="mb-3">
        <label for="yzjWebHookUrl" class="form-label">{{ $t("YZJ Webhook URL") }}<span style="color: red;"><sup>*</sup></span></label>
        <input id="yzjWebHookUrl" v-model="$parent.notification.yzjWebHookUrl" type="url" class="form-control" required />
        <i18n-t class="form-text" keypath="wayToGetTeamsURL">
            <a href="https://www.yunzhijia.com/opendocs/docs.html#/tutorial/index/robot" target="_blank">{{ $t("here") }}</a>
        </i18n-t>
    </div>
    <div class="mb-3">
        <label for="yzjToken" class="form-label">{{ $t("YZJ Robot Token") }}<span style="color: red;"><sup>*</sup></span></label>
        <HiddenInput id="yzjToken" v-model="$parent.notification.yzjToken" :required="true" autocomplete="new-password"></HiddenInput>
        <i18n-t class="form-text" keypath="wayToGetLineNotifyToken">
            <a href="https://www.yunzhijia.com/opendocs/docs.html#/server-api/im/index?id=%e6%8e%a5%e5%8f%a3%e5%9c%b0%e5%9d%80%e5%92%8c%e6%8e%88%e6%9d%83%e7%a0%81" target="_blank">{{ $t("here") }}</a>
        </i18n-t>
    </div>
</template>
<script setup lang="ts">
import HiddenInput from "../HiddenInput.vue";
</script>