-
Notifications
You must be signed in to change notification settings - Fork 3.2k
fix(app): fix issue with reinstall openresty failed #8265
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -273,19 +273,46 @@ | |
| <table> | ||
| <tbody> | ||
| <tr v-if="defaultLink != ''"> | ||
| <td> | ||
| <td v-if="installed.httpPort > 0"> | ||
| <el-button | ||
| type="primary" | ||
| link | ||
| @click=" | ||
| toLink( | ||
| defaultLink + | ||
| 'http://' + | ||
| defaultLink + | ||
| ':' + | ||
| installed.httpPort, | ||
| ) | ||
| " | ||
| > | ||
| {{ defaultLink + ':' + installed.httpPort }} | ||
| {{ | ||
| 'http://' + | ||
| defaultLink + | ||
| ':' + | ||
| installed.httpPort | ||
| }} | ||
| </el-button> | ||
| </td> | ||
| <td v-if="installed.httpsPort > 0"> | ||
| <el-button | ||
| type="primary" | ||
| link | ||
| @click=" | ||
| toLink( | ||
| 'https://' + | ||
| defaultLink + | ||
| ':' + | ||
| installed.httpsPort, | ||
| ) | ||
| " | ||
| > | ||
| {{ | ||
| 'https://' + | ||
| defaultLink + | ||
| ':' + | ||
| installed.httpsPort | ||
| }} | ||
| </el-button> | ||
| </td> | ||
| </tr> | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 Here are some suggestions:
Overall, the changes address a common mistake in UI development related to conditional rendering and URL construction, ensuring robustness against invalid inputs. |
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,12 +14,6 @@ | |
| <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> | ||
| <template #prepend> | ||
| <el-select v-model="protocol" placeholder="Select" class="p-w-100" disabled> | ||
| <el-option label="HTTP" value="http://" /> | ||
| <el-option label="HTTPS" value="https://" /> | ||
| </el-select> | ||
| </template> | ||
| <template #append> | ||
| <el-button @click="setDefaultDomain()" icon="Setting"> | ||
| {{ $t('commons.button.set') }} | ||
|
|
@@ -54,35 +48,15 @@ const config = ref({ | |
| }); | ||
| const loading = ref(false); | ||
| const configForm = ref(); | ||
| const protocol = ref('http://'); | ||
| const domainRef = ref(); | ||
| const useCustomApp = ref(false); | ||
|
|
||
| function getUrl(url: string) { | ||
| const regex = /^(https?:\/\/)(.*)/; | ||
| const match = url.match(regex); | ||
| if (match) { | ||
| const protocol = match[1]; | ||
| const remainder = match[2]; | ||
| return { | ||
| protocol: protocol, | ||
| remainder: remainder, | ||
| }; | ||
| } else { | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
| const search = async () => { | ||
| loading.value = true; | ||
| try { | ||
| const res = await getAppStoreConfig(); | ||
| if (res.data.defaultDomain != '') { | ||
| const url = getUrl(res.data.defaultDomain); | ||
| if (url) { | ||
| config.value.defaultDomain = url.remainder; | ||
| protocol.value = url.protocol; | ||
| } | ||
| config.value.defaultDomain = res.data.defaultDomain; | ||
| } | ||
| } catch (error) { | ||
| } finally { | ||
|
|
@@ -93,7 +67,6 @@ const search = async () => { | |
| const setDefaultDomain = () => { | ||
| domainRef.value.acceptParams({ | ||
| domain: config.value.defaultDomain, | ||
| protocol: protocol.value, | ||
| }); | ||
| }; | ||
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've reviewed the code and identified several improvements to make it more robust, clean, and efficient:
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:
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The provided code is almost identical between the two versions, differing only in the placement of the call to
createAllWebsitesWAFConfig. Here are my recommendations:processDefaultConfigthat 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:
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.