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.

Sign up for GitHub

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

Jump to bottom

feat(pg-v5): credentials adding capabilities for numbered essential plans #2513

Merged
merged 12 commits into from
Oct 24, 2023
Merged

feat(pg-v5): credentials adding capabilities for numbered essential plans #2513

merged 12 commits into from
Oct 24, 2023

Conversation

brittanyrjones
Copy link
Contributor

@brittanyrjones brittanyrjones commented Oct 13, 2023 •
edited
Loading

Fix UX around pg:credentials for essential addons. This feature is unsupported in Essential-tier, but supported in legacy essentials. We should include messaging around that. Additionally, don't let users rotate or grab URLs for credentials they cannot create.

Before on an essential-0:
Screenshot 2023-10-17 at 6 06 40 PM

Destroy flow:
Screenshot 2023-10-17 at 6 10 42 PM

After on an essential-0:
Screenshot 2023-10-17 at 6 09 05 PM

Destroy flow:
Screenshot 2023-10-17 at 6 13 10 PM

Testing backwards compatibility
Before changes on a legacy mini:
Screenshot 2023-10-17 at 5 34 59 PM

Screenshot 2023-10-17 at 5 36 28 PM

Screenshot 2023-10-17 at 5 38 16 PM

Destroy flow:
Screenshot 2023-10-17 at 6 16 20 PM

After changes on a legacy mini:
Screenshot 2023-10-17 at 5 39 30 PM

Screenshot 2023-10-17 at 5 40 09 PM

Screenshot 2023-10-17 at 5 41 15 PM

Destroy flow:
Screenshot 2023-10-17 at 6 18 11 PM

@brittanyrjones brittanyrjones temporarily deployed to AcceptanceTests October 13, 2023 15:45 — with GitHub Actions Inactive
@brittanyrjones brittanyrjones temporarily deployed to AcceptanceTests October 13, 2023 15:45 — with GitHub Actions Inactive
@brittanyrjones brittanyrjones temporarily deployed to AcceptanceTests October 13, 2023 15:45 — with GitHub Actions Inactive
@brittanyrjones brittanyrjones marked this pull request as ready for review October 13, 2023 15:45
@brittanyrjones brittanyrjones requested a review from a team as a code owner October 13, 2023 15:45
@brittanyrjones brittanyrjones temporarily deployed to AcceptanceTests October 13, 2023 16:17 — with GitHub Actions Inactive
@brittanyrjones brittanyrjones temporarily deployed to AcceptanceTests October 13, 2023 16:17 — with GitHub Actions Inactive
@brittanyrjones brittanyrjones temporarily deployed to AcceptanceTests October 13, 2023 16:17 — with GitHub Actions Inactive
@brittanyrjones brittanyrjones temporarily deployed to AcceptanceTests October 13, 2023 16:29 — with GitHub Actions Inactive
@brittanyrjones brittanyrjones temporarily deployed to AcceptanceTests October 13, 2023 16:29 — with GitHub Actions Inactive
@brittanyrjones brittanyrjones temporarily deployed to AcceptanceTests October 13, 2023 16:29 — with GitHub Actions Inactive
@brittanyrjones brittanyrjones temporarily deployed to AcceptanceTests October 13, 2023 17:31 — with GitHub Actions Inactive
@brittanyrjones brittanyrjones temporarily deployed to AcceptanceTests October 13, 2023 17:31 — with GitHub Actions Inactive
@brittanyrjones brittanyrjones temporarily deployed to AcceptanceTests October 13, 2023 17:31 — with GitHub Actions Inactive
@brittanyrjones brittanyrjones temporarily deployed to AcceptanceTests October 13, 2023 17:33 — with GitHub Actions Inactive
@brittanyrjones brittanyrjones temporarily deployed to AcceptanceTests October 13, 2023 17:33 — with GitHub Actions Inactive
@brittanyrjones brittanyrjones temporarily deployed to AcceptanceTests October 13, 2023 17:33 — with GitHub Actions Inactive
@brittanyrjones brittanyrjones temporarily deployed to AcceptanceTests October 17, 2023 21:40 — with GitHub Actions Inactive
@brittanyrjones brittanyrjones temporarily deployed to AcceptanceTests October 17, 2023 21:40 — with GitHub Actions Inactive
@brittanyrjones brittanyrjones temporarily deployed to AcceptanceTests October 17, 2023 21:40 — with GitHub Actions Inactive
@brittanyrjones brittanyrjones temporarily deployed to AcceptanceTests October 17, 2023 22:09 — with GitHub Actions Inactive
@brittanyrjones brittanyrjones temporarily deployed to AcceptanceTests October 17, 2023 22:09 — with GitHub Actions Inactive
@brittanyrjones brittanyrjones temporarily deployed to AcceptanceTests October 17, 2023 22:09 — with GitHub Actions Inactive
brittanyrjones
brittanyrjones commented Oct 18, 2023
View reviewed changes
Copy link
Contributor Author

