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

[dashboard] Admin dashboard #3723

Merged
merged 1 commit into from
Apr 5, 2021

Conversation

svenefftinge
Copy link
Member

@svenefftinge svenefftinge commented Apr 2, 2021

Adds the admin dashboard to the new dashboard.

How to test:

  • create a user with 'admin' flag (default in dev staging)
  • you should see the admin menu on top.
  • try search for users or workspaces.

@svenefftinge svenefftinge force-pushed the sefftinge/dashboard-admin-dashboard-3359 branch 4 times, most recently from ff27e38 to 47694f5 Compare April 2, 2021 16:55
@svenefftinge
Copy link
Member Author

svenefftinge commented Apr 3, 2021

/werft run

👍 started the job as gitpod-build-sefftinge-dashboard-admin-dashboard-3359.7

@svenefftinge svenefftinge force-pushed the sefftinge/dashboard-admin-dashboard-3359 branch 2 times, most recently from e4ec8bb to fe9bed9 Compare April 4, 2021 07:42
@svenefftinge
Copy link
Member Author

svenefftinge commented Apr 4, 2021

/werft run

👍 started the job as gitpod-build-sefftinge-dashboard-admin-dashboard-3359.10

@svenefftinge
Copy link
Member Author

svenefftinge commented Apr 4, 2021

/werft run

👍 started the job as gitpod-build-sefftinge-dashboard-admin-dashboard-3359.11

@svenefftinge svenefftinge force-pushed the sefftinge/dashboard-admin-dashboard-3359 branch from fe9bed9 to 1a7bd2c Compare April 4, 2021 15:39
@svenefftinge svenefftinge marked this pull request as ready for review April 4, 2021 15:39
@@ -42,23 +42,26 @@ export class LicenseEvaluator {
}

