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

Update sshkey page #2554

Merged
merged 24 commits into from
Apr 21, 2024
Merged

Update sshkey page #2554

merged 24 commits into from
Apr 21, 2024

Conversation

samaradel
Copy link
Contributor

@samaradel samaradel commented Apr 14, 2024

Description

  • Remove Active keys table
  • Add all keys into one table
  • generate a name for keys that haven't a name
  • Reduce the size of titles
  • prevent opening the dialog of ssh key details by clicking on the active checkbox
  • Generate name when the key is imported without name

Changes

image

Related Issues

Checklist

  • Tests included
  • Build pass
  • Documentation
  • Code format and docstrings
  • Screenshots/Video attached (needed for UI changes)

- Add all keys in one table
- generate name for keys which havnt a name
- Reduce size of titles
- prevent opening the dialog of sshkey details on click on active checkbox
Copy link
Contributor

@AlaaElattar AlaaElattar left a comment

Choose a reason for hiding this comment

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

Great Job

Copy link
Contributor

@Mahmoud-Emad Mahmoud-Emad left a comment

Choose a reason for hiding this comment

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

Good job ya Samar,

  • as we agreed we have to migrate the old key once the user logs in in the background.

image

  • The SSH key can be without a name, so we need to update the validation rule to accept keys with no names.

image

  • I deleted the name when importing a new SSH key and it doesn't generate a name for me, it displayed as -, in this case, I think we should generate a name on behalf of the user as we displayed it in the input hint.

image

  • Should we enable the loader while activating/deactivating the SSH key?
Screencast.from.04-15-2024.08.52.33.AM.webm
  • I deleted a key then generated a new one and got the deleted key and the generated key with the same ID

image

Screencast.from.04-15-2024.08-57-15.AM.mp4
  • When deleted the SSH key name when generating new one I expect that you'll generate another one for me but got a dash -

image

@samaradel samaradel marked this pull request as draft April 15, 2024 07:17
samaradel and others added 13 commits April 16, 2024 11:59
- Migrate keys on login/connect
- Fix sshkeyData type
- The validation rule was modified to accept keys without names

- Take the key name when importing an 'SSH key' if exists or generate a new one

- Migrating the old ssh key after activating the 'profile manager'

- Created a class named 'SSHKeyData' that holds all 'SSH' functionality, to avoid code duplication since there are some functions that need to be imported into other places

- Disable the 'Import', 'Generate', and 'Export' buttons while 'Deleteing', 'Creating', and 'Updating' keys to avoid errors

- Wait on the chain to finish the task then update the table of 'ssh keys'
@Mahmoud-Emad Mahmoud-Emad marked this pull request as ready for review April 18, 2024 07:47
Copy link
Contributor

@0oM4R 0oM4R left a comment

Choose a reason for hiding this comment

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

great job, we just need to pay attention to error handling.

@zaelgohary
Copy link
Contributor

Can we change the info to be "Manage SSH keys easily, switch between them, and activate or deactivate keys as needed for accessing deployed machines. Simplify key distribution and effectively manage access to nodes."

Facilitating access to deployed machines involves the incorporation or adaptation of SSH keys, with the
flexibility to manage multiple keys seamlessly, allowing users to switch between them. Moreover, users can
activate individual keys or enable them all, streamlining the process of distributing them to the machines and
effectively managing accessibility to the deployed nodes.

@Mahmoud-Emad Mahmoud-Emad marked this pull request as draft April 18, 2024 14:08
@zaelgohary
Copy link
Contributor

The lower export button doesn't work as expected.

export_ssh.webm

- Rename the tab of 'SSH Key' to 'SSH Keys' to match the new feature

- Generate a unique name/key and check if any of them exists on the profile keys

- Check the user balance if enough or not before updating the chain and notify the user

- Rename the text area and input fields

- Migrate and take the old key in deployment if the balance isn't enough to avoid errors
@Mahmoud-Emad Mahmoud-Emad marked this pull request as ready for review April 18, 2024 18:01
- Removed the hard-coded cost balance and make it variable instead
- Updated the @click:row event
- Removed unneeded alert message
Copy link
Contributor

@zaelgohary zaelgohary left a comment

Choose a reason for hiding this comment

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

Good job, Emad!

Copy link
Contributor

@0oM4R 0oM4R left a comment

Choose a reason for hiding this comment

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

I still have an unhanded errors on low balance account
image

Copy link
Contributor

@0oM4R 0oM4R left a comment

Choose a reason for hiding this comment

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

On low balance, an error is handled, but the status has changed!
After reloading the page, the status went back to active. 
also i think the toast should have more friendly message
Screencast from 21 أبر, 2024 EET 11:21:09 ص.webm

packages/playground/src/utils/ssh.ts Outdated Show resolved Hide resolved
@Mahmoud-Emad Mahmoud-Emad requested a review from 0oM4R April 21, 2024 12:28
Copy link
Contributor

@0oM4R 0oM4R left a comment

Choose a reason for hiding this comment

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

Thanks ya Omda great job

@samaradel samaradel merged commit 2c8c89b into development Apr 21, 2024
3 checks passed
@samaradel samaradel deleted the development_sshkeys branch April 21, 2024 13:27
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.

6 participants