feat: [M3-7164] - Update AGLB Certificates UI #9711
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bnussman-akamai
requested review from
dwiley-akamai and
tyler-akamai
and removed request for
a team
|
I need to update the e2es still! |
abailly-akamai
approved these changes
Sep 25, 2023
| /* | ||
| * - Confirms Load Balancer certificate upload UI flow using mocked API requests. | ||
| * - Confirms that TLS and Service Target certificates can be uploaded. | ||
| * - Confirms that certificates table update to reflects uploaded certificates. | ||
| */ | ||
| it('can upload a Load Balancer Certificate', () => { | ||
| it('can upload a tls certificate', () => { |
There was a problem hiding this comment.
nitpick: capitalize TLS since it is everywhere else
| <Typography> | ||
| Used by the load balancer to accept responses from your endpoints in | ||
| your Service Target. This is the certificate installed on your | ||
| Endpoints. Apply these certificate(s) in the{' '} |
There was a problem hiding this comment.
"This is the certificate installed on your Endpoints. Apply these certificate(s) in the{' '}"
This is a weird mix of singular and plural
| ) : ( | ||
| <Typography> | ||
| Certificate used by your load balancer to terminate the connection | ||
| and decrypt request from client. Apply these certificate(s) in the |
dwiley-akamai
approved these changes
Sep 25, 2023
|
|
||
| const id = Number(loadbalancerId); | ||
|
|
||
| const { data: loadbalancer } = useLoadBalancerQuery(id); | ||
|
|
||
| const tabs = [ | ||
| { | ||
| routeName: `/loadbalancers/${id}/summary`, | ||
| component: LoadBalancerSummary, | ||
| path: 'summary', |
There was a problem hiding this comment.
(this is present in develop too)
this can be replicated by going {BASE_URL}/loadbalancers/certificates/summary or something similar that's not a legitimate path.
Seems like we should be displaying a Not Found or redirecting back to the landing page?
|
Maybe add some padding for the header texts and the 'Upload Certificate' button when width < 960px. |
|
It looks a lot cleaner, great job! I checked:
|
tyler-akamai
approved these changes
Sep 26, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description 📝
Major Changes 🔄
logsfrom Load BalancersLoadBalancerstoLoad Balancersdownstream)ca)Upload TLS CertificateUpload Service Target CertificatePreview 📷
How to test 🧪
http://localhost:3000/loadbalancers/0/certificatepage