Address code review feedback: add cache expiration, error handling, and GitHub Enterprise support
Co-authored-by: blackboxprogramming <118287761+blackboxprogramming@users.noreply.github.com>
This commit is contained in:
@@ -174,7 +174,12 @@ function createEmojiAgentRouter(options = {}) {
|
|||||||
};
|
};
|
||||||
|
|
||||||
if (onEscalation) {
|
if (onEscalation) {
|
||||||
|
try {
|
||||||
await onEscalation(context, eventData);
|
await onEscalation(context, eventData);
|
||||||
|
} catch (error) {
|
||||||
|
console.error(`⚠️ Escalation callback error: ${error.message}`);
|
||||||
|
result.data.callbackError = error.message;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
console.log(
|
console.log(
|
||||||
@@ -218,7 +223,12 @@ function createEmojiAgentRouter(options = {}) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
if (onStatusUpdate) {
|
if (onStatusUpdate) {
|
||||||
|
try {
|
||||||
await onStatusUpdate(context, eventData);
|
await onStatusUpdate(context, eventData);
|
||||||
|
} catch (error) {
|
||||||
|
console.error(`⚠️ Status update callback error: ${error.message}`);
|
||||||
|
result.data.callbackError = error.message;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
console.log(
|
console.log(
|
||||||
|
|||||||
@@ -158,19 +158,36 @@ function reactionToStatus(reaction) {
|
|||||||
/**
|
/**
|
||||||
* Create a project status sync handler
|
* Create a project status sync handler
|
||||||
* @param {Object} octokit - GitHub Octokit instance
|
* @param {Object} octokit - GitHub Octokit instance
|
||||||
|
* @param {Object} options - Options
|
||||||
|
* @param {number} options.cacheTTL - Cache TTL in milliseconds (default: 5 minutes)
|
||||||
* @returns {Object} - Handler methods
|
* @returns {Object} - Handler methods
|
||||||
*/
|
*/
|
||||||
function createProjectStatusSync(octokit) {
|
function createProjectStatusSync(octokit, options = {}) {
|
||||||
|
const cacheTTL = options.cacheTTL || 5 * 60 * 1000; // Default 5 minutes
|
||||||
let cachedProjectFields = null;
|
let cachedProjectFields = null;
|
||||||
let cachedProjectId = null;
|
let cachedProjectId = null;
|
||||||
|
let cacheTimestamp = null;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Clear the project fields cache
|
||||||
|
*/
|
||||||
|
function clearCache() {
|
||||||
|
cachedProjectFields = null;
|
||||||
|
cachedProjectId = null;
|
||||||
|
cacheTimestamp = null;
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Fetch and cache project fields
|
* Fetch and cache project fields
|
||||||
* @param {string} projectId - GitHub Project node ID
|
* @param {string} projectId - GitHub Project node ID
|
||||||
|
* @param {boolean} forceRefresh - Force refresh the cache
|
||||||
* @returns {Promise<Array>} - Project fields
|
* @returns {Promise<Array>} - Project fields
|
||||||
*/
|
*/
|
||||||
async function getProjectFields(projectId) {
|
async function getProjectFields(projectId, forceRefresh = false) {
|
||||||
if (cachedProjectId === projectId && cachedProjectFields) {
|
const now = Date.now();
|
||||||
|
const cacheExpired = cacheTimestamp && (now - cacheTimestamp) > cacheTTL;
|
||||||
|
|
||||||
|
if (!forceRefresh && !cacheExpired && cachedProjectId === projectId && cachedProjectFields) {
|
||||||
return cachedProjectFields;
|
return cachedProjectFields;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -180,6 +197,7 @@ function createProjectStatusSync(octokit) {
|
|||||||
|
|
||||||
cachedProjectId = projectId;
|
cachedProjectId = projectId;
|
||||||
cachedProjectFields = result.node?.fields?.nodes || [];
|
cachedProjectFields = result.node?.fields?.nodes || [];
|
||||||
|
cacheTimestamp = now;
|
||||||
return cachedProjectFields;
|
return cachedProjectFields;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -306,6 +324,7 @@ function createProjectStatusSync(octokit) {
|
|||||||
syncIssueStatusFromEmoji,
|
syncIssueStatusFromEmoji,
|
||||||
syncIssueStatusFromReaction,
|
syncIssueStatusFromReaction,
|
||||||
findStatusFieldValue,
|
findStatusFieldValue,
|
||||||
|
clearCache,
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -8,6 +8,10 @@ async function main() {
|
|||||||
const token = process.env.GITHUB_TOKEN;
|
const token = process.env.GITHUB_TOKEN;
|
||||||
const dryRun = process.env.DRY_RUN === "true";
|
const dryRun = process.env.DRY_RUN === "true";
|
||||||
|
|
||||||
|
// Support GitHub Enterprise with GITHUB_API_URL, default to public GitHub
|
||||||
|
const apiUrl = process.env.GITHUB_API_URL || "https://api.github.com";
|
||||||
|
const graphqlUrl = `${apiUrl}/graphql`;
|
||||||
|
|
||||||
if (!token) {
|
if (!token) {
|
||||||
console.error("❌ GITHUB_TOKEN environment variable is required");
|
console.error("❌ GITHUB_TOKEN environment variable is required");
|
||||||
process.exit(1);
|
process.exit(1);
|
||||||
@@ -24,11 +28,12 @@ async function main() {
|
|||||||
|
|
||||||
console.log(`📊 Generating weekly emoji digest for ${owner}/${repoName}`);
|
console.log(`📊 Generating weekly emoji digest for ${owner}/${repoName}`);
|
||||||
console.log(`🔧 Dry run: ${dryRun}`);
|
console.log(`🔧 Dry run: ${dryRun}`);
|
||||||
|
console.log(`🌐 API URL: ${apiUrl}`);
|
||||||
|
|
||||||
// Create a minimal octokit-like client
|
// Create a minimal octokit-like client
|
||||||
const octokit = {
|
const octokit = {
|
||||||
graphql: async (query, variables) => {
|
graphql: async (query, variables) => {
|
||||||
const response = await fetch("https://api.github.com/graphql", {
|
const response = await fetch(graphqlUrl, {
|
||||||
method: "POST",
|
method: "POST",
|
||||||
headers: {
|
headers: {
|
||||||
Authorization: `Bearer ${token}`,
|
Authorization: `Bearer ${token}`,
|
||||||
|
|||||||
@@ -404,8 +404,13 @@ function createWeeklyEmojiDigest(octokit) {
|
|||||||
const weekStart = new Date();
|
const weekStart = new Date();
|
||||||
weekStart.setDate(weekStart.getDate() - 7);
|
weekStart.setDate(weekStart.getDate() - 7);
|
||||||
|
|
||||||
// Extract emoji counts from all issues
|
// Extract emoji counts from all issues once (optimization: reuse for escalation check)
|
||||||
const allCounts = issues.map((issue) => extractIssueEmojis(issue));
|
const issuesWithCounts = issues.map((issue) => ({
|
||||||
|
issue,
|
||||||
|
counts: extractIssueEmojis(issue),
|
||||||
|
}));
|
||||||
|
|
||||||
|
const allCounts = issuesWithCounts.map(({ counts }) => counts);
|
||||||
const overallCounts = aggregateCounts(allCounts);
|
const overallCounts = aggregateCounts(allCounts);
|
||||||
const overallHeatmap = generateHeatmap(overallCounts);
|
const overallHeatmap = generateHeatmap(overallCounts);
|
||||||
|
|
||||||
@@ -413,12 +418,10 @@ function createWeeklyEmojiDigest(octokit) {
|
|||||||
const agentGroups = groupIssuesByAgent(issues);
|
const agentGroups = groupIssuesByAgent(issues);
|
||||||
const agentHeatmaps = generateAgentHeatmaps(agentGroups);
|
const agentHeatmaps = generateAgentHeatmaps(agentGroups);
|
||||||
|
|
||||||
// Find issues with escalations
|
// Find issues with escalations (reuse pre-computed counts)
|
||||||
const topEscalations = issues
|
const topEscalations = issuesWithCounts
|
||||||
.filter((issue) => {
|
.filter(({ counts }) => counts.escalation > 0)
|
||||||
const counts = extractIssueEmojis(issue);
|
.map(({ issue }) => issue)
|
||||||
return counts.escalation > 0;
|
|
||||||
})
|
|
||||||
.slice(0, 5);
|
.slice(0, 5);
|
||||||
|
|
||||||
const digestData = {
|
const digestData = {
|
||||||
|
|||||||
@@ -136,6 +136,7 @@ describe("project-status-sync", () => {
|
|||||||
expect(sync).toHaveProperty("updateProjectItemStatus");
|
expect(sync).toHaveProperty("updateProjectItemStatus");
|
||||||
expect(sync).toHaveProperty("syncIssueStatusFromEmoji");
|
expect(sync).toHaveProperty("syncIssueStatusFromEmoji");
|
||||||
expect(sync).toHaveProperty("syncIssueStatusFromReaction");
|
expect(sync).toHaveProperty("syncIssueStatusFromReaction");
|
||||||
|
expect(sync).toHaveProperty("clearCache");
|
||||||
});
|
});
|
||||||
|
|
||||||
it("should cache project fields", async () => {
|
it("should cache project fields", async () => {
|
||||||
|
|||||||
@@ -1,10 +0,0 @@
|
|||||||
import { defineConfig } from "vitest/config";
|
|
||||||
import react from "@vitejs/plugin-react";
|
|
||||||
export default defineConfig({
|
|
||||||
plugins: [react()],
|
|
||||||
test: {
|
|
||||||
environment: "jsdom",
|
|
||||||
setupFiles: "./vitest.setup.ts",
|
|
||||||
globals: true
|
|
||||||
}
|
|
||||||
});
|
|
||||||
Reference in New Issue
Block a user