Copilot commented on code in PR #820:
URL:
https://github.com/apache/skywalking-banyandb/pull/820#discussion_r2450671414
##########
ui/src/components/Property/Editor.vue:
##########
@@ -93,54 +91,40 @@
},
};
if (operator === 'create') {
- createProperty(param)
- .then((res) => {
- if (res.status === 200) {
- ElMessage({
- message: 'successed',
- type: 'success',
- duration: 5000,
- });
- $bus.emit('refreshAside');
- $bus.emit('deleteGroup', formData.group);
- openResourses();
- }
- })
- .catch((err) => {
- ElMessage({
- message: 'Please refresh and try again. Error: ' + err,
- type: 'error',
- duration: 3000,
- });
- })
- .finally(() => {
- $loadingClose();
+ const response = await createProperty(param);
+ $loadingClose();
+ if (response.error) {
+ ElMessage.error({
+ message: `Failed to create property: ${response.error.message}`,
+ type: 'error',
});
+ return;
+ }
+ ElMessage.success({
+ message: 'Create successed',
+ type: 'success',
+ });
+ $bus.emit('refreshAside');
+ $bus.emit('deleteResource', formData.name);
Review Comment:
Inconsistent event emission: In create mode, line 108 emits `deleteResource`
with `formData.name`, while line 126 in edit mode emits with the same
parameter. However, line 107 in create mode emits `deleteGroup` with
`formData.group`. The create path should emit `deleteGroup` instead of
`deleteResource` to match the original behavior where it was
`$bus.emit('deleteGroup', formData.group);`.
```suggestion
$bus.emit('deleteGroup', formData.group);
```
##########
ui/src/components/IndexRule/index.vue:
##########
@@ -61,15 +60,15 @@
$loadingCreate();
const result = await getSecondaryDataModel(data.type, data.group,
data.name);
$loadingClose();
- if (!(result.data && result.data.indexRule)) {
+ if (!result.indexRule) {
ElMessage({
message: `Please refresh and try again.`,
type: 'error',
duration: 3000,
});
Review Comment:
Inconsistent error handling: This code checks for missing data but doesn't
check for `result.error` first like other refactored components. Add error
check: `if (result.error) { ElMessage.error({ message: ..., type: 'error' });
return; }` before checking for missing indexRule data.
##########
ui/src/components/GroupTree/index.vue:
##########
@@ -140,99 +139,86 @@
});
// init data
- function getGroupLists() {
+ async function getGroupLists() {
filterText.value = '';
loading.value = true;
- getGroupList().then((res) => {
- if (res.status === 200) {
- data.groupLists = res.data.group.filter((d) =>
CatalogToGroupType[d.catalog] === props.type);
- let promise = data.groupLists.map((item) => {
- const type = props.type;
- const name = item.metadata.name;
- return new Promise((resolve, reject) => {
- getAllTypesOfResourceList(type, name)
- .then((res) => {
- if (res.status === 200) {
- item.children = res.data[type];
- resolve();
- }
- })
- .catch((err) => {
- reject(err);
- });
- });
- });
- if (SupportedIndexRuleTypes.includes(props.type)) {
- const promiseIndexRule = data.groupLists.map((item) => {
- const name = item.metadata.name;
- return new Promise((resolve, reject) => {
- getindexRuleList(name)
- .then((res) => {
- if (res.status === 200) {
- item.indexRule = res.data.indexRule;
- resolve();
- }
- })
- .catch((err) => {
- reject(err);
- });
- });
- });
- const promiseIndexRuleBinding = data.groupLists.map((item) => {
- const name = item.metadata.name;
- return new Promise((resolve, reject) => {
- getindexRuleBindingList(name)
- .then((res) => {
- if (res.status === 200) {
- item.indexRuleBinding = res.data.indexRuleBinding;
- resolve();
- }
- })
- .catch((err) => {
- reject(err);
- });
- });
- });
- promise = promise.concat(promiseIndexRule);
- promise = promise.concat(promiseIndexRuleBinding);
- }
- if (props.type === 'measure') {
- const TopNAggregationRule = data.groupLists.map((item) => {
- const name = item.metadata.name;
- return new Promise((resolve, reject) => {
- getTopNAggregationList(name)
- .then((res) => {
- if (res.status === 200) {
- item.topNAggregation = res.data.topNAggregation;
- resolve();
- }
- })
- .catch((err) => {
- reject(err);
- });
- });
- });
- promise = promise.concat(TopNAggregationRule);
+ const res = await getGroupList();
+ if (res.error) {
+ loading.value = false;
+ return;
+ }
+ data.groupLists = res.group.filter((d) => CatalogToGroupType[d.catalog]
=== props.type);
+ let promise = data.groupLists.map((item) => {
+ const name = item.metadata.name;
+ return new Promise(async(resolve, reject) => {
Review Comment:
Antipattern: Wrapping an async function in `new Promise` is redundant. The
async function already returns a promise. Remove the `new Promise` wrapper and
directly use `async () => { ... }`. Change to: `return (async () => { ... })()`
or refactor to avoid the immediately-invoked async function pattern.
##########
ui/src/components/Editor/index.vue:
##########
@@ -222,60 +222,62 @@
};
$bus.emit('AddTabs', add);
}
- function initData() {
- if (data.operator === 'edit' && data.form.group && data.form.name) {
- $loadingCreate();
- getStreamOrMeasure(data.type, data.form.group, data.form.name)
- .then((res) => {
- if (res.status === 200) {
- data.form.indexMode = res.data[String(data.type)]?.indexMode ||
false;
- const tagFamilies = res.data[String(data.type)]?.tagFamilies || [];
- const entity = res.data[String(data.type)]?.entity?.tagNames || [];
- const shardingKey =
res.data[String(data.type)]?.shardingKey?.tagNames || [];
- const arr = [];
- tagFamilies.forEach((item) => {
- item.tags.forEach((tag) => {
- const entityIndex = entity.findIndex((entityItem) => {
- return entityItem === tag.name;
- });
- const shardingKeyIndex =
shardingKey.findIndex((shardingKeyItem) => {
- return shardingKeyItem === tag.name;
- });
- const obj = {
- tagFamily: item.name,
- tag: tag.name,
- type: tag.type,
- entity: entityIndex >= 0 ? true : false,
- shardingKey: shardingKeyIndex >= 0 ? true : false,
- };
- arr.push(obj);
- });
- });
- tagEditorRef.value.setTagFamilies(arr);
- if (data.type === 'measure') {
- const fields = res.data[data.type + ''].fields;
- const intervalArr = res.data[data.type + ''].interval.split('');
- let interval = 0;
- let intervalUnit = '';
- intervalArr.forEach((char) => {
- let code = char.charCodeAt();
- if (code >= 48 && code < 58) {
- interval = interval * 10 + (char - 0);
- } else {
- intervalUnit = intervalUnit + char;
- }
- });
- data.form.interval = interval;
- data.form.intervalUnit = intervalUnit;
- fieldEditorRef.value.setFields(fields);
- }
- data.form.modRevision = res.data[data.type +
''].metadata.modRevision;
- }
- })
- .finally(() => {
- $loadingClose();
+ async function initData() {
+ if (data.operator !== 'edit' && !data.form.group && !data.form.name) {
Review Comment:
Logic error in condition: The condition `data.operator !== 'edit' &&
!data.form.group && !data.form.name` will skip initialization for create mode
even when group and name are set. This should be `data.operator !== 'edit' ||
!data.form.group || !data.form.name` to properly guard against missing data in
edit mode. The original logic was `if (data.operator === 'edit' &&
data.form.group && data.form.name)`.
```suggestion
if (data.operator !== 'edit' || !data.form.group || !data.form.name) {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
