-
Notifications
You must be signed in to change notification settings - Fork 2.9k
feat: Increase version restrictions for node switching #7631
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 |
|---|---|---|
|
|
@@ -227,5 +227,6 @@ export namespace Setting { | |
| id: number; | ||
| addr: string; | ||
| status: string; | ||
| version: string; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1743,6 +1743,10 @@ const message = { | |
| upgradeNow: 'Upgrade now', | ||
| source: 'Download source', | ||
| hasNewVersion: 'New version Available', | ||
| versionHigher: | ||
| 'Detected that node {0} version is higher than the main node, switching is not supported at this time. Please upgrade the main node system version and try again!', | ||
| versionLower: | ||
| 'Detected that node {0} version is lower than the main node, switching is not supported at this time. Please upgrade the system version of this node and try again!', | ||
|
|
||
| about: 'About', | ||
| project: 'Project Address', | ||
|
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 has no known irregularities or issues, but it can be optimized to improve readability and maintainability. Here's how: Change const message =
{
upgradeNow: 'Upgrade now',
source: 'Download source',
hasNewVersion: 'New version Available',Remove redundant comments from some lines like: versionHigher:
...
'Please upgrade the main node system version and try again!',Replace string concatenation expressions with template literals if they are used frequently. For example, instead of These changes will make the code cleaner while keeping its functionality intact. |
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,10 +8,8 @@ | |
| > | ||
| <Logo :isCollapse="isCollapse" /> | ||
| <div class="el-dropdown-link flex justify-between items-center"> | ||
| <el-button type="text" @click="openChangeNode" @mouseenter="openChangeNode"> | ||
| <span> | ||
| {{ loadCurrentName() }} | ||
| </span> | ||
| <el-button link class="ml-4" @click="openChangeNode" @mouseenter="openChangeNode"> | ||
| {{ loadCurrentName() }} | ||
| </el-button> | ||
| <div> | ||
| <el-dropdown | ||
|
|
@@ -25,11 +23,12 @@ | |
| <template #dropdown> | ||
| <el-dropdown-menu> | ||
| <el-dropdown-item command="local"> | ||
| <SvgIcon class="ico" iconName="p-host" /> | ||
| <el-button link icon="CircleCheck" type="success" /> | ||
| {{ $t('terminal.local') }} | ||
| </el-dropdown-item> | ||
| <el-dropdown-item v-for="item in nodes" :key="item.name" :command="item.name"> | ||
| <SvgIcon class="ico" iconName="p-host" /> | ||
| <el-button v-if="item.status === 'Healthy'" link icon="CircleCheck" type="success" /> | ||
| <el-button v-else link icon="Warning" type="danger" /> | ||
| {{ item.name }} | ||
| </el-dropdown-item> | ||
| </el-dropdown-menu> | ||
|
|
@@ -74,16 +73,18 @@ import { logOutApi } from '@/api/modules/auth'; | |
| import i18n from '@/lang'; | ||
| import { DropdownInstance, ElMessageBox } from 'element-plus'; | ||
| import { GlobalStore, MenuStore } from '@/store'; | ||
| import { MsgSuccess } from '@/utils/message'; | ||
| import { MsgError, MsgSuccess } from '@/utils/message'; | ||
| import { isString } from '@vueuse/core'; | ||
| import { getSettingInfo, listNodeOptions } from '@/api/modules/setting'; | ||
| import { countExecutingTask } from '@/api/modules/log'; | ||
| import { compareVersion } from '@/utils/version'; | ||
|
|
||
| const route = useRoute(); | ||
| const menuStore = MenuStore(); | ||
| const globalStore = GlobalStore(); | ||
| const nodes = ref([]); | ||
| const nodeChangeRef = ref<DropdownInstance>(); | ||
| const version = ref(); | ||
| defineProps({ | ||
| menuRouter: { | ||
| type: Boolean, | ||
|
|
@@ -172,8 +173,32 @@ const loadNodes = async () => { | |
| }); | ||
| }; | ||
| const changeNode = (command: string) => { | ||
| globalStore.currentNode = command || 'local'; | ||
| location.reload(); | ||
| if (globalStore.currentNode === command) { | ||
| return; | ||
| } | ||
| if (command == 'local') { | ||
| globalStore.currentNode = command || 'local'; | ||
| location.reload(); | ||
| return; | ||
| } | ||
| for (const item of nodes.value) { | ||
| if (item.name == command) { | ||
| if (version.value == item.version) { | ||
| globalStore.currentNode = command || 'local'; | ||
| location.reload(); | ||
| return; | ||
| } | ||
| let compareItem = compareVersion(item.version, version.value); | ||
| if (compareItem) { | ||
| MsgError(i18n.global.t('setting.versionHigher', [command])); | ||
| return; | ||
| } | ||
| if (!compareItem) { | ||
| MsgError(i18n.global.t('setting.versionLower', [command])); | ||
| return; | ||
| } | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| function extractLabels(node: Node, result: string[]): void { | ||
|
|
@@ -195,6 +220,7 @@ function getCheckedLabels(json: Node): string[] { | |
|
|
||
| const search = async () => { | ||
| const res = await getSettingInfo(); | ||
| version.value = res.data.systemVersion; | ||
| const json: Node = JSON.parse(res.data.xpackHideMenu); | ||
| if (json.isCheck === false) { | ||
| json.children.forEach((child: any) => { | ||
|
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. There seems to be two main sections where potential improvements are needed for clarity and consistency:
Also, regarding style and formatting suggestions: This could potentially be:
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. There is no notable difference in syntax, variable names, or function usage between the given code snippets. However, there might be more meaningful ways to optimize these scripts based on current best practices and coding standards such as using The most important thing to note here is if there's any typo present which could potentially lead to bugs without user awareness. For instance, consider variables named Remember, while reviewing JS code, always ensure it adheres to current standards and security policies to make sure it performs as intended across environments including both production and testing phases. |
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| export function compareVersion(version1: string, version2: string): boolean { | ||
| const v1s = extractNumbers(version1); | ||
| const v2s = extractNumbers(version2); | ||
|
|
||
| const maxLen = Math.max(v1s.length, v2s.length); | ||
| v1s.push(...new Array(maxLen - v1s.length).fill('0')); | ||
| v2s.push(...new Array(maxLen - v2s.length).fill('0')); | ||
|
|
||
| for (let i = 0; i < maxLen; i++) { | ||
| const v1 = parseInt(v1s[i], 10); | ||
| const v2 = parseInt(v2s[i], 10); | ||
| if (v1 !== v2) { | ||
| return v1 > v2; | ||
| } | ||
| } | ||
| return true; | ||
| } | ||
|
|
||
| function extractNumbers(version: string): string[] { | ||
| const numbers: string[] = []; | ||
| let start = -1; | ||
| for (let i = 0; i < version.length; i++) { | ||
| if (isDigit(version[i])) { | ||
| if (start === -1) { | ||
| start = i; | ||
| } | ||
| } else { | ||
| if (start !== -1) { | ||
| numbers.push(version.slice(start, i)); | ||
| start = -1; | ||
| } | ||
| } | ||
| } | ||
| if (start !== -1) { | ||
| numbers.push(version.slice(start)); | ||
| } | ||
| return numbers; | ||
| } | ||
|
|
||
| function isDigit(char: string): boolean { | ||
| return /^\d$/.test(char); | ||
| } | ||
|
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 does not appear to have any immediate syntax errors. However, there are some improvements that could be made:
Here's an updated version: function extractNumbers(version: string): string[] {
const numbers: string[] = [];
let startAtLastChar = false;
for (let i = 0; i < version.length; i++) {
const char = version[i];
// If digit at current position matches the last non-digit character,
// ensure it has its starting index set correctly before appending the number part.
if (!startAtLastChar && isDigit(char)) {
startAtLastChar = { startAtThisPosition: i };
if (numbers[0]) {
console.log('StartAtFirstPos', startAtFirstPos)
} else if (numbers && numbers[numbers.length-1].length === 0) {
console.log('startOnNewLine')
numbers.push('');
}
}
// Only append a number part when we've reached end of current line
if (char !== ',' && !isDigit(char)) break;
if (!startAtLastChar) continue;
if (typeof startAtLastChar.startAtThisPosition == 'undefined') continue ;To summarize the key improvements from these suggestions: * Extract just one group of consecutive characters between commas as "digits" instead of trying to find groups of numbers across multiple lines or spaces
* Make sure that each number starts itself (on separate lines), ensuring you handle both leading zeros properly
```
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 provided code snippet contains several inconsistencies and errors that require attention before being released. Here's an overview of these issues:
For instance, to address these concerns, you would need to implement explicit validations such as checking the inputs for alphabetic/numeric values rather than relying solely on regular expression matching. Additionally, implementing basic type safety checks could prevent runtime errors during compile time. Here’s a simplified approach with improved error handling and additional optimizations: import { strict as assert } from 'assert';
/**
* Compares two version strings based on how numerically each number part should be compared when concatenated together,
* considering negative sign prefixing as necessary.
*
* @param version1 The first version string representing the major/minor/patch numbers of interest.
* @param version2 The second version string also representing the same information.
* @returns true if there is no mismatch between version parts (e.g., if both versions can be safely added directly),
* false otherwise due to invalid input or numerical anomalies leading to incorrect comparisons.
*/
function isValidCompareVersion(version1: string, version2: string): boolean {
const partsA = extractPartsFromVersions(version1);
const partsB = extractPartsFromVersions(version2);
// Validate input types
if (!Array.isArray(partsA) || !Array.isArray(partsB)) {
throw new Error("Inputs must be arrays");
}
// Compare valid elements by concatenating them together
for (const parts of [partsA, partsB]) {
if (isValidElementsByCombiningElements(partsWithMajorMinorsToArrays(parts))) {
return true; // No mismatches found between consecutive sections after combining their individual parts
}
}
return false; // Some section(s) had mismatches
}
// Helper functions used in comparison logic
/**
* Extracts all digit characters from a given string, returns empty array if none available.
* @param str String containing digits to process.
* @returns An array of extracted digits.
*/
function extractDigits(str: string): string[] {
return str.match(/\d+/g)?.map((digit) => digit.trim()) ?? [];
}
function isValidElementsByCombiningElements(elements: string[]): boolean {
try {
return combineIntoValidStringsAndValidateResult(elements);
} catch (_) {} // Catch any unexpected issues encountered while attempting to validate combined results
return false;
}
// Main comparison logic for validity check of version strings
function combineIntoValidStringsAndValidateResult(combinedElements: string[]): boolean {
const sortedCombinedElements = [...combinedElements].sort();
let hasMismatches = false;
// Loop through combined elements to ensure every element is valid
for (let i = 1; i < sortedCombinedElements.length; i++) {
if (parseInt(sortedCombinedElements[i], 10) <= parseInt(sortedCombinedElements[i - 1], 10)) {
hasMismatches = true;
}
}
return !hasMismatches && sortedCombinedElements[0].length == 6 ? true : false;
}Keep refining this implementation as per specific requirements and consider adding proper error messages to assist in diagnosing problems related to inconsistent data structures within version strings. This revised solution now adheres more strictly to good coding practices aiming at maintaining efficiency and robustness over its entire lifecycle from initial development till release into production. |
||
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.
Based on my knowledge up to 2021-09-01, the provided code appears fine with no apparent issues or need for changes. It seems quite standardized within its current context of a project addressing node versions among other things and offers clear options like upgrading now, downloading source files, etc. However, I would still recommend running it through some basic linting tools to ensure there might be specific improvements that can enhance its efficiency or legibility (if you're using this kind of code).