appsmithorg / appsmith

Platform to build admin panels, internal tools, and dashboards. Integrates with 25+ databases and any API.
https://www.appsmith.com
Apache License 2.0
34.73k stars 3.76k forks source link

[Bug]: Snowflake key pair auth - Private key does not get removed upon removing from modal #34763

Open shadabbuchh opened 4 months ago

shadabbuchh commented 4 months ago

Is there an existing issue for this?

Description

If we try to remove the uploaded private key by clicking on the x icon in the upload modal, the key does not get removed.

Steps To Reproduce

  1. Upload a private key.
  2. Remove it by clicking on the x icon.

Expected: The key should be removed. Actual: The key is not removed.

Public Sample App

No response

Environment

Release

Severity

Medium (Frustrating UX)

Issue video log

https://jam.dev/c/85b3ab80-09ec-4303-8e0e-e7ab1b746b58

Version

Cloud

rohan-arthur commented 4 months ago

generic issue with file upload dialog. Also can be seen in these 3 postgres fields

Screenshot 2024-07-08 at 11 41 32
Jagadeesh-90 commented 4 months ago

@shadabbuchh @rohan-arthur When i try to reproduce this issue I couldn't find the private-key encrypted field and authentication dropdown to reproduce. image

i could see it in postgre SQL file upload dialogue but not in snowflake,can i resolve the same in postgre SQL.

daaliachhak17 commented 4 months ago

https://zemoso.atlassian.net/browse/TNOSB-450- ticket created on Jira

Jagadeesh-90 commented 4 months ago

@rohan-arthur @shadabbuchh @sneha122

Approach to Solve: Approach1:Need to update the value passed to the filepicker when we click on the close icon. In filePicker when it calls onChange then we update the prop value as empty.

Approach2: Add a Temporary State for Base64 File Data: Introduce a new state variable to store the base64 encoded file data.

Modify the FileUploader Function: Adjust the FileUploader function to read the file data using a FileReader and store the result in the temporary state.

Update the UseEffect Hook: Implement a useEffect hook that updates props.input.onChange when the modal's open state changes to false. Ensure this useEffect checks whether the modal is closed and the file data is available before updating props.input.onChange.

Handle Modal Close Event: Modify the Modal component to set the isOpen state to false when it is closed. This will trigger the useEffect hook to update props.input.onChange.

Clean Up on File Removal: Ensure that file removal logic also clears the temporary state storing the base64 data, so props.input.onChange is reset appropriately when no file is selected.

Nikhil-Nandagopal commented 3 months ago

Bumping this issue up to prod because it should have made it's way there by now if it was not fixed

sneha122 commented 3 months ago

@Jagadeesh-90 Thanks a lot for the detailed explanation of the solution approach, I would recommend going with the first approach, as this is simpler and UI also reflects correct state all the time, in this case when file gets removed from file picker, we should update the props.input to contain empty value in background, with first approach we can achieve this.

As for second approach, the problem I see with this is, file selected / removed state will not be visible in the background input when modal is open, as we will be only updating this background state on modal closure. Hope this makes sense.

Jagadeesh-90 commented 3 months ago

@sneha122 in Approach 1, when I select and upload the first file, then attempt to upload a second file and click on the cross icon, the first file is deselected and the file picker field becomes empty but when i cancel uploading the 2nd file the deafult 1st file is not visible. However, I was able to achieve the desired behavior using Approach 2.

sneha122 commented 3 months ago

@sneha122 in Approach 1, when I select and upload the first file, then attempt to upload a second file and click on the cross icon, the first file is deselected and the file picker field becomes empty but when i cancel uploading the 2nd file the deafult 1st file is not visible. However, I was able to achieve the desired behavior using Approach 2.

Let me check this, I am triggering DP on the PR you have raised, will check all cases there once and let you know

Jagadeesh-90 commented 3 months ago

Oky Sneha

sneha122 commented 3 months ago

@sneha122 in Approach 1, when I select and upload the first file, then attempt to upload a second file and click on the cross icon, the first file is deselected and the file picker field becomes empty but when i cancel uploading the 2nd file the deafult 1st file is not visible. However, I was able to achieve the desired behavior using Approach 2.

@Jagadeesh-90 Could you please explain this with a video? how I can reproduce this case

Jagadeesh-90 commented 3 months ago

Hi @sneha122 this is the vedio to reproduce it https://drive.google.com/file/d/1tOVlUJAeCq8hVcDwIU0CB0Cu0j3KychX/view?usp=sharing

sneha122 commented 3 months ago

Thanks a lot @Jagadeesh-90, One thing here is that file picker dialog allows us to upload the file and once uploaded it only has close button, it does not upload or update button to update this file selected in the form UI. If you see even in your video, you had to click outside in order to close the modal and get the uploaded file in form UI, this interaction may not be obvious to end user and hence on uploading file, it's important to show uploaded file in the form UI in the background (Approach 1).

Jagadeesh-90 commented 3 months ago

So let i do with approach1?

sneha122 commented 3 months ago

So let i do with approach1?

yes

Jagadeesh-90 commented 2 months ago

Hi @sneha122 re-raised the pr with approach1 https://github.com/appsmithorg/appsmith/pull/36270

sneha122 commented 2 months ago

Hi @sneha122 re-raised the pr with approach1 #36270

Thank you so much @Jagadeesh-90, I will review this either in this or next week.