Create new RDP service #3029
Create new RDP service #3029
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
| /** | ||
| * Returns the available RDP clients | ||
| */ | ||
| handle('getRdpClients', async () => []); |
There was a problem hiding this comment.
All the below four handlers are placeholders only. Logic will be added in a separate IPC handler PR
There was a problem hiding this comment.
Are these unit tests useful? It seems like we're mocking what is being returned and then verify that our mocks are being returned and not actual logic being tested.
There was a problem hiding this comment.
Good point. Yeah it does just check the mocked data is returned correctly. I will update the tests
| * The preferred RDP client set by the user. | ||
| * @type {string|null} | ||
| */ | ||
| @tracked _preferredRdpClient = null; |
There was a problem hiding this comment.
We should use private fields for this if they're meant to be internal
There was a problem hiding this comment.
Ahh yes thanks! I will update it
| return this.preferredRdpClient && this.preferredRdpClient !== 'none'; | ||
| } | ||
|
|
||
| get rdpClients() { |
There was a problem hiding this comment.
Are these getters necessary? It seems users should just call the underlying function e.g. getRdpClients rather than a getter that actually is async but doesn't make it obvious to the consumer that it's async
| this._rdpClients = await this.ipc.invoke('getRdpClients'); | ||
| return this._rdpClients; | ||
| } catch (error) { | ||
| console.error('Failed to fetch RDP clients:', error); |
There was a problem hiding this comment.
We normally should log these instead (though they may already be being logged in the )handle?
There was a problem hiding this comment.
I was wondering the same thing too. Maybe electron-log would be a good fit here?
There was a problem hiding this comment.
May be we can keep logging in the handlers only and remove them from the service.
| * the proxy details and constructs the appropriate RDP connection parameters. | ||
| * @param {string} session_id - The ID of the active session | ||
| */ | ||
| async launchRdpClient(session_id) { |
There was a problem hiding this comment.
| async launchRdpClient(session_id) { | |
| async launchRdpClient(sessionId) { |
We normally use camelCase for standard JS that's not JSON from an API
3 participants
Description
This PR focuses on creating a new RDP service for RDP client launch in DC feature, that helps keep the RDP client detection and launch, and setting user's preferred client logic centralized for reusability in components and controllers.
For more context, refer RDP service section of RFC.
🎟️ Jira ticket
Screenshots (if appropriate)
How to Test
Checklist
a11y-testslabel to run a11y audit tests if neededPCI review checklist
Examples of changes to security controls include using new access control methods, adding or removing logging pipelines, etc.