diff --git a/background.ts b/background.ts new file mode 100644 index 00000000..f29ada3f --- /dev/null +++ b/background.ts @@ -0,0 +1,83 @@ +import { ChromeApi } from "./src/chrome/api"; +import { chromeApiSingleton } from "./src/chrome/implementation"; +import { buildEnvironment } from "./src/environment/implementation"; +import { CrossScriptMessenger } from "./src/messaging/api"; +import { Core } from "./src/state/core"; + +// This is the entry point of the service worker of the Chrome extension. +console.debug("Service worker running..."); + +const chromeApi = chromeApiSingleton; +const env = buildEnvironment(chromeApiSingleton); +const core = new Core(env); + +selfUpdateAsap(chromeApi); +refreshOnUpdate(chromeApi); +refreshRegulary(chromeApi); +refreshOnDemand(env.messenger); + +async function triggerRefresh() { + const result = await chromeApi.storage.session.get(["refreshing"]); + const refreshing = result?.refreshing ?? false; + if (refreshing) { + return; + } + try { + chromeApi.storage.session.set({ refreshing: true }); + await core.load(); + if (!core.token) { + return; + } + await core.refreshPullRequests(); + } finally { + chromeApi.storage.session.set({ refreshing: false }); + } +} + +/** + * Automatically reloads the extension as soon as an update is available. + */ +function selfUpdateAsap(chromeApi: ChromeApi) { + chromeApi.runtime.onUpdateAvailable.addListener(() => { + console.debug("Update available"); + chromeApi.runtime.reload(); + }); +} + +/** + * Refreshes pull requests when the extension is installed or updated. + */ +function refreshOnUpdate(chromeApi: ChromeApi) { + chromeApi.runtime.onInstalled.addListener(() => { + console.debug("Extension installed"); + triggerRefresh().catch(console.error); + }); +} + +/** + * Refreshes pull requests at regular intervals. + */ +function refreshRegulary(chromeApi: ChromeApi) { + chromeApi.alarms.create({ + periodInMinutes: 10, + }); + chromeApi.alarms.onAlarm.addListener((alarm) => { + console.debug("Alarm triggered", alarm); + triggerRefresh().catch(console.error); + }); +} + +/** + * Refreshes pull requests when requested by the user (e.g. after entering a new token). + */ +function refreshOnDemand(messenger: CrossScriptMessenger) { + messenger.listen((message) => { + console.debug("Message received", message); + if (message.kind === "refresh") { + triggerRefresh().catch(console.error); + } + if (message.kind === "restart") { + chromeApi.runtime.reload(); + } + }); +} diff --git a/manifest.json b/manifest.json index 97e8e63f..f7341efc 100644 --- a/manifest.json +++ b/manifest.json @@ -1,15 +1,14 @@ { - "manifest_version": 2, - "name": "PR Monitor", - "version": "0.0.0", - "description": "Get notified when you receive a pull request on GitHub.", - "permissions": ["alarms", "notifications", "storage"], - "optional_permissions": ["tabs"], + "manifest_version": 3, + "name": "Github PR Tracker", + "version": "1.0.2", + "description": "Better track Github PRs", + "permissions": ["alarms", "storage"], "background": { - "scripts": ["background.js"], - "persistent": true + "service_worker": "./background.js", + "type": "module" }, - "browser_action": { + "action": { "default_popup": "index.html#popup" }, "icons": { diff --git a/package.json b/package.json index 33d0b74b..572c5f99 100644 --- a/package.json +++ b/package.json @@ -3,17 +3,13 @@ "dependencies": { "@emotion/react": "^11.10.0", "@emotion/styled": "^11.10.0", - "@fortawesome/fontawesome-svg-core": "^6.1.2", - "@fortawesome/free-solid-svg-icons": "^6.1.2", - "@fortawesome/react-fontawesome": "^0.2.0", "@octokit/plugin-throttling": "^4.3.2", + "@octokit/request": "^8.1.5", "@octokit/rest": "^19.0.4", - "@types/lodash": "^4.14.184", - "assert-never": "^1.2.1", + "@primer/octicons-react": "^19.8.0", "bootstrap": "^5.2.0", "graphql": "^16.6.0", "graphql-request": "^4.3.0", - "lodash": "^4.17.21", "mobx": "^6.6.1", "mobx-react-lite": "^3.4.0", "moment": "^2.29.4", diff --git a/screencasts/latest.gif b/screencasts/latest.gif deleted file mode 100644 index 1ee22e03..00000000 Binary files a/screencasts/latest.gif and /dev/null differ diff --git a/screencasts/v0.5.1.gif b/screencasts/v0.5.1.gif deleted file mode 100644 index 1ee22e03..00000000 Binary files a/screencasts/v0.5.1.gif and /dev/null differ diff --git a/src/background.ts b/src/background.ts deleted file mode 100644 index 6f156c18..00000000 --- a/src/background.ts +++ /dev/null @@ -1,92 +0,0 @@ -import { ChromeApi } from "./chrome/api"; -import { chromeApiSingleton } from "./chrome/implementation"; -import { Context } from "./environment/api"; -import { buildEnvironment } from "./environment/implementation"; -import { CrossScriptMessenger } from "./messaging/api"; -import { Core } from "./state/core"; - -// This is the entry point of the background script of the Chrome extension. -console.debug("Background entry point running..."); -const chromeApi = chromeApiSingleton; -const env = buildEnvironment(chromeApiSingleton); -setUpBackgroundScript(chromeApi, env); - -let refreshing = false; - -function setUpBackgroundScript(chromeApi: ChromeApi, context: Context) { - selfUpdateAsap(chromeApi); - refreshOnUpdate(chromeApi, triggerRefresh); - refreshRegulary(chromeApi, triggerRefresh); - refreshOnDemand(context.messenger, triggerRefresh); - const core = new Core(context); - - async function triggerRefresh() { - if (refreshing) { - return; - } - try { - refreshing = true; - await core.load(); - if (!core.token) { - return; - } - await core.refreshPullRequests(); - } finally { - refreshing = false; - } - } -} - -/** - * Automatically reloads the extension as soon as an update is available. - */ -function selfUpdateAsap(chromeApi: ChromeApi) { - chromeApi.runtime.onUpdateAvailable.addListener(() => { - console.debug("Update available"); - chrome.runtime.reload(); - }); -} - -/** - * Refreshes pull requests when the extension is installed or updated. - */ -function refreshOnUpdate( - chromeApi: ChromeApi, - triggerRefresh: () => Promise -) { - chromeApi.runtime.onInstalled.addListener(() => { - console.debug("Extension installed"); - triggerRefresh().catch(console.error); - }); -} - -/** - * Refreshes pull requests at regular intervals. - */ -function refreshRegulary( - chromeApi: ChromeApi, - triggerRefresh: () => Promise -) { - chromeApi.alarms.create({ - periodInMinutes: 3, - }); - chromeApi.alarms.onAlarm.addListener((alarm) => { - console.debug("Alarm triggered", alarm); - triggerRefresh().catch(console.error); - }); -} - -/** - * Refreshes pull requests when requested by the user (e.g. after entering a new token). - */ -function refreshOnDemand( - messenger: CrossScriptMessenger, - triggerRefresh: () => Promise -) { - messenger.listen((message) => { - console.debug("Message received", message); - if (message.kind === "refresh") { - triggerRefresh().catch(console.error); - } - }); -} diff --git a/src/badge/implementation.ts b/src/badge/implementation.ts index 730fc3b3..2ade429b 100644 --- a/src/badge/implementation.ts +++ b/src/badge/implementation.ts @@ -10,10 +10,10 @@ export function buildBadger(chromeApi: ChromeApi): Badger { } function updateBadge(chromeApi: ChromeApi, state: BadgeState) { - chromeApi.browserAction.setBadgeText({ + chromeApi.action.setBadgeText({ text: badgeLabel(state), }); - chromeApi.browserAction.setBadgeBackgroundColor({ + chromeApi.action.setBadgeBackgroundColor({ color: badgeColor(state), }); } diff --git a/src/chrome/fake-chrome.ts b/src/chrome/fake-chrome.ts index 35c8c03e..439da829 100644 --- a/src/chrome/fake-chrome.ts +++ b/src/chrome/fake-chrome.ts @@ -6,14 +6,14 @@ import { ChromeApi, ChromeStorageItems } from "./api"; * outside of a Chrome extension. */ const partialFakeChrome: RecursivePartial = { - browserAction: { - setBadgeText(details: chrome.browserAction.BadgeTextDetails) { - console.log("chrome.browserAction.setBadgeText", details); + action: { + setBadgeText(details: chrome.action.BadgeTextDetails) { + console.log("chrome.action.setBadgeText", details); }, setBadgeBackgroundColor( - details: chrome.browserAction.BadgeBackgroundColorDetails + details: chrome.action.BadgeBackgroundColorDetails ) { - console.log("chrome.browserAction.setBadgeBackgroundColor", details); + console.log("chrome.action.setBadgeBackgroundColor", details); }, }, runtime: { diff --git a/src/components/IgnoredRepositories.tsx b/src/components/IgnoredRepositories.tsx deleted file mode 100644 index 887203ad..00000000 --- a/src/components/IgnoredRepositories.tsx +++ /dev/null @@ -1,65 +0,0 @@ -import styled from "@emotion/styled"; -import flatten from "lodash/flatten"; -import { observer } from "mobx-react-lite"; -import React from "react"; -import { Card } from "react-bootstrap"; -import { Core } from "../state/core"; -import { MediumButton } from "./design/Button"; -import { Header } from "./design/Header"; -import { Link } from "./design/Link"; - -const Container = styled.div` - margin-bottom: 16px; -`; - -const Item = styled.div` - padding: 8px; -`; - -const Remove = styled(MediumButton)` - margin-left: 0; - margin-right: 8px; -`; - -export const IgnoredRepositories = observer((props: { core: Core }) => { - const ignored = props.core.muteConfiguration.ignored || {}; - if (Object.keys(ignored).length === 0) { - return <>; - } - return ( - -
Ignored repositories
- - {...flatten( - Object.entries(ignored).map(([owner, config]) => - config.kind === "ignore-all" ? ( - - props.core.unmuteOwner(owner)}>+ - {`${owner}/*`} - - ) : ( - config.repoNames.map((repo) => ( - - - props.core.unmuteRepository({ owner, name: repo }) - } - > - + - - {`${owner}/${repo}`} - - )) - ) - ) - )} - - - ); -}); diff --git a/src/components/NewCommitsToggle.tsx b/src/components/NewCommitsToggle.tsx deleted file mode 100644 index 45757a78..00000000 --- a/src/components/NewCommitsToggle.tsx +++ /dev/null @@ -1,33 +0,0 @@ -import styled from "@emotion/styled"; -import { observer } from "mobx-react-lite"; -import React from "react"; - -export interface NewCommitsToggleProps { - toggled: boolean; - onToggle(): void; -} - -export const NewCommitsToggle = observer((props: NewCommitsToggleProps) => { - return ( - - - Include new commits - - ); -}); - -const Container = styled.label` - padding: 8px; - margin: 0; - display: flex; - flex-direction: row; - align-items: center; -`; - -const NewCommitsCheckbox = styled.input` - margin-right: 8px; -`; diff --git a/src/components/Popup.tsx b/src/components/Popup.tsx index 7bc2a9af..88278d7e 100644 --- a/src/components/Popup.tsx +++ b/src/components/Popup.tsx @@ -1,22 +1,23 @@ -import styled from "@emotion/styled"; -import { FontAwesomeIcon } from "@fortawesome/react-fontawesome"; import { observer } from "mobx-react-lite"; import React, { useState } from "react"; import { Badge, Tab, Tabs } from "react-bootstrap"; -import { Filter } from "../filtering/filters"; -import { isRunningAsPopup } from "../popup-environment"; +import { Filter, FilteredPullRequests } from "../filtering/filters"; import { Core } from "../state/core"; -import { PullRequest, ref } from "../storage/loaded-state"; -import { MuteType } from "../storage/mute-configuration"; -import { Link } from "./design/Link"; -import { Row } from "./design/Row"; -import { IgnoredRepositories } from "./IgnoredRepositories"; import { Loader } from "./Loader"; -import { NewCommitsToggle } from "./NewCommitsToggle"; import { PullRequestList } from "./PullRequestList"; -import { Settings } from "./Settings"; import { Status } from "./Status"; -import { WhitelistedTeams } from "./WhitelistedTeams"; +import { isRunningAsPopup } from "../popup-environment"; +import { Link } from "./design/Link"; +import styled from "@emotion/styled"; +import { CopyIcon } from "@primer/octicons-react"; +import { Settings } from "./Settings"; + +const FullScreenLink = styled(Link)` + opacity: 0.7; + &:hover { + opacity: 1; + } +`; export interface PopupProps { core: Core; @@ -27,199 +28,185 @@ export interface PopupState { } export const Popup = observer((props: PopupProps) => { + const { core } = props; + const { filteredPullRequests: prs } = core ?? {}; + const [state, setState] = useState({ - currentFilter: Filter.INCOMING, + currentFilter: Filter.ALL, }); - const onOpenAll = () => { - const pullRequests = props.core.filteredPullRequests - ? props.core.filteredPullRequests[state.currentFilter] - : []; - for (const pullRequest of pullRequests) { - onOpen(pullRequest.htmlUrl); - } - }; - const onOpen = (pullRequestUrl: string) => { - props.core.openPullRequest(pullRequestUrl).catch(console.error); - }; - - const onMute = (pullRequest: PullRequest, muteType: MuteType) => { - props.core.mutePullRequest(ref(pullRequest), muteType); - }; - - const onUnmute = (pullRequest: PullRequest) => { - props.core.unmutePullRequest(ref(pullRequest)); + core.openPullRequest(pullRequestUrl).catch(console.error); }; - const onToggleNewCommitsNotification = () => { - props.core.toggleNewCommitsNotificationSetting(); - }; - - const onToggleOnlyDirectRequests = () => { - props.core.toggleOnlyDirectRequestsSetting(); - }; - - const onChangeWhitelistedTeams = (teamsText: string) => { - const teams = teamsText - .split(",") - .map((s) => s.trim()) - .filter((s) => s.length); - props.core.onChangeWhitelistedTeamsSetting(teams); - }; - - if (props.core.overallStatus !== "loaded") { + if (core.overallStatus !== "loaded") { return ; } return ( <> - - - {isRunningAsPopup() && ( - - - - )} - - {props.core.token && + {core.token && // Don't show the list if there was an error, we're not refreshing // anymore (because of the error) and we don't have any loaded state. - !( - props.core.lastError && - !props.core.refreshing && - !props.core.loadedState - ) && ( - <> - setState({ currentFilter: key as Filter })} + !(core.lastError && !core.refreshing && !core.loadedState) && ( +
+
- - Incoming PRs{" "} - {props.core.filteredPullRequests && ( - 0 - ? "danger" - : "secondary" - } - > - {props.core.filteredPullRequests.incoming.length} - - )} - - } - eventKey={Filter.INCOMING} - /> - - Muted{" "} - {props.core.filteredPullRequests && ( - - {props.core.filteredPullRequests.muted.length} - - )} - - } - eventKey={Filter.MUTED} - /> - - Already reviewed{" "} - {props.core.filteredPullRequests && ( - - {props.core.filteredPullRequests.reviewed.length} - - )} - - } - eventKey={Filter.REVIEWED} - /> - - My PRs{" "} - {props.core.filteredPullRequests && ( - - {props.core.filteredPullRequests.mine.length} - - )} - - } - eventKey={Filter.MINE} - /> - - - - - - ) - } - pullRequests={ - props.core.filteredPullRequests - ? props.core.filteredPullRequests[state.currentFilter] - : null - } - emptyMessage={ - state.currentFilter === Filter.INCOMING - ? `Nothing to review, yay!` - : `There's nothing to see here.` - } - mutingConfiguration={ - state.currentFilter === Filter.INCOMING - ? "allow-muting" - : state.currentFilter === Filter.MUTED - ? "allow-unmuting" - : "none" - } - onOpenAll={onOpenAll} + setState({ currentFilter: key as Filter })} + > + + All{" "} + {prs?.needsReview && ( + + {prs.needsReview.length} + + )} + + } + eventKey={Filter.ALL} + /> + + Needs Review{" "} + {prs?.needsReview && ( + + {prs.needsReview.length} + + )} + + } + eventKey={Filter.NEEDS_REVIEW} + /> + + Needs Revision{" "} + {prs?.needsRevision && ( + + {prs.needsRevision.length} + + )} + + } + eventKey={Filter.NEEDS_REVISION} + /> + + My PRs{" "} + {prs?.mine && ( + {prs.mine.length} + )} + + } + eventKey={Filter.MINE} + /> + + {isRunningAsPopup() && ( + + + + )} +
+ - +
)} - - +
+ +
); }); -const FullScreenLink = styled(Link)` - padding: 16px; - opacity: 0.7; +function headerForFilter(filter: Filter): string { + switch (filter) { + case Filter.NEEDS_REVIEW: + return "Needs Review"; + case Filter.NEEDS_REVISION: + return "Needs Revision"; + case Filter.MINE: + return "My PRs"; + case Filter.RECENTLY_MERGED: + return "My Recently Merged PRs"; + default: + return "Invalid Filter"; + } +} - &:hover { - opacity: 1; +function PullRequests({ + core, + filter, + prs, + onOpen, +}: { + core: Core; + filter: Filter; + prs: FilteredPullRequests | null; + onOpen: (pullRequestUrl: string) => void; +}): JSX.Element { + if (filter === Filter.ALL) { + const filters: Array = [ + Filter.NEEDS_REVIEW, + Filter.NEEDS_REVISION, + Filter.MINE, + Filter.RECENTLY_MERGED, + ]; + return ( +
+
+ +
+ {filters.map((filter: Filter, index: number) => { + return ( +
+
{headerForFilter(filter)}
+ +
+ ); + })} +
+ ); } -`; + + return ( + + ); +} diff --git a/src/components/PullRequestItem.tsx b/src/components/PullRequestItem.tsx index 7f595964..43ec4b85 100644 --- a/src/components/PullRequestItem.tsx +++ b/src/components/PullRequestItem.tsx @@ -1,56 +1,39 @@ import styled from "@emotion/styled"; -import { FontAwesomeIcon } from "@fortawesome/react-fontawesome"; import { observer } from "mobx-react-lite"; import React from "react"; -import { Dropdown } from "react-bootstrap"; import { EnrichedPullRequest } from "../filtering/enriched-pull-request"; -import { isRunningAsPopup } from "../popup-environment"; -import { PullRequest } from "../storage/loaded-state"; -import { MuteType } from "../storage/mute-configuration"; -import { SmallButton } from "./design/Button"; import { PullRequestStatus } from "./PullRequestStatus"; +import { CommentIcon } from "@primer/octicons-react"; +import moment from "moment"; const PullRequestBox = styled.a` display: flex; flex-direction: row; justify-content: space-between; text-decoration: none; - border-bottom: 1px solid #eee; + border-top: 1px solid #ddd; cursor: pointer; + padding: 12px; + + &:first-child { + border-top: none; + border-radius: 8px 8px 0 0; + } &:last-child { - border-bottom: none; + border-radius: 0 0 8px 8px; + } + + &:first-child:last-child { + border-radius: 8px; } &:hover { - background: #eef5ff; + background: #eef5ff !important; text-decoration: none; } `; -const Info = styled.div` - display: flex; - flex-direction: column; - justify-content: space-between; -`; - -const Title = styled.div` - color: #000; - padding: 8px; -`; - -const ContextSummary = styled.div` - white-space: nowrap; - overflow: hidden; - text-overflow: ellipsis; - font-size: 0.9em; - padding: 8px; -`; - -const ChangeSummary = styled.span` - margin-left: 8px; -`; - const LinesAdded = styled.span` color: #22863a; `; @@ -67,17 +50,7 @@ const Repo = styled.span` color: #555; `; -const AuthorWidth = "80px"; -const AuthorAvatarSize = "40px"; - -const AuthorBox = styled.div` - display: flex; - flex-direction: column; - align-items: center; - justify-content: center; - width: ${AuthorWidth}; - padding: 8px; -`; +const AuthorAvatarSize = "20px"; const AuthorAvatar = styled.img` width: ${AuthorAvatarSize}; @@ -86,159 +59,91 @@ const AuthorAvatar = styled.img` border-radius: 50%; `; -const AuthorLogin = styled.div` - white-space: nowrap; - text-overflow: ellipsis; - overflow: hidden; - font-size: 0.9em; - color: #555; - max-width: ${AuthorWidth}; -`; - -const InlineDropdown = styled(Dropdown)` - display: inline-block; - margin: 0 8px; - - .dropdown-menu { - font-size: 14px; - } - - .dropdown-item { - padding: 4px 16px 3px 36px; - } -`; - -const Icon = styled(FontAwesomeIcon)` - position: absolute; - margin-left: -24px; - margin-top: 2px; -`; - export interface PullRequestItemProps { pullRequest: EnrichedPullRequest; - mutingConfiguration: "allow-muting" | "allow-unmuting" | "none"; onOpen(pullRequestUrl: string): void; - onMute(pullRequest: PullRequest, muteType: MuteType): void; - onUnmute(pullRequest: PullRequest): void; } -export const PullRequestItem = observer((props: PullRequestItemProps) => { - const open = (e: React.MouseEvent) => { - props.onOpen(props.pullRequest.htmlUrl); - e.preventDefault(); - }; +export const PullRequestItem = observer( + ({ onOpen, pullRequest }: PullRequestItemProps) => { + const open = (e: React.MouseEvent) => { + onOpen(pullRequest.htmlUrl); + e.preventDefault(); + }; - const preventDefault = (e: React.MouseEvent) => { - e.preventDefault(); - e.stopPropagation(); - }; + return ( + +
+
+
+ +
+ {pullRequest.title} + {" (#"} + {pullRequest.pullRequestNumber} + {")"} +
+
+
+ {moment(pullRequest.updatedAt).fromNow()} +
+
+
+
+ + {pullRequest.repoOwner}/{pullRequest.repoName} + +
+ +{pullRequest.changeSummary.additions} + + -{pullRequest.changeSummary.deletions} + + + @{pullRequest.changeSummary.changedFiles} + +
+ {pullRequest.comments.length} +
+
+
+ +
+
+ + ); + } +); - const createMuteHandler = (muteType: MuteType) => { - return () => { - props.onMute(props.pullRequest, muteType); - }; - }; - - const unmute = (e: React.MouseEvent) => { - props.onUnmute(props.pullRequest); - e.preventDefault(); - e.stopPropagation(); - }; - - return ( - - - - {props.pullRequest.title} - {props.mutingConfiguration === "allow-muting" && ( - <InlineDropdown onClick={preventDefault} align="end"> - <Dropdown.Toggle - as={SmallButton} - id={`mute-dropdown-${props.pullRequest.nodeId}`} - > - <FontAwesomeIcon icon="bell-slash" /> - </Dropdown.Toggle> - <Dropdown.Menu> - <Dropdown.Item - onClick={createMuteHandler("next-comment-by-author")} - > - <Icon icon="reply" /> - Mute until next comment by author - </Dropdown.Item> - <Dropdown.Item onClick={createMuteHandler("next-update")}> - <Icon icon="podcast" /> - Mute until any update by author - </Dropdown.Item> - {props.pullRequest.draft && ( - <Dropdown.Item onClick={createMuteHandler("not-draft")}> - <Icon icon="pen" /> - Mute until not draft - </Dropdown.Item> - )} - <Dropdown.Item onClick={createMuteHandler("1-hour")}> - <Icon icon="clock" /> - Mute for 1 hour - </Dropdown.Item> - <Dropdown.Item onClick={createMuteHandler("forever")}> - <Icon icon="ban" /> - Mute forever - </Dropdown.Item> - <Dropdown.Item onClick={createMuteHandler("repo")}> - Ignore PRs in{" "} - <b>{`${props.pullRequest.repoOwner}/${props.pullRequest.repoName}`}</b> - </Dropdown.Item> - <Dropdown.Item onClick={createMuteHandler("owner")}> - Ignore all PRs in repositories owned by{" "} - <b>{props.pullRequest.repoOwner}</b> - </Dropdown.Item> - </Dropdown.Menu> - </InlineDropdown> - )} - {props.mutingConfiguration === "allow-unmuting" && ( - <SmallButton title="Unmute" onClick={unmute}> - <FontAwesomeIcon icon="bell" /> - </SmallButton> - )} - - - - - {props.pullRequest.repoOwner}/{props.pullRequest.repoName} (# - {props.pullRequest.pullRequestNumber}) - - {props.pullRequest.changeSummary && - (() => { - const adds = props.pullRequest.changeSummary.additions; - const dels = props.pullRequest.changeSummary.deletions; - const files = props.pullRequest.changeSummary.changedFiles; - return ( - - +{adds} - -{dels} - @{files} - - ); - })()} - - - {props.pullRequest.author && ( - - {props.pullRequest.author && ( - - )} - {props.pullRequest.author.login} - - )} - - ); -}); +function itemBgColor(pr: EnrichedPullRequest): string { + if (pr.isMerged) { + return "#CBC3E3"; + } + if (pr.draft) { + return "#fff"; + } + if (pr.state.approved) { + return "#d9fee5"; + } + if (!pr.state.changesRequested && moreThanOneDayAgo(pr.updatedAt)) { + return "#ffeae9"; + } + return "#fff"; +} + +function moreThanOneDayAgo(timestamp: string) { + return moment().diff(moment(timestamp), "days") >= 1; +} diff --git a/src/components/PullRequestList.tsx b/src/components/PullRequestList.tsx index aa9b9307..c73e99e8 100644 --- a/src/components/PullRequestList.tsx +++ b/src/components/PullRequestList.tsx @@ -2,34 +2,21 @@ import styled from "@emotion/styled"; import { observer } from "mobx-react-lite"; import React from "react"; import { EnrichedPullRequest } from "../filtering/enriched-pull-request"; -import { PullRequest } from "../storage/loaded-state"; -import { MuteType } from "../storage/mute-configuration"; -import { Link } from "./design/Link"; import { Paragraph } from "./design/Paragraph"; import { Loader } from "./Loader"; import { PullRequestItem } from "./PullRequestItem"; const List = styled.div` border: 1px solid #ddd; - border-radius: 0 8px 8px 8px; + border-radius: 8px; background: #fff; - margin-bottom: 16px; -`; - -const OpenAllParagraph = styled(Paragraph)` - text-align: center; - color: #777; `; export interface PullRequestListProps { pullRequests: EnrichedPullRequest[] | null; emptyMessage: string; - mutingConfiguration: "allow-muting" | "allow-unmuting" | "none"; header: React.ReactNode; - onOpenAll(): void; onOpen(pullRequestUrl: string): void; - onMute(pullRequest: PullRequest, muteType: MuteType): void; - onUnmute(pullRequest: PullRequest): void; } export const PullRequestList = observer((props: PullRequestListProps) => { @@ -46,17 +33,9 @@ export const PullRequestList = observer((props: PullRequestListProps) => { ))} - {props.pullRequests.length > 1 && ( - - Open them all - - )} )} diff --git a/src/components/PullRequestStatus.tsx b/src/components/PullRequestStatus.tsx index a8b1d8bd..0fa4c4b0 100644 --- a/src/components/PullRequestStatus.tsx +++ b/src/components/PullRequestStatus.tsx @@ -1,119 +1,99 @@ -import styled from "@emotion/styled"; import { observer } from "mobx-react-lite"; import React from "react"; import { Badge } from "react-bootstrap"; import { EnrichedPullRequest } from "../filtering/enriched-pull-request"; -import { - IncomingState, - OutgoingState, - PullRequestState, -} from "../filtering/status"; +import { PullRequestState } from "../filtering/status"; import { CheckStatus } from "../github-api/api"; -const StateBox = styled.div` - padding: 0 8px; -`; - -const SpacedBadge = styled(Badge)` - margin-right: 4px; -`; - const UNREVIEWED = ( - + Unreviewed - + ); const AUTHOR_REPLIED = ( - + Author replied - -); - -const NEW_COMMIT = ( - - New commit - + ); const DRAFT = ( - + Draft - + ); -const MERGEABLE = ( - - Mergeable - +const MERGED = ( + + Merged + ); -const APPROVED_BY_EVERONE = ( - - Approved by everyone - +const APPROVED = ( + + Approved + ); const CHECK_STATUS_PASSED = ( - - Checks Pass - + + Tests + ); const CHECK_STATUS_FAILED = ( - - Checks Fail - + + Tests + ); const CHECK_STATUS_PENDING = ( - - Checks Pending - + + Tests + ); const CHANGES_REQUESTED = ( - + Changes requested - + ); -const WAITING_FOR_REVIEW = ( - - Waiting for review - -); - -const NO_REVIEWER_ASSIGNED = ( - - No reviewer assigned - +const NEEDS_REVIEW = ( + + Needs review + ); export const PullRequestStatus = observer( ({ pullRequest }: { pullRequest: EnrichedPullRequest }) => { const badges = getBadges(pullRequest.state); if (badges.length > 0) { - return {badges}; + return ( +
+ {badges} +
+ ); } return <>; } ); function getBadges(state: PullRequestState): JSX.Element[] { - const badges: JSX.Element[] = []; - if (state.draft) { - badges.push(DRAFT); - } switch (state.kind) { case "incoming": - badges.push(...getIncomingStateBadges(state)); - break; + return getIncomingStateBadges(state); case "outgoing": - badges.push(...getOutgoingStateBadges(state)); - break; - default: - // Do nothing. + return getOutgoingStateBadges(state); } - return badges; + return []; } function getCheckStatusBadge(checkStatus?: CheckStatus): JSX.Element[] { @@ -131,41 +111,43 @@ function getCheckStatusBadge(checkStatus?: CheckStatus): JSX.Element[] { } } -function getIncomingStateBadges(state: IncomingState): JSX.Element[] { +function getIncomingStateBadges(state: PullRequestState): JSX.Element[] { const badges: JSX.Element[] = []; - badges.push(...getCheckStatusBadge(state.checkStatus)); - if (state.newReviewRequested) { + if (state.draft) { + badges.push(DRAFT); + } else if (state.newReviewRequested) { badges.push(UNREVIEWED); - return badges; + } else if (state.changesRequested) { + badges.push(CHANGES_REQUESTED); } if (state.authorResponded) { badges.push(AUTHOR_REPLIED); } - if (state.newCommit) { - badges.push(NEW_COMMIT); - } + + badges.push(...getCheckStatusBadge(state.checkStatus)); + return badges; } -function getOutgoingStateBadges(state: OutgoingState): JSX.Element[] { +function getOutgoingStateBadges(state: PullRequestState): JSX.Element[] { const badges: JSX.Element[] = []; - badges.push(...getCheckStatusBadge(state.checkStatus)); - if (state.mergeable) { - badges.push(MERGEABLE); - } - if (state.approvedByEveryone) { - badges.push(APPROVED_BY_EVERONE); + if (state.isMerged) { + badges.push(MERGED); + return badges; + } else if (state.draft) { + badges.push(DRAFT); + } else if (state.approved) { + badges.push(APPROVED); } else if (state.changesRequested) { badges.push(CHANGES_REQUESTED); } else { - badges.push(WAITING_FOR_REVIEW); - if (state.noReviewers) { - badges.push(NO_REVIEWER_ASSIGNED); - } + badges.push(NEEDS_REVIEW); } + badges.push(...getCheckStatusBadge(state.checkStatus)); + return badges; } diff --git a/src/components/Settings.tsx b/src/components/Settings.tsx index 305f97f1..3efe262d 100644 --- a/src/components/Settings.tsx +++ b/src/components/Settings.tsx @@ -4,7 +4,6 @@ import React, { FormEvent, useRef, useState } from "react"; import { Core } from "../state/core"; import { LargeButton } from "./design/Button"; import { Center } from "./design/Center"; -import { Header } from "./design/Header"; import { Link } from "./design/Link"; import { Paragraph } from "./design/Paragraph"; import { Row } from "./design/Row"; @@ -71,7 +70,6 @@ export const Settings = observer((props: SettingsProps) => { return ( <> -
Settings
{!editing ? ( props.core.loadedState ? ( @@ -80,7 +78,6 @@ export const Settings = observer((props: SettingsProps) => { {props.core.loadedState.userLogin || "unknown"} - . Update token diff --git a/src/components/Status.tsx b/src/components/Status.tsx index 5ab7c174..4803ae13 100644 --- a/src/components/Status.tsx +++ b/src/components/Status.tsx @@ -2,7 +2,6 @@ import styled from "@emotion/styled"; import { observer } from "mobx-react-lite"; import moment from "moment"; import React from "react"; -import { Alert } from "react-bootstrap"; import { SmallButton } from "./design/Button"; import { Core } from "../state/core"; @@ -11,44 +10,57 @@ export interface StatusProps { } const StatusContainer = styled.div` + background: #fff; flex-grow: 1; + border: 1px solid #ddd; + border-radius: 0 8px 8px 8px; + padding: 12px; `; export const Status = observer((props: StatusProps) => { - let lastUpdated; - if (props.core.loadedState && props.core.loadedState.startRefreshTimestamp) { - lastUpdated = ( -
- Last updated{" "} - {moment(props.core.loadedState.startRefreshTimestamp).fromNow()} - {". "} - {props.core.refreshing ? ( - "Refreshing..." - ) : ( - { - props.core.triggerBackgroundRefresh(); - }} - > - Refresh - - )} -
- ); - } + const { core } = props; + const { loadedState } = core ?? {}; + return ( - - {props.core.lastError ? ( - -
Error: {props.core.lastError}
- {lastUpdated} - + + {core.lastError ? ( +
+
Error: {core.lastError}
+ +
) : ( - - {lastUpdated || - (props.core.refreshing ? "Loading..." : "Welcome to PR Monitor!")} - +
+ { + core.refreshing ? +
+ Refreshing... +
: + + } + { + core.refreshing ? +
)} ); }); + +function Button({onClick, title}: {onClick: () => void, title: string}): JSX.Element { + return ( + + {title} + + ) +} + +function LastUpdated({timestamp}: {timestamp: moment.MomentInput}): JSX.Element { + return ( +
+ Last updated{' '}{moment(timestamp).fromNow()} +
+ ) +} diff --git a/src/components/WhitelistedTeams.tsx b/src/components/WhitelistedTeams.tsx deleted file mode 100644 index cc55a092..00000000 --- a/src/components/WhitelistedTeams.tsx +++ /dev/null @@ -1,89 +0,0 @@ -import styled from "@emotion/styled"; -import { observer } from "mobx-react-lite"; -import React, { useRef, useState } from "react"; -import { LargeButton } from "./design/Button"; - -export interface WhitelistedTeamsProps { - onlyDirectRequestsToggled: boolean; - whitelistedTeams: string[]; - userLogin?: string; - onToggleOnlyDirectRequests(): void; - onChangeWhitelistedTeams(text: string): void; -} - -export const WhitelistedTeams = observer((props: WhitelistedTeamsProps) => { - const defaultWhitelistedTeams = props.whitelistedTeams.join(", "); - const [whitelistedTeams, setWhitelistedTeams] = useState( - defaultWhitelistedTeams - ); - const inputRef = useRef(null); - const handleWhitelistedTeamsChange = (e: React.FormEvent) => { - e.preventDefault(); - if (!inputRef.current) { - return; - } - - setWhitelistedTeams(inputRef.current.value); - }; - const handleApplyWhitelistedTeamsChange = (e: React.FormEvent) => { - e.preventDefault(); - props.onChangeWhitelistedTeams(whitelistedTeams); - }; - return ( - - -
- Only directly requested - {props.userLogin && ( - - {" "} - (@{props.userLogin}){" "} - - )}{" "} - and whitelisted teams - {props.onlyDirectRequestsToggled && ( -
- - - Apply - -
- )} -
- - ); -}); - -const OnlyDirectRequestsCheckbox = styled.input` - margin-right: 8px; -`; - -const OnlyDirectRequestsToggle = styled.label` - padding: 8px; - margin: 0; - display: flex; - flex-direction: row; - align-items: center; -`; - -const WhitelistedTeamsInput = styled.input` - flex-grow: 1; - padding: 4px 8px; - margin-right: 8px; - - &:focus { - outline-color: #2ee59d; - } -`; diff --git a/src/environment/api.ts b/src/environment/api.ts index 570620a9..2e6253b0 100644 --- a/src/environment/api.ts +++ b/src/environment/api.ts @@ -1,14 +1,12 @@ import { Badger } from "../badge/api"; import { GitHubLoader } from "../loading/api"; import { CrossScriptMessenger } from "../messaging/api"; -import { Notifier } from "../notifications/api"; import { Store } from "../storage/api"; import { TabOpener } from "../tabs/api"; export interface Context { store: Store; githubLoader: GitHubLoader; - notifier: Notifier; badger: Badger; messenger: CrossScriptMessenger; tabOpener: TabOpener; diff --git a/src/environment/implementation.ts b/src/environment/implementation.ts index d8614f75..09e85469 100644 --- a/src/environment/implementation.ts +++ b/src/environment/implementation.ts @@ -2,21 +2,18 @@ import { buildBadger } from "../badge/implementation"; import { ChromeApi } from "../chrome/api"; import { buildGitHubLoader } from "../loading/implementation"; import { buildMessenger } from "../messaging/implementation"; -import { buildNotifier } from "../notifications/implementation"; import { buildStore } from "../storage/implementation"; import { buildTabOpener } from "../tabs/implementation"; import { Context } from "./api"; export function buildEnvironment(chromeApi: ChromeApi): Context { - const store = buildStore(chromeApi); const getCurrentTime = () => Date.now(); return { - store, + store: buildStore(chromeApi), githubLoader: buildGitHubLoader(), - notifier: buildNotifier(chromeApi), badger: buildBadger(chromeApi), messenger: buildMessenger(chromeApi), - tabOpener: buildTabOpener(chromeApi, store, getCurrentTime), + tabOpener: buildTabOpener(chromeApi), getCurrentTime, isOnline: () => navigator.onLine, }; diff --git a/src/filtering/filters.ts b/src/filtering/filters.ts index cbf1292b..712bee58 100644 --- a/src/filtering/filters.ts +++ b/src/filtering/filters.ts @@ -1,37 +1,13 @@ -import { Context } from "../environment/api"; import { PullRequest } from "../storage/loaded-state"; -import { MuteConfiguration } from "../storage/mute-configuration"; import { EnrichedPullRequest } from "./enriched-pull-request"; -import { isMuted, MutedResult } from "./muted"; -import { isReviewRequired, pullRequestState } from "./status"; +import { PullRequestState, pullRequestState } from "./status"; export enum Filter { - /** - * Filter that only shows pull requests that need a review (either because - * they are new, or they have been updated since the last review), ignoring - * any muted PRs unless they no longer match the muting criteria. - */ - INCOMING = "incoming", - - /** - * Filter that shows pull requests that need a review but have been muted. - */ - MUTED = "muted", - - /** - * Filter that shows open pull requests that have already been reviewed. - */ - REVIEWED = "reviewed", - - /** - * Filter that shows the user's own open pull requests. - */ + ALL = "all", MINE = "mine", - - /** - * Filter that contains ignored PRs. - */ - IGNORED = "ignored", + NEEDS_REVIEW = "needsReview", + NEEDS_REVISION = "needsRevision", + RECENTLY_MERGED = "recentlyMerged", } export type FilteredPullRequests = { @@ -39,47 +15,43 @@ export type FilteredPullRequests = { }; export function filterPullRequests( - context: Context, userLogin: string, - openPullRequests: PullRequest[], - muteConfiguration: MuteConfiguration + openPullRequests: PullRequest[] ): FilteredPullRequests { const enrichedPullRequests = openPullRequests.map((pr) => ({ state: pullRequestState(pr, userLogin), ...pr, })); - const notifyNewCommits = !!muteConfiguration.notifyNewCommits; - const onlyDirectRequests = !!muteConfiguration.onlyDirectRequests; - const whitelistedTeams = muteConfiguration.whitelistedTeams || []; + + const mine = enrichedPullRequests.filter( + (pr) => pr.author?.login === userLogin && !pr.isMerged + ); + const recentlyMerged = enrichedPullRequests.filter( + (pr) => pr.author?.login === userLogin && pr.isMerged + ); + const needsReview = enrichedPullRequests.filter( + (pr) => userLogin !== pr.author?.login && !areChangesRequested(pr.state) + ); + const needsRevision = enrichedPullRequests.filter( + (pr) => userLogin !== pr.author?.login && areChangesRequested(pr.state) + ); + const all = [...mine, ...recentlyMerged, ...needsReview, ...needsRevision]; + return { - incoming: enrichedPullRequests.filter( - (pr) => - isReviewRequired( - pr.state, - notifyNewCommits, - onlyDirectRequests, - whitelistedTeams - ) && isMuted(context, pr, muteConfiguration) === MutedResult.VISIBLE - ), - muted: enrichedPullRequests.filter( - (pr) => - isReviewRequired( - pr.state, - notifyNewCommits, - onlyDirectRequests, - whitelistedTeams - ) && isMuted(context, pr, muteConfiguration) === MutedResult.MUTED - ), - reviewed: enrichedPullRequests.filter( - (pr) => - pr.state.kind === "incoming" && - !pr.state.newReviewRequested && - (!pr.state.newCommit || !notifyNewCommits) && - !pr.state.authorResponded - ), - mine: enrichedPullRequests.filter((pr) => pr.author?.login === userLogin), - ignored: enrichedPullRequests.filter( - (pr) => isMuted(context, pr, muteConfiguration) === MutedResult.INVISIBLE - ), + all, + mine, + needsReview, + needsRevision, + recentlyMerged, }; } + +function areChangesRequested(state: PullRequestState) { + switch (state.kind) { + case "incoming": + case "outgoing": + return state.changesRequested; + default: + return false; + } +} diff --git a/src/filtering/muted.ts b/src/filtering/muted.ts deleted file mode 100644 index 01cd49ca..00000000 --- a/src/filtering/muted.ts +++ /dev/null @@ -1,68 +0,0 @@ -import { Context } from "../environment/api"; -import { PullRequest } from "../storage/loaded-state"; -import { MuteConfiguration } from "../storage/mute-configuration"; -import { - getLastAuthorCommentTimestamp, - getLastAuthorUpdateTimestamp, -} from "./timestamps"; - -/** - * Returns whether the pull request is muted. - */ -export function isMuted( - context: Context, - pr: PullRequest, - muteConfiguration: MuteConfiguration -): MutedResult { - const currentTime = context.getCurrentTime(); - for (const [owner, ignoreConfiguration] of Object.entries( - muteConfiguration.ignored || {} - )) { - if (pr.repoOwner !== owner) { - continue; - } - switch (ignoreConfiguration.kind) { - case "ignore-only": - if (ignoreConfiguration.repoNames.indexOf(pr.repoName) !== -1) { - return MutedResult.INVISIBLE; - } - break; - case "ignore-all": - return MutedResult.INVISIBLE; - } - } - for (const muted of muteConfiguration.mutedPullRequests) { - if ( - muted.repo.owner === pr.repoOwner && - muted.repo.name === pr.repoName && - muted.number === pr.pullRequestNumber - ) { - // It's a match. - switch (muted.until.kind) { - case "next-update": - const updatedSince = - getLastAuthorUpdateTimestamp(pr) > muted.until.mutedAtTimestamp; - return updatedSince ? MutedResult.VISIBLE : MutedResult.MUTED; - case "next-comment-by-author": - const commentedSince = - getLastAuthorCommentTimestamp(pr) > muted.until.mutedAtTimestamp; - return commentedSince ? MutedResult.VISIBLE : MutedResult.MUTED; - case "not-draft": - return pr.draft ? MutedResult.MUTED : MutedResult.VISIBLE; - case "specific-time": - return currentTime >= muted.until.unmuteAtTimestamp - ? MutedResult.VISIBLE - : MutedResult.MUTED; - case "forever": - return MutedResult.MUTED; - } - } - } - return MutedResult.VISIBLE; -} - -export enum MutedResult { - VISIBLE, - MUTED, - INVISIBLE, -} diff --git a/src/filtering/repos-pushed-after.spec.ts b/src/filtering/repos-pushed-after.spec.ts deleted file mode 100644 index 4b46df55..00000000 --- a/src/filtering/repos-pushed-after.spec.ts +++ /dev/null @@ -1,39 +0,0 @@ -import { repoWasPushedAfter } from "./repos-pushed-after"; - -describe("repoWasPushedAfter", () => { - it("always returns true when pushedAfter = null", () => { - const predicate = repoWasPushedAfter(null); - expect( - predicate({ - owner: "test", - name: "test", - pushedAt: "2012-03-15T17:00:11Z", - }) - ).toBe(true); - expect( - predicate({ - owner: "test", - name: "test", - pushedAt: "2052-03-15T17:00:11Z", - }) - ).toBe(true); - }); - - it("filters correctly", () => { - const predicate = repoWasPushedAfter("2012-03-15T17:00:11Z"); - expect( - predicate({ - owner: "test", - name: "test", - pushedAt: "2010-03-15T17:00:11Z", - }) - ).toBe(false); - expect( - predicate({ - owner: "test", - name: "test", - pushedAt: "2020-03-15T17:00:11Z", - }) - ).toBe(true); - }); -}); diff --git a/src/filtering/repos-pushed-after.ts b/src/filtering/repos-pushed-after.ts deleted file mode 100644 index f1fa3b50..00000000 --- a/src/filtering/repos-pushed-after.ts +++ /dev/null @@ -1,16 +0,0 @@ -import { Repo } from "../storage/loaded-state"; - -/** - * Returns a predicate that will return true if the repo was pushed after the given timestamp. - * - * If `pushedAt` is null, the predicate will always return true. - */ -export function repoWasPushedAfter( - pushedAt: string | null -): (repo: Repo) => boolean { - if (!pushedAt) { - return () => true; - } - const minPushedAt = new Date(pushedAt); - return (repo: Repo) => new Date(repo.pushedAt) > minPushedAt; -} diff --git a/src/filtering/status.spec.ts b/src/filtering/status.spec.ts index 31843cae..d51f654d 100644 --- a/src/filtering/status.spec.ts +++ b/src/filtering/status.spec.ts @@ -190,8 +190,7 @@ describe("pullRequestState", () => { draft: false, noReviewers: true, changesRequested: false, - mergeable: false, - approvedByEveryone: false, + approved: false, }); expect( @@ -204,8 +203,7 @@ describe("pullRequestState", () => { draft: true, noReviewers: true, changesRequested: false, - mergeable: false, - approvedByEveryone: false, + approved: false, }); expect( @@ -222,8 +220,7 @@ describe("pullRequestState", () => { draft: false, noReviewers: false, changesRequested: false, - mergeable: false, - approvedByEveryone: false, + approved: false, }); expect( @@ -234,7 +231,6 @@ describe("pullRequestState", () => { .addReview("kevin", "CHANGES_REQUESTED") .addComment("fwouts") .addReview("kevin", "APPROVED") - .mergeable() .seenAs("fwouts") .build(), "fwouts" @@ -244,8 +240,7 @@ describe("pullRequestState", () => { draft: false, noReviewers: false, changesRequested: false, - mergeable: true, - approvedByEveryone: false, + approved: false, }); expect( @@ -266,8 +261,7 @@ describe("pullRequestState", () => { draft: false, noReviewers: false, changesRequested: false, - mergeable: false, - approvedByEveryone: true, + approved: true, }); expect( @@ -288,8 +282,7 @@ describe("pullRequestState", () => { draft: false, noReviewers: false, changesRequested: false, - mergeable: false, - approvedByEveryone: false, + approved: false, }); expect( @@ -310,8 +303,7 @@ describe("pullRequestState", () => { draft: false, noReviewers: false, changesRequested: false, - mergeable: false, - approvedByEveryone: false, + approved: false, }); }); }); diff --git a/src/filtering/status.ts b/src/filtering/status.ts index a3c22ed1..bbeeafcb 100644 --- a/src/filtering/status.ts +++ b/src/filtering/status.ts @@ -1,10 +1,9 @@ import { CheckStatus } from "../github-api/api"; import { PullRequest, ReviewState } from "../storage/loaded-state"; -import { userPreviouslyReviewed } from "./reviewed"; import { getLastAuthorCommentTimestamp, - getLastCommitTimestamp, getLastReviewOrCommentTimestamp, + getLastReviewTimestamp, } from "./timestamps"; /** @@ -17,12 +16,6 @@ export function pullRequestState( if (pr.author?.login === currentUserLogin) { return outgoingPullRequestState(pr, currentUserLogin); } - if (!pr.reviewRequested && !userPreviouslyReviewed(pr, currentUserLogin)) { - return { - kind: "not-involved", - draft: pr.draft === true, - }; - } return incomingPullRequestState(pr, currentUserLogin); } @@ -36,32 +29,8 @@ function incomingPullRequestState( ); const hasNewCommentByAuthor = lastReviewOrCommentFromCurrentUser < getLastAuthorCommentTimestamp(pr); - const hasNewCommit = - lastReviewOrCommentFromCurrentUser < getLastCommitTimestamp(pr); const hasReviewed = lastReviewOrCommentFromCurrentUser > 0; - return { - kind: "incoming", - draft: pr.draft === true, - newReviewRequested: !hasReviewed, - authorResponded: hasReviewed && hasNewCommentByAuthor, - newCommit: hasReviewed && hasNewCommit, - directlyAdded: (pr.requestedReviewers || []).includes(currentUserLogin), - teams: pr.requestedTeams || [], - checkStatus: pr.checkStatus, - }; -} -function outgoingPullRequestState( - pr: PullRequest, - currentUserLogin: string -): PullRequestState { - const lastReviewOrCommentFromCurrentUserTimestamp = - getLastReviewOrCommentTimestamp(pr, currentUserLogin); - const lastCommitTimestamp = getLastCommitTimestamp(pr); - const lastActionByCurrentUserTimestamp = Math.max( - lastReviewOrCommentFromCurrentUserTimestamp, - lastCommitTimestamp - ); const stateByUser = new Map(); // Keep track of the last known state of reviews left by others. @@ -71,7 +40,7 @@ function outgoingPullRequestState( } const submittedAt = new Date(review.submittedAt).getTime(); if ( - submittedAt < lastActionByCurrentUserTimestamp && + submittedAt < getLastReviewTimestamp(pr, review.authorLogin) && review.state === "CHANGES_REQUESTED" ) { // This change request may not be relevant anymore because the author has @@ -87,149 +56,61 @@ function outgoingPullRequestState( } } - // Ensure that anyone who commented without leaving a review is counted too. - for (const comment of pr.comments) { - if (comment.authorLogin === currentUserLogin) { + const states = new Set(stateByUser.values()); + return { + kind: "incoming", + draft: pr.draft === true, + newReviewRequested: !hasReviewed || states.has("PENDING"), + authorResponded: hasReviewed && hasNewCommentByAuthor, + checkStatus: pr.checkStatus, + changesRequested: states.has("CHANGES_REQUESTED") || !pr.reviewRequested, + approved: false, + isMerged: pr.isMerged, + }; +} + +function outgoingPullRequestState( + pr: PullRequest, + currentUserLogin: string +): PullRequestState { + const stateByUser = new Map(); + for (const review of pr.reviews) { + if (review.authorLogin === currentUserLogin || !review.submittedAt) { continue; } - if (!stateByUser.has(comment.authorLogin)) { - stateByUser.set(comment.authorLogin, "COMMENTED"); + const submittedAt = new Date(review.submittedAt).getTime(); + if ( + submittedAt < getLastReviewTimestamp(pr, review.authorLogin) && + review.state === "CHANGES_REQUESTED" + ) { + stateByUser.set(review.authorLogin, "PENDING"); + continue; + } + if (!stateByUser.has(review.authorLogin) || review.state !== "COMMENTED") { + stateByUser.set(review.authorLogin, review.state); } - } - - // Requested reviewers are specifically reviewers who haven't posted a review - // after a request. This can also be the case when they had previously - // reviewed, but the author requested another review from them. - for (const requestedReviewer of pr.requestedReviewers || []) { - stateByUser.set(requestedReviewer, "PENDING"); } const states = new Set(stateByUser.values()); return { kind: "outgoing", draft: pr.draft === true, - noReviewers: stateByUser.size === 0, - changesRequested: states.has("CHANGES_REQUESTED"), - mergeable: pr.mergeable === true, - approvedByEveryone: states.has("APPROVED") && states.size === 1, + changesRequested: states.has("CHANGES_REQUESTED") || !pr.reviewRequested, + approved: states.has("APPROVED"), checkStatus: pr.checkStatus, + newReviewRequested: states.has("PENDING"), + authorResponded: false, + isMerged: pr.isMerged, }; } -export type PullRequestState = IncomingState | NotInvolvedState | OutgoingState; - -/** - * The current user is involved in the PR, either because they are a reviewer or - * because they've added comments. - */ -export interface IncomingState { - kind: "incoming"; - - /** - * True if the PR is a draft. - */ - draft: boolean; - - /** - * True if a review has been requested from the user, but they haven't - * submitted any review or comments on the PR yet. - */ - newReviewRequested: boolean; - - /** - * True if the author posted a comment after a review or comment was - * previously submitted by the user. - */ +export type PullRequestState = { + approved: boolean; authorResponded: boolean; - - /** - * True if a new commit was added to the PR after a review or comment was - * previously submitted by the user. - */ - newCommit: boolean; - - /** - * True if a review has been requested for the current user, and not just included indirectly via a team. - */ - directlyAdded: boolean; - - /** - * List of team names requested. - */ - teams: string[]; - - /** - * Current check status of tests. - */ - checkStatus?: CheckStatus; -} - -/** - * The current user is not involved in the PR. That is, they are not a - * reviewer and haven't posted any comments. - */ -export interface NotInvolvedState { - kind: "not-involved"; - - /** - * True if the PR is a draft. - */ - draft: boolean; -} - -/** - * The PR that authored by the current user. - */ -export interface OutgoingState { - kind: "outgoing"; - - /** - * True if the PR is a draft. - */ - draft: boolean; - - /** - * True if the PR has no reviewers yet. - */ - noReviewers: boolean; - - /** - * True if the PR received review comments which need to be addressed either - * by responding or adding new commits. - */ changesRequested: boolean; - - /** - * True if GitHub indicates that the PR can be merged. - */ - mergeable: boolean; - - /** - * True if the PR was approved by all reviewers. - */ - approvedByEveryone: boolean; - - /** - * Current check status of tests. - */ + draft: boolean; + kind: "incoming" | "outgoing"; + newReviewRequested: boolean; + isMerged: boolean; checkStatus?: CheckStatus; -} - -export function isReviewRequired( - state: PullRequestState, - notifyNewCommits: boolean, - onlyDirectRequests: boolean, - whitelistedTeams: string[] -) { - const inWhitelistedTeams = - state.kind === "incoming" && - state.teams.some((team) => whitelistedTeams.includes(team)); - const v = - state.kind === "incoming" && - (!onlyDirectRequests || state.directlyAdded || inWhitelistedTeams) && - (state.newReviewRequested || - state.authorResponded || - (notifyNewCommits && state.newCommit)); - - return v; -} +}; diff --git a/src/filtering/testing.ts b/src/filtering/testing.ts index 64544c06..f3b5e326 100644 --- a/src/filtering/testing.ts +++ b/src/filtering/testing.ts @@ -1,35 +1,22 @@ import { Context } from "../environment/api"; import { PullRequest } from "../storage/loaded-state"; -import { MuteConfiguration } from "../storage/mute-configuration"; import { Filter, filterPullRequests } from "./filters"; export function getFilteredBucket( - context: Context, + _context: Context, userLogin: string, - muteConfiguration: MuteConfiguration, pr: PullRequest ) { const filteredPullRequests = filterPullRequests( - context, userLogin, [pr], - muteConfiguration ); const filters: Filter[] = []; - if (filteredPullRequests.incoming.length > 0) { - filters.push(Filter.INCOMING); - } - if (filteredPullRequests.muted.length > 0) { - filters.push(Filter.MUTED); - } - if (filteredPullRequests.reviewed.length > 0) { - filters.push(Filter.REVIEWED); + if (filteredPullRequests.needsReview.length > 0) { + filters.push(Filter.NEEDS_REVIEW); } if (filteredPullRequests.mine.length > 0) { filters.push(Filter.MINE); } - if (filteredPullRequests.ignored.length > 0) { - filters.push(Filter.IGNORED); - } return filters; } diff --git a/src/filtering/timestamps.ts b/src/filtering/timestamps.ts index ee0868f1..1993bb83 100644 --- a/src/filtering/timestamps.ts +++ b/src/filtering/timestamps.ts @@ -1,10 +1,7 @@ import { PullRequest } from "../storage/loaded-state"; export function getLastUpdateTimestamp(pr: PullRequest) { - let prTimestamp = Math.max( - new Date(pr.updatedAt).getTime(), - getLastCommitTimestamp(pr) - ); + let prTimestamp = new Date(pr.updatedAt).getTime(); for (const comment of pr.comments) { prTimestamp = Math.max(prTimestamp, new Date(comment.createdAt).getTime()); } @@ -17,13 +14,6 @@ export function getLastUpdateTimestamp(pr: PullRequest) { return prTimestamp; } -export function getLastAuthorUpdateTimestamp(pr: PullRequest): number { - return Math.max( - getLastAuthorCommentTimestamp(pr), - getLastCommitTimestamp(pr) - ); -} - export function getLastAuthorCommentTimestamp(pr: PullRequest): number { if (!pr.author) { return 0; @@ -59,14 +49,22 @@ export function getLastReviewOrCommentTimestamp( return lastCommentedTime; } -export function getLastCommitTimestamp(pr: PullRequest): number { - let lastCommitTime = 0; - for (const commit of pr.commits || []) { - if (!commit.createdAt) { +export function getLastReviewTimestamp( + pr: PullRequest, + login: string +): number { + let lastReviewedTime = 0; + for (const review of pr.reviews) { + if (review.state === "PENDING") { continue; } - const createdAt = new Date(commit.createdAt).getTime(); - lastCommitTime = Math.max(lastCommitTime, createdAt); + if (!review.submittedAt) { + continue; + } + const submittedAt = new Date(review.submittedAt).getTime(); + if (review.authorLogin === login) { + lastReviewedTime = Math.max(lastReviewedTime, submittedAt); + } } - return lastCommitTime; + return lastReviewedTime; } diff --git a/src/github-api/api.ts b/src/github-api/api.ts index e5f9702b..1d3938a4 100644 --- a/src/github-api/api.ts +++ b/src/github-api/api.ts @@ -25,6 +25,8 @@ export interface GitHubApi { > >; + loadPullRequestChangeSummary(pr: PullRequestReference): Promise; + /** * Returns the details of a pull request. */ @@ -51,18 +53,22 @@ export interface GitHubApi { >; /** - * Returns the full list of commits for a pull request. + * Returns the full list of comments for a pull request. */ - loadCommits( + loadReviewComments( pr: PullRequestReference ): Promise< - GetResponseDataTypeFromEndpointMethod + GetResponseDataTypeFromEndpointMethod< + Octokit["pulls"]["listReviewComments"] + > >; /** * Returns the current status fields for a pull request. */ loadPullRequestStatus(pr: PullRequestReference): Promise; + + loadIsMerged(pr: PullRequestReference): Promise; } // Ref: https://docs.github.com/en/graphql/reference/enums#pullrequestreviewdecision diff --git a/src/github-api/implementation.ts b/src/github-api/implementation.ts index afb28a6a..ced9710d 100644 --- a/src/github-api/implementation.ts +++ b/src/github-api/implementation.ts @@ -1,104 +1,117 @@ -import { throttling } from "@octokit/plugin-throttling"; -import { Octokit } from "@octokit/rest"; import { GitHubApi } from "./api"; -import { GraphQLClient, gql } from "graphql-request"; - -const ThrottledOctokit = Octokit.plugin(throttling); -const graphQLEndpoint = "https://api.github.com/graphql"; - -interface ThrottlingOptions { - method: string; - url: string; -} +import { request } from "@octokit/request"; export function buildGitHubApi(token: string): GitHubApi { - const octokit: Octokit = new ThrottledOctokit({ - auth: `token ${token}`, - // https://developer.github.com/v3/pulls/#list-pull-requests - // Enable Draft Pull Request API. - previews: ["shadow-cat"], - throttle: { - onRateLimit: (retryAfter: number, options: ThrottlingOptions, _: Octokit, retryCount: number) => { - console.warn( - `Request quota exhausted for request ${options.method} ${options.url}` - ); - // Only retry twice. - if (retryCount < 2) { - console.log(`Retrying after ${retryAfter} seconds!`); - return true; - } - return false; - }, - onSecondaryRateLimit: (retryAfter: number, options: ThrottlingOptions, _: Octokit, retryCount: number) => { - console.warn( - `Secondary Rate Limit detected for request ${options.method} ${options.url}` - ); - // Only retry twice. - if (retryCount < 2) { - console.log(`Retrying after ${retryAfter} seconds!`); - return true; - } - return false; - }, - }, - }); - - const graphQLClient = new GraphQLClient(graphQLEndpoint, { - headers: { - Authorization: `Bearer ${token}`, - }, - }); - return { - async loadAuthenticatedUser() { - const response = await octokit.users.getAuthenticated({}); + async loadAuthenticatedUser(): Promise { + const response = await request(`GET /user`, { + headers: { + authorization: `Bearer ${token}`, + }, + org: "octokit", + type: "private", + }); return response.data; }, - searchPullRequests(query) { - return octokit.paginate( - octokit.search.issuesAndPullRequests.endpoint.merge({ - q: `is:pr ${query}`, - }) + async searchPullRequests(query): Promise { + const response = await request(`GET /search/issues`, { + headers: { + authorization: `Bearer ${token}`, + }, + q: `is:pr ${query}`, + org: "octokit", + type: "private", + }); + return response.data.items; + }, + async loadPullRequestDetails(pr): Promise { + const response = await request( + `GET /repos/${pr.repo.owner}/${pr.repo.name}/pulls/${pr.number}`, + { + headers: { + authorization: `Bearer ${token}`, + }, + org: "octokit", + type: "private", + } ); + return response.data; }, - async loadPullRequestDetails(pr) { - const response = await octokit.pulls.get({ - owner: pr.repo.owner, - repo: pr.repo.name, - pull_number: pr.number, - }); + async loadPullRequestChangeSummary(pr): Promise { + const response = await request( + `GET /repos/${pr.repo.owner}/${pr.repo.name}/pulls/${pr.number}/files`, + { + headers: { + authorization: `Bearer ${token}`, + }, + org: "octokit", + type: "private", + } + ); return response.data; }, - loadReviews(pr) { - return octokit.paginate( - octokit.pulls.listReviews.endpoint.merge({ - owner: pr.repo.owner, - repo: pr.repo.name, - pull_number: pr.number, - }) + async loadReviews(pr): Promise { + const response = await request( + `GET /repos/${pr.repo.owner}/${pr.repo.name}/pulls/${pr.number}/reviews`, + { + headers: { + authorization: `Bearer ${token}`, + }, + org: "octokit", + type: "private", + } ); + return response.data; }, - loadComments(pr) { - return octokit.paginate( - octokit.issues.listComments.endpoint.merge({ - owner: pr.repo.owner, - repo: pr.repo.name, - issue_number: pr.number, - }) + async loadComments(pr): Promise { + const response = await request( + `GET /repos/${pr.repo.owner}/${pr.repo.name}/issues/${pr.number}/comments`, + { + headers: { + authorization: `Bearer ${token}`, + }, + org: "octokit", + type: "private", + } ); + return response.data ?? []; }, - loadCommits(pr) { - return octokit.paginate( - octokit.pulls.listCommits.endpoint.merge({ - owner: pr.repo.owner, - repo: pr.repo.name, - pull_number: pr.number, - }) + async loadReviewComments(pr): Promise { + const response = await request( + `GET /repos/${pr.repo.owner}/${pr.repo.name}/pulls/${pr.number}/comments`, + { + headers: { + authorization: `Bearer ${token}`, + }, + org: "octokit", + type: "private", + } ); + return response.data ?? []; + }, + async loadIsMerged(pr): Promise { + try { + const response = await request( + `GET /repos/${pr.repo.owner}/${pr.repo.name}/pulls/${pr.number}/merge`, + { + headers: { + authorization: `Bearer ${token}`, + }, + org: "octokit", + type: "private", + } + ); + return response.status === 204; + } catch (e) { + return false; + } }, - loadPullRequestStatus(pr) { - const query = gql` - query { + async loadPullRequestStatus(pr) { + const response = await request("POST /graphql", { + headers: { + authorization: `Bearer ${token}`, + }, + query: `query { repository(owner: "${pr.repo.owner}", name: "${pr.repo.name}") { pullRequest(number: ${pr.number}) { reviewDecision @@ -113,19 +126,18 @@ export function buildGitHubApi(token: string): GitHubApi { } } } - } - `; - - return graphQLClient.request(query).then((response) => { - const result = response.repository.pullRequest; - const reviewDecision = result.reviewDecision; - const checkStatus = - result.commits.nodes?.[0]?.commit.statusCheckRollup?.state; - return { - reviewDecision, - checkStatus, - }; + }`, + variables: { + login: "octokit", + }, }); + + const pullRequest = response.data.data.repository.pullRequest; + return { + reviewDecision: pullRequest.reviewDecision, + checkStatus: + pullRequest.commits.nodes?.[0]?.commit.statusCheckRollup?.state, + }; }, }; } diff --git a/src/loading/implementation.ts b/src/loading/implementation.ts index 6c5f4c71..813718f4 100644 --- a/src/loading/implementation.ts +++ b/src/loading/implementation.ts @@ -11,7 +11,7 @@ export function buildGitHubLoader(): GitHubLoader { async function load(token: string): Promise { const githubApi = buildGitHubApi(token); const user = await githubApi.loadAuthenticatedUser(); - const openPullRequests = await refreshOpenPullRequests(githubApi, user.login); + const openPullRequests = await refreshOpenPullRequests(githubApi); const sorted = [...openPullRequests].sort((a, b) => { return getLastUpdateTimestamp(b) - getLastUpdateTimestamp(a); }); diff --git a/src/loading/internal/pull-requests.ts b/src/loading/internal/pull-requests.ts index a99fd842..f6eeb1a7 100644 --- a/src/loading/internal/pull-requests.ts +++ b/src/loading/internal/pull-requests.ts @@ -5,14 +5,234 @@ import { PullRequestStatus, RepoReference, } from "../../github-api/api"; -import { nonEmptyItems } from "../../helpers"; import { Comment, - Commit, PullRequest, Review, ReviewState, } from "../../storage/loaded-state"; +import moment from "moment"; + +export function testPRs(): Promise { + return Promise.all([ + { + nodeId: "123", + htmlUrl: "", + repoOwner: "dbharris", + repoName: "test-repo", + pullRequestNumber: 1098, + updatedAt: "9 December 2023", + author: { + login: "dbharris2", + avatarUrl: + "https://cdn.iconscout.com/icon/free/png-256/free-eevee-eievui-pokemon-cartoon-game-video-pokemongo-32216.png", + }, + changeSummary: { + changedFiles: 6, + additions: 134, + deletions: 344, + }, + title: "Codemod old API to new API", + draft: false, + reviewRequested: true, + requestedReviewers: [], + requestedTeams: [], + reviews: [ + { + authorLogin: "someone-else", + state: "APPROVED", + submittedAt: "10 December 2023", + }, + ], + comments: [ + { authorLogin: "", createdAt: "" }, + { authorLogin: "", createdAt: "" }, + ], + reviewDecision: "APPROVED", + checkStatus: "SUCCESS", + isMerged: false, + }, + { + nodeId: "123", + htmlUrl: "", + repoOwner: "dbharris", + repoName: "test-repo", + pullRequestNumber: 1234, + updatedAt: "10 December 2023", + author: { + login: "dbharris2", + avatarUrl: + "https://cdn.iconscout.com/icon/free/png-256/free-eevee-eievui-pokemon-cartoon-game-video-pokemongo-32216.png", + }, + changeSummary: { + changedFiles: 27, + additions: 732, + deletions: 640, + }, + title: "Use new endpoint on homepage", + draft: false, + reviewRequested: true, + requestedReviewers: [], + requestedTeams: [], + reviews: [], + comments: [ + { authorLogin: "", createdAt: "" }, + { authorLogin: "", createdAt: "" }, + ], + reviewDecision: "REVIEW_REQUIRED", + checkStatus: "SUCCESS", + isMerged: false, + }, + { + nodeId: "123", + htmlUrl: "", + repoOwner: "dbharris", + repoName: "test-repo", + pullRequestNumber: 1308, + updatedAt: "10 December 2023", + author: { + login: "dbharris2", + avatarUrl: + "https://cdn.iconscout.com/icon/free/png-256/free-eevee-eievui-pokemon-cartoon-game-video-pokemongo-32216.png", + }, + changeSummary: { + changedFiles: 27, + additions: 732, + deletions: 640, + }, + title: "Use new routing API", + draft: false, + reviewRequested: false, + requestedReviewers: [], + requestedTeams: [], + reviews: [{ authorLogin: "dbharris2", state: "CHANGES_REQUESTED" }], + comments: [ + { authorLogin: "", createdAt: "" }, + { authorLogin: "", createdAt: "" }, + ], + reviewDecision: "CHANGES_REQUESTED", + checkStatus: "SUCCESS", + isMerged: false, + }, + { + nodeId: "123", + htmlUrl: "", + repoOwner: "dbharris", + repoName: "test-repo", + pullRequestNumber: 1023, + updatedAt: "8 December 2023", + author: { + login: "dbharris2", + avatarUrl: + "https://cdn.iconscout.com/icon/free/png-256/free-eevee-eievui-pokemon-cartoon-game-video-pokemongo-32216.png", + }, + changeSummary: { + changedFiles: 4, + additions: 12, + deletions: 64, + }, + title: "Update unit tests for nav bar", + draft: true, + reviewRequested: true, + requestedReviewers: [], + requestedTeams: [], + reviews: [], + comments: [ + { authorLogin: "", createdAt: "" }, + { authorLogin: "", createdAt: "" }, + ], + reviewDecision: "REVIEW_REQUIRED", + checkStatus: "FAILURE", + isMerged: false, + }, + { + nodeId: "123", + htmlUrl: "", + repoOwner: "dbharris", + repoName: "test-repo", + pullRequestNumber: 1521, + updatedAt: "10 December 2023", + author: { + login: "someone-else", + avatarUrl: + "https://daily.pokecommunity.com/wp-content/uploads/2018/11/pokemon_icon_092_00.png", + }, + changeSummary: { + changedFiles: 7, + additions: 112, + deletions: 32, + }, + title: "Ship new accounts page", + draft: false, + reviewRequested: true, + requestedReviewers: [], + requestedTeams: [], + reviews: [{ authorLogin: "dbharris2", state: "CHANGES_REQUESTED" }], + comments: [{ authorLogin: "dbharris2", createdAt: "10 December 2023" }], + reviewDecision: "REVIEW_REQUIRED", + checkStatus: "SUCCESS", + isMerged: false, + }, + { + nodeId: "123", + htmlUrl: "", + repoOwner: "dbharris", + repoName: "test-repo", + pullRequestNumber: 1521, + updatedAt: "9 December 2023", + author: { + login: "someone-else", + avatarUrl: + "https://daily.pokecommunity.com/wp-content/uploads/2018/11/pokemon_icon_092_00.png", + }, + changeSummary: { + changedFiles: 7, + additions: 112, + deletions: 32, + }, + title: "Fix bug when scrolling through accounts", + draft: false, + reviewRequested: true, + requestedReviewers: [], + requestedTeams: [], + reviews: [], + comments: [ + { authorLogin: "", createdAt: "" }, + { authorLogin: "", createdAt: "" }, + ], + reviewDecision: "REVIEW_REQUIRED", + checkStatus: "SUCCESS", + isMerged: false, + }, + { + nodeId: "123", + htmlUrl: "", + repoOwner: "dbharris", + repoName: "test-repo", + pullRequestNumber: 1771, + updatedAt: "9 December 2023", + author: { + login: "another-someone", + avatarUrl: "https://www.icons101.com/icon_ico/id_60018/025_Pikachu.ico", + }, + changeSummary: { + changedFiles: 4, + additions: 12, + deletions: 64, + }, + title: "Add caching to queries", + draft: false, + reviewRequested: false, + requestedReviewers: [], + requestedTeams: [], + reviews: [{ authorLogin: "dbharris2", state: "CHANGES_REQUESTED" }], + comments: [{ authorLogin: "dbharris2", createdAt: "10 December 2023" }], + reviewDecision: "CHANGES_REQUESTED", + checkStatus: "SUCCESS", + isMerged: false, + }, + ]); +} /** * Refreshes the list of pull requests for a list of repositories. @@ -22,28 +242,45 @@ import { * hundred repositories or many pull requests opened. */ export async function refreshOpenPullRequests( - githubApi: GitHubApi, - userLogin: string + githubApi: GitHubApi ): Promise { - // Note: each query should specifically exclude the previous ones so we don't end up having - // to deduplicate PRs across lists. + // if (true) { + // return testPRs(); + // } + + // As long as a review is requested for you (even if changes have been requested), then it's reviewable const reviewRequestedPullRequests = await githubApi.searchPullRequests( - `review-requested:${userLogin} -author:${userLogin} is:open archived:false` + `-author:@me -is:draft is:open -review:approved review-requested:@me` + ); + const needsRevisionPullRequests = await githubApi.searchPullRequests( + `-author:@me -is:draft is:open review:changes_requested involves:@me` ); - const commentedPullRequests = await githubApi.searchPullRequests( - `commenter:${userLogin} -author:${userLogin} -review-requested:${userLogin} is:open archived:false` + const myPullRequests = await githubApi.searchPullRequests( + `author:@me is:open` ); - const ownPullRequests = await githubApi.searchPullRequests( - `author:${userLogin} is:open archived:false` + const myRecentlyMergedPullRequests = await githubApi.searchPullRequests( + `author:@me is:closed archived:false` ); + return Promise.all([ ...reviewRequestedPullRequests.map((pr) => updateCommentsAndReviews(githubApi, pr, true) ), - ...commentedPullRequests.map((pr) => - updateCommentsAndReviews(githubApi, pr) + // Remove PRs that needs revision but have a review requested of you + ...needsRevisionPullRequests + .filter( + (nrpr) => + !reviewRequestedPullRequests.find( + (rrpr) => nrpr.number === rrpr.number + ) + ) + .map((pr) => updateCommentsAndReviews(githubApi, pr)), + ...myPullRequests.map((pr) => + updateCommentsAndReviews(githubApi, pr, true) ), - ...ownPullRequests.map((pr) => updateCommentsAndReviews(githubApi, pr)), + ...myRecentlyMergedPullRequests + .filter((pr) => moment().diff(moment(pr.updated_at), "days") < 1) + .map((pr) => updateCommentsAndReviews(githubApi, pr)), ]); } @@ -58,13 +295,14 @@ async function updateCommentsAndReviews( number: rawPullRequest.number, }; const [ - freshPullRequestDetails, + freshChangeSummary, freshReviews, freshComments, - freshCommits, + freshReviewComments, pullRequestStatus, + isMerged, ] = await Promise.all([ - githubApi.loadPullRequestDetails(pr), + githubApi.loadPullRequestChangeSummary(pr), githubApi.loadReviews(pr).then((reviews) => reviews.map((review) => ({ authorLogin: review.user ? review.user.login : "", @@ -78,34 +316,37 @@ async function updateCommentsAndReviews( createdAt: comment.created_at, })) ), - githubApi.loadCommits(pr).then((commits) => - commits.map((commit) => ({ - authorLogin: commit.author ? commit.author.login : "", - createdAt: commit.commit.author?.date, + githubApi.loadReviewComments(pr).then((comments) => + comments.map((comment) => ({ + authorLogin: comment.user ? comment.user.login : "", + createdAt: comment.created_at, })) ), githubApi.loadPullRequestStatus(pr), + githubApi.loadIsMerged(pr), ]); return pullRequestFromResponse( rawPullRequest, - freshPullRequestDetails, + freshChangeSummary, freshReviews, freshComments, - freshCommits, + freshReviewComments, isReviewRequested, - pullRequestStatus + pullRequestStatus, + isMerged ); } function pullRequestFromResponse( response: RestEndpointMethodTypes["search"]["issuesAndPullRequests"]["response"]["data"]["items"][number], - details: RestEndpointMethodTypes["pulls"]["get"]["response"]["data"], + changeSummary: any, reviews: Review[], comments: Comment[], - commits: Commit[], + reviewComments: Comment[], reviewRequested: boolean, - status: PullRequestStatus + status: PullRequestStatus, + isMerged: boolean ): PullRequest { const repo = extractRepo(response); return { @@ -120,25 +361,24 @@ function pullRequestFromResponse( avatarUrl: response.user.avatar_url, }, changeSummary: { - changedFiles: details.changed_files, - additions: details.additions, - deletions: details.deletions, + changedFiles: changeSummary.length, + additions: changeSummary.reduce( + (total: number, curr: any) => total + curr.additions, + 0 + ), + deletions: changeSummary.reduce( + (total: number, curr: any) => total + curr.deletions, + 0 + ), }, title: response.title, draft: response.draft, - mergeable: details.mergeable || false, reviewRequested, - requestedReviewers: nonEmptyItems( - details.requested_reviewers?.map((reviewer) => reviewer?.login) - ), - requestedTeams: nonEmptyItems( - details.requested_teams?.map((team) => team?.name) - ), reviews, - comments, - commits, + comments: [...comments, ...reviewComments], reviewDecision: status.reviewDecision, checkStatus: status.checkStatus, + isMerged, }; } diff --git a/src/messaging/api.ts b/src/messaging/api.ts index e69b7f31..6eb20f7e 100644 --- a/src/messaging/api.ts +++ b/src/messaging/api.ts @@ -9,4 +9,7 @@ export type Message = } | { kind: "reload"; + } + | { + kind: "restart"; }; diff --git a/src/notifications/api.ts b/src/notifications/api.ts deleted file mode 100644 index 37f68772..00000000 --- a/src/notifications/api.ts +++ /dev/null @@ -1,11 +0,0 @@ -import { EnrichedPullRequest } from "../filtering/enriched-pull-request"; - -export interface Notifier { - notify( - pullRequests: EnrichedPullRequest[], - alreadyNotifiedPullRequestUrls: Set - ): void; - registerClickListener(clickListener: NotifierClickListener): void; -} - -export type NotifierClickListener = (pullRequestUrl: string) => void; diff --git a/src/notifications/implementation.ts b/src/notifications/implementation.ts deleted file mode 100644 index c5bf90b8..00000000 --- a/src/notifications/implementation.ts +++ /dev/null @@ -1,95 +0,0 @@ -import { ChromeApi } from "../chrome/api"; -import { EnrichedPullRequest } from "../filtering/enriched-pull-request"; -import { Notifier } from "./api"; - -export function buildNotifier(chromeApi: ChromeApi): Notifier { - return { - notify(pullRequests, notifiedPullRequestUrls) { - showNotificationForNewPullRequests( - chromeApi, - pullRequests, - notifiedPullRequestUrls - ); - }, - registerClickListener(clickListener: (pullRequestUrl: string) => void) { - // Notification IDs are always pull request URLs (see below). - chromeApi.notifications.onClicked.addListener((notificationId) => { - clickListener(notificationId); - chromeApi.notifications.clear(notificationId); - }); - }, - }; -} - -/** - * Shows a notification for each pull request that we haven't yet notified about. - */ -async function showNotificationForNewPullRequests( - chromeApi: ChromeApi, - pullRequests: EnrichedPullRequest[], - notifiedPullRequestUrls: Set -) { - for (const pullRequest of pullRequests) { - if (!notifiedPullRequestUrls.has(pullRequest.htmlUrl)) { - console.log(`Showing ${pullRequest.htmlUrl}`); - showNotification(chromeApi, pullRequest); - } else { - console.log(`Filtering ${pullRequest.htmlUrl}`); - } - } -} - -/** - * Shows a notification if the pull request is new. - */ -function showNotification( - chromeApi: ChromeApi, - pullRequest: EnrichedPullRequest -) { - // We set the notification ID to the URL so that we simply cannot have duplicate - // notifications about the same pull request and we can easily open a browser tab - // to this pull request just by knowing the notification ID. - const notificationId = pullRequest.htmlUrl; - - // Chrome supports requireInteraction, but it crashes Firefox. - const supportsRequireInteraction = - navigator.userAgent.toLowerCase().indexOf("firefox") === -1; - chromeApi.notifications.create(notificationId, { - type: "basic", - iconUrl: "images/GitHub-Mark-120px-plus.png", - title: getTitle(pullRequest), - message: getMessage(pullRequest), - contextMessage: getContextMessage(pullRequest), - ...(supportsRequireInteraction ? { requireInteraction: true } : {}), - }); -} - -function getTitle(pullRequest: EnrichedPullRequest): string { - switch (pullRequest.state.kind) { - case "incoming": - if (pullRequest.state.newReviewRequested) { - return "New pull request"; - } else if (pullRequest.state.newCommit) { - return `Pull request updated`; - } else if (pullRequest.state.authorResponded) { - return `${pullRequest.author?.login || "unknown"} commented`; - } - break; - case "outgoing": - if (pullRequest.state.approvedByEveryone) { - return `Pull request approved by everyone`; - } else if (pullRequest.state.changesRequested) { - return `New changes requested`; - } - break; - } - throw new Error(`Well, this should never happen.`); -} - -function getMessage(pullRequest: EnrichedPullRequest): string { - return pullRequest.title; -} - -function getContextMessage(pullRequest: EnrichedPullRequest): string { - return `${pullRequest.repoOwner}/${pullRequest.repoName}`; -} diff --git a/src/popup.tsx b/src/popup.tsx index f1e30e13..35418d2e 100644 --- a/src/popup.tsx +++ b/src/popup.tsx @@ -1,13 +1,4 @@ import { css, Global } from "@emotion/react"; -import { library } from "@fortawesome/fontawesome-svg-core"; -import { faBan } from "@fortawesome/free-solid-svg-icons/faBan"; -import { faBell } from "@fortawesome/free-solid-svg-icons/faBell"; -import { faBellSlash } from "@fortawesome/free-solid-svg-icons/faBellSlash"; -import { faClock } from "@fortawesome/free-solid-svg-icons/faClock"; -import { faClone } from "@fortawesome/free-solid-svg-icons/faClone"; -import { faPen } from "@fortawesome/free-solid-svg-icons/faPen"; -import { faPodcast } from "@fortawesome/free-solid-svg-icons/faPodcast"; -import { faReply } from "@fortawesome/free-solid-svg-icons/faReply"; import "bootstrap/dist/css/bootstrap.min.css"; import React from "react"; import ReactDOM from "react-dom"; @@ -16,15 +7,6 @@ import { Popup } from "./components/Popup"; import { buildEnvironment } from "./environment/implementation"; import { Core } from "./state/core"; -library.add(faBan); -library.add(faBell); -library.add(faBellSlash); -library.add(faClock); -library.add(faPodcast); -library.add(faReply); -library.add(faPen); -library.add(faClone); - const env = buildEnvironment(chromeApiSingleton); const core = new Core(env); core.load().catch(console.error); diff --git a/src/state/core.ts b/src/state/core.ts index edd5531d..69919918 100644 --- a/src/state/core.ts +++ b/src/state/core.ts @@ -7,17 +7,7 @@ import { FilteredPullRequests, filterPullRequests } from "../filtering/filters"; -import { PullRequestReference, RepoReference } from "../github-api/api"; -import { LoadedState, PullRequest } from "../storage/loaded-state"; -import { - addMute, - MuteConfiguration, - MuteType, - NOTHING_MUTED, - removeOwnerMute, - removePullRequestMute, - removeRepositoryMute -} from "../storage/mute-configuration"; +import { LoadedState } from "../storage/loaded-state"; export class Core { private readonly context: Context; @@ -26,8 +16,6 @@ export class Core { @observable refreshing = false; @observable token: string | null = null; @observable loadedState: LoadedState | null = null; - @observable muteConfiguration = NOTHING_MUTED; - @observable notifiedPullRequestUrls = new Set(); @observable lastError: string | null = null; constructor(context: Context) { @@ -39,9 +27,6 @@ export class Core { this.load(); } }); - this.context.notifier.registerClickListener((url) => - this.openPullRequest(url).catch(console.error) - ); } async load() { @@ -49,19 +34,12 @@ export class Core { if (this.token !== null) { this.refreshing = await this.context.store.currentlyRefreshing.load(); this.lastError = await this.context.store.lastError.load(); - this.notifiedPullRequestUrls = new Set( - await this.context.store.notifiedPullRequests.load() - ); - this.muteConfiguration = - await this.context.store.muteConfiguration.load(); this.loadedState = await this.context.store.lastCheck.load(); } else { this.refreshing = false; this.lastError = null; this.token = null; this.loadedState = null; - this.notifiedPullRequestUrls = new Set(); - this.muteConfiguration = NOTHING_MUTED; } this.overallStatus = "loaded"; this.updateBadge(); @@ -72,9 +50,7 @@ export class Core { await this.context.store.token.save(token); await this.saveRefreshing(false); await this.saveError(null); - await this.saveNotifiedPullRequests([]); await this.saveLoadedState(null); - await this.saveMuteConfiguration(NOTHING_MUTED); await this.load(); this.triggerBackgroundRefresh(); } @@ -97,15 +73,6 @@ export class Core { startRefreshTimestamp, ...(await this.context.githubLoader(this.token, this.loadedState)), }); - const notifyAboutPullRequests = [ - ...(this.unreviewedPullRequests || []), - ...(this.actionRequiredOwnPullRequests || []), - ]; - await this.context.notifier.notify( - notifyAboutPullRequests, - this.notifiedPullRequestUrls - ); - await this.saveNotifiedPullRequests(notifyAboutPullRequests); this.saveError(null); } catch (e: any) { this.saveError(e.message); @@ -121,59 +88,6 @@ export class Core { await this.context.tabOpener.openPullRequest(pullRequestUrl); } - async mutePullRequest(pullRequest: PullRequestReference, muteType: MuteType) { - await this.saveMuteConfiguration( - addMute(this.context, this.muteConfiguration, pullRequest, muteType) - ); - this.updateBadge(); - } - - async unmutePullRequest(pullRequest: PullRequestReference) { - await this.saveMuteConfiguration( - removePullRequestMute(this.muteConfiguration, pullRequest) - ); - this.updateBadge(); - } - - async unmuteOwner(owner: string) { - await this.saveMuteConfiguration( - removeOwnerMute(this.muteConfiguration, owner) - ); - this.updateBadge(); - } - - async unmuteRepository(repo: RepoReference) { - await this.saveMuteConfiguration( - removeRepositoryMute(this.muteConfiguration, repo) - ); - this.updateBadge(); - } - - async toggleNewCommitsNotificationSetting() { - await this.saveMuteConfiguration({ - ...this.muteConfiguration, - notifyNewCommits: !this.muteConfiguration.notifyNewCommits, - }); - this.updateBadge(); - } - - async toggleOnlyDirectRequestsSetting() { - const newOnlyDirectRequests = !this.muteConfiguration.onlyDirectRequests; - await this.saveMuteConfiguration({ - ...this.muteConfiguration, - onlyDirectRequests: newOnlyDirectRequests, - }); - this.updateBadge(); - } - - async onChangeWhitelistedTeamsSetting(teams: string[]) { - await this.saveMuteConfiguration({ - ...this.muteConfiguration, - whitelistedTeams: teams, - }); - this.updateBadge(); - } - @computed get filteredPullRequests(): FilteredPullRequests | null { const lastCheck = this.loadedState; @@ -181,17 +95,15 @@ export class Core { return null; } return filterPullRequests( - this.context, lastCheck.userLogin, lastCheck.openPullRequests, - this.muteConfiguration ); } @computed get unreviewedPullRequests(): EnrichedPullRequest[] | null { return this.filteredPullRequests - ? this.filteredPullRequests[Filter.INCOMING] + ? this.filteredPullRequests[Filter.NEEDS_REVIEW] : null; } @@ -201,18 +113,11 @@ export class Core { ? this.filteredPullRequests[Filter.MINE].filter( (pr) => pr.state.kind === "outgoing" && - (pr.state.approvedByEveryone || pr.state.changesRequested) + (pr.state.approved || pr.state.changesRequested) ) : null; } - private async saveNotifiedPullRequests(pullRequests: PullRequest[]) { - this.notifiedPullRequestUrls = new Set(pullRequests.map((p) => p.htmlUrl)); - await this.context.store.notifiedPullRequests.save( - Array.from(this.notifiedPullRequestUrls) - ); - } - private async saveError(error: string | null) { this.lastError = error; await this.context.store.lastError.save(error); @@ -228,11 +133,6 @@ export class Core { await this.context.store.lastCheck.save(lastCheck); } - private async saveMuteConfiguration(muteConfiguration: MuteConfiguration) { - this.muteConfiguration = muteConfiguration; - await this.context.store.muteConfiguration.save(muteConfiguration); - } - private updateBadge() { let badgeState: BadgeState; const unreviewedPullRequests = this.unreviewedPullRequests; @@ -271,6 +171,19 @@ export class Core { } } + triggerRestart() { + this.context.messenger.send({ + kind: "restart", + }); + + // Note: this is a hack in place because outside of a Chrome extension (ie + // when developing with webpack dev server), we don't have a background + // script that will refresh. + if (process.env.NODE_ENV === "development") { + this.refreshPullRequests().catch(console.error); + } + } + private triggerReload() { this.context.messenger.send({ kind: "reload", diff --git a/src/storage/api.ts b/src/storage/api.ts index bb45b8d5..e993f0b1 100644 --- a/src/storage/api.ts +++ b/src/storage/api.ts @@ -1,5 +1,4 @@ import { LoadedState } from "../storage/loaded-state"; -import { MuteConfiguration } from "./mute-configuration"; export interface Store { /** @@ -17,11 +16,6 @@ export interface Store { */ currentlyRefreshing: ValueStorage; - /** - * Storage of the currently muted pull requests. - */ - muteConfiguration: ValueStorage; - /** * Storage of the URLs of pull requests that we have already notified the user about. * @@ -34,11 +28,6 @@ export interface Store { * Storage of the user's provided GitHub token. */ token: ValueStorage; - - /** - * Storage of the last timestamp we requested the "tabs" permission. - */ - lastRequestForTabsPermission: ValueStorage; } export interface ValueStorage { diff --git a/src/storage/implementation.ts b/src/storage/implementation.ts index 316251e3..f3f3e22c 100644 --- a/src/storage/implementation.ts +++ b/src/storage/implementation.ts @@ -5,7 +5,6 @@ import { chromeValueStorageWithDefault, } from "./internal/chrome-value-storage"; import { LoadedState } from "./loaded-state"; -import { NOTHING_MUTED } from "./mute-configuration"; export function buildStore(chromeApi: ChromeApi): Store { return { @@ -16,20 +15,11 @@ export function buildStore(chromeApi: ChromeApi): Store { "currentlyRefreshing", false ), - muteConfiguration: chromeValueStorageWithDefault( - chromeApi, - "mute", - NOTHING_MUTED - ), notifiedPullRequests: chromeValueStorageWithDefault( chromeApi, "lastSeenPullRequests", [] ), token: chromeValueStorage(chromeApi, "gitHubApiToken"), - lastRequestForTabsPermission: chromeValueStorage( - chromeApi, - "lastRequestForTabsPermission" - ), }; } diff --git a/src/storage/loaded-state.ts b/src/storage/loaded-state.ts index 623fcdd9..80a6ddd5 100644 --- a/src/storage/loaded-state.ts +++ b/src/storage/loaded-state.ts @@ -66,7 +66,6 @@ export interface PullRequest { }; title: string; draft?: boolean; - mergeable?: boolean; /** * Whether a review is requested from the current user. */ @@ -79,10 +78,10 @@ export interface PullRequest { requestedTeams?: string[]; reviews: Review[]; comments: Comment[]; - commits?: Commit[]; reviewDecision: ReviewDecision; checkStatus?: CheckStatus; + isMerged: boolean; } export interface Comment { diff --git a/src/storage/mute-configuration.spec.ts b/src/storage/mute-configuration.spec.ts deleted file mode 100644 index d93309c1..00000000 --- a/src/storage/mute-configuration.spec.ts +++ /dev/null @@ -1,410 +0,0 @@ -import { buildTestingEnvironment } from "../environment/testing/fake"; -import { PullRequestReference, RepoReference } from "../github-api/api"; -import { - addMute, - MuteConfiguration, - removeOwnerMute, - removePullRequestMute, - removeRepositoryMute, -} from "./mute-configuration"; - -const FAKE_CURRENT_TIME = 3; - -const REPO: RepoReference = { - owner: "zenclabs", - name: "prmonitor", -}; - -const PR: PullRequestReference = { - repo: REPO, - number: 1, -}; - -const OTHER_PR_SAME_REPO: PullRequestReference = { - repo: REPO, - number: 2, -}; - -const OTHER_PR_DIFFERENT_REPO: PullRequestReference = { - repo: { - owner: "zenclabs", - name: "spot", - }, - number: 1, -}; - -const OTHER_PR_DIFFERENT_OWNER: PullRequestReference = { - repo: { - owner: "fwouts", - name: "prmonitor", - }, - number: 1, -}; - -describe("MuteConfiguration", () => { - describe("addMute", () => { - test("next-update", () => { - const env = buildTestingEnvironment(); - env.currentTime = FAKE_CURRENT_TIME; - expect(addMute(env, createEmptyConfig(), PR, "next-update")).toEqual({ - mutedPullRequests: [ - { - ...PR, - until: { - kind: "next-update", - mutedAtTimestamp: FAKE_CURRENT_TIME, - }, - }, - ], - ignored: {}, - }); - }); - test("1-hour", () => { - const env = buildTestingEnvironment(); - env.currentTime = FAKE_CURRENT_TIME; - expect(addMute(env, createEmptyConfig(), PR, "1-hour")).toEqual({ - mutedPullRequests: [ - { - ...PR, - until: { - kind: "specific-time", - unmuteAtTimestamp: FAKE_CURRENT_TIME + 3600 * 1000, - }, - }, - ], - ignored: {}, - }); - }); - test("forever", () => { - const env = buildTestingEnvironment(); - env.currentTime = FAKE_CURRENT_TIME; - expect(addMute(env, createEmptyConfig(), PR, "forever")).toEqual({ - mutedPullRequests: [ - { - ...PR, - until: { - kind: "forever", - }, - }, - ], - ignored: {}, - }); - }); - test("repo", () => { - const env = buildTestingEnvironment(); - env.currentTime = FAKE_CURRENT_TIME; - expect(addMute(env, createEmptyConfig(), PR, "repo")).toEqual({ - mutedPullRequests: [], - ignored: { - zenclabs: { - kind: "ignore-only", - repoNames: ["prmonitor"], - }, - }, - }); - }); - test("owner", () => { - const env = buildTestingEnvironment(); - env.currentTime = FAKE_CURRENT_TIME; - expect(addMute(env, createEmptyConfig(), PR, "owner")).toEqual({ - mutedPullRequests: [], - ignored: { - zenclabs: { - kind: "ignore-all", - }, - }, - }); - }); - it("does not duplicate", () => { - const env = buildTestingEnvironment(); - env.currentTime = 1; - - let muteConfiguration = createEmptyConfig(); - - // Mute for an hour. - muteConfiguration = addMute(env, muteConfiguration, PR, "1-hour"); - expect(muteConfiguration).toEqual({ - mutedPullRequests: [ - { - ...PR, - until: { - kind: "specific-time", - unmuteAtTimestamp: 3600001, - }, - }, - ], - ignored: {}, - }); - - // One hour is elapsed. - env.currentTime = 4000000; - - // Mute until next update. - muteConfiguration = addMute(env, muteConfiguration, PR, "next-update"); - - // Ensure there is only one mute for this pull request. - expect(muteConfiguration).toEqual({ - mutedPullRequests: [ - { - ...PR, - until: { - kind: "next-update", - mutedAtTimestamp: 4000000, - }, - }, - ], - ignored: {}, - }); - }); - }); - - describe("removePullRequestMute", () => { - it("removes matching mutes", () => { - expect( - removePullRequestMute( - { - mutedPullRequests: [ - { - ...PR, - until: { - kind: "next-update", - mutedAtTimestamp: 1, - }, - }, - { - ...OTHER_PR_SAME_REPO, - until: { - kind: "forever", - }, - }, - { - ...OTHER_PR_DIFFERENT_REPO, - until: { - kind: "forever", - }, - }, - { - ...PR, - until: { - kind: "forever", - }, - }, - { - ...OTHER_PR_DIFFERENT_OWNER, - until: { - kind: "forever", - }, - }, - ], - ignored: {}, - }, - PR - ) - ).toEqual({ - mutedPullRequests: [ - { - ...OTHER_PR_SAME_REPO, - until: { - kind: "forever", - }, - }, - { - ...OTHER_PR_DIFFERENT_REPO, - until: { - kind: "forever", - }, - }, - { - ...OTHER_PR_DIFFERENT_OWNER, - until: { - kind: "forever", - }, - }, - ], - ignored: {}, - }); - }); - it("does not fail when empty", () => { - expect(removePullRequestMute(createEmptyConfig(), PR)).toEqual({ - mutedPullRequests: [], - ignored: {}, - }); - }); - }); - - describe("removeOwnerMute", () => { - it("removes when ignore-all", () => { - expect( - removeOwnerMute( - { - mutedPullRequests: [], - ignored: { - zenclabs: { - kind: "ignore-all", - }, - fwouts: { - kind: "ignore-only", - repoNames: ["codetree"], - }, - }, - }, - "zenclabs" - ) - ).toEqual({ - mutedPullRequests: [], - ignored: { - fwouts: { - kind: "ignore-only", - repoNames: ["codetree"], - }, - }, - }); - }); - it("removes when ignore-only", () => { - expect( - removeOwnerMute( - { - mutedPullRequests: [], - ignored: { - zenclabs: { - kind: "ignore-only", - repoNames: ["prmonitor"], - }, - fwouts: { - kind: "ignore-only", - repoNames: ["codetree"], - }, - }, - }, - "zenclabs" - ) - ).toEqual({ - mutedPullRequests: [], - ignored: { - fwouts: { - kind: "ignore-only", - repoNames: ["codetree"], - }, - }, - }); - }); - it("does not fail when empty", () => { - expect( - removeOwnerMute( - { - mutedPullRequests: [], - }, - "zenclabs" - ) - ).toEqual({ - mutedPullRequests: [], - ignored: {}, - }); - }); - }); - - describe("removeRepositoryMute", () => { - it("removes only repository when there are more ignored repositories", () => { - expect( - removeRepositoryMute( - { - mutedPullRequests: [], - ignored: { - zenclabs: { - kind: "ignore-only", - repoNames: ["spot", "prmonitor"], - }, - fwouts: { - kind: "ignore-only", - repoNames: ["codetree"], - }, - }, - }, - REPO - ) - ).toEqual({ - mutedPullRequests: [], - ignored: { - zenclabs: { - kind: "ignore-only", - repoNames: ["spot"], - }, - fwouts: { - kind: "ignore-only", - repoNames: ["codetree"], - }, - }, - }); - }); - it("removes ignore configuration when no more ignored repositories", () => { - expect( - removeRepositoryMute( - { - mutedPullRequests: [], - ignored: { - fwouts: { - kind: "ignore-only", - repoNames: ["codetree"], - }, - }, - }, - REPO - ) - ).toEqual({ - mutedPullRequests: [], - ignored: { - fwouts: { - kind: "ignore-only", - repoNames: ["codetree"], - }, - }, - }); - }); - it("defaults to removing when ignore-all", () => { - expect( - removeRepositoryMute( - { - mutedPullRequests: [], - ignored: { - zenclabs: { - kind: "ignore-all", - }, - fwouts: { - kind: "ignore-only", - repoNames: ["codetree"], - }, - }, - }, - REPO - ) - ).toEqual({ - mutedPullRequests: [], - ignored: { - fwouts: { - kind: "ignore-only", - repoNames: ["codetree"], - }, - }, - }); - }); - it("does not fail when empty", () => { - expect( - removeRepositoryMute( - { - mutedPullRequests: [], - }, - REPO - ) - ).toEqual({ - mutedPullRequests: [], - ignored: {}, - }); - }); - }); -}); - -function createEmptyConfig(): MuteConfiguration { - return { - mutedPullRequests: [], - }; -} diff --git a/src/storage/mute-configuration.ts b/src/storage/mute-configuration.ts deleted file mode 100644 index bd9f8fd3..00000000 --- a/src/storage/mute-configuration.ts +++ /dev/null @@ -1,253 +0,0 @@ -import assertNever from "assert-never"; -import cloneDeep from "lodash/cloneDeep"; -import { Context } from "../environment/api"; -import { PullRequestReference, RepoReference } from "../github-api/api"; - -export const NOTHING_MUTED: MuteConfiguration = { - mutedPullRequests: [], - ignored: {}, - notifyNewCommits: false, - onlyDirectRequests: false, - whitelistedTeams: [], -}; - -export interface MuteConfiguration { - // Note: it's expected that a specific pull request only occurs once in the - // list. - // - // We could have used a dictionary keyed by owner/name/number instead, but - // hey, this was hacked up too quickly and now it's persisted in people's - // local storage so let's leave it for now until there's a pressing need for a - // breaking change (we could do a smooth transition with a bit of effort). - mutedPullRequests: MutedPullRequest[]; - - ignored?: { - [owner: string]: IgnoreConfiguration; - }; - - notifyNewCommits?: boolean; - - onlyDirectRequests?: boolean; - - whitelistedTeams?: string[]; -} - -export function addMute( - context: Context, - config: MuteConfiguration, - pullRequest: PullRequestReference, - muteType: MuteType -): MuteConfiguration { - const muteConfiguration = { - // Remove any previous mute of this PR. - mutedPullRequests: [ - ...config.mutedPullRequests.filter( - (pr) => - pr.repo.owner !== pullRequest.repo.owner || - pr.repo.name !== pullRequest.repo.name || - pr.number !== pullRequest.number - ), - ], - ignored: cloneDeep(config.ignored || {}), - }; - // Add the new mute. - switch (muteType) { - case "next-comment-by-author": - muteConfiguration.mutedPullRequests.push({ - ...pullRequest, - until: { - kind: "next-comment-by-author", - mutedAtTimestamp: context.getCurrentTime(), - }, - }); - break; - case "next-update": - muteConfiguration.mutedPullRequests.push({ - ...pullRequest, - until: { - kind: "next-update", - mutedAtTimestamp: context.getCurrentTime(), - }, - }); - break; - case "not-draft": - muteConfiguration.mutedPullRequests.push({ - ...pullRequest, - until: { - kind: "not-draft", - }, - }); - break; - case "1-hour": - muteConfiguration.mutedPullRequests.push({ - ...pullRequest, - until: { - kind: "specific-time", - unmuteAtTimestamp: context.getCurrentTime() + 3600 * 1000, - }, - }); - break; - case "forever": - muteConfiguration.mutedPullRequests.push({ - ...pullRequest, - until: { - kind: "forever", - }, - }); - break; - case "repo": - const existingIgnoreConfig = - muteConfiguration.ignored[pullRequest.repo.owner]; - if (existingIgnoreConfig && existingIgnoreConfig.kind === "ignore-only") { - existingIgnoreConfig.repoNames.push(pullRequest.repo.name); - } else { - muteConfiguration.ignored[pullRequest.repo.owner] = { - kind: "ignore-only", - repoNames: [pullRequest.repo.name], - }; - } - break; - case "owner": - muteConfiguration.ignored[pullRequest.repo.owner] = { - kind: "ignore-all", - }; - break; - default: - throw assertNever(muteType); - } - return muteConfiguration; -} - -export function removePullRequestMute( - config: MuteConfiguration, - pullRequest: PullRequestReference -): MuteConfiguration { - return { - mutedPullRequests: config.mutedPullRequests.filter( - (pr) => - pr.repo.owner !== pullRequest.repo.owner || - pr.repo.name !== pullRequest.repo.name || - pr.number !== pullRequest.number - ), - ignored: config.ignored || {}, - }; -} - -export function removeOwnerMute( - config: MuteConfiguration, - owner: string -): MuteConfiguration { - const ignored = cloneDeep(config.ignored || {}); - delete ignored[owner]; - return { - ...config, - ignored, - }; -} - -export function removeRepositoryMute( - config: MuteConfiguration, - repo: RepoReference -): MuteConfiguration { - const ignored = cloneDeep(config.ignored || {}); - const ownerConfig = ignored[repo.owner]; - if (ownerConfig) { - switch (ownerConfig.kind) { - case "ignore-all": - delete ignored[repo.owner]; - break; - case "ignore-only": - ownerConfig.repoNames = ownerConfig.repoNames.filter( - (repoName) => repoName !== repo.name - ); - if (ownerConfig.repoNames.length === 0) { - delete ignored[repo.owner]; - } - break; - } - } - return { - ...config, - ignored, - }; -} - -export type MuteType = - | "next-update" - | "next-comment-by-author" - | "1-hour" - | "not-draft" - | "forever" - | "repo" - | "owner"; - -export interface MutedPullRequest { - repo: { - owner: string; - name: string; - }; - number: number; - until: MutedUntil; -} - -export type MutedUntil = - | MutedUntilNextUpdateByAuthor - | MutedUntilNextCommentByAuthor - | MutedUntilNotDraft - | MutedUntilSpecificTime - | MutedForever; - -export interface MutedUntilNextUpdateByAuthor { - kind: "next-update"; - - /** - * The timestamp at which the PR was muted. - * - * Any update (commit or comment by the author) after this timestamp will make - * the PR re-appear. - */ - mutedAtTimestamp: number; -} - -export interface MutedUntilNextCommentByAuthor { - kind: "next-comment-by-author"; - - /** - * The timestamp at which the PR was muted. - * - * Any comment by the author after this timestamp will make the PR re-appear. - */ - mutedAtTimestamp: number; -} - -export interface MutedUntilNotDraft { - kind: "not-draft"; -} - -export interface MutedUntilSpecificTime { - kind: "specific-time"; - - /** - * The timestamp at which the PR should no longer be muted. - * - * The PR will stay muted even if the author updates it. - */ - unmuteAtTimestamp: number; -} - -export interface MutedForever { - kind: "forever"; -} - -export type IgnoreConfiguration = - | IgnoreConfigurationAllRepositories - | IgnoreConfigurationSpecificRepositories; - -export interface IgnoreConfigurationAllRepositories { - kind: "ignore-all"; -} - -export interface IgnoreConfigurationSpecificRepositories { - kind: "ignore-only"; - repoNames: string[]; -} diff --git a/src/tabs/implementation.ts b/src/tabs/implementation.ts index ac686435..44643eed 100644 --- a/src/tabs/implementation.ts +++ b/src/tabs/implementation.ts @@ -1,64 +1,12 @@ import { ChromeApi } from "../chrome/api"; -import { Store } from "../storage/api"; import { TabOpener } from "./api"; -export function buildTabOpener( - chromeApi: ChromeApi, - store: Store, - getCurrentTime: () => number -): TabOpener { +export function buildTabOpener(chromeApi: ChromeApi): TabOpener { return { async openPullRequest(pullRequestUrl: string) { - const lastRequestTimestamp = - await store.lastRequestForTabsPermission.load(); - if (lastRequestTimestamp !== null) { - // We requested the permission before already. Let's not be persistent. - chromeApi.permissions.getAll((permissions) => { - const granted = permissions.permissions - ? permissions.permissions.indexOf("tabs") !== -1 - : false; - openTab(chromeApi, pullRequestUrl, granted); - }); - } else { - await store.lastRequestForTabsPermission.save(getCurrentTime()); - chromeApi.permissions.request( - { - permissions: ["tabs"], - }, - (granted) => { - openTab(chromeApi, pullRequestUrl, granted); - } - ); - } + chromeApi.tabs.create({ + url: pullRequestUrl, + }); }, }; } - -function openTab( - chromeApi: ChromeApi, - pullRequestUrl: string, - permissionGranted: boolean -) { - if (permissionGranted) { - chromeApi.tabs.query({}, (tabs) => { - const existingTab = tabs.find((tab) => - tab.url ? tab.url.startsWith(pullRequestUrl) : false - ); - if (existingTab) { - chromeApi.tabs.highlight({ - windowId: existingTab.windowId, - tabs: existingTab.index, - }); - chromeApi.windows.update(existingTab.windowId, { focused: true }); - } else { - chromeApi.tabs.create({ - url: pullRequestUrl, - }); - } - }); - } else { - chromeApi.tabs.create({ - url: pullRequestUrl, - }); - } -} diff --git a/src/testing/fake-pr.ts b/src/testing/fake-pr.ts index 10cd1559..1ed74dd9 100644 --- a/src/testing/fake-pr.ts +++ b/src/testing/fake-pr.ts @@ -28,7 +28,6 @@ class FakePullRequestBuilder { private _reviews: Review[] = []; private _commits: Commit[] = []; private _draft = false; - private _mergeable = false; private _nextTimestamp = 1; @@ -54,11 +53,6 @@ class FakePullRequestBuilder { return this; } - mergeable() { - this._mergeable = true; - return this; - } - reviewRequested(logins: string[], teams?: string[]) { this._reviewerLogins = logins; this._reviewerTeams = teams || []; @@ -121,7 +115,6 @@ class FakePullRequestBuilder { pullRequestNumber: this._ref.number, nodeId: `${this._ref.repo.owner}/${this._ref.repo.name}/${this._ref.number}`, draft: this._draft, - mergeable: this._mergeable, updatedAt: "1 June 2019", htmlUrl: `http://github.com/${this._ref.repo.owner}/${this._ref.repo.name}/${this._ref.number}`, reviewRequested: reviewRequested, @@ -129,7 +122,6 @@ class FakePullRequestBuilder { requestedTeams: this._reviewerTeams, comments: this._comments, reviews: this._reviews, - commits: this._commits, reviewDecision: "REVIEW_REQUIRED", }; } diff --git a/webpack.config.js b/webpack.config.js index 4527dcd7..d0b60276 100644 --- a/webpack.config.js +++ b/webpack.config.js @@ -32,7 +32,7 @@ module.exports = { ], }, entry: { - background: "./src/background.ts", + background: "./background.ts", popup: "./src/popup.tsx", }, resolve: { @@ -50,7 +50,7 @@ module.exports = { }, compress: true, hot: true, - port: 9000, + port: 9001, }, plugins: [ new HtmlWebpackPlugin({ diff --git a/yarn.lock b/yarn.lock index e2879507..514786b6 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1056,32 +1056,6 @@ minimatch "^3.1.2" strip-json-comments "^3.1.1" -"@fortawesome/fontawesome-common-types@6.1.2": - version "6.1.2" - resolved "https://registry.yarnpkg.com/@fortawesome/fontawesome-common-types/-/fontawesome-common-types-6.1.2.tgz#c1095b1bbabf19f37f9ff0719db38d92a410bcfe" - integrity sha512-wBaAPGz1Awxg05e0PBRkDRuTsy4B3dpBm+zreTTyd9TH4uUM27cAL4xWyWR0rLJCrRwzVsQ4hF3FvM6rqydKPA== - -"@fortawesome/fontawesome-svg-core@^6.1.2": - version "6.1.2" - resolved "https://registry.yarnpkg.com/@fortawesome/fontawesome-svg-core/-/fontawesome-svg-core-6.1.2.tgz#11e2e8583a7dea75d734e4d0e53d91c63fae7511" - integrity sha512-853G/Htp0BOdXnPoeCPTjFrVwyrJHpe8MhjB/DYE9XjwhnNDfuBCd3aKc2YUYbEfHEcBws4UAA0kA9dymZKGjA== - dependencies: - "@fortawesome/fontawesome-common-types" "6.1.2" - -"@fortawesome/free-solid-svg-icons@^6.1.2": - version "6.1.2" - resolved "https://registry.yarnpkg.com/@fortawesome/free-solid-svg-icons/-/free-solid-svg-icons-6.1.2.tgz#491d668b8a6603698d0ce1ac620f66fd22b74c84" - integrity sha512-lTgZz+cMpzjkHmCwOG3E1ilUZrnINYdqMmrkv30EC3XbRsGlbIOL8H9LaNp5SV4g0pNJDfQ4EdTWWaMvdwyLiQ== - dependencies: - "@fortawesome/fontawesome-common-types" "6.1.2" - -"@fortawesome/react-fontawesome@^0.2.0": - version "0.2.0" - resolved "https://registry.yarnpkg.com/@fortawesome/react-fontawesome/-/react-fontawesome-0.2.0.tgz#d90dd8a9211830b4e3c08e94b63a0ba7291ddcf4" - integrity sha512-uHg75Rb/XORTtVt7OS9WoK8uM276Ufi7gCzshVWkUJbHhh3svsUUeqXerrM96Wm7fRiDzfKRwSoahhMIkGAYHw== - dependencies: - prop-types "^15.8.1" - "@humanwhocodes/config-array@^0.10.4": version "0.10.4" resolved "https://registry.yarnpkg.com/@humanwhocodes/config-array/-/config-array-0.10.4.tgz#01e7366e57d2ad104feea63e72248f22015c520c" @@ -1413,6 +1387,15 @@ is-plain-object "^5.0.0" universal-user-agent "^6.0.0" +"@octokit/endpoint@^9.0.0": + version "9.0.2" + resolved "https://registry.yarnpkg.com/@octokit/endpoint/-/endpoint-9.0.2.tgz#f9098bf15b893ac30c144c5e77da0322ad41b008" + integrity sha512-qhKW8YLIi+Kmc92FQUFGr++DYtkx/1fBv+Thua6baqnjnOsgBYJDCvWZR1YcINuHGOEQt416WOfE+A/oG60NBQ== + dependencies: + "@octokit/types" "^12.0.0" + is-plain-object "^5.0.0" + universal-user-agent "^6.0.0" + "@octokit/graphql@^5.0.0": version "5.0.1" resolved "https://registry.yarnpkg.com/@octokit/graphql/-/graphql-5.0.1.tgz#a06982514ad131fb6fbb9da968653b2233fade9b" @@ -1432,6 +1415,11 @@ resolved "https://registry.yarnpkg.com/@octokit/openapi-types/-/openapi-types-14.0.0.tgz#949c5019028c93f189abbc2fb42f333290f7134a" integrity sha512-HNWisMYlR8VCnNurDU6os2ikx0s0VyEjDYHNS/h4cgb8DeOxQ0n72HyinUtdDVxJhFy3FWLGl0DJhfEWk3P5Iw== +"@octokit/openapi-types@^19.0.2": + version "19.0.2" + resolved "https://registry.yarnpkg.com/@octokit/openapi-types/-/openapi-types-19.0.2.tgz#d72778fe2f6151314b6f0201fbc771bb741276fc" + integrity sha512-8li32fUDUeml/ACRp/njCWTsk5t17cfTM1jp9n08pBrqs5cDFJubtjsSnuz56r5Tad6jdEPJld7LxNp9dNcyjQ== + "@octokit/plugin-paginate-rest@^4.0.0": version "4.1.0" resolved "https://registry.yarnpkg.com/@octokit/plugin-paginate-rest/-/plugin-paginate-rest-4.1.0.tgz#670ac9ac369448c69a2371bfcd7e2b37d95534f2" @@ -1469,6 +1457,15 @@ deprecation "^2.0.0" once "^1.4.0" +"@octokit/request-error@^5.0.0": + version "5.0.1" + resolved "https://registry.yarnpkg.com/@octokit/request-error/-/request-error-5.0.1.tgz#277e3ce3b540b41525e07ba24c5ef5e868a72db9" + integrity sha512-X7pnyTMV7MgtGmiXBwmO6M5kIPrntOXdyKZLigNfQWSEQzVxR4a4vo49vJjTWX70mPndj8KhfT4Dx+2Ng3vnBQ== + dependencies: + "@octokit/types" "^12.0.0" + deprecation "^2.0.0" + once "^1.4.0" + "@octokit/request@^6.0.0": version "6.2.1" resolved "https://registry.yarnpkg.com/@octokit/request/-/request-6.2.1.tgz#3ceeb22dab09a29595d96594b6720fc14495cf4e" @@ -1481,6 +1478,17 @@ node-fetch "^2.6.7" universal-user-agent "^6.0.0" +"@octokit/request@^8.1.5": + version "8.1.5" + resolved "https://registry.yarnpkg.com/@octokit/request/-/request-8.1.5.tgz#902ae9565117a1dc410d10b5dbc44c4d27a89b71" + integrity sha512-zVKbNbX1xUluD9ZR4/tPs1yuYrK9xeh5fGZUXA6u04XGsTvomg0YO8/ZUC0FqAd49hAOEMFPAVUTh+2lBhOhLA== + dependencies: + "@octokit/endpoint" "^9.0.0" + "@octokit/request-error" "^5.0.0" + "@octokit/types" "^12.0.0" + is-plain-object "^5.0.0" + universal-user-agent "^6.0.0" + "@octokit/rest@^19.0.4": version "19.0.4" resolved "https://registry.yarnpkg.com/@octokit/rest/-/rest-19.0.4.tgz#fd8bed1cefffa486e9ae46a9dc608ce81bcfcbdd" @@ -1491,6 +1499,13 @@ "@octokit/plugin-request-log" "^1.0.4" "@octokit/plugin-rest-endpoint-methods" "^6.0.0" +"@octokit/types@^12.0.0": + version "12.3.0" + resolved "https://registry.yarnpkg.com/@octokit/types/-/types-12.3.0.tgz#e3f8bc53f65ef551e19cc1a0fea15adadec17d2d" + integrity sha512-nJ8X2HRr234q3w/FcovDlA+ttUU4m1eJAourvfUUtwAWeqL8AsyRqfnLvVnYn3NFbUnsmzQCzLNdFerPwdmcDQ== + dependencies: + "@octokit/openapi-types" "^19.0.2" + "@octokit/types@^7.0.0", "@octokit/types@^7.1.1": version "7.1.1" resolved "https://registry.yarnpkg.com/@octokit/types/-/types-7.1.1.tgz#a30fd6ca3279d59d532fa75583d65d93b7588e6d" @@ -1515,6 +1530,11 @@ resolved "https://registry.yarnpkg.com/@popperjs/core/-/core-2.11.6.tgz#cee20bd55e68a1720bdab363ecf0c821ded4cd45" integrity sha512-50/17A98tWUfQ176raKiOGXuYpLyyVMkxxG6oylzL3BPOlA6ADGdK7EYunSa4I064xerltq9TGXs8HmOk5E+vw== +"@primer/octicons-react@^19.8.0": + version "19.8.0" + resolved "https://registry.yarnpkg.com/@primer/octicons-react/-/octicons-react-19.8.0.tgz#f76ec3b3c0bea0c6ceb147d7f2d989e5f149f7e5" + integrity sha512-2Z+D7xTloFTLQVRUEbg0pQpe6aTL9RR+8RqBhjkrF+BFuVdM1ENOyjywaGEO7DIkPU5Zxlv0gxSlD85LQxL+sw== + "@react-aria/ssr@^3.2.0": version "3.3.0" resolved "https://registry.yarnpkg.com/@react-aria/ssr/-/ssr-3.3.0.tgz#25e81daf0c7a270a4a891159d8d984578e4512d8" @@ -1747,11 +1767,6 @@ resolved "https://registry.yarnpkg.com/@types/json-schema/-/json-schema-7.0.11.tgz#d421b6c527a3037f7c84433fd2c4229e016863d3" integrity sha512-wOuvG1SN4Us4rez+tylwwwCV1psiNVOkJeM3AUWUNWg/jDQY2+HE/444y5gc+jBmRqASOm2Oeh5c1axHobwRKQ== -"@types/lodash@^4.14.184": - version "4.14.184" - resolved "https://registry.yarnpkg.com/@types/lodash/-/lodash-4.14.184.tgz#23f96cd2a21a28e106dc24d825d4aa966de7a9fe" - integrity sha512-RoZphVtHbxPZizt4IcILciSWiC6dcn+eZ8oX9IWEYfDMcocdd42f7NPI6fQj+6zI8y4E0L7gu2pcZKLGTRaV9Q== - "@types/mime@*": version "3.0.1" resolved "https://registry.yarnpkg.com/@types/mime/-/mime-3.0.1.tgz#5f8f2bca0a5863cb69bc0b0acd88c96cb1d4ae10" @@ -2258,11 +2273,6 @@ array.prototype.flatmap@^1.3.0: es-abstract "^1.19.2" es-shim-unscopables "^1.0.0" -assert-never@^1.2.1: - version "1.2.1" - resolved "https://registry.yarnpkg.com/assert-never/-/assert-never-1.2.1.tgz#11f0e363bf146205fb08193b5c7b90f4d1cf44fe" - integrity sha512-TaTivMB6pYI1kXwrFlEhLeGfOqoDNdTxjCdwRfFFkEA30Eu+k48W34nlok2EYWJfFFzqaEmichdNM7th6M5HNw== - asynckit@^0.4.0: version "0.4.0" resolved "https://registry.yarnpkg.com/asynckit/-/asynckit-0.4.0.tgz#c79ed97f7f34cb8f2ba1bc9790bcc366474b4b79"