public validate(): { msg?: string, valid: boolean } {
const v = validate(this.instanceID);
if (v.valid) {
// const v = validate(this.instanceID);
Copy link
Member Author

Choose a reason for hiding this comment

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

TODO: revert these changes

@svenefftinge svenefftinge force-pushed the sefftinge/dashboard-admin-dashboard-3359 branch from 1a7bd2c to 5aec9c7 Compare April 4, 2021 15:44
@svenefftinge svenefftinge requested a review from gtsiolis April 4, 2021 15:44
@gtsiolis
Copy link
Contributor

gtsiolis commented Apr 5, 2021

/werft run

👍 started the job as gitpod-build-sefftinge-dashboard-admin-dashboard-3359.14

Copy link
Contributor

@gtsiolis gtsiolis left a comment

Choose a reason for hiding this comment

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

Estupendo! Great to this coming to life. 🌟

Two points to consider:

  1. For some reason, I cannot start a workspace anymore. 💡
  2. As long as we have the functionality in place it should be good to ship. 🚢

left.push({
title: 'Admin',
link: '/admin',
alternatives: adminMenu.flatMap(e => e.link)
Copy link
Contributor

Choose a reason for hiding this comment

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

thought: I assume this hides the Admin menu for unauthorized users, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, you can test it by logging in with another user and then remove the admin role from it.

<path fillRule="evenodd" clipRule="evenodd" d="M6 2a4 4 0 100 8 4 4 0 000-8zM0 6a6 6 0 1110.89 3.477l4.817 4.816a1 1 0 01-1.414 1.414l-4.816-4.816A6 6 0 010 6z" fill="#A8A29E" />
</svg>
</div>
<input className="border-0" type="text" placeholder="Search in context urls" onKeyDown={(ke) => ke.key === 'Enter' && search() } onChange={(v) => { setSearchTerm(v.target.value) }} />
Copy link
Contributor

Choose a reason for hiding this comment

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

thought: This is probably more accurate but could suffice as Search Workspaces. Let's keep it as is!

Suggested change
<input className="border-0" type="text" placeholder="Search in context urls" onKeyDown={(ke) => ke.key === 'Enter' && search() } onChange={(v) => { setSearchTerm(v.target.value) }} />
<input className="border-0" type="text" placeholder="Search context URLs" onKeyDown={(ke) => ke.key === 'Enter' && search() } onChange={(v) => { setSearchTerm(v.target.value) }} />

Copy link
Member Author

Choose a reason for hiding this comment

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

I made it explicit, because it only searches in context urls so for instance not in workspace ids.

function WorkspaceEntry(p: { ws: WorkspaceAndInstance }) {
return <Link to={'/admin/workspaces/' + p.ws.workspaceId}>
<div className="rounded-xl whitespace-nowrap flex space-x-2 py-6 px-6 w-full justify-between hover:bg-gray-100 focus:bg-gitpod-kumquat-light group">
<div className="pr-3 self-center w-1/12">
Copy link
Contributor

Choose a reason for hiding this comment

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

issue: This is making the tooltip show up slightly offset.

Screenshot 2021-04-05 at 10 18 45 AM

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems fixed in the preview environment. 🧀

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I fixed it already in my workspace :-)

<div className="font-medium text-gray-800 truncate hover:text-blue-600">{p.ws.workspaceId}</div>
<div className="text-sm overflow-ellipsis truncate text-gray-400">{getProject(WorkspaceAndInstance.toWorkspace(p.ws))}</div>
</div>
<div className="flex w-4/12 self-center">
Copy link
Contributor

Choose a reason for hiding this comment

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

issue: Some columns overlap and the list is also overflowing from the container on medium viewports! 🙀

Column Container
Screenshot 2021-04-05 at 10 17 21 AM container

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems fixed in the preview environment. 🧀

</div>
<input className="border-0" type="text" placeholder="Search Users" onKeyDown={(ke) => ke.key === 'Enter' && search() } onChange={(v) => { setSearchTerm(v.target.value) }} />
</div>
<button className="" disabled={searching} onClick={search}>Start Search</button>
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: No need to prefix this with Start.

Suggested change
<button className="" disabled={searching} onClick={search}>Start Search</button>
<button className="" disabled={searching} onClick={search}>Search</button>

<p>{getProject(WorkspaceAndInstance.toWorkspace(workspace))}</p>
</div>
<button className="secondary ml-3" disabled={activity} onClick={reload}>Reload Data</button>
<button className="danger ml-3" disabled={activity || workspace.phase === 'stopped'} onClick={stopWorkspace}>Stop Workspaces</button>
Copy link
Contributor

Choose a reason for hiding this comment

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

question: Shouldn't this read Stop Workspace?

Suggested change
<button className="danger ml-3" disabled={activity || workspace.phase === 'stopped'} onClick={stopWorkspace}>Stop Workspaces</button>
<button className="danger ml-3" disabled={activity || workspace.phase === 'stopped'} onClick={stopWorkspace}>Stop Workspace</button>

<div className="w-5/12">Created</div>
</div>
</div>
{searchResult.rows.filter(u => u.identities.length > 0).map(u => <UserEntry user={u} />)}
Copy link
Contributor

Choose a reason for hiding this comment

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

thought: I still find this empty data set on load slightly confusing but I understand this helps us avoid some performance costs. Let's leave it as is for now. 🎯

Copy link
Member Author

Choose a reason for hiding this comment

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

We should possibly show an empty list place holder (later).

return <div className={`ml-3 text-sm text-${p.color}-600 truncate bg-${p.color}-100 px-1.5 py-0.5 rounded-md my-auto`}>{p.text}</div>;
}

export function Property(p: { name: string, children: string | ReactChild, action?: { label: string, onClick: () => void } }) {
Copy link
Contributor

Choose a reason for hiding this comment

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

praise: Nice abstraction here!

{p.name}
</div>
<div className="text-lg text-gray-600 font-semibold truncate">
{p.children}
Copy link
Contributor

Choose a reason for hiding this comment

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

issue: Latest Instance ID bring in some overflow-scroll. Known?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it is on purpose so I can actually see and copy the whole ID.

<div className="flex"><h3>{workspace.workspaceId}</h3><span className="my-auto ml-3"><WorkspaceStatusIndicator instance={WorkspaceAndInstance.toInstance(workspace)} /></span></div>
<p>{getProject(WorkspaceAndInstance.toWorkspace(workspace))}</p>
</div>
<button className="secondary ml-3" disabled={activity} onClick={reload}>Reload Data</button>
Copy link
Contributor

Choose a reason for hiding this comment

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

question: No more Download Workspace button?

Copy link
Member Author

Choose a reason for hiding this comment

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

It doesn't work AFAIK

@svenefftinge svenefftinge force-pushed the sefftinge/dashboard-admin-dashboard-3359 branch from 5aec9c7 to a56ef7e Compare April 5, 2021 09:42
@svenefftinge svenefftinge merged commit bf5e0de into main Apr 5, 2021
@svenefftinge svenefftinge deleted the sefftinge/dashboard-admin-dashboard-3359 branch April 5, 2021 09:52
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.

2 participants