@brittanyrjones brittanyrjones left a comment

Choose a reason for hiding this comment

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

It may be good to change the messaging for numbered essentials around destroying the default credential. Right now it says default credential cant be destroyed, but perhaps it should be "You can’t perform this operation on Essential-tier databases."

brittanyrjones
brittanyrjones commented Oct 18, 2023
View reviewed changes
packages/pg-v5/commands/credentials/destroy.js Outdated
@@ -15,8 +15,9 @@ async function run(context, heroku) {
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It may be good to change the messaging for numbered essentials around destroying the default credential. Right now it says default credential cant be destroyed, but perhaps it should be "You can’t perform this operation on Essential-tier databases."?

Copy link
Member

Choose a reason for hiding this comment

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

agreed. do you mind adding that in this PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

Has any of this language been checked with CX?

@brittanyrjones brittanyrjones temporarily deployed to AcceptanceTests October 18, 2023 20:49 — with GitHub Actions Inactive
@brittanyrjones brittanyrjones temporarily deployed to AcceptanceTests October 18, 2023 20:49 — with GitHub Actions Inactive
@brittanyrjones brittanyrjones temporarily deployed to AcceptanceTests October 18, 2023 20:49 — with GitHub Actions Inactive
k80bowman
k80bowman previously requested changes Oct 20, 2023
View reviewed changes
Copy link
Contributor

@k80bowman k80bowman left a comment

Choose a reason for hiding this comment

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

Before merging this PR, I'd like to be sure that the language changes have been checked with CX.

@k80bowman k80bowman dismissed their stale review October 20, 2023 20:33

Unblocking so this can be merged while I'm out.

@brittanyrjones brittanyrjones temporarily deployed to AcceptanceTests October 20, 2023 20:33 — with GitHub Actions Inactive
@brittanyrjones brittanyrjones temporarily deployed to AcceptanceTests October 20, 2023 20:33 — with GitHub Actions Inactive
@brittanyrjones brittanyrjones temporarily deployed to AcceptanceTests October 20, 2023 20:33 — with GitHub Actions Inactive
@jdowning jdowning temporarily deployed to AcceptanceTests October 24, 2023 18:38 — with GitHub Actions Inactive
@jdowning jdowning temporarily deployed to AcceptanceTests October 24, 2023 18:38 — with GitHub Actions Inactive
@jdowning jdowning temporarily deployed to AcceptanceTests October 24, 2023 18:38 — with GitHub Actions Inactive
@jdowning jdowning temporarily deployed to AcceptanceTests October 24, 2023 18:52 — with GitHub Actions Inactive
@jdowning jdowning temporarily deployed to AcceptanceTests October 24, 2023 18:52 — with GitHub Actions Inactive
@jdowning jdowning temporarily deployed to AcceptanceTests October 24, 2023 18:52 — with GitHub Actions Inactive
@ryandagg ryandagg merged commit 51dc3ac into main Oct 24, 2023
8 checks passed
@ryandagg ryandagg deleted the bj-creds-essentials branch October 24, 2023 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jdowning jdowning jdowning approved these changes

@ryandagg ryandagg ryandagg approved these changes

@k80bowman k80bowman k80bowman left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@brittanyrjones @jdowning @ryandagg @k80bowman