fix(app): fix issue with reinstall openresty failed

[zhengkunwang223]

No description provided.

[f2c-ci-robot]

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[wanghe-fit2cloud]

The code appears to fix an issue where the HTTP button was not being shown when there is no valid installation information, but it may still be problematic if defaultLink is not provided or has invalid values. Additionally, it's good practice to validate the ports before attempting to create URLs since they should ideally be numbers greater than zero.

Here are some suggestions:

1. Validate Ports: Before constructing the URL in the <el-button>'s @click handler, ensure that both HTTP and HTTPS port properties of installed have valid numeric values (greater than zero).

2. Check Link Existence: The condition inside <td v-if="defaultLink != ''"> ensures that at least one link component will appear, even if neither HTTP nor HTTPS ports have valid data. However, this might not be optimal for clarity; you can consider showing only one type of link based on whether the other exists.

3. Use Template Literals: Your use of template literals ({{ }}) within templates is correct, which helps with readability and prevents escaping issues.

4. Comments for Clarity: Adding comments could help clarify the purpose of certain sections of the code, especially if the logic becomes more complex without additional documentation.

Overall, the changes address a common mistake in UI development related to conditional rendering and URL construction, ensuring robustness against invalid inputs.

[wanghe-fit2cloud]

I've reviewed the code and identified several improvements to make it more robust, clean, and efficient:

1. Remove getUrl Function: The function is not used anywhere in the component and can be removed.

2. Consolidate URL Handling: Move the logic to handle default domain retrieval into the search method, where it's currently used but not consistently applied across all components/functions.

3. Simplify Protocol Management: Since there are only two options ('HTTP' and 'HTTPS'), consider using a simple enum or switch statement instead of setting a separate ref variable.

4. Use TypeScript Types for Configuation Object: Ensure that the data returned from getAppStoreConfig() matches what is expected with appropriate types, such as string.

Here’s an optimized version of your code considering these changes:

<template>
  <el-form :model="config">
    <el-col :xs="24" :sm="20" :md="15" :lg="12" :xl="12">
      <el-form-item :label="$t('app.defaultWebDomain')" prop="defaultDomain">
        <el-input v-model="config.defaultDomain" disabled>
          <!-- No need for prepend slot -->
          <template #append>
            <el-button @click="setDefaultDomain()" icon="Setting">{{ $t('commons.button.set') }}</el-button>
          </template>
        </el-input>
      </el-form-item>
    </el-col>
    <!-- ... (rest of the template remains unchanged) ... -->
  </el-form>
</template>

<script lang='ts'>
import { defineComponent, ref } from 'vue';
import { getAppStoreConfig } from '@/api'; // Adjust import path according your project structure

export default defineComponent({
  setup() {
    const loading = ref(false);
    const configForm = ref();
    const config = ref({ defaultDomain: '' }); // Initialize with empty string
    const useCustomApp = ref(false);

    const search = async () => {
      try {
        loading.value = true;
        const res = await getAppStoreConfig();
        if (res.data.defaultDomain && !res.data.defaultDomain.startsWith('/')) {
          config.value.defaultDomain = '/' + res.data.defaultDomain; // Add leading slash if missing
          /* You might want to update the protocol here based on response */
        }
      } catch (error) {
        console.error(error); // Consider better error handling
      } finally {
        loading.value = false;
      }
    };

    const setDefaultDomain = () => {
      domainRef.value.acceptParams({
        domain: config.value.defaultDomain.replace(/^\/|\/$/, ''), // Strip optional leading/trailing slashes
        /* Use custom protocol handling or leave blank if always HTTPS */
      });
    };

    // Fetch config when needed or load page initially if required
    search();

    return {
      loading,
      config Form,
      config,
      search,
      setDefaultDomain,
      useCustomApp
    };
  }
});
</script>

Key Changes:

• Removed unnecessary getUrl function.
• Combined default domain retrieval logic into the search method.
• Simplified setDefaultDomain by stripping optional trailing/leading slashes.
• Used a ref object for configuration (config) and initialized it with an empty string.

[wanghe-fit2cloud]

The provided code is almost identical between the two versions, differing only in the placement of the call to createAllWebsitesWAFConfig. Here are my recommendations:

• Refactor Code: Consider refactoring this code into a single function called processDefaultConfig that handles both cases where no default website is present and when it exists. This would reduce redundancy and make the code cleaner.

Here's an example of how you might refactor the code:

func processDefaultConfig(appInstall *model.AppInstall) error {
    installDir := appInstall.GetPath()
    defaultConfigPath := path.Join(installDir, "conf", "default", "00.default.conf")
    fileOp := files.NewFileOp()

    if !hasDefaultWebsite {
        return nil
    }

    content, err := fileOp.GetContent(defaultConfigPath)
    if err != nil {
        return err
    }

    newContent := strings.ReplaceAll(string(content), "default_server", "")
    if err := fileOp.WriteFile(defaultConfigPath, strings.NewReader(newContent), constant.FilePerm); err != nil {
        return err
    }

    if websites == nil {
        return createAllWebsitesWAFConfig(websites)
    } else {
        // Proceed with other processing using websites
        // ...
    }

    return nil
}

// Example usage:
if hasDefaultWebsite {
    if err := processDefaultConfig(appInstall); err != nil {
        return err
    }
} else {
    // Handle case without predefined configurations
    // ...
}

This refactoring makes sense because most of the logic remains similar whether there is a default website defined or not, thus reducing the duplication in the original code.

[sonarqubecloud]

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[wanghe-fit2cloud]

/lgtm

[wanghe-fit2cloud]

/approve

[f2c-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: wanghe-fit2cloud

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [wanghe-fit2cloud]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment