Skip to content

Fix event parsing in activity plugin #1760

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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Fix event parsing in activity plugin #1760

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c6bc3c8
Filter event types before parsing them
EndBug Oct 29, 2025
8c95f5f
Get commit data for PushEvent from GitHub API
EndBug Oct 29, 2025
deb1c30
Misc fixes
EndBug Oct 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 68 additions & 36 deletions source/plugins/activity/index.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@ export default async function({login, data, rest, q, account, imports}, {enabled
return null

//Context
let context = {mode: "user"}
let context = {mode:"user"}
if (q.repo) {
console.debug(`metrics/compute/${login}/plugins > activity > switched to repository mode`)
const {owner, repo} = data.user.repositories.nodes.map(({name: repo, owner: {login: owner}}) => ({repo, owner})).shift()
context = {...context, mode: "repository", owner, repo}
const {owner, repo} = data.user.repositories.nodes.map(({name:repo, owner:{login:owner}}) => ({repo, owner})).shift()
context = {...context, mode:"repository", owner, repo}
}

//Load inputs
Expand All @@ -29,134 +29,167 @@ export default async function({login, data, rest, q, account, imports}, {enabled
try {
for (let page = 1; page <= pages; page++) {
console.debug(`metrics/compute/${login}/plugins > activity > loading page ${page}/${pages}`)
events.push(...(context.mode === "repository" ? await rest.activity.listRepoEvents({owner: context.owner, repo: context.repo}) : await rest.activity.listEventsForAuthenticatedUser({username: login, per_page: 100, page})).data)
events.push(...(context.mode === "repository" ? await rest.activity.listRepoEvents({owner:context.owner, repo:context.repo}) : await rest.activity.listEventsForAuthenticatedUser({username:login, per_page:100, page})).data)
}
}
catch {
console.debug(`metrics/compute/${login}/plugins > activity > no more page to load`)
}
console.debug(`metrics/compute/${login}/plugins > activity > ${events.length} events loaded`)

const payloadTypesToCustomTypes = {
CommitCommentEvent:"comment",
CreateEvent:"ref/create",
DeleteEvent:"ref/delete",
ForkEvent:"fork",
GollumEvent:"wiki",
IssueCommentEvent:"comment",
IssuesEvent:"issue",
MemberEvent:"member",
PublicEvent:"public",
PullRequestEvent:"pr",
PullRequestReviewEvent:"review",
PullRequestReviewCommentEvent:"comment",
PushEvent:"push",
ReleaseEvent:"release",
WatchEvent:"star",
}

//Extract activity events
const activity = (await Promise.all(
events
.filter(({actor}) => account === "organization" ? true : actor.login?.toLocaleLowerCase() === login.toLocaleLowerCase())
.filter(({created_at}) => Number.isFinite(days) ? new Date(created_at) > new Date(Date.now() - days * 24 * 60 * 60 * 1000) : true)
.filter(event => visibility === "public" ? event.public : true)
.map(async ({type, payload, actor: {login: actor}, repo: {name: repo}, created_at}) => {
.map(event => ({event, customType:payloadTypesToCustomTypes[event.type]}))
.filter(({customType}) => !!customType) //Ignore events with an unknown type
.filter(({customType}) => filter.includes("all") || filter.includes(customType)) //Filter events based on user preference
.map(({event}) => event) //Discard customType, it will be re-assigned
.map(async ({type, payload, actor:{login:actor}, repo:{name:repo}, created_at}) => {
//See https://docs.github.com/en/free-pro-team@latest/developers/webhooks-and-events/github-event-types
const timestamp = new Date(created_at)
if (!imports.filters.repo(repo, skipped))
return null

//Get custom type from the previously delcared mapping, so that it acts as a single source of truth
const customType = payloadTypesToCustomTypes[type]
if (!customType) throw new Error(`Missing event mapping for type: ${type}`)

switch (type) {
//Commented on a commit
case "CommitCommentEvent": {
if (!["created"].includes(payload.action))
return null
const {comment: {user: {login: user}, commit_id: sha, body: content}} = payload
const {comment:{user:{login:user}, commit_id:sha, body:content}} = payload
if (!imports.filters.text(user, ignored))
return null
return {type: "comment", on: "commit", actor, timestamp, repo, content: await imports.markdown(content, {mode: markdown, codelines}), user, mobile: null, number: sha.substring(0, 7), title: ""}
return {type:customType, on:"commit", actor, timestamp, repo, content:await imports.markdown(content, {mode:markdown, codelines}), user, mobile:null, number:sha.substring(0, 7), title:""}
}
//Created a git branch or tag
case "CreateEvent": {
const {ref: name, ref_type: type} = payload
return {type: "ref/create", actor, timestamp, repo, ref: {name, type}}
const {ref:name, ref_type:type} = payload
return {type:customType, actor, timestamp, repo, ref:{name, type}}
}
//Deleted a git branch or tag
case "DeleteEvent": {
const {ref: name, ref_type: type} = payload
return {type: "ref/delete", actor, timestamp, repo, ref: {name, type}}
const {ref:name, ref_type:type} = payload
return {type:customType, actor, timestamp, repo, ref:{name, type}}
}
//Forked repository
case "ForkEvent": {
const {forkee: {full_name: forked}} = payload
return {type: "fork", actor, timestamp, repo, forked}
const {forkee:{full_name:forked}} = payload
return {type:customType, actor, timestamp, repo, forked}
}
//Wiki changes
case "GollumEvent": {
const {pages} = payload
return {type: "wiki", actor, timestamp, repo, pages: pages.map(({title}) => title)}
return {type:customType, actor, timestamp, repo, pages:pages.map(({title}) => title)}
}
//Commented on an issue
case "IssueCommentEvent": {
if (!["created"].includes(payload.action))
return null
const {issue: {user: {login: user}, title, number}, comment: {body: content, performed_via_github_app: mobile}} = payload
const {issue:{user:{login:user}, title, number}, comment:{body:content, performed_via_github_app:mobile}} = payload
if (!imports.filters.text(user, ignored))
return null
return {type: "comment", on: "issue", actor, timestamp, repo, content: await imports.markdown(content, {mode: markdown, codelines}), user, mobile, number, title}
return {type:customType, on:"issue", actor, timestamp, repo, content:await imports.markdown(content, {mode:markdown, codelines}), user, mobile, number, title}
}
//Issue event
case "IssuesEvent": {
if (!["opened", "closed", "reopened"].includes(payload.action))
return null
const {action, issue: {user: {login: user}, title, number, body: content}} = payload
const {action, issue:{user:{login:user}, title, number, body:content}} = payload
if (!imports.filters.text(user, ignored))
return null
return {type: "issue", actor, timestamp, repo, action, user, number, title, content: await imports.markdown(content, {mode: markdown, codelines})}
return {type:customType, actor, timestamp, repo, action, user, number, title, content:await imports.markdown(content, {mode:markdown, codelines})}
}
//Activity from repository collaborators
case "MemberEvent": {
if (!["added"].includes(payload.action))
return null
const {member: {login: user}} = payload
const {member:{login:user}} = payload
if (!imports.filters.text(user, ignored))
return null
return {type: "member", actor, timestamp, repo, user}
return {type:customType, actor, timestamp, repo, user}
}
//Made repository public
case "PublicEvent": {
return {type: "public", actor, timestamp, repo}
return {type:customType, actor, timestamp, repo}
Copy link

@oddstr13 oddstr13 Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain the rationale behind these changes? Seem unrelated to the stated goal of this PR.

Copy link
Author

@EndBug EndBug Oct 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah sure!

The activity plugin is not working because of an error that happens when parsing events of type PushEvent. The issue will occur even if the user is not interested in disaplying push events, because the filtering happens after the parsing

To make the plugin a bit more robust, I've decided to filter the events beforehand, by mapping the type first, then filtering, then parsing the events: that means that in the eventuality that the plugin breaks again for a certain type of events, the user could temporarily disable those to get the stats back running with just a config change, instead of having to wait for a patch.

To avoid having a mismatch I've moved the custom types to a single map

Copy link
Author

@EndBug EndBug Oct 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw, I've just updated the issue description and title: I opened this in a rush and was admittedly pretty poor 😅

}
//Pull requests events
case "PullRequestEvent": {
if (!["opened", "closed"].includes(payload.action))
return null
const {action, pull_request: {user: {login: user}, title, number, body: content, additions: added, deletions: deleted, changed_files: changed, merged}} = payload
const {action, pull_request:{user:{login:user}, title, number, body:content, additions:added, deletions:deleted, changed_files:changed, merged}} = payload
if (!imports.filters.text(user, ignored))
return null
return {type: "pr", actor, timestamp, repo, action: (action === "closed") && (merged) ? "merged" : action, user, title, number, content: await imports.markdown(content, {mode: markdown, codelines}), lines: {added, deleted}, files: {changed}}
return {type:customType, actor, timestamp, repo, action:(action === "closed") && (merged) ? "merged" : action, user, title, number, content:await imports.markdown(content, {mode:markdown, codelines}), lines:{added, deleted}, files:{changed}}
}
//Reviewed a pull request
case "PullRequestReviewEvent": {
const {review: {state: review}, pull_request: {user: {login: user}, number, title}} = payload
const {review:{state:review}, pull_request:{user:{login:user}, number, title}} = payload
if (!imports.filters.text(user, ignored))
return null
return {type: "review", actor, timestamp, repo, review, user, number, title}
return {type:customType, actor, timestamp, repo, review, user, number, title}
}
//Commented on a pull request
case "PullRequestReviewCommentEvent": {
if (!["created"].includes(payload.action))
return null
const {pull_request: {user: {login: user}, title, number}, comment: {body: content, performed_via_github_app: mobile}} = payload
const {pull_request:{user:{login:user}, title, number}, comment:{body:content, performed_via_github_app:mobile}} = payload
if (!imports.filters.text(user, ignored))
return null
return {type: "comment", on: "pr", actor, timestamp, repo, content: await imports.markdown(content, {mode: markdown, codelines}), user, mobile, number, title}
return {type:customType, on:"pr", actor, timestamp, repo, content:await imports.markdown(content, {mode:markdown, codelines}), user, mobile, number, title}
}
//Pushed commits
case "PushEvent": {
let {size, commits, ref} = payload
commits = commits.filter(({author: {email}}) => imports.filters.text(email, ignored))
let {size, ref, head, before} = payload
const [owner, repoName] = repo.split("/")

const res = await rest.repos.compareCommitsWithBasehead({owner, repo:repoName, basehead:`${before}...${head}`})
let {commits} = res.data

commits = commits.filter(({author:{email}}) => imports.filters.text(email, ignored))
if (!commits.length)
return null
if (commits.slice(-1).pop()?.message.startsWith("Merge branch "))
if (commits.slice(-1).pop()?.commit.message.startsWith("Merge branch "))
commits = commits.slice(-1)
return {type: "push", actor, timestamp, repo, size, branch: ref.match(/refs.heads.(?<branch>.*)/)?.groups?.branch ?? null, commits: commits.reverse().map(({sha, message}) => ({sha: sha.substring(0, 7), message}))}

return {type:customType, actor, timestamp, repo, size, branch:ref.match(/refs.heads.(?<branch>.*)/)?.groups?.branch ?? null, commits:commits.reverse().map(({sha, message}) => ({sha:sha.substring(0, 7), message}))}
}
//Released
case "ReleaseEvent": {
if (!["published"].includes(payload.action))
return null
const {action, release: {name, tag_name, prerelease, draft, body: content}} = payload
return {type: "release", actor, timestamp, repo, action, name: name || tag_name, prerelease, draft, content: await imports.markdown(content, {mode: markdown, codelines})}
const {action, release:{name, tag_name, prerelease, draft, body:content}} = payload
return {type:customType, actor, timestamp, repo, action, name:name || tag_name, prerelease, draft, content:await imports.markdown(content, {mode:markdown, codelines})}
}
//Starred a repository
case "WatchEvent": {
if (!["started"].includes(payload.action))
return null
const {action} = payload
return {type: "star", actor, timestamp, repo, action}
return {type:customType, actor, timestamp, repo, action}
}
//Unknown event
default: {
Expand All @@ -166,11 +199,10 @@ export default async function({login, data, rest, q, account, imports}, {enabled
}),
))
.filter(event => event)
.filter(event => filter.includes("all") || filter.includes(event.type))
.slice(0, limit)

//Results
return {timestamps, events: activity}
return {timestamps, events:activity}
}
//Handle errors
catch (error) {
Expand Down
Loading