diff --git a/manifest.json b/manifest.json index 663473e4..304d129a 100644 --- a/manifest.json +++ b/manifest.json @@ -2,7 +2,8 @@ "name": "PR Monitor", "version": "0.0.0", "description": "Get notified when you receive a pull request on GitHub.", - "permissions": ["alarms", "notifications", "storage", "tabs"], + "permissions": ["alarms", "notifications", "storage"], + "optional_permissions": ["tabs"], "background": { "scripts": ["background.js"], "persistent": true diff --git a/src/chrome/fake-chrome.ts b/src/chrome/fake-chrome.ts index 4ffe314b..e1279247 100644 --- a/src/chrome/fake-chrome.ts +++ b/src/chrome/fake-chrome.ts @@ -39,6 +39,16 @@ export const fakeChrome = (>{ } } }, + permissions: { + request(_permissions, callback) { + if (callback) { + callback(true); + } + }, + getAll(callback) { + callback({}); + } + }, storage: { // To simulate chrome.storage.local, we simply fall back to the localStorage API. local: { @@ -63,6 +73,9 @@ export const fakeChrome = (>{ tabs: { query(_queryInfo, callback) { callback([]); + }, + create(properties) { + window.open(properties.url); } } }) as ChromeApi; diff --git a/src/components/Popup.tsx b/src/components/Popup.tsx index b5e666f7..7dbba180 100644 --- a/src/components/Popup.tsx +++ b/src/components/Popup.tsx @@ -128,7 +128,7 @@ export class Popup extends Component { } private onOpen = (pullRequestUrl: string) => { - this.props.core.openPullRequest(pullRequestUrl); + this.props.core.openPullRequest(pullRequestUrl).catch(console.error); }; private onMute = (pullRequest: PullRequest) => { diff --git a/src/environment/implementation.ts b/src/environment/implementation.ts index f28d8a4c..984527ef 100644 --- a/src/environment/implementation.ts +++ b/src/environment/implementation.ts @@ -8,13 +8,14 @@ import { buildTabOpener } from "../tabs/implementation"; import { Environment } from "./api"; export function buildEnvironment(chromeApi: ChromeApi): Environment { + const store = buildStore(chromeApi); return { - store: buildStore(chromeApi), + store, githubLoader: buildGitHubLoader(), notifier: buildNotifier(chromeApi), badger: buildBadger(chromeApi), messenger: buildMessenger(chromeApi), - tabOpener: buildTabOpener(chromeApi), + tabOpener: buildTabOpener(chromeApi, store), isOnline: () => navigator.onLine }; } diff --git a/src/environment/testing/fake.ts b/src/environment/testing/fake.ts index 5586395d..4c1ae85e 100644 --- a/src/environment/testing/fake.ts +++ b/src/environment/testing/fake.ts @@ -39,7 +39,8 @@ function fakeStore() { lastCheck: fakeStorage(null), muteConfiguration: fakeStorage(NOTHING_MUTED), notifiedPullRequests: fakeStorage([]), - token: fakeStorage(null) + token: fakeStorage(null), + lastRequestForTabsPermission: fakeStorage(null) }; } @@ -121,7 +122,7 @@ function fakeTabOpener() { const openedUrls: string[] = []; return { openedUrls, - openPullRequest(pullRequestUrl: string) { + async openPullRequest(pullRequestUrl: string) { openedUrls.push(pullRequestUrl); } }; diff --git a/src/state/core.ts b/src/state/core.ts index aa9b04d5..6c002456 100644 --- a/src/state/core.ts +++ b/src/state/core.ts @@ -28,7 +28,9 @@ export class Core { this.load(); } }); - this.env.notifier.registerClickListener(url => this.openPullRequest(url)); + this.env.notifier.registerClickListener(url => + this.openPullRequest(url).catch(console.error) + ); } async load() { @@ -95,8 +97,8 @@ export class Core { } } - openPullRequest(pullRequestUrl: string) { - this.env.tabOpener.openPullRequest(pullRequestUrl); + async openPullRequest(pullRequestUrl: string) { + await this.env.tabOpener.openPullRequest(pullRequestUrl); } async mutePullRequest(pullRequest: { diff --git a/src/storage/api.ts b/src/storage/api.ts index e47e2eac..229bc621 100644 --- a/src/storage/api.ts +++ b/src/storage/api.ts @@ -29,6 +29,11 @@ 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 b0136beb..a355c321 100644 --- a/src/storage/implementation.ts +++ b/src/storage/implementation.ts @@ -21,6 +21,10 @@ export function buildStore(chromeApi: ChromeApi): Store { "lastSeenPullRequests", [] ), - token: chromeValueStorage(chromeApi, "gitHubApiToken") + token: chromeValueStorage(chromeApi, "gitHubApiToken"), + lastRequestForTabsPermission: chromeValueStorage( + chromeApi, + "lastRequestForTabsPermission" + ) }; } diff --git a/src/tabs/api.ts b/src/tabs/api.ts index 25bdecf1..7c5ab07c 100644 --- a/src/tabs/api.ts +++ b/src/tabs/api.ts @@ -2,5 +2,5 @@ * TabOpener opens tabs intelligently (reusing them when possible). */ export interface TabOpener { - openPullRequest(pullRequestUrl: string): void; + openPullRequest(pullRequestUrl: string): Promise; } diff --git a/src/tabs/implementation.ts b/src/tabs/implementation.ts index 2579e4cf..89b6fedb 100644 --- a/src/tabs/implementation.ts +++ b/src/tabs/implementation.ts @@ -1,21 +1,57 @@ import { ChromeApi } from "../chrome/api"; +import { Store } from "../storage/api"; import { TabOpener } from "./api"; -export function buildTabOpener(chromeApi: ChromeApi): TabOpener { +export function buildTabOpener(chromeApi: ChromeApi, store: Store): TabOpener { return { - openPullRequest(pullRequestUrl: string) { - chromeApi.tabs.query({}, tabs => { - const existingTab = tabs.find(tab => - tab.url ? tab.url.startsWith(pullRequestUrl) : false + 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(Date.now()); + chromeApi.permissions.request( + { + permissions: ["tabs"] + }, + granted => { + openTab(chromeApi, pullRequestUrl, granted); + } ); - if (existingTab) { - chromeApi.tabs.highlight({ - tabs: existingTab.index - }); - } else { - window.open(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({ + tabs: existingTab.index + }); + } else { + chrome.tabs.create({ + url: pullRequestUrl + }); + } + }); + } else { + chrome.tabs.create({ + url: pullRequestUrl + }); + } +}