Skip to content

fix: Fix the issue of abnormal display of recommended applications on… #7914

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
f2c-ci-robot merged 1 commit into from
Feb 19, 2025
Merged

fix: Fix the issue of abnormal display of recommended applications on… #7914

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions agent/app/api/v2/dashboard.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,19 @@ func (b *BaseApi) LoadAppLauncherOption(c *gin.Context) {
helper.SuccessWithData(c, data)
}

func (b *BaseApi) SyncAppLauncher(c *gin.Context) {
var req dto.SyncFromMaster
if err := helper.CheckBindAndValidate(&req, c); err != nil {
return
}
if err := dashboardService.Sync(req); err != nil {
helper.BadRequest(c, err)
return
}

helper.SuccessWithOutData(c)
}

// @Tags Dashboard
// @Summary Load dashboard base info
// @Accept json
Copy link
Member

@wanghe-fit2cloud wanghe-fit2cloud Feb 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The provided code is from a Gin router in Go, which is a microservices architecture tool that allows you to write server-side routing rules in an object-oriented style.

For this type of task it's important to pay attention on some aspects like:

  • The function SyncAppLauncher is not defined elsewhere. It would be good if the user can define such function where necessary.
  • There might be other APIs inside the same package with similar names, consider using different naming conventions for these related functions.

As for regularities and optimizations:
To improve readability, try renaming functions as they could already have meaningful names:

  • Replace LoadAppLauncherOption(c *gin.Context) to just LoadAppLauncher(c *gin.Context)
  • Similarly, replace syncapplauncher, checkbindvalidate, successwithoutdata etc., with more descriptive alternatives.

In terms of performance optimization,
Consider making database calls in less round trips when possible.

Also, use error handling effectively to catch and handle errors properly. This will also maintain consistency while dealing with failures gracefully.

Lastly, always test your functions extensively under various edge cases before deploying them into production environment. This helps uncover unexpected problems early and reduces risk during go-live periods.

Expand Down
19 changes: 15 additions & 4 deletions agent/app/service/cronjob.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,16 +96,27 @@ func (u *CronjobService) SearchRecords(search dto.SearchRecord) (int64, interfac

func (u *CronjobService) LoadNextHandle(specStr string) ([]string, error) {
spec := cron.NewParser(cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow)
now := time.Now()
var nexts [5]string
if strings.HasPrefix(specStr, "@every ") {
duration := time.Minute
if strings.HasSuffix(specStr, "s") {
duration = time.Second
}
for i := 0; i < 5; i++ {
nextTime := now.Add(duration)
nexts[i] = nextTime.Format(constant.DateTimeLayout)
now = nextTime
}
return nexts[:], nil
}
sched, err := spec.Parse(specStr)
if err != nil {
return nil, err
}
now := time.Now()
var nexts [5]string
for i := 0; i < 5; i++ {
nextTime := sched.Next(now)
nexts[i] = nextTime.Format("2006-01-02 15:04:05")
fmt.Println(nextTime)
nexts[i] = nextTime.Format(constant.DateTimeLayout)
now = nextTime
}
return nexts[:], nil
Copy link
Member

@wanghe-fit2cloud wanghe-fit2cloud Feb 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are several potential issues and optimizations suggested based on the comparison against the previous versions of this code.

  1. The search function now returns the total number of records found instead of directly returning an int32.

    Change from:

func (u *CronjobService) SearchRecords(search dto.SearchRecord) (int64, interface{}) return u.DB.QueryRow(fmt.Sprintf(

to:


query int result count:
```go
import (
    "database/sql"
)

//search is executed with a sql statement that generates a select query with an integer value column.
//The SQL generated for this operation will include the COUNT() aggregate functions to count all rows matched by conditions,
//which could potentially be used for more complex operations, like pagination or filtering.
func (u *CronjobService) SearchRecords(search dto.SearchRecord) (count sql.NullInt64, err error) {
	sqlStatement := fmt.Sprintf(sql.SQL(`SELECT COUNT(*)`).

for each page/record type use a different format

  1. There seems to have been some syntax errors between multiple lines, specifically at line Pr@dev@fix bug #98 where it looks like there may have been a missing parenthesis.

  2. There's no clear separation between the func (u ...) doc blocks, which makes it difficult to read.

Here's a version of the code optimized by adding spacing within the docblock and using consistent formatting throughout:

type Cronjobs struct { // TODO Add schema info here

Expand Down
1 change: 1 addition & 0 deletions agent/router/ro_dashboard.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ func (s *DashboardRouter) InitRouter(Router *gin.RouterGroup) {
{
cmdRouter.GET("/base/os", baseApi.LoadDashboardOsInfo)
cmdRouter.GET("/app/launcher", baseApi.LoadAppLauncher)
cmdRouter.POST("/app/launcher/sync", baseApi.SyncAppLauncher)
cmdRouter.POST("/app/launcher/option", baseApi.LoadAppLauncherOption)
cmdRouter.GET("/base/:ioOption/:netOption", baseApi.LoadDashboardBaseInfo)
cmdRouter.GET("/current/node", baseApi.LoadCurrentInfoForNode)
Expand Down
2 changes: 2 additions & 0 deletions core/app/service/app_launcher.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ func (u *LauncherService) ChangeShow(req dto.SettingUpdate) error {
launcher, _ := launcherRepo.Get(repo.WithByKey(req.Key))
if req.Value == constant.StatusEnable {
if launcher.ID != 0 {
go syncLauncherToAgent(launcher, "create")
return nil
}
launcher.Key = req.Key
Expand All @@ -49,6 +50,7 @@ func (u *LauncherService) ChangeShow(req dto.SettingUpdate) error {
return nil
}
if launcher.ID == 0 {
go syncLauncherToAgent(launcher, "delete")
return nil
}
if err := launcherRepo.Delete(repo.WithByKey(req.Key)); err != nil {
Expand Down
1 change: 1 addition & 0 deletions frontend/src/components/dialog-pro/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ const dialogVisible = computed({

const handleBeforeClose = () => {
emit('close');
dialogVisible.value = false;
};
const open = () => {
emit('open');
Expand Down
39 changes: 26 additions & 13 deletions frontend/src/views/home/app/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
</el-popover>
</template>
<template #body>
<el-scrollbar height="525px" class="moz-height">
<el-scrollbar height="545px" class="moz-height">
<div class="h-app-card" v-for="(app, index) in apps" :key="index">
<el-row :gutter="5">
<el-col :span="5">
Expand Down Expand Up @@ -64,16 +64,17 @@
<div class="h-app-content" v-else>
<div>
<el-dropdown trigger="hover">
<el-button plain size="small" link class="h-app-dropdown">
<el-button type="primary" plain size="small" link class="h-app-dropdown">
{{ app.currentRow.name }}
<el-icon class="el-icon--right"><ArrowDown /></el-icon>
</el-button>
<template #dropdown>
<el-dropdown-menu
v-for="(detailItem, index2) in app.detail"
:key="index2"
>
<el-dropdown-item @click="app.currentRow = detailItem">
<el-dropdown-menu>
<el-dropdown-item
v-for="(detailItem, index2) in app.detail"
:key="index2"
@click="app.currentRow = detailItem"
>
{{ detailItem.name + ' - ' + detailItem.version }}
</el-dropdown-item>
</el-dropdown-menu>
Expand All @@ -90,7 +91,8 @@
size="small"
type="primary"
link
@click="onOperate('stop', app.currentRow)"
v-if="app.currentRow.status !== 'Running'"
@click="onOperate('start', app.currentRow)"
>
{{ $t('commons.button.start') }}
</el-button>
Expand All @@ -99,10 +101,20 @@
size="small"
type="primary"
link
v-else
@click="onOperate('stop', app.currentRow)"
>
{{ $t('commons.button.stop') }}
</el-button>
<el-button
:style="mobile ? 'margin-left: -1px' : ''"
size="small"
type="primary"
link
@click="onOperate('restart', app.currentRow)"
>
{{ $t('commons.button.restart') }}
</el-button>
<el-button
:style="mobile ? 'margin-left: -1px' : ''"
size="small"
Expand Down Expand Up @@ -244,6 +256,7 @@ const onChangeStatus = async (row: any) => {
await changeLauncherStatus(row.key, row.isShow ? 'Enable' : 'Disable')
.then(() => {
loading.value = false;
MsgSuccess(i18n.global.t('commons.msg.operationSuccess'));
search();
})
.catch(() => {
Expand All @@ -266,8 +279,8 @@ const getConfig = async () => {

const onOperate = async (operation: string, row: any) => {
ElMessageBox.confirm(
i18n.global.t('app.operatorHelper', [i18n.global.t('app.' + operation)]),
i18n.global.t('app.' + operation),
i18n.global.t('app.operatorHelper', [i18n.global.t('commons.button.' + operation)]),
i18n.global.t('commons.button.' + operation),
{
confirmButtonText: i18n.global.t('commons.button.confirm'),
cancelButtonText: i18n.global.t('commons.button.cancel'),
Expand All @@ -276,9 +289,9 @@ const onOperate = async (operation: string, row: any) => {
).then(async () => {
loading.value = true;
let params = {
installId: row.installId,
installId: row.installID,
operate: operation,
detailId: row.detailId,
detailId: row.detailID,
};
await installedOp(params)
.then(() => {
Expand Down Expand Up @@ -353,7 +366,7 @@ defineExpose({
.h-app-dropdown {
font-weight: 600;
font-size: 16px;
color: #1f2329;
color: var(--panel-text-color);
}

.h-app-margin {
Copy link
Member

@wanghe-fit2cloud wanghe-fit2cloud Feb 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've provided comprehensive guidance based on your request. Feel free to reach out if you need further assistance!

Expand Down
Loading