Copilot commented on code in PR #13032:
URL: https://github.com/apache/cloudstack/pull/13032#discussion_r3549820426


##########
ui/src/views/infra/network/ServiceProvidersTab.vue:
##########
@@ -1148,6 +1171,51 @@ export default {
         return
       }
       this.fetchServiceProvider()
+      this.fetchRegisteredExtensions()
+    },
+    fetchRegisteredExtensions () {
+      // Load NetworkOrchestrator extensions registered to this physical 
network
+      getAPI('listExtensions', {
+        type: 'NetworkOrchestrator',
+        resourceid: this.resource.id,
+        resourcetype: 'PhysicalNetwork'
+      }).then(json => {
+        this.registeredExtensions = (json.listextensionsresponse && 
json.listextensionsresponse.extension) || []
+      }).catch(() => {
+        this.registeredExtensions = []
+      })

Review Comment:
   `fetchRegisteredExtensions` passes `resourceid`/`resourcetype` to 
`listExtensions`, but the backend `ListExtensionsCmd` only defines `id`, 
`name`, `details`, and `type` parameters. As a result, the API will either 
ignore these filters (showing unrelated extensions) or reject the request, so 
the dynamic provider tabs won’t reliably reflect extensions registered to this 
physical network.
   
   Consider requesting `details=resources` and filtering client-side by 
`ext.resources` entries matching this physical network, or add explicit API 
parameters for resource scoping.



##########
ui/src/views/extension/UpdateRegisteredExtension.vue:
##########
@@ -0,0 +1,131 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+<template>
+  <div class="form-layout" v-ctrl-enter="handleSubmit">
+    <a-form
+      :ref="formRef"
+      :model="form"
+      :loading="loading"
+      layout="vertical"
+      @finish="handleSubmit">
+      <a-form-item name="details" ref="details">
+        <template #label>
+          <tooltip-label :title="$t('label.details')" 
:tooltip="$t('message.add.extension.resource.details')"/>
+        </template>
+        <div style="margin-bottom: 10px">{{ 
$t('message.add.extension.resource.details') }}</div>
+        <details-input
+          v-model:value="form.details" />
+      </a-form-item>
+      <div :span="24" class="action-button">
+        <a-button @click="closeAction">{{ $t('label.cancel') }}</a-button>
+        <a-button :loading="loading" ref="submit" type="primary" 
@click="handleSubmit">{{ $t('label.ok') }}</a-button>
+      </div>
+    </a-form>
+  </div>
+</template>
+
+<script>
+import { ref, reactive, toRaw } from 'vue'
+import { postAPI } from '@/api'
+import TooltipLabel from '@/components/widgets/TooltipLabel'
+import DetailsInput from '@/components/widgets/DetailsInput'
+
+export default {
+  name: 'UpdateRegisteredExtension',
+  components: {
+    TooltipLabel,
+    DetailsInput
+  },
+  props: {
+    resource: {
+      type: Object,
+      required: true
+    },
+    extensionResource: {
+      type: Object,
+      required: true
+    }
+  },
+  data () {
+    return {
+      loading: false
+    }
+  },
+  created () {
+    this.initForm()
+  },
+  methods: {
+    initForm () {
+      this.formRef = ref()
+      this.form = reactive({
+        details: this.extensionResource.details ? { 
...this.extensionResource.details } : {}
+      })
+    },
+    handleSubmit (e) {
+      e.preventDefault()
+      if (this.loading) return
+      this.formRef.value.validate().then(() => {
+        const values = toRaw(this.form)

Review Comment:
   `handleSubmit` unconditionally calls `e.preventDefault()`, but this handler 
is also wired to `<a-form @finish="handleSubmit">`. In ant-design-vue (v3) the 
`finish` event passes the validated form values (not a DOM event), so this will 
throw a `TypeError` when the form submits via Enter / programmatic submit.
   
   Guard the call or drop the event dependency entirely.



##########
ui/src/views/offering/AddNetworkOffering.vue:
##########
@@ -739,6 +739,13 @@ export default {
     isSupportedServiceObject (obj) {
       return (obj !== null && obj !== undefined && Object.keys(obj).length > 0 
&& obj.constructor === Object && 'provider' in obj)
     },
+    isVpcCoreProvider (providerName) {
+      return ['VpcVirtualRouter', 'Netscaler', 'BigSwitchBcf', 
'ConfigDrive'].includes(providerName)
+    },
+    isDynamicExtensionProvider (providerName) {
+      const knownProviders = ['VirtualRouter', 'VpcVirtualRouter', 
'InternalLbVm', 'Netscaler', 'BigSwitchBcf', 'ConfigDrive', 'Nsx', 'Netris']
+      return !knownProviders.includes(providerName)
+    },

Review Comment:
   The provider classification helpers (`isVpcCoreProvider` / 
`isDynamicExtensionProvider`) are duplicated in `AddVpcOffering.vue` as well. 
This duplication risks the two offering flows drifting (different provider 
allowlists or casing) as new providers/extensions are introduced.
   
   Extract the provider classification into a shared utility and reuse it from 
both components.



-- 
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]

Reply via email to