Skip to content
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

Vue3: Edit components can fail to relay errors #12298

Closed
16 tasks done
rak-phillip opened this issue Oct 18, 2024 · 1 comment · Fixed by #12299
Closed
16 tasks done

Vue3: Edit components can fail to relay errors #12298

rak-phillip opened this issue Oct 18, 2024 · 1 comment · Fixed by #12299
Assignees
Labels
area/vue3 Bugs and technical debts as outcome to the Vue3 migration kind/tech-debt Technical debt QA/manual-test Indicates issue requires manually testing
Milestone

Comments

@rak-phillip
Copy link
Member

rak-phillip commented Oct 18, 2024

Edit components could possibly fail to render error messages on failure. See #11937 for more details on how this error manifests.

The components identified are:

  • shell/edit/management.cattle.io.user.vue
  • shell/edit/nodeDriver.vue
  • shell/edit/kontainerDriver.vue
  • shell/edit/fleet.cattle.io.cluster.vue
  • shell/edit/cis.cattle.io.clusterscanbenchmark.vue
  • shell/edit/helm.cattle.io.projecthelmchart.vue
  • shell/edit/k8s.cni.cncf.io.networkattachmentdefinition.vue
  • shell/edit/management.cattle.io.projectroletemplatebinding.vue
  • shell/edit/monitoring.coreos.com.route.vue
  • shell/edit/constraints.gatekeeper.sh.constraint/index.vue
  • shell/edit/logging-flow/index.vue
  • shell/edit/monitoring.coreos.com.alertmanagerconfig/index.vue
  • shell/edit/monitoring.coreos.com.alertmanagerconfig/receiverConfig.vue
  • shell/edit/monitoring.coreos.com.prometheusrule/index.vue
  • shell/edit/monitoring.coreos.com.receiver/index.vue
  • shell/edit/networking.istio.io.destinationrule/index.vue

Notes

The following components were reviewed, but not modified:

  • shell/edit/fleet.cattle.io.clustergroup.vue
    Already contains inheritAttrs: false
  • shell/edit/auth/azuread.vue
    Not required - root component wrapped in div
  • shell/edit/auth/ldap/index.vue
    Not required - root component wrapped in div
  • shell/edit/auth/saml.vue
    Not required - root component wrapped in div
  • shell/edit/auth/oidc.vue
    Not required - root component wrapped in div
  • shell/edit/auth/googleoauth.vue
    Not required - root component wrapped in div
  • shell/edit/auth/github.vue
    Not required - root component wrapped in div
  • shell/edit/logging.banzaicloud.io.output/index.vue
    Not required - root component wrapped in div
  • shell/edit/provisioning.cattle.io.cluster/import.vue (this one matches pattern... but works without inheritAttrs)
    The component tree for this one is as follows:
      <resource-detail>
        <async-component-wrapper>
          <cru-cluster>
            <cru-resource>
              <import>
    
    The shell/edit/provisioning.cattle.io.cluster/index.vue component already contains inheritAttrs: false . Import.vue is also rendered by this component, but not as a root-level ancestor; therefore, the fallthrough attributes issue is a non-issue in this case.

Original Message

Looks like this same issue appears in a number of different places (some aren't fixed by inheritAttrs: false,)

Standout - These need to be confirmed or fixed in 2.10.0

  • shell/edit/management.cattle.io.user.vue
  • shell/edit/auth/azuread.vue
  • shell/edit/auth/ldap/index.vue
  • shell/edit/auth/saml.vue
  • shell/edit/auth/oidc.vue
  • shell/edit/auth/googleoauth.vue
  • shell/edit/auth/github.vue
  • shell/edit/nodeDriver.vue
  • shell/edit/kontainerDriver.vue

Debatable - These need to be confirmed but not maybe fixed in 2.10.0

  • shell/edit/fleet.cattle.io.cluster.vue
  • shell/edit/fleet.cattle.io.clustergroup.vue

Others - These need to be confirmed and fixed at some point

  • shell/edit/cis.cattle.io.clusterscanbenchmark.vue
  • shell/edit/helm.cattle.io.projecthelmchart.vue
  • shell/edit/k8s.cni.cncf.io.networkattachmentdefinition.vue
  • shell/edit/management.cattle.io.projectroletemplatebinding.vue
  • shell/edit/monitoring.coreos.com.route.vue
  • shell/edit/constraints.gatekeeper.sh.constraint/index.vue
  • shell/edit/logging-flow/index.vue
  • shell/edit/logging.banzaicloud.io.output/index.vue
  • shell/edit/monitoring.coreos.com.alertmanagerconfig/index.vue
  • shell/edit/monitoring.coreos.com.alertmanagerconfig/receiverConfig.vue
  • shell/edit/monitoring.coreos.com.prometheusrule/index.vue
  • shell/edit/monitoring.coreos.com.receiver/index.vue
  • shell/edit/networking.istio.io.destinationrule/index.vue

Of Note

  • shell/edit/provisioning.cattle.io.cluster/import.vue (this one matches pattern... but works without inheritAttrs)
  • The hack was already put in a number of affected places by Fixed edit as yaml not working #11803

@rak-phillip i'll leave it up to you on how to split those up, and where it leaves this PR

Originally posted by @richard-cox in #11956 (comment)

@rak-phillip rak-phillip added area/vue3 Bugs and technical debts as outcome to the Vue3 migration kind/tech-debt Technical debt labels Oct 18, 2024
@rak-phillip rak-phillip self-assigned this Oct 18, 2024
@rak-phillip rak-phillip added this to the v2.10.0 milestone Oct 18, 2024
@github-actions github-actions bot added QA/dev-automation Issues that engineers have written automation around so QA doesn't have look at this and removed kind/bug kind/tech-debt Technical debt area/vue3 Bugs and technical debts as outcome to the Vue3 migration labels Oct 18, 2024
@yonasberhe23 yonasberhe23 added QA/manual-test Indicates issue requires manually testing area/vue3 Bugs and technical debts as outcome to the Vue3 migration kind/tech-debt Technical debt and removed QA/dev-automation Issues that engineers have written automation around so QA doesn't have look at this labels Oct 21, 2024
@yonasberhe23 yonasberhe23 self-assigned this Oct 25, 2024
@yonasberhe23
Copy link
Contributor

Tested in:

  • Rancher v2.10-fcb4b0e12dd94e908dcc39c4f941a660f22e5d51-head
  • Dashboard master 78df731

Errors display as expected in create/edit mode, yaml editor included in testing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/vue3 Bugs and technical debts as outcome to the Vue3 migration kind/tech-debt Technical debt QA/manual-test Indicates issue requires manually testing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants