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

feat: Extract certificate details #35

Merged
merged 10 commits into from
Jul 1, 2024
Merged

feat: Extract certificate details #35

merged 10 commits into from
Jul 1, 2024

Conversation

saltiyazan
Copy link
Contributor

@saltiyazan saltiyazan commented Jun 28, 2024

Description

  • Improves the code to extract certificates
  • Improves the code to compare certificates
  • Display certificate details

Both certificate and csr details are displayed, as there is no guarantee that those are equal.
Discrepancies between csr and cert fields are highlighted in red.
The details are responsive and they go into one column on small screens.
The expiry date is coloured green, it turns red when we are within one day into expiry and beyond.

Screenshot 2024-07-01 at 13 40 46

Screenshot 2024-07-01 at 13 40 55

Screenshot 2024-07-01 at 13 39 05

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that validate the behaviour of the software
  • I validated that new and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Improves the code to extract certificates
Improves the code to compare certificates
Display certificate details
@saltiyazan saltiyazan changed the title feat: Extract certificate details [WIP] feat: Extract certificate details Jun 28, 2024
@saltiyazan saltiyazan changed the title [WIP] feat: Extract certificate details feat: Extract certificate details Jul 1, 2024
@saltiyazan saltiyazan marked this pull request as ready for review July 1, 2024 10:37
@saltiyazan saltiyazan requested a review from a team as a code owner July 1, 2024 10:37
Copy link
Contributor

@kayra1 kayra1 left a comment

Choose a reason for hiding this comment

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

I really like how it looks. I especially like that if the common name is very long the certificate details wrap.

There are still some suggestions though.

ui/src/app/certificate_requests/row.tsx Outdated Show resolved Hide resolved
ui/src/app/certificate_requests/row.tsx Outdated Show resolved Hide resolved
ui/src/app/certificate_requests/row.tsx Outdated Show resolved Hide resolved
ui/src/app/utils.ts Outdated Show resolved Hide resolved
@saltiyazan saltiyazan requested a review from kayra1 July 1, 2024 12:36
Copy link
Contributor

@kayra1 kayra1 left a comment

Choose a reason for hiding this comment

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

I think we can pass on the highlighting for now.

@saltiyazan saltiyazan merged commit 85fd6ec into main Jul 1, 2024
11 checks passed
@saltiyazan saltiyazan deleted the TLSENG-296 branch July 1, 2024 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants