Secret changes dialog added #274 #318

harshith-1008 · 2024-07-31T14:18:29Z

🔍 Overview

This pull request solves issue Add detailed diffs of changes made to secrets #274.
Feature: Functionality which allows user to see the changes made to environment secrets.

💡 Proposed Changes

Used GenericDialog component to show the changes made by user
Added info button which onClick shows dialog
Created type SecretChange for the secret changes
Created a component DisplayChanges which returns changes of a particular secret
Also limited key value to 50 characters so that it does overflow out of the dialogue

🖼️ Screenshots or Demo

Before

After

📝 Release Notes

User now can see the changes made to the environment secrets

❓ Open Questions

Is there anyway I can enhance the UI more?

🧪 Testing

Add a secret
Change an attribute of a pre-existing secret
Remove a tag from the secret and add other tags

🎯 Reviewer Focus

You can just make any desired changes and click on the info button to view the changes

➕ Additional Context

issue #274

✨ How to Test the Changes Locally

Pull the branch
log in to your phase console account
head over to an environment of any app and start testing

💚 Did You...

Ensure linting passes (code style checks)?
Update dependencies and lockfiles (if required)
Regenerate graphql schema and types (if required)
Verify the app builds locally?
Manually test the changes on different browsers/devices?

harshith-1008 · 2024-08-01T07:51:43Z

@rohan-chaturvedi
If possible, could you please look into this pr?

rohan-chaturvedi · 2024-08-01T07:57:44Z

@rohan-chaturvedi If possible, could you please look into this pr?

Sure @harshith-1008 , will take a look as soon as I can!

rohan-chaturvedi · 2024-08-09T15:43:23Z

@harshith-1008 Could you fix the filename please?

harshith-1008 · 2024-08-11T14:48:36Z

@harshith-1008 Could you fix the filename please?

@rohan-chaturvedi for some reason vscode isnt allowing me to keep this license file because folder name license already exists, can you please guide me to restore it?

rohan-chaturvedi · 2024-08-11T15:02:18Z

@harshith-1008 Could you fix the filename please?

@rohan-chaturvedi for some reason vscode isnt allowing me to keep this license file because folder name license already exists, can you please guide me to restore it?

Thanks a lot for the screenshot! Now I understand why this was a problem 😅

I'll raise a PR to rename the license folder in a bit, that should resolve it.

harshith-1008 · 2024-08-11T15:11:08Z

@rohan-chaturvedi I have managed to rename the file through GitHub as I wasnt Able to do in Vscode. The file is renamed now.

rohan-chaturvedi · 2024-08-12T14:17:06Z

Hey @harshith-1008 , a couple of updates:

We've updated the UX for deleting secrets. Secrets can now be "staged" for delete, and are only deleted when clicking on "Deploy". See the PR for a full demo: refactor: misc optimizations and ux improvements to secrets editing #340
This means we need to add Deleted secrets to the changed secrets dialog
I was working on a cleaner design for this dialog for a while. I've made a Figma mock here.

Here's a screenshot as well:

You can grab the colors from Figma. The tailwind colors are:

Dialog title: text-zinc-900 dark:text-zinc-900
Dialog subtitle: text-zinc-500
Created: text-emerald-400 (Backgrounds: bg-emerald-400/20 dark:bg-emerald-400/10)
Updated: text-amber-400 (Backgrounds: bg-amber-400/20 dark:bg-amber-400/10)
Deleted: text-red-400 (Backgrounds: bg-red-400/20 dark:bg-red-400/10)

Sorry for the constant design updates! Lmk if you have any questions, we can hop on a slack huddle and discuss it as well.

harshith-1008 · 2024-08-15T11:05:31Z

@rohan-chaturvedi I tried changing the UI, Can you guide me how to override text-color of tags? In Tag component the styling is (text-zinc-700 dark:text-zinc-200).

I tried this method but it isn't working. and should I decrease the height and width of tags?

rohan-chaturvedi · 2024-08-15T13:06:58Z

@rohan-chaturvedi I tried changing the UI, Can you guide me how to override text-color of tags? In Tag component the styling is (text-zinc-700 dark:text-zinc-200).

I tried this method but it isn't working. and should I decrease the height and width of tags?

@harshith-1008 You could add a variant prop to the Tag component that takes a string of type: 'normal' | 'deleted' | 'created'. Apply the styles based on the value of this prop. Check out the Button component to see how a variant can be added. Make the 'normal' default value of the prop, so that you won't have to add this prop wherever the Tag is currently used.

Lets keep the height and width of the tags for now.

harshith-1008 · 2024-08-16T05:13:51Z

@rohan-chaturvedi can you look into these changes? I have kept the tags width and height same.

rohan-chaturvedi · 2024-08-16T05:52:06Z

@rohan-chaturvedi can you look into these changes? I have kept the tags width and height same.

Looking good! Only issue I can see is that monospace font isn't used in the right places. Could you take a look at that?

harshith-1008 · 2024-08-16T10:16:46Z

@rohan-chaturvedi changed font irregularities.

rohan-chaturvedi · 2024-08-16T10:21:33Z

@rohan-chaturvedi changed font irregularities.

Use normal font for the change type ("Created", "Updated" etc)
Use monospace for everything else (field names and values)

harshith-1008 · 2024-08-16T10:53:12Z

@rohan-chaturvedi changed font irregularities.

Use normal font for the change type ("Created", "Updated" etc)

Use monospace for everything else (field names and values)

@rohan-chaturvedi Except change type, all other texts changed to font-mono.

rohan-chaturvedi · 2024-08-16T13:54:03Z

@harshith-1008 Nice! Lmk when the code changes are pushed, will take a look. Lets get this merged!

harshith-1008 · 2024-08-17T01:50:34Z

@rohan-chaturvedi I have pushed changes.

rohan-chaturvedi

@harshith-1008 Awesome work! This is working perfectly 👌

Just a few code changes and minor fixes required. Also, could you please create a separate component for this? Call is "DeployPreview" or something and put it in components/environments/secrets

rohan-chaturvedi · 2024-08-30T06:22:57Z

frontend/app/[team]/apps/[app]/environments/[environment]/[[...path]]/page.tsx

+                    {secretsToDelete?.map(secretId => {
+                      const deletedSecret = serverSecrets.find(secret => secret.id === secretId);
+                      return (
+                        <div className="flex items-center space-x-2 mb-6">


Suggested change

<div className="flex items-center space-x-2 mb-6">

<div key={secretId} className="flex items-center space-x-2 mb-6">

Add a key for elements rendered in a list

rohan-chaturvedi · 2024-08-30T06:26:58Z

frontend/app/[team]/apps/[app]/environments/[environment]/[[...path]]/page.tsx

+    const removedTags = change.tags?.old?.filter(tag => 
+      !change.tags?.new?.some(newTag => newTag.id === tag.id)
+    ) || [];
+
+    const addedTags = change.tags?.new?.filter(tag => 
+      !change.tags?.old?.some(oldTag => oldTag.id === tag.id)
+    ) || [];


Suggested change

const removedTags = change.tags?.old?.filter(tag =>

!change.tags?.new?.some(newTag => newTag.id === tag.id)

) || [];

const addedTags = change.tags?.new?.filter(tag =>

!change.tags?.old?.some(oldTag => oldTag.id === tag.id)

) || [];

const removedTags =

change.tags?.old?.filter(

(tag: SecretTagType) =>

!change.tags?.new?.some((newTag: SecretTagType) => newTag.id === tag.id)

) || []

const addedTags =

change.tags?.new?.filter(

(tag: SecretTagType) =>

!change.tags?.old?.some((oldTag: SecretTagType) => oldTag.id === tag.id)

) || []

Add the SecretTagType annotation here

rohan-chaturvedi · 2024-08-30T06:44:11Z

frontend/app/[team]/apps/[app]/environments/[environment]/[[...path]]/page.tsx

+                  onClose={() => setIsChangesModalOpen(false)}
+                  buttonContent={<FaInfoCircle />}
+                >
+                  <div className="flex flex-col space-y-2">


Suggested change

<div className="flex flex-col space-y-2">

<div className="flex flex-col space-y-2 max-h-[85vh] overflow-auto">

Add a max height to handle overflow where there are a large number of changes

harshith-1008 · 2024-08-30T15:40:28Z

@rohan-chaturvedi I have pushed new changes. Can you look into it ?

rohan-chaturvedi · 2024-08-31T08:08:03Z

Great work! Sorry for the lengthy review 😅

Secret changes dialog added

fff1fb0

rohan-chaturvedi self-requested a review August 1, 2024 07:57

harshith-1008 and others added 2 commits August 2, 2024 07:59

added license

9a164d8

Merge branch 'main' into feat-show-secret-changes

b6ba946

Merge branch 'phasehq:main' into feat-show-secret-changes

00803a5

Update and rename LICENSE.txt to LICENSE

716d82f

rohan-chaturvedi mentioned this pull request Aug 11, 2024

chore: rename license folder to licensing #341

Merged

3 tasks

harshith-1008 added 2 commits August 13, 2024 08:25

Merge branch 'main' into feat-show-secret-changes

7aca339

Merge branch 'main' into feat-show-secret-changes

256758a

harshith-1008 and others added 2 commits August 16, 2024 10:45

Merge branch 'main' into feat-show-secret-changes

f6335fe

figma design changes

8e51842

fixed font irregularities

00ee51e

fixed font

1cb01d3

renamed getclientSecrets to its original name

1124dc9

Merge branch 'main' into feat-show-secret-changes

b0d38df

rohan-chaturvedi requested changes Aug 30, 2024

View reviewed changes

Created DeployPreview component

ec2eb86

rohan-chaturvedi self-requested a review August 31, 2024 08:06

rohan-chaturvedi approved these changes Aug 31, 2024

View reviewed changes

rohan-chaturvedi merged commit 4389320 into phasehq:main Aug 31, 2024
2 of 3 checks passed

	<div className="flex items-center space-x-2 mb-6">
	<div key={secretId} className="flex items-center space-x-2 mb-6">

	<div className="flex flex-col space-y-2">
	<div className="flex flex-col space-y-2 max-h-[85vh] overflow-auto">

Secret changes dialog added #274 #318

Secret changes dialog added #274 #318

Uh oh!

Conversation

harshith-1008 commented Jul 31, 2024

🔍 Overview

💡 Proposed Changes

🖼️ Screenshots or Demo

📝 Release Notes

❓ Open Questions

🧪 Testing

🎯 Reviewer Focus

➕ Additional Context

✨ How to Test the Changes Locally

💚 Did You...

Uh oh!

harshith-1008 commented Aug 1, 2024

Uh oh!

rohan-chaturvedi commented Aug 1, 2024

Uh oh!

rohan-chaturvedi commented Aug 9, 2024

Uh oh!

harshith-1008 commented Aug 11, 2024

Uh oh!

rohan-chaturvedi commented Aug 11, 2024

Uh oh!

harshith-1008 commented Aug 11, 2024

Uh oh!

rohan-chaturvedi commented Aug 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

harshith-1008 commented Aug 15, 2024

Uh oh!

rohan-chaturvedi commented Aug 15, 2024

Uh oh!

harshith-1008 commented Aug 16, 2024

Uh oh!

rohan-chaturvedi commented Aug 16, 2024

Uh oh!

harshith-1008 commented Aug 16, 2024

Uh oh!

rohan-chaturvedi commented Aug 16, 2024

Uh oh!

harshith-1008 commented Aug 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rohan-chaturvedi commented Aug 16, 2024

Uh oh!

harshith-1008 commented Aug 17, 2024

Uh oh!

rohan-chaturvedi left a comment

Choose a reason for hiding this comment

Uh oh!

rohan-chaturvedi Aug 30, 2024

Choose a reason for hiding this comment

Uh oh!

rohan-chaturvedi Aug 30, 2024

Choose a reason for hiding this comment

Uh oh!

rohan-chaturvedi Aug 30, 2024

Choose a reason for hiding this comment

Uh oh!

harshith-1008 commented Aug 30, 2024

Uh oh!

rohan-chaturvedi commented Aug 31, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rohan-chaturvedi commented Aug 12, 2024 •

edited

Loading

harshith-1008 commented Aug 16, 2024 •

edited

Loading

rohan-chaturvedi commented Aug 31, 2024 •

edited

Loading