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.

Sign up for GitHub

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

Revise Prebuilds Page #4970

Merged
merged 12 commits into from
Aug 2, 2021
Merged

Revise Prebuilds Page #4970

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
c42eb8e
[projects] remove settings page
AlexTugarev Jul 27, 2021
b90fa62
[projects] fixes truncate commit message
AlexTugarev Jul 27, 2021
7c160f9
[projects] fix short hash rendering
AlexTugarev Jul 27, 2021
4a5751c
[projects] remove status filter
AlexTugarev Jul 27, 2021
7fe5cdc
[projects] remove "cancel prebuild" item
AlexTugarev Jul 27, 2021
130ed5c
[projects] rename overview page
AlexTugarev Jul 27, 2021
c8c7b92
[project] add menu spacing
AlexTugarev Jul 27, 2021
40af349
[projects] add contextURL for branches
AlexTugarev Jul 27, 2021
7a18a52
[projects] fix search on overview page
AlexTugarev Jul 27, 2021
1eecdd3
[projects] add default badge
AlexTugarev Jul 27, 2021
1f24982
[prebuilds] speed up query for active branches
AlexTugarev Jul 27, 2021
88b36e9
[projects] increase repo listing limit
AlexTugarev Jul 29, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions components/dashboard/src/App.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ const Projects = React.lazy(() => import(/* webpackPrefetch: true */ './projects
const Project = React.lazy(() => import(/* webpackPrefetch: true */ './projects/Project'));
gtsiolis marked this conversation as resolved.
Show resolved Hide resolved
gtsiolis marked this conversation as resolved.
Show resolved Hide resolved
gtsiolis marked this conversation as resolved.
Show resolved Hide resolved
const Prebuilds = React.lazy(() => import(/* webpackPrefetch: true */ './projects/Prebuilds'));
const Prebuild = React.lazy(() => import(/* webpackPrefetch: true */ './projects/Prebuild'));
const Settings = React.lazy(() => import(/* webpackPrefetch: true */ './projects/Settings'));
const InstallGitHubApp = React.lazy(() => import(/* webpackPrefetch: true */ './prebuilds/InstallGitHubApp'));
const FromReferrer = React.lazy(() => import(/* webpackPrefetch: true */ './FromReferrer'));
const UserSearch = React.lazy(() => import(/* webpackPrefetch: true */ './admin/UserSearch'));
Expand Down Expand Up @@ -214,9 +213,6 @@ function App() {
if (resourceOrPrebuild === "prebuilds") {
return <Prebuilds />;
}
if (resourceOrPrebuild === "settings") {
return <Settings />;
}
return resourceOrPrebuild ? <Prebuild /> : <Project />;
}} />
</Route>)}
Expand Down
6 changes: 1 addition & 5 deletions components/dashboard/src/Menu.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,6 @@ export default function Menu() {
title: 'Prebuilds',
link: `/${team.slug}/${projectName}/prebuilds`
},
{
title: 'Settings',
link: `/${team.slug}/${projectName}/settings`
},
{
title: 'Configure',
link: `/${team.slug}/${projectName}/configure`
Expand Down Expand Up @@ -201,7 +197,7 @@ export default function Menu() {
}

return <>
<header className={`lg:px-28 px-10 flex flex-col pt-4 space-y-4 ${isMinimalUI ? 'pb-4' : ''}`}>
<header className={`lg:px-28 px-10 flex flex-col pt-4 space-y-4 ${isMinimalUI || !!prebuildId ? 'pb-4' : ''}`}>
<div className="flex h-10">
<div className="flex justify-between items-center pr-3">
<Link to="/">
Expand Down
9 changes: 2 additions & 7 deletions components/dashboard/src/projects/Prebuilds.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,14 +116,9 @@ export default function () {
}

const formatDate = (date: string | undefined) => {
return date ? moment(date).fromNow() : "–";
return date ? moment(date).fromNow() : "";
}

const formatHash = (longHash: string) => {
return longHash ? longHash.substring(0, 8) : "–";
}


return <>
<Header title="Prebuilds" subtitle={`View all recent prebuilds for all active branches.`} />
AlexTugarev marked this conversation as resolved.
Show resolved Hide resolved
<div className="lg:px-28 px-10">
AlexTugarev marked this conversation as resolved.
Show resolved Hide resolved
AlexTugarev marked this conversation as resolved.
Show resolved Hide resolved
Expand Down Expand Up @@ -166,7 +161,7 @@ export default function () {
<ItemField className="flex items-center">
<div>
<div className="text-base text-gray-500 dark:text-gray-50 font-medium mb-1">{shortCommitMessage(p.changeTitle)}</div>
<p>{p.changeAuthorAvatar && <img className="rounded-full w-4 h-4 inline-block align-text-bottom mr-2" src={p.changeAuthorAvatar || ''} alt={p.changeAuthor} />}Authored {formatDate(p.changeDate)} · {formatHash(p.changeHash)}</p>
<p>{p.changeAuthorAvatar && <img className="rounded-full w-4 h-4 inline-block align-text-bottom mr-2" src={p.changeAuthorAvatar || ''} alt={p.changeAuthor} />}Authored {formatDate(p.changeDate)} · {p.changeHash?.substring(0, 8)}</p>
</div>
</ItemField>
<ItemField className="flex items-center">
Expand Down
55 changes: 16 additions & 39 deletions components/dashboard/src/projects/Project.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,16 @@
*/

import moment from "moment";
import { PrebuildInfo, PrebuiltWorkspaceState, Project } from "@gitpod/gitpod-protocol";
import { PrebuildInfo, Project } from "@gitpod/gitpod-protocol";
import { useContext, useEffect, useState } from "react";
import { useHistory, useLocation, useRouteMatch } from "react-router";
import Header from "../components/Header";
import DropDown, { DropDownEntry } from "../components/DropDown";
import { ItemsList, Item, ItemField, ItemFieldContextMenu } from "../components/ItemsList";
import { getGitpodService, gitpodHostUrl } from "../service/service";
import { TeamsContext, getCurrentTeam } from "../teams/teams-context";
import { prebuildStatusIcon, prebuildStatusLabel } from "./Prebuilds";
import { ContextMenuEntry } from "../components/ContextMenu";
import { toRemoteURL } from "./render-utils";
import { shortCommitMessage } from "./render-utils";

export default function () {
const history = useHistory();
Expand All @@ -31,7 +30,6 @@ export default function () {
const [lastPrebuilds, setLastPrebuilds] = useState<Map<string, PrebuildInfo>>(new Map());

const [searchFilter, setSearchFilter] = useState<string | undefined>();
const [statusFilter, setStatusFilter] = useState<PrebuiltWorkspaceState | undefined>();

useEffect(() => {
updateProject();
Expand Down Expand Up @@ -78,38 +76,15 @@ export default function () {
onClick: () => onNewWorkspace(branch)
});
entries.push({
title: "Trigger Prebuild",
title: "Rerun Prebuild",
onClick: () => triggerPrebuild(branch),
separator: true
});
entries.push({
title: "Cancel Prebuild",
customFontStyle: 'text-red-600 dark:text-red-400 hover:text-red-800 dark:hover:text-red-300',
onClick: () => { }
})
return entries;
}

const statusFilterEntries = () => {
const entries: DropDownEntry[] = [];
entries.push({
title: 'All',
onClick: () => setStatusFilter(undefined)
});
entries.push({
title: 'READY',
onClick: () => setStatusFilter("available")
});
return entries;
}

const lastPrebuild = (branch: Project.BranchDetails) => lastPrebuilds.get(branch.name);

const filter = (branch: Project.BranchDetails) => {
const prebuild = lastPrebuild(branch);
if (statusFilter && prebuild && statusFilter !== prebuild.status) {
return false;
}
if (searchFilter && `${branch.changeTitle} ${branch.name}`.toLowerCase().includes(searchFilter.toLowerCase()) === false) {
return false;
}
Expand All @@ -130,8 +105,12 @@ export default function () {
history.push(`/${team?.slug}/${projectName}/${pb.id}`);
}

const formatDate = (date: string | undefined) => {
return date ? moment(date).fromNow() : "";
}

return <>
<Header title={project?.name || ""} subtitle={toRemoteURL(project?.cloneUrl || "")} />
<Header title={"Branches"} subtitle={<h2>View recent active branches for <a className="text-gray-400 hover:text-gray-600" href={project?.cloneUrl}>{project?.name}</a>.</h2>} />
<div className="lg:px-28 px-10">
<div className="flex mt-8">
<div className="flex">
Expand All @@ -142,7 +121,6 @@ export default function () {
</div>
<div className="flex-1" />
<div className="py-3 pl-3">
<DropDown prefix="Prebuild Status: " contextMenuWidth="w-32" entries={statusFilterEntries()} />
</div>
</div>
<ItemsList className="mt-2">
Expand All @@ -159,40 +137,39 @@ export default function () {
</ItemField>
</Item>
{branches.map((branch, index) => {
if (!filter(branch)) {
return undefined;
}
Comment on lines +140 to +142
Copy link
Contributor

@jankeromnes jankeromnes Jul 29, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Nit: Wouldn't it be cleaner to filter out branches before the .map? E.g. like so:

{branches.filter(filter).map((branch, index) => {
    // ...

Copy link
Member Author

Choose a reason for hiding this comment

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

sounds good! will apply in the following PR.


const branchName = branch.name;
const prebuild = lastPrebuild(branch);

const avatar = branch.changeAuthorAvatar && <img className="rounded-full w-4 h-4 inline-block align-text-bottom mr-2" src={branch.changeAuthorAvatar || ''} alt={branch.changeAuthor} />;
const fakeShortHash = branch.changeHash.substring(0, 10);
const statusIcon = prebuild?.status && prebuildStatusIcon(prebuild.status);
const status = prebuild?.status && prebuildStatusLabel(prebuild.status);
console.log(`status for ${branchName} is ${prebuild?.status} (${lastPrebuilds.size})`)
if (!filter(branch)) {
// return undefined;
}
return <Item key={`branch-${index}-${branchName}`} className="grid grid-cols-3 group">
<ItemField className="flex items-center">
<div>
<div className="text-base text-gray-900 dark:text-gray-50 font-medium mb-1">
{branchName}
{branch.isDefault && (<span className="ml-2 self-center rounded-xl py-0.5 px-2 text-sm bg-blue-50 text-blue-400">DEFAULT</span>)}
jankeromnes marked this conversation as resolved.
Show resolved Hide resolved
AlexTugarev marked this conversation as resolved.
Show resolved Hide resolved
</div>
<p>Updated _ minutes ago</p>
</div>
</ItemField>
<ItemField className="flex items-center">
<div>
<div className="text-base text-gray-500 dark:text-gray-50 font-medium mb-1">{branch.changeTitle}</div>
<p>{avatar}Authored {moment(branch.changeDate).fromNow()} · {fakeShortHash}</p>
<div className="text-base text-gray-500 dark:text-gray-50 font-medium mb-1">{shortCommitMessage(branch.changeTitle)}</div>
<p>{avatar}Authored {formatDate(branch.changeDate)} · {branch.changeHash?.substring(0, 8)}</p>
AlexTugarev marked this conversation as resolved.
Show resolved Hide resolved
</div>
</ItemField>
<ItemField className="flex items-center">
<div className="text-base text-gray-900 dark:text-gray-50 font-medium uppercase mb-1 cursor-pointer" onClick={() => prebuild && openPrebuild(prebuild)}>
{prebuild ? (<><div className="inline-block align-text-bottom mr-2 w-4 h-4">{statusIcon}</div>{status}</>) : (<span>–</span>)}
{prebuild ? (<><div className="inline-block align-text-bottom mr-2 w-4 h-4">{statusIcon}</div>{status}</>) : (<span> </span>)}
</div>
<span className="flex-grow" />
<a href={gitpodHostUrl.withContext(`${branch.url}`).toString()}>
<button className={`primary mr-2 py-2 ${branch.isDefault ? "" : "opacity-0"} group-hover:opacity-100`}>New Workspace</button>
<button className={`primary mr-2 py-2 opacity-0 group-hover:opacity-100`}>New Workspace</button>
</a>
<ItemFieldContextMenu className="py-0.5" menuEntries={branchContextMenu(branch)} />
</ItemField>
Expand Down
34 changes: 0 additions & 34 deletions components/dashboard/src/projects/Settings.tsx
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion components/dashboard/src/projects/render-utils.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,6 @@ export function toRemoteURL(cloneURL: string) {
}

export function shortCommitMessage(message: string) {
const firstLine = message.split("/n")[0];
const firstLine = message.split("\n")[0];
AlexTugarev marked this conversation as resolved.
Show resolved Hide resolved
return firstLine.length > 50 ? firstLine.substring(0, 45) + " …" : firstLine;
}
1 change: 1 addition & 0 deletions components/gitpod-protocol/src/protocol.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1011,6 +1011,7 @@ export interface Repository {
export interface Branch {
name: string;
commit: CommitInfo;
htmlUrl: string;
AlexTugarev marked this conversation as resolved.
Show resolved Hide resolved
}

export interface CommitInfo {
Expand Down
2 changes: 1 addition & 1 deletion components/server/ee/src/github/github-app-support.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export class GitHubAppSupport {
const listReposForInstallation = async (installation: RestEndpointMethodTypes["apps"]["getUserInstallation"]["response"]) => {
const sub = await probot.auth(installation.data.id);
try {
const accessibleRepos = await sub.apps.listReposAccessibleToInstallation();
const accessibleRepos = await sub.apps.listReposAccessibleToInstallation({ per_page: 100 });
return accessibleRepos.data.repositories.map(r => {
return {
name: r.name,
Expand Down
10 changes: 7 additions & 3 deletions components/server/ee/src/workspace/gitpod-server-impl.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1461,7 +1461,7 @@ export class GitpodServerEEImpl extends GitpodServerImpl<GitpodClient, GitpodSer
return repositories;
}

async triggerPrebuild(projectId: string, branch: string): Promise<void> {
async triggerPrebuild(projectId: string, branchName: string): Promise<void> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I understand that public is the default, but maybe not every developer that ever looks at this code will know that. Also, I think it's slightly dangerous to be inconsistent in this file (e.g. if you see both public async and async, you might accidentally think the second is private by default).

Please either respect the file-local convention (i.e. explicit public keyword), or refactor the entire file in a separate commit (i.e. remove all the public prefixes).

const user = this.checkAndBlockUser("triggerPrebuild");

const project = await this.projectsService.getProject(projectId);
Expand All @@ -1473,7 +1473,11 @@ export class GitpodServerEEImpl extends GitpodServerImpl<GitpodClient, GitpodSer
const span = opentracing.globalTracer().startSpan("triggerPrebuild");
span.setTag("userId", user.id);

const contextURL = `${project.cloneUrl.replace(".git", "")}/tree/${branch}`; // just a quick hack!
const branchDetails = await this.projectsService.getBranchDetails(user, project, branchName);
if (branchDetails.length !== 1) {
log.debug({ userId: user.id }, 'Cannot find branch details.', { project, branchName });
}
const contextURL = branchDetails[0].url;

const context = await this.contextParser.handle({ span }, user, contextURL) as CommitContext;

Expand All @@ -1482,7 +1486,7 @@ export class GitpodServerEEImpl extends GitpodServerImpl<GitpodClient, GitpodSer
cloneURL: project.cloneUrl,
commit: context.revision,
user,
branch,
branch: branchName,
project
});
}
Expand Down
5 changes: 5 additions & 0 deletions components/server/src/bitbucket/bitbucket-repository-provider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ export class BitbucketRepositoryProvider implements RepositoryProvider {
return { host, owner, name, cloneUrl, description, avatarUrl, webUrl };
}

async getBranch(user: User, owner: string, repo: string, branch: string): Promise<Branch> {
// todo
throw new Error("not implemented");
Copy link
Contributor

Choose a reason for hiding this comment

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

Q: When will we implement the Bitbucket and GitLab methods?

Copy link
Member Author

Choose a reason for hiding this comment

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

Planing to implement GitLab support tomorrow.

No plans for BitBucket so far. TODO: remove from selection list for the time being.

}

async getBranches(user: User, owner: string, repo: string): Promise<Branch[]> {
// todo
return [];
Expand Down
22 changes: 21 additions & 1 deletion components/server/src/github/api.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,20 @@ export class GitHubRestApi {
return response.data;
}

public async getBranch(user: User, params: RestEndpointMethodTypes["repos"]["getBranch"]["parameters"]): Promise<Branch> {
const key = `getBranch:${params.owner}/${params.owner}/${params.branch}:${user.id}`;
const getBranchResponse = (await this.runWithCache(key, user, (api) => api.repos.getBranch(params))) as RestEndpointMethodTypes["repos"]["getBranch"]["response"];
const { commit: { sha }, name, _links: { html } } = getBranchResponse.data;

const commit = await this.getCommit(user, { ...params, ref: sha });

return {
name,
commit,
htmlUrl: html
};
}

public async getBranches(user: User, params: RestEndpointMethodTypes["repos"]["listBranches"]["parameters"]): Promise<Branch[]> {
const key = `getBranches:${params.owner}/${params.owner}:${user.id}`;
const listBranchesResponse = (await this.runWithCache(key, user, (api) => api.repos.listBranches(params))) as RestEndpointMethodTypes["repos"]["listBranches"]["response"];
Expand All @@ -255,9 +269,15 @@ export class GitHubRestApi {
for (const branch of listBranchesResponse.data) {
const { commit: { sha } } = branch;
const commit = await this.getCommit(user, { ...params, ref: sha });

const key = `getBranch:${params.owner}/${params.owner}/${params.branch}:${user.id}`;
const getBranchResponse = (await this.runWithCache(key, user, (api) => api.repos.listBranches(params))) as RestEndpointMethodTypes["repos"]["getBranch"]["response"];
const htmlUrl = getBranchResponse.data._links.html;

result.push({
name: branch.name,
commit
commit,
htmlUrl
});
}

Expand Down
Loading