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

Added Helper Text To the Token Field #498

Merged

Conversation

DavidResende0
Copy link
Member

@DavidResende0 DavidResende0 commented Jul 7, 2023

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
Copy link
Member Author

@miq-bot assign @Fryguy

@DavidResende0
Copy link
Member Author

@agrare

@DavidResende0 DavidResende0 force-pushed the add-helper-text-to-token-field branch 2 times, most recently from e3ad31b to 594d70e Compare July 7, 2023 18:26
@@ -203,6 +203,7 @@ def self.params_for_create
:id => "authentications.bearer.auth_key",
:name => "authentications.bearer.auth_key",
:label => "Token",
:helperText => _('Note that the Default, Metrics, and Alert Endpoints, if enabled, must be revalidated every time the token is changed or updated'),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me - another option:

Suggested change
:helperText => _('Note that the Default, Metrics, and Alert Endpoints, if enabled, must be revalidated every time the token is changed or updated'),
:helperText => _('NOTE: The Default, Metrics, and Alert Endpoints, if enabled, must be revalidated every time the token is changed or updated'),

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dehawkins512 or @rwellon can you give suggestions for wording here, or if it's ok give it a 👍 ?

@DavidResende0 DavidResende0 force-pushed the add-helper-text-to-token-field branch from 594d70e to b23d0d2 Compare July 7, 2023 18:45
@agrare
Copy link
Member

agrare commented Jul 7, 2023

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

@agrare
Copy link
Member

agrare commented Jul 7, 2023

@kbrock it seems like ManageIQ/manageiq@f7f3bdb 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
Copy link

rwellon commented Jul 10, 2023

@Fryguy
Suggested text:

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

@DavidResende0 DavidResende0 force-pushed the add-helper-text-to-token-field branch from b23d0d2 to 05ae0a5 Compare July 10, 2023 14:59
@agrare
Copy link
Member

agrare commented Jul 10, 2023

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
Copy link
Member Author

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
Copy link
Member

Fryguy commented Jul 10, 2023

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
Copy link
Member

agrare commented Jul 10, 2023

so the wording needs to work for both.

So how about "when adding or changing the token"

@kbrock
Copy link
Member

kbrock commented Jul 14, 2023

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

@agrare
Copy link
Member

agrare commented Jul 26, 2023

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

@DavidResende0 DavidResende0 force-pushed the add-helper-text-to-token-field branch from 05ae0a5 to 5d26725 Compare July 26, 2023 14:42
@DavidResende0
Copy link
Member Author

@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
Copy link
Member

miq-bot commented Jul 26, 2023

Checked commit DavidResende0@5d26725 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. 🍪

@agrare agrare assigned agrare and unassigned Fryguy Jul 26, 2023
@agrare agrare merged commit 6f5fb86 into ManageIQ:master Jul 26, 2023
3 checks passed
@Fryguy
Copy link
Member

Fryguy commented Jul 26, 2023

Backported to petrosian in commit dd8188e.

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)

Fryguy pushed a commit that referenced this pull request Jul 26, 2023
…ield

Added Helper Text To the Token Field

(cherry picked from commit 6f5fb86)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants