Fix uptime & ping badge duration #4850

ZhaoQi99 · 2024-06-12T15:40:09Z

⚠️⚠️⚠️ 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]:

I have read and understand the pull request rules.

Description

Fixes uptime & ping badge duration not working when generate badge.
Now if you use input 25 as duration , you will get an error badge.

Unfortunately, it still can't implement displaying Uptime(30d) after I fixed this. I think #2808 may be a good solution for this.
Btw, It seems that there is a label variable in BadgeGeneratorDialog that is not used.

uptime-kuma/src/components/BadgeGeneratorDialog.vue

Lines 29 to 32 in bc25b71

    
           <div v-if=" (parameters[badge.type || 'null'] || [] ).includes('label') " class="mb-3"> 
        
               <label for="label" class="form-label">{{ $t("Badge Label") }}</label> 
        
               <input id="label" v-model="badge.label" type="text" class="form-control"> 
        
           </div>

Also,labelSuffix seems to be designed only for default label. And I don't know the purpose of this design.🤔

uptime-kuma/server/routers/api-router.js

Lines 266 to 269 in bc25b71

    
           badgeValues.label = filterAndJoin([ 
        
               labelPrefix, 
        
               label ?? `Uptime (${requestedDuration}${labelSuffix})`, 
        
           ]);

Type of change

Please delete any options that are not relevant.

Bug fix (non-breaking change which fixes an issue)

Checklist

My code follows the style guidelines of this project
I ran ESLint and other linters for modified files
I have performed a self-review of my own code and tested it
I have commented my code, particularly in hard-to-understand areas (including JSDoc for methods)
My changes generates no new warnings
My code needed automated testing. I have added them (this is optional task)

Screenshots (if any)

Header	after

CommanderStorm

Hi,

About the screenshots, I am bit unsure which are before and wich are after the Pr. Could you clairify this?

See comments below

server/uptime-calculator.js

server/routers/api-router.js

CommanderStorm · 2024-06-12T16:42:27Z

server/routers/api-router.js

