louislam / uptime-kuma

A fancy self-hosted monitoring tool
https://uptime.kuma.pet
MIT License
60.3k stars 5.39k forks source link

feat(monitor condition): Remove the empty default condition creation #5354

Open Ionys320 opened 1 week ago

Ionys320 commented 1 week ago

⚠️⚠️⚠️ Since we do not accept all types of pull requests and do not want to waste your time. Please be sure that you have read pull request rules: https://github.com/louislam/uptime-kuma/blob/master/CONTRIBUTING.md#can-i-create-a-pull-request-for-uptime-kuma

Tick the checkbox if you understand [x]:

Description

The first PR related to monitors conditions were adding an empty condition. This has been removed since an empty condition disallow monitor saving. Please note this merge may be unwanted the default condition were a feature (but maybe unwished after all?).

E2E has been updated to avoid considering we have a default condition (all conditions count has been reduced by 1, etc).

Fixes #5303

Type of change

Please delete any options that are not relevant.

Checklist

CommanderStorm commented 1 week ago

My concearn in https://github.com/louislam/uptime-kuma/pull/5048#discussion_r1730428445 was that

Currently, I think a good percentage of people would not find this functionality and would be then somewhat confused..

=> we need to find a better way to add this only for new monitors. Sadly, I have not had the time to debug how to differentiated the 1.0 and 2.0 DNS monitors and if they are different.

CommanderStorm commented 1 week ago

Likely extending the dns monitor instead to include an "Record Exists" functionality or setting a wildcard as the default would be better.

Ionys320 commented 6 days ago

Regarding DNS, adding a default condition may be done during the migration from v1 to v2, no?

CommanderStorm commented 6 days ago

Yes, that is a possibility

Ionys320 commented 6 days ago

Likely extending the dns monitor instead to include an "Record Exists" functionality or setting a wildcard as the default would be better.

I prefer the default wildcard. Using defaut values allows us to keep the conditions system for future monitors without needing too much work.