-
-
Notifications
You must be signed in to change notification settings - Fork 51
Stop auto-opening settings on browser update #127
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThe onInstalled listener now forwards installation details to the install handler, which accepts the details and passes them into the database check. The database check uses the provided InstalledDetails to decide whether to open the options page only on first start for "install" or "update". Manifests and popup version text were bumped to 6.0.1. Changes
Sequence Diagram(s)sequenceDiagram
participant Runtime as chrome.runtime
participant BG as background.ts
participant Install as install.ts
participant DB as database.ts
Runtime->>BG: onInstalled(installedDetails)
activate BG
BG->>Install: handleExtensionInstalled(installedDetails)
deactivate BG
activate Install
Install->>DB: checkIfUpdateNeeded(true, installedDetails)
deactivate Install
activate DB
DB->>DB: if firstStart && installedDetails?.reason in {"install","update"}\n -> open options page
deactivate DB
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (2)
🔇 Additional comments (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/scripts/background/database.ts (1)
38-38: Simplify the optional parameter syntax.The parameter can use TypeScript's optional parameter syntax instead of explicit union with
undefinedand default value.Apply this diff:
-export async function checkIfUpdateNeeded(firstStart = false, addonInstallReason:chrome.runtime.InstalledDetails | undefined = undefined): Promise<void> { +export async function checkIfUpdateNeeded(firstStart = false, addonInstallReason?: chrome.runtime.InstalledDetails): Promise<void> {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/scripts/background.ts(1 hunks)src/scripts/background/database.ts(2 hunks)src/scripts/background/install.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/scripts/background.ts (1)
src/scripts/background/install.ts (1)
handleExtensionInstalled(6-24)
src/scripts/background/install.ts (1)
src/scripts/background/database.ts (1)
checkIfUpdateNeeded(38-68)
🪛 Biome (2.1.2)
src/scripts/background/database.ts
[error] 65-65: Unexpected constant condition.
(lint/correctness/noConstantCondition)
🪛 GitHub Actions: CI/CD Pipeline
src/scripts/background/database.ts
[error] 65-65: ESLint: Unexpected constant condition. (no-constant-condition)
🪛 GitHub Check: Lint and Type Check
src/scripts/background/database.ts
[failure] 65-65:
Unexpected constant condition
🪛 GitHub Check: SonarCloud Code Analysis
src/scripts/background/database.ts
[warning] 65-65: Prefer using an optional chain expression instead, as it's more concise and easier to read.
🔇 Additional comments (2)
src/scripts/background.ts (1)
24-26: LGTM - Correctly captures installation reason.The listener now properly captures and forwards the installation reason to the handler function.
src/scripts/background/install.ts (1)
6-6: LGTM - Properly threads installation reason.The function signature and call to
checkIfUpdateNeededcorrectly propagate the installation reason through the call chain.Also applies to: 18-18
|
|
@coderabbitai generate docstrings |
|
Warning Docstrings generation is disabled for your repository or organization. |
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 4650 @shadowwwind, I'll generate JSDoc docstrings for all the functions in the modified TypeScript files. src/scripts/background/database.tsimport { API_HEADERS, DEFAULT_API_URL } from '../constants';
import { getLocal, setLocal } from '../lib/chromeStorage';
import type { DatabaseEntry } from './types';
/**
* Resolves the API URL from local storage or returns the default API URL.
*
* @returns {Promise<string>} The API URL to use for database requests
*/
export async function resolveApiUrl(): Promise<string> {
const data = await getLocal('api');
const api = data['api'];
if (typeof api === 'string' && api.length > 0) {
return api;
}
return DEFAULT_API_URL;
}
/**
* Downloads the latest database from the API and stores it in local storage.
* Sets a badge error text if the request fails.
*
* @param {string} [apiUrl] - Optional API URL to use; if not provided, resolves from storage
* @returns {Promise<void>}
*/
export async function downloadDatabase(apiUrl?: string): Promise<void> {
const targetApi = apiUrl ?? (await resolveApiUrl());
try {
const response = await fetch(`https://${targetApi}/appdb/version/v2`, {
headers: API_HEADERS,
});
if (response.status >= 300) {
chrome.action.setBadgeText({ text: `err ${response.status}` });
return;
}
const data = (await response.json()) as DatabaseEntry[];
await setLocal({
db: data,
lastModified: new Date().toISOString(),
});
} catch (error) {
console.error('Failed to download database', error);
}
}
/**
* Checks if the database needs to be updated based on the configured interval.
* Downloads a new database if needed and optionally opens the settings page on first start.
*
* @param {boolean} [firstStart=false] - Whether this is the first start after installation/update
* @param {chrome.runtime.InstalledDetails} [addonInstallReason] - Details about why the extension was installed
* @returns {Promise<void>}
*/
export async function checkIfUpdateNeeded(firstStart = false, addonInstallReason?:chrome.runtime.InstalledDetails): Promise<void> {
const data = await getLocal([
'db',
'lastModified',
'interval',
'api',
]);
const api = await resolveApiUrlFromData(data);
const db = data['db'] as DatabaseEntry[] | undefined;
const lastModifiedRaw = data['lastModified'];
if (db && typeof lastModifiedRaw === 'string') {
const intervalDays = computeIntervalDays(data['interval']);
const lastModified = new Date(lastModifiedRaw);
const today = new Date();
const diffTime = Math.abs(today.getTime() - lastModified.getTime());
const diffDays = Math.ceil(diffTime / (1000 * 60 * 60 * 24));
if (diffDays < intervalDays) {
return;
}
}
await downloadDatabase(api);
if (firstStart && addonInstallReason && ["update", "install"].includes(addonInstallReason?.reason)) {
chrome.runtime.openOptionsPage();
}
}
/**
* Computes the interval in days from a raw value stored in settings.
* Adds 1 to the stored value or returns a default of 8 days if invalid.
*
* @param {unknown} rawValue - The raw interval value from storage
* @returns {number} The computed interval in days
*/
function computeIntervalDays(rawValue: unknown): number {
const DEFAULT_INTERVAL = 8;
if (typeof rawValue === 'number') {
return rawValue + 1;
}
if (typeof rawValue === 'string') {
const parsed = Number(rawValue);
if (!Number.isNaN(parsed)) {
return parsed + 1;
}
}
return DEFAULT_INTERVAL;
}
/**
* Resolves the API URL from the provided data object or falls back to resolveApiUrl().
*
* @param {Record<string, unknown>} data - The data object containing potential API URL
* @returns {Promise<string>} The resolved API URL
*/
async function resolveApiUrlFromData(data: Record<string, unknown>): Promise<string> {
const api = data['api'];
if (typeof api === 'string' && api.length > 0) {
return api;
}
return resolveApiUrl();
}src/scripts/background/install.tsimport { setLocal } from '../lib/chromeStorage';
import { checkIfUpdateNeeded } from './database';
import { donationReminderAllowed } from './donation';
import { initializePageAction } from './pageAction';
/**
* Handles the extension installation or update event.
* Initializes default settings, checks for database updates, and initializes the page action for the active tab.
*
* @param {chrome.runtime.InstalledDetails} reason - Details about why the extension was installed (install, update, etc.)
* @returns {Promise<void>}
*/
export async function handleExtensionInstalled(reason:chrome.runtime.InstalledDetails): Promise<void> {
const donationAllowed = donationReminderAllowed(navigator.userAgent);
await setLocal({
themeHeader: true,
sentry: false,
displayDonationReminder: {
active: false,
allowedPlattform: donationAllowed,
},
});
await checkIfUpdateNeeded(true, reason);
const [activeTab] = await queryActiveTab();
if (activeTab) {
await initializePageAction(activeTab);
}
}
/**
* Queries for the currently active tab in the current window.
*
* @returns {Promise<chrome.tabs.Tab[]>} Array containing the active tab (if any)
*/
async function queryActiveTab(): Promise<chrome.tabs.Tab[]> {
return new Promise((resolve) => {
chrome.tabs.query({ active: true, currentWindow: true }, (tabs) => {
resolve(tabs);
});
});
}These docstrings provide clear documentation for:
The docstrings follow JSDoc conventions and will be recognized by TypeScript tooling for IntelliSense and documentation generation. |



fixes #126
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.