@@ -253,7 +249,7 @@ router.get("/api/badge/:id/uptime/:duration?", cache("5 minutes"), async (reques
            badgeValues.color = badgeConstants.naColor;
        } else {
            const uptimeCalculator = await UptimeCalculator.getUptimeCalculator(requestedMonitorId);
-            const uptime = overrideValue ?? uptimeCalculator.getDataByDuration(requestedDuration).uptime;
+            const uptime = overrideValue ?? uptimeCalculator.getDataByDuration(`${requestedDuration}h`).uptime;


please move this back up.
Currently, I don't think you are handling this correctly.

Think about cases where I supply 30d => this is now handled as 30dh..

Currently, BadgeGeneratorDialog does not support selecting time units. And I mistakenly thought that all durations would be treated as hour. For backward compatibility reasons, I will reconsider this.

We have not yet looked deeply at that part of the UI.
Related #4500

server/uptime-calculator.js

ZhaoQi99 · 2024-06-12T16:59:24Z

Hi,

About the screenshots, I am bit unsure which are before and wich are after the Pr. Could you clairify this?

See comments below

Hi! Now it can't generate badge correctly.

And both of two screenshots are the results after PR.

ZhaoQi99 · 2024-06-19T05:13:51Z

@CommanderStorm Hi! PTAL

Co-authored-by: Frank Elsinga <frank@elsinga.de>

server/uptime-calculator.js

CommanderStorm

Works as advertised.

There are two semi-related issues (we can fix them in different PRs if you'd prefer, feedback on this would be appreciated):

The default labelSuffix does not make huge sense anymore
The frontend badge generator does not allow specifiying the suffixes. This means that things like /api/badge/1/uptime/6666 get rejected because the hours is larger than 1400

ZhaoQi99 · 2024-07-04T02:45:31Z

Is it ready for merge？🤔

CommanderStorm · 2024-07-04T14:19:49Z

There are two semi-related issues (we can fix them in different PRs if you'd prefer, feedback on this would be appreciated)

I am still wating for feedback on these ^^

ZhaoQi99 · 2024-07-04T16:27:31Z

There are two semi-related issues (we can fix them in different PRs if you'd prefer, feedback on this would be appreciated)

I am still wating for feedback on these ^^

Sorry for my oversight, I think that it would be better if we fixed them in separate PRs.

CommanderStorm requested changes Jun 12, 2024

View reviewed changes

CommanderStorm added area:core issues describing changes to the core of uptime kuma needs:work this PR needs work labels Jun 12, 2024

CommanderStorm added this to the 2.0.0 milestone Jun 12, 2024

CommanderStorm added the releaseblocker blocking bugs encountered with a new release label Jun 12, 2024

CommanderStorm mentioned this pull request Jun 12, 2024

v2.0.0-Release <- Read this for performance problems #4500

Open

16 tasks

ZhaoQi99 force-pushed the master branch from 8347349 to 133da30 Compare June 13, 2024 07:33

ZhaoQi99 requested a review from CommanderStorm June 13, 2024 07:33

ZhaoQi99 changed the title ~~Fix status & ping badge duration~~ Fix uptime & ping badge duration Jun 13, 2024

This comment was marked as resolved.

Sign in to view

ZhaoQi99 force-pushed the master branch from 9d42d22 to 8382a25 Compare June 13, 2024 15:48

ZhaoQi99 requested a review from CommanderStorm June 13, 2024 15:49

ZhaoQi99 force-pushed the master branch 2 times, most recently from c9088e8 to 977ff73 Compare June 13, 2024 18:00

ZhaoQi99 and others added 7 commits June 20, 2024 10:50

fix: 🐛 status & ping badge duration

e5ff86e

fix: 🐛 badge requestedDuration

53c4bba

Update server/routers/api-router.js

7270cac

Co-authored-by: Frank Elsinga <frank@elsinga.de>

Update server/routers/api-router.js

f63d364

Co-authored-by: Frank Elsinga <frank@elsinga.de>

Update server/routers/api-router.js

3f425dc

Co-authored-by: Frank Elsinga <frank@elsinga.de>

Update server/routers/api-router.js

58533e8

Co-authored-by: Frank Elsinga <frank@elsinga.de>

perf: ⚡️ revert some overengineer code

4941b17

ZhaoQi99 force-pushed the master branch from 977ff73 to 4941b17 Compare June 20, 2024 02:50

CommanderStorm added needs:review this PR needs a review by maintainers or other community members and removed needs:work this PR needs work labels Jun 24, 2024

CommanderStorm reviewed Jun 24, 2024

View reviewed changes

server/uptime-calculator.js Show resolved Hide resolved

CommanderStorm approved these changes Jun 24, 2024

View reviewed changes

CommanderStorm removed the needs:review this PR needs a review by maintainers or other community members label Jun 29, 2024

Merge branch 'master' into master

1ef6106

CommanderStorm merged commit 1a5a1a6 into louislam:master Jul 4, 2024
18 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix uptime & ping badge duration #4850

Fix uptime & ping badge duration #4850

ZhaoQi99 commented Jun 12, 2024 •

edited

Loading

CommanderStorm left a comment

CommanderStorm Jun 12, 2024

ZhaoQi99 Jun 12, 2024 •

edited

Loading

CommanderStorm Jun 12, 2024

ZhaoQi99 commented Jun 12, 2024

This comment was marked as resolved.

ZhaoQi99 commented Jun 19, 2024

CommanderStorm left a comment

ZhaoQi99 commented Jul 4, 2024

CommanderStorm commented Jul 4, 2024

ZhaoQi99 commented Jul 4, 2024

	<div v-if=" (parameters[badge.type \|\| 'null'] \|\| [] ).includes('label') " class="mb-3">
	<label for="label" class="form-label">{{ $t("Badge Label") }}</label>
	<input id="label" v-model="badge.label" type="text" class="form-control">
	</div>

	badgeValues.label = filterAndJoin([
	labelPrefix,
	label ?? `Uptime (${requestedDuration}${labelSuffix})`,
	]);

Fix uptime & ping badge duration #4850

Fix uptime & ping badge duration #4850

Conversation

ZhaoQi99 commented Jun 12, 2024 • edited Loading

Description

Type of change

Checklist

Screenshots (if any)

CommanderStorm left a comment

Choose a reason for hiding this comment

CommanderStorm Jun 12, 2024

Choose a reason for hiding this comment

ZhaoQi99 Jun 12, 2024 • edited Loading

Choose a reason for hiding this comment

CommanderStorm Jun 12, 2024

Choose a reason for hiding this comment

ZhaoQi99 commented Jun 12, 2024

This comment was marked as resolved.

ZhaoQi99 commented Jun 19, 2024

CommanderStorm left a comment

Choose a reason for hiding this comment

ZhaoQi99 commented Jul 4, 2024

CommanderStorm commented Jul 4, 2024

ZhaoQi99 commented Jul 4, 2024

ZhaoQi99 commented Jun 12, 2024 •

edited

Loading

ZhaoQi99 Jun 12, 2024 •

edited

Loading