feat: dockge set/update agent friendly name #414

lohrbini · 2024-02-01T07:29:20Z

⚠️⚠️⚠️ 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/dockge/blob/master/CONTRIBUTING.md

Tick the checkbox if you understand [x]:

I have read and understand the pull request rules.

Disclaimer

I am not a Software programmer and only have basic understandings on any Programming Languages used in the project, so please be patient and maybe assist me with further development of this feature

Description

Since dockge agents is still a beta feature and in the future maybe more and more remote instances will be added I wanted to enable the option for setting a friendly name for a remote instance. If the name is not set via the prompt. The default agent.url will be displayed
Fixes #345

Type of change

New feature (non-breaking change which adds functionality)
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 generate no new warnings
My code needed automated testing. I have added them (this is optional task)

Screenshots

Add the option to set a friendly name to a dockge remote agent. If the field is not used the `agent.url` will be displayed

cyril59310 · 2024-02-01T18:10:57Z

After testing your PR, I have two small remarks:

Could you add the translation key "Friendly Name" to the en.json file?
It would be interesting for the friendly name to be applied here as well

lohrbini · 2024-02-01T18:33:31Z

Hey @cyril59310,

thanks for your feedback.

The translation can be added - for sure.
~~Could you maybe point out, where the friendlyName is missing exactly ?~~ Figured out where you want to add the friendlyName

Currently trying to refactor the code a bit to enable renaming - which seems more difficult as expected ..

cyril59310 · 2024-02-01T18:49:02Z

In the file

frontend/src/lang/en.json ,

please add "Friendly Name": "Friendly Name" so that translators can translate it into other languages using our Weblate tool.

adding `Friendly Name` to en.json

allow updates and removal of `friendly name`

lohrbini · 2024-02-01T21:35:38Z

@cyril59310 point 2 of your request is now also done. Only need to update codebase and remove all debug options

frontend/src/mixins/socket.ts

adding the `friendlyname` option to the running and paused container

cyril59310 · 2024-02-02T17:12:42Z

Thank you for taking my feedback.
I noticed afterwards that here too, the friendly name could be added.

when create a new stack via UI the friendly name should also be displayed

lohrbini · 2024-02-03T15:04:13Z

Thanks again @cyril59310 . This is now also implemented

Merge branch 'feat/dockge-agents-friendly-name' into feat/dockge-agents-friendly-name-editable

add a query to update the friendlyname correctly

remove edit-pen for controller which cannot have a friendlyname for now

lohrbini · 2024-02-06T07:03:34Z

Finally I was able to get the update feature working. Friendly names are now updated properly by a small dialog

@larswmh maybe need help this time with syntaxing/linting issues for sqlite statement. Or even a better solution than a "hardcoded" query

fix linting issues

fix tsc check by updating var type

remove if condition and update to proper values to prevent duplicates

cyril59310 · 2024-02-06T17:53:01Z

I made some suggestions in order to add translation keys for our translators.

louislam · 2024-02-06T21:59:01Z

backend/migrations/2023-12-20-2117-agent-table.ts

@@ -7,6 +7,7 @@ export async function up(knex: Knex): Promise<void> {
        table.string("url", 255).notNullable().unique();
        table.string("username", 255).notNullable();
        table.string("password", 255).notNullable();
+        table.string("friendlyname", 255);


This file cannot be modified. Have to create a new file.

Migration guide:
https://github.com/louislam/uptime-kuma/tree/master/db/knex_migrations

Also the naming convention, it should be friendly_name.

Will come back later, as I would like to finish the next milestone of Uptime Kuma first.

louislam · 2024-02-06T22:03:46Z

backend/agent-manager.ts

     */
-    async add(url : string, username : string, password : string) : Promise<Agent> {
+    async add(url : string, username : string, password : string, friendlyname : string) : Promise<Agent> {


Name Conventions

Javascript/Typescript: camelCaseType

SQLite: snake_case (Underscore)

CSS/SCSS: kebab-case (Dash)

https://github.com/louislam/dockge/blob/master/CONTRIBUTING.md#name-conventions

all namings has been updated

nathan815

Nice feature!

Have some code feedback

nathan815 · 2024-04-23T04:18:56Z

backend/agent-manager.ts

+     */
+
+    async update(friendlyname : string, updatedFriendlyName : string) {
+        await R.exec("UPDATE agent SET friendlyname = " + updatedFriendlyName + " WHERE friendlyname = " + friendlyname + "");


SQL injection vuln here - if name has quotes this will break.

You can use parameter bindings to build queries safely:

await R.exec("UPDATE agent SET friendlyname = ? WHERE friendlyname = ?", [ updatedFriendlyName, friendlyname ]);

https://github.com/louislam/redbean-node/blob/master/docs/Query.md

Instead of a raw query, you could utilize the redbean ORM methods. See: https://github.com/louislam/redbean-node/blob/master/docs/Create-Update-Bean.md

Something like this if by ID (primary key):

const agent = await R.load('agent', id); agent.friendlyName = friendlyName; await R.store(agent);

Or by URL:

const agent = await R.findOne('agent', ' url = ? ', [url]); agent.friendlyName = friendlyName; await R.store(agent);

Thanks for the input. Added this with update by url and started a small "discussion"
#414 (comment)

nathan815 · 2024-04-23T04:31:25Z

backend/agent-manager.ts

+     * @param updatedFriendlyName
+     */
+
+    async update(friendlyname : string, updatedFriendlyName : string) {


I would consider accepting the agent's id or the url here for identifying the agent to update instead of friendly name.

ID is the primary key, so thats probably best to use for updating the agent. But URL is always set and has a unique constraint, so it will work too.

I agree with nathan, I would refactor to pass id around for the agent

sure it should be the id ? it is currently not passed in the agentList and needs to be added. If a host will be added and deleted it will still increase and not release a taken number. The url would be the "better" identifier ?
( just a small comment. I am not against refactoring this to id but while updating this MR I was thinking about)

nathan815 · 2024-04-23T04:34:49Z

backend/models/agent.ts

+            friendlyname: this.friendlyname,
+            updatedFriendlyName: this.updatedFriendlyName


Why is updatedFriendlyName also here on the model? Shouldn't just friendlyName be sufficient?

I forgot to update this file while adding the other suggestions but it still worked with the correct variables. Where is this needed 🤔

lohrbini · 2024-05-22T19:12:16Z

Long time no see. Will have a look the next days on all your feedback. Many thanks 🙏🏻

kayakyakr

I appreciate this addition. Short of hacking the docker swarm mechanics to do node auto-discovery, I think is a fairly nice quality of life improvement.

Left some comments on some changes I'd make to make this prod-ready.

kayakyakr · 2024-06-04T17:25:39Z

backend/agent-manager.ts

+     * @param updatedFriendlyName
+     */
+
+    async update(friendlyname : string, updatedFriendlyName : string) {


I agree with nathan, I would refactor to pass id around for the agent

kayakyakr · 2024-06-04T17:27:12Z

backend/models/agent.ts

@@ -23,6 +23,8 @@ export class Agent extends BeanModel {
            url: this.url,
            username: this.username,
            endpoint: this.endpoint,
+            friendlyname: this.friendlyname,


recommendation: you should consider just naming this field name. There are no conflicts using the simpler version of the field name.

kayakyakr · 2024-06-04T17:28:37Z

backend/socket-handlers/manage-agent-socket-handler.ts

+                if (typeof(updatedFriendlyName) !== "string") {
+                    throw new Error("FriendlyName must be a string");
+                }


the more you know: since this is typescript, you shouldn't actually need to typecheck the input value for friendly name. We already know it's a string from before.

kayakyakr · 2024-06-04T17:30:36Z

frontend/src/pages/DashboardHome.vue

+                            </template>
+
+                            <!-- Edit FriendlyName  -->
+                            <font-awesome-icon v-if="agent.friendlyname !== '' && agent.friendlyname !== ''" icon="pen-to-square" @click="showEditAgentFriendlynameDialog[agent.friendlyname] = !showEditAgentFriendlynameDialog[agent.friendlyname]" />


suggestion: I'm not sure why you're checking for blank string twice here, but if you switch to passing around agent.id you will be able to edit a name no matter if the server was created with friendly name or not, so the v-if will be unnecessary.

kayakyakr · 2024-06-04T17:33:44Z

frontend/src/pages/DashboardHome.vue

+                            <!-- Edit Dialog -->
+                            <BModal v-model="showEditAgentFriendlynameDialog[agent.friendlyname]" :no-close-on-backdrop="true" :close-on-esc="true" :okTitle="$t('Update Friendlyname')" okVariant="info" @ok="updateFriendlyname(agent.friendlyname, agent.updatedFriendlyName)">
+                                <label for="Update Friendlyname" class="form-label">Current value: {{ $t(agent.friendlyname) }}</label>
+                                <input id="updatedFriendlyName" v-model="agent.updatedFriendlyName" type="text" class="form-control" optional>


suggestion: similar to nathan's comments up above, if you are finding agents by their ID, you can just set agent.name here and use that to update the value. You also then don't have to show Current value: in the label, because the input will have the current value. You may want to show agent.url as part of the label, though, in case input name is blank.

kayakyakr · 2024-06-04T17:41:18Z

frontend/src/pages/DashboardHome.vue

+                            <template v-if="$root.agentStatusList[endpoint]">
+                                <span v-if="endpoint === '' && agent.friendlyname === ''" class="badge bg-secondary me-2">Controller</span>
+                                <span v-else-if="agent.friendlyname === ''" :href="agent.url">{{ endpoint }}</span>
+                                <span v-else :href="agent.url">{{ agent.friendlyname }}</span>
+                            </template>


Couple of suggestions here:

We can use the fact that an empty string is falsy to shorten this chain.

You probably don't want to be modifying things so drastically. If it's working, then a small change will do.

Suggested change

<template v-if="$root.agentStatusList[endpoint]">

Controller

{{ endpoint }}

{{ agent.friendlyname }}

</template>

{{ $t("currentEndpoint") }}

<a v-else :href="agent.url" target="_blank">{{ agent.name || endpoint }}</a>

mon5termatt · 2024-06-21T05:11:16Z

would love for this to be added!

updating the merge request by adding the suggestions from reviewer

update agent.ts file by renaming the variables and added suggestion

lohrbini · 2024-08-06T16:11:26Z

Ready & happy about a second review :)

&& could need help with that one failing CI pipeline

updated variables in language file

larswmh · 2024-08-14T10:51:44Z

frontend/src/pages/DashboardHome.vue

+                                <span v-if="endpoint === '' && agent.name === ''" class="badge bg-secondary me-2">Controller</span>
+                                <span v-else-if="agent.name === ''" :href="agent.url">{{ endpoint }}</span>
+                                <span v-else :href="agent.url">{{ agent.name }}</span>


Suggested change

Controller

{{ endpoint }}

{{ agent.name }}

Controller

{{ endpoint }}

{{ agent.name }}

Add spacing from controller badge to edit icon for agent url and agent name display.

Before:

After:

Thanks for the feedback. Updated in latest commit

larswmh · 2024-08-14T10:53:27Z

backend/agent-manager.ts

@@ -76,12 +76,15 @@ export class AgentManager {
     * @param url
     * @param username
     * @param password
+     * @param name
+     * @param updatedName


Suggested change

* @param updatedName

Parameter updatedName is not used in this function, can be removed

Thanks for the feedback. Updated in latest commit

XenoUniv3rse · 2024-08-17T12:50:59Z

on which tag do i need to be to have this feature?

lohrbini · 2024-08-17T13:06:28Z

on which tag do i need to be to have this feature?

It is planned to be included in the next release v1.5.0 . To use this feature you need to be on my branch and build the image on your own

implement suggestions from review

add missing comma in en.json

feat: dockge agent friendly name

c0fc76a

Add the option to set a friendly name to a dockge remote agent. If the field is not used the `agent.url` will be displayed

lohrbini added 2 commits February 1, 2024 18:51

docs(en): add missing new item

11f5fff

adding `Friendly Name` to en.json

DRAFT: feat(friendlyName): make friendlyname configurable

54e158f

allow updates and removal of `friendly name`

larswmh reviewed Feb 2, 2024

View reviewed changes

frontend/src/mixins/socket.ts Outdated Show resolved Hide resolved

lohrbini force-pushed the feat/dockge-agents-friendly-name branch from 6529f6d to 96b5205 Compare February 2, 2024 06:17

feat(containerTerminal): add friendly name

8f142af

adding the `friendlyname` option to the running and paused container

lohrbini force-pushed the feat/dockge-agents-friendly-name branch from 96b5205 to 8f142af Compare February 2, 2024 06:20

feat(compose): add friendly names

62ceb44

when create a new stack via UI the friendly name should also be displayed

lohrbini added 3 commits February 6, 2024 06:53

chore(deps): update from upstream

5885665

Merge branch 'feat/dockge-agents-friendly-name' into feat/dockge-agents-friendly-name-editable

fix(agent-manager): add update query

2b64b3f

add a query to update the friendlyname correctly

fix(DashboardHome): remove edit-pen for controller

a8b75fb

remove edit-pen for controller which cannot have a friendlyname for now

lohrbini changed the title ~~feat: dockge agent friendly name~~ feat: dockge set/update agent friendly name Feb 6, 2024

lohrbini added 3 commits February 6, 2024 07:30

style(agent-manager): liniting

9748469

fix linting issues

fix(manage-agent-socket-handler): type

052cf17

fix tsc check by updating var type

fix(DashboardHome): duplicate code

dcc48d3

remove if condition and update to proper values to prevent duplicates

lohrbini force-pushed the feat/dockge-agents-friendly-name branch from 1e688f8 to dcc48d3 Compare February 6, 2024 07:55

louislam reviewed Feb 6, 2024

View reviewed changes

louislam added this to the 1.5.0 milestone Feb 6, 2024

louislam added the question Further information is requested label Feb 6, 2024

louislam reviewed Feb 6, 2024

View reviewed changes

nathan815 reviewed Apr 23, 2024

View reviewed changes

kayakyakr reviewed Jun 4, 2024

View reviewed changes

lohrbini added 2 commits August 6, 2024 16:04

fix: pull-request suggestions

825b727

updating the merge request by adding the suggestions from reviewer

fix(suggestion): added missing change

49eda70

update agent.ts file by renaming the variables and added suggestion

chore(lang): updated en.json

1c72c23

updated variables in language file

lohrbini requested review from nathan815 and kayakyakr August 6, 2024 16:28

lohrbini requested review from louislam and larswmh August 14, 2024 09:19

larswmh reviewed Aug 14, 2024

View reviewed changes

fix: suggestions

5b2ac56

implement suggestions from review

lohrbini requested a review from larswmh August 20, 2024 13:13

lohrbini added 2 commits November 6, 2024 12:47

Merge branch 'master' into feat/dockge-agents-friendly-name

29cc257

fix(style): missing comma

70bcc3f

add missing comma in en.json

		friendlyname: this.friendlyname,
		updatedFriendlyName: this.updatedFriendlyName

feat: dockge set/update agent friendly name #414

Are you sure you want to change the base?

feat: dockge set/update agent friendly name #414

Conversation

lohrbini commented Feb 1, 2024 • edited Loading

Disclaimer

Description

Type of change

Screenshots

cyril59310 commented Feb 1, 2024 • edited Loading

lohrbini commented Feb 1, 2024 • edited Loading

cyril59310 commented Feb 1, 2024

lohrbini commented Feb 1, 2024 • edited Loading

cyril59310 commented Feb 2, 2024

lohrbini commented Feb 3, 2024

lohrbini commented Feb 6, 2024 • edited Loading

cyril59310 commented Feb 6, 2024 • edited Loading

louislam Feb 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nathan815 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lohrbini commented May 22, 2024

kayakyakr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kayakyakr Jun 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mon5termatt commented Jun 21, 2024

lohrbini commented Aug 6, 2024 • edited Loading

larswmh Aug 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

larswmh Aug 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

XenoUniv3rse commented Aug 17, 2024

lohrbini commented Aug 17, 2024

lohrbini commented Feb 1, 2024 •

edited

Loading

cyril59310 commented Feb 1, 2024 •

edited

Loading

lohrbini commented Feb 1, 2024 •

edited

Loading

lohrbini commented Feb 1, 2024 •

edited

Loading

lohrbini commented Feb 6, 2024 •

edited

Loading

cyril59310 commented Feb 6, 2024 •

edited

Loading

louislam Feb 6, 2024 •

edited

Loading

kayakyakr Jun 4, 2024 •

edited

Loading

lohrbini commented Aug 6, 2024 •

edited

Loading

larswmh Aug 14, 2024 •

edited

Loading

larswmh Aug 14, 2024 •

edited

Loading