console icon indicating copy to clipboard operation
console copied to clipboard

Add detailed diffs of changes made to secrets

Open rohan-chaturvedi opened this issue 1 year ago • 16 comments

Is your feature request related to a problem?

Making changes to secrets currently shows a "You have undeployed changes to this environment." warning banner and an amber highlight to affected secrets, but it can be difficult to determine what changes were made other than which secrets are affected.

2024-06-16 20-44-40.webm

Describe the solution you'd like

Clicking the warning banner should show a modal with details of secrets affected, with diffs of specific properties changed, such as tags, comments, etc. For example:

  • Modified GOOGLE_CLIENT_ID
    • changed key
    • added tag
  • AddedTEST
  • Modified SESSION_COOKIE_DOMAIN
    • changed comment

Ideally, something inspired by VSCode's diffs would be great:

image

rohan-chaturvedi avatar Jun 16 '24 15:06 rohan-chaturvedi

hey, I would like to work on this issue. @rohan-chaturvedi

harshith-1008 avatar Jul 08 '24 03:07 harshith-1008

hey, I would like to work on this issue. @rohan-chaturvedi

Great, its all yours!

rohan-chaturvedi avatar Jul 08 '24 07:07 rohan-chaturvedi

Screenshot 2024-07-11 at 12 09 50 PM Screenshot 2024-07-11 at 12 14 03 PM

Hey @rohan-chaturvedi just wanted a little clarification about the UI, for testing I have created a dropdown ui, should I make a separate pop up screen/page for the changes or should I just try to make the current dropdown look cleaner?

harshith-1008 avatar Jul 11 '24 06:07 harshith-1008

Hey @harshith-1008 , this is a good start! However I think we should avoid this "accordion" style behavior, since it shifts the entire rest of the UI down. Instead, how about trying a modal dialog? We have a GenericDialog component you could use: https://github.com/phasehq/console/blob/main/frontend/components/common/GenericDialog.tsx

rohan-chaturvedi avatar Jul 11 '24 06:07 rohan-chaturvedi

thanks for the feedback, ill try to implement it.

harshith-1008 avatar Jul 11 '24 07:07 harshith-1008

@rohan-chaturvedi I tried using genericDialogue component, but it expects button variant props, if I pass button variant then the alert changes to green box around the text. I have changed the component such that it accepts an empty string as props for genericDialogue, is it fine? Screenshot 2024-07-25 at 1 10 17 PM

harshith-1008 avatar Jul 25 '24 07:07 harshith-1008

@rohan-chaturvedi I tried using genericDialogue component, but it expects button variant props, if I pass button variant then the alert changes to green box around the text. I have changed the component such that it accepts an empty string as props for genericDialogue, is it fine? Screenshot 2024-07-25 at 1 10 17 PM

@harshith-1008 that should be fine, just make sure that its not a breaking change for other components that use it. I'll take a look in detail once you have a PR. Once thing you could try is adding an i icon in an outline or secondary button as the modal trigger rather than the entire alert text :thinking:

rohan-chaturvedi avatar Jul 25 '24 08:07 rohan-chaturvedi

Screenshot 2024-07-27 at 10 47 12 PM Screenshot 2024-07-27 at 10 47 23 PM

@rohan-chaturvedi I hope this UI is fine.

harshith-1008 avatar Jul 27 '24 17:07 harshith-1008

This is a great start! A few suggestions:

  • Add a bit more vertical spacing between items
  • Newly added secrets should show as "Added CHECKING" rather than "Modified 'Added CHECKING'"
  • Use MONOSPACE fonts for key names
  • Highlight the specific field that was changed, perhaps by using a higher font weight like font-medium or a different font color, (maybe font-mono as well?)
  • Explore the FontAwesome Icons and try and use some icons to denote modifications vs additions

rohan-chaturvedi avatar Jul 27 '24 17:07 rohan-chaturvedi

Screenshot 2024-07-28 at 12 34 14 AM

@rohan-chaturvedi is this ui okay?

harshith-1008 avatar Jul 27 '24 19:07 harshith-1008

That looks awesome! A few notes:

  • Only use font-mono for key names or field names. For example: "Modified AWS_SECRET_ACCESS_KEY"

  • Make sure the color-coding logic is consistent. Green makes sense for a new secret, but for modified secrets there's both blue and yellow. I'd suggest just picking one (yellow makes more sense IMO)

  • Could we show details of the changes here? For example:

  • Modified AWS_SECRET_ACCESS_KEY

    • Changed value
      • aCRAMarEbFC3Q5c24pi7AVMIt6TaCfHeFZ4KCf/a -> JBgoiFGNdbutADANDPmjp[OMKOJHduafdKAD

You could use the secret history UI as a baseline for this as well:

image

rohan-chaturvedi avatar Jul 27 '24 19:07 rohan-chaturvedi

@harshith-1008 The SecretPropertyDiffs component could be useful for rendering details of changes

rohan-chaturvedi avatar Jul 27 '24 19:07 rohan-chaturvedi

@rohan-chaturvedi just one small clarification, the font-mono should be just on keys name right? not the properties name right?

harshith-1008 avatar Jul 27 '24 19:07 harshith-1008

Yep, I think that will work well

rohan-chaturvedi avatar Jul 27 '24 20:07 rohan-chaturvedi

Screenshot 2024-07-31 at 11 00 50 AM

@rohan-chaturvedi Is this UI fine?

harshith-1008 avatar Jul 31 '24 05:07 harshith-1008

@harshith-1008 This is good progress! The UI still needs a bit of work though. Could you tag me on the PR when its ready? It will be easier to suggest changes to the styles.

rohan-chaturvedi avatar Jul 31 '24 07:07 rohan-chaturvedi