-
Notifications
You must be signed in to change notification settings - Fork 29
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
feat: avatar from github map #195
Changes from 6 commits
f295cb9
4196b7b
9e1568e
ec54146
db222ef
2688bca
5dfa1b8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,116 +1,33 @@ | ||
import { Octokit } from "@octokit/rest"; | ||
import { getGitHubAccessToken } from "../getters/get-github-access-token"; | ||
import { getImageFromCache, saveImageToCache } from "../getters/get-indexed-db"; | ||
import { renderErrorInModal } from "../rendering/display-popup-modal"; | ||
import { organizationImageCache } from "./fetch-issues-full"; | ||
import { GitHubIssue } from "../github-types"; | ||
import { taskManager } from "../home"; | ||
export const ubiquityAvatarUrl = "https://avatars.githubusercontent.com/u/76412717?v=4"; | ||
|
||
// Map to track ongoing avatar fetches | ||
const pendingFetches: Map<string, Promise<Blob | void>> = new Map(); | ||
export type OrgNameAndAvatarUrl = { | ||
ownerName: string; | ||
avatar_url?: string; | ||
}; | ||
|
||
// Fetches the avatar for a given organization from GitHub either from cache, indexedDB or GitHub API | ||
export async function fetchAvatar(orgName: string): Promise<Blob | void> { | ||
// Check if the avatar is already cached in memory | ||
const cachedAvatar = organizationImageCache.get(orgName); | ||
if (cachedAvatar) { | ||
return cachedAvatar; | ||
} | ||
|
||
// If there's a pending fetch for this organization, wait for it to complete | ||
if (pendingFetches.has(orgName)) { | ||
return pendingFetches.get(orgName); | ||
} | ||
|
||
// Start the fetch process and store the promise in the pending fetches map | ||
// It will try to fetch from IndexedDB first, then from GitHub organizations, and finally from GitHub users, returning in the first successful step | ||
const fetchPromise = (async () => { | ||
// Step 1: Try to get the avatar from IndexedDB | ||
const avatarBlob = await getImageFromCache({ dbName: "GitHubAvatars", storeName: "ImageStore", orgName: `avatarUrl-${orgName}` }); | ||
if (avatarBlob) { | ||
organizationImageCache.set(orgName, avatarBlob); // Cache it in memory | ||
return avatarBlob; | ||
} | ||
|
||
const octokit = new Octokit({ auth: await getGitHubAccessToken() }); | ||
|
||
// Step 2: No avatar in IndexedDB, fetch from GitHub | ||
try { | ||
const { | ||
data: { avatar_url: avatarUrl }, | ||
} = await octokit.rest.orgs.get({ org: orgName }); | ||
|
||
if (avatarUrl) { | ||
const response = await fetch(avatarUrl); | ||
const blob = await response.blob(); | ||
|
||
// Cache the fetched avatar in both memory and IndexedDB | ||
await saveImageToCache({ | ||
dbName: "GitHubAvatars", | ||
storeName: "ImageStore", | ||
keyName: "name", | ||
orgName: `avatarUrl-${orgName}`, | ||
avatarBlob: blob, | ||
}); | ||
export async function fetchPartnerAvatars(): Promise<OrgNameAndAvatarUrl[]> { | ||
const response = await fetch("https://raw.githubusercontent.com/ubiquity/devpool-directory/__STORAGE__/devpool-partner-avatars.json"); | ||
const jsonData = await response.json(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have mixed feelings on your approach. The normal operating procedure is to create a pull request and link it to this task. Editing a production system is highly risky and prone to breakage. In the future you should work off of your own forks for a proof of concept. ubiquity/devpool-directory@da89c2b Mixed feelings, though, because I could see myself doing the same thing given that we're the only users of this directory at the moment. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. mb, I only did that cause the change was independent from the rest of the system. so it was self contained. even if it failed it wouldnt compromise any other section, but I agree its dumb |
||
return jsonData; | ||
} | ||
|
||
organizationImageCache.set(orgName, blob); | ||
return blob; | ||
} | ||
} catch (orgError) { | ||
console.warn(`Failed to fetch avatar from organization ${orgName}: ${orgError}`); | ||
} | ||
// A global map of partner {ownerName => avatarUrl} | ||
const partnerAvatarMap = new Map<string, string>(); | ||
|
||
// Step 3: Try fetching from GitHub users if the organization lookup failed | ||
try { | ||
const { | ||
data: { avatar_url: avatarUrl }, | ||
} = await octokit.rest.users.getByUsername({ username: orgName }); | ||
export function fetchAvatar(orgName: string): string | undefined { | ||
return partnerAvatarMap.get(orgName.toLowerCase()); | ||
} | ||
|
||
export async function fetchAvatars() { | ||
try { | ||
const partnerData = await fetchPartnerAvatars(); | ||
|
||
partnerData.forEach(({ ownerName, avatar_url: avatarUrl }) => { | ||
if (avatarUrl) { | ||
const response = await fetch(avatarUrl); | ||
const blob = await response.blob(); | ||
|
||
// Cache the fetched avatar in both memory and IndexedDB | ||
await saveImageToCache({ | ||
dbName: "GitHubAvatars", | ||
storeName: "ImageStore", | ||
keyName: "name", | ||
orgName: `avatarUrl-${orgName}`, | ||
avatarBlob: blob, | ||
}); | ||
|
||
organizationImageCache.set(orgName, blob); | ||
return blob; | ||
partnerAvatarMap.set(ownerName.toLowerCase(), avatarUrl); | ||
} | ||
} catch (innerError) { | ||
renderErrorInModal(innerError as Error, `All tries failed to fetch avatar for ${orgName}: ${innerError}`); | ||
} | ||
})(); | ||
|
||
pendingFetches.set(orgName, fetchPromise); | ||
|
||
// Wait for the fetch to complete | ||
try { | ||
const result = await fetchPromise; | ||
return result; | ||
} finally { | ||
// Remove the pending fetch once it completes | ||
pendingFetches.delete(orgName); | ||
}); | ||
} catch (error) { | ||
console.error("Failed to load partner avatars:", error); | ||
} | ||
} | ||
|
||
// fetches avatars for all tasks (issues) cached. it will fetch only once per organization, remaining are returned from cache | ||
export async function fetchAvatars() { | ||
const cachedTasks = taskManager.getTasks(); | ||
|
||
// fetches avatar for each organization for each task, but fetchAvatar() will only fetch once per organization, remaining are returned from cache | ||
const avatarPromises = cachedTasks.map(async (task: GitHubIssue) => { | ||
const [orgName] = task.repository_url.split("/").slice(-2); | ||
if (orgName) { | ||
return fetchAvatar(orgName); | ||
} | ||
return Promise.resolve(); | ||
}); | ||
|
||
await Promise.allSettled(avatarPromises); | ||
} |
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.
I feel like its better practice to not make property name optional, but instead to initialize it as
null
if it is being set later in the program.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.
good catch it doesnt even need to be nullable