ManageIQ / manageiq-providers-kubernetes

ManageIQ plugin for the Kubernetes provider.
https://kubernetes.io/
Apache License 2.0
7 stars 63 forks source link

Added Helper Text To the Token Field #498

Closed DavidResende0 closed 1 year ago

DavidResende0 commented 1 year ago

Adds some text to the Token Field notifying the user that the Default, Metrics, and Alerts endpoints must be revalidated anytime the Token is updated if that endpoint is enabled.

Before: image

After: image

The long term goal is to add a clearer visual cue that illustrates this fact to the user.

DavidResende0 commented 1 year ago

@miq-bot assign @Fryguy

DavidResende0 commented 1 year ago

@agrare

agrare commented 1 year ago

Spec failures look unrelated, failing on master for the last two days

agrare commented 1 year ago

@kbrock it seems like https://github.com/ManageIQ/manageiq/commit/f7f3bdb9d052e0b535efa88b90ddea5f48c84362 caused the ./spec/models/manageiq/providers/kubernetes/container_manager/metrics_capture/rollup_spec.rb spec to start failing, haven't looked into it at all yet just FYI

rwellon commented 1 year ago

@Fryguy Suggested text:

 :helperText => _('Note: If enabled, the Default, Metrics, and Alert Endpoints must be revalidated whenever the token is changed or updated'),
agrare commented 1 year ago

whenever the token is changed or updated

What is the difference between changed/updated? I think we could go with one or the other but not both

DavidResende0 commented 1 year ago

whenever the token is changed or updated

What is the difference between changed/updated? I think we could go with one or the other but not both

@rwellon, @agrare Which one do you think is better?

Fryguy commented 1 year ago

Good question - We can probably choose one or the other. @rwellon Opinion here?

FWIW, I know @DavidResende0 was trying to handle both "new" and "edit" screens, so the wording needs to work for both.

agrare commented 1 year ago

so the wording needs to work for both.

So how about "when adding or changing the token"

kbrock commented 1 year ago

think this test failure has been fixed if you rebase this PR.

agrare commented 1 year ago

@DavidResende0 can you update the text per https://github.com/ManageIQ/manageiq-providers-kubernetes/pull/498#issuecomment-1629168783

DavidResende0 commented 1 year ago

@DavidResende0 can you update the text per #498 (comment)

New text -> Note: If enabled, the Default, Metrics, and Alert Endpoints must be revalidated when adding or changing the token

miq-bot commented 1 year ago

Checked commit https://github.com/DavidResende0/manageiq-providers-kubernetes/commit/5d267255dd902444d8de0549ca9c02984d288a55 with ruby 2.6.10, rubocop 1.28.2, haml-lint 0.35.0, and yamllint 1 file checked, 0 offenses detected Everything looks fine. :cookie:

Fryguy commented 1 year ago

Backported to petrosian in commit dd8188e5df8fb1c9810c96f36ff9d07f3ccd704c.

commit dd8188e5df8fb1c9810c96f36ff9d07f3ccd704c
Author: Adam Grare <adam@grare.com>
Date:   Wed Jul 26 12:42:59 2023 -0400

    Merge pull request #498 from DavidResende0/add-helper-text-to-token-field

    Added Helper Text To the Token Field

    (cherry picked from commit 6f5fb86495405f2a2259f1291a5240f46f3510a1)