generated from ubiquity/ts-template
-
Notifications
You must be signed in to change notification settings - Fork 18
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: notify self assign #24
Merged
gentlementlegen
merged 86 commits into
ubiquity-os-marketplace:development
from
gentlementlegen:feat/notify-self-assign
Sep 7, 2024
Merged
Changes from 85 commits
Commits
Show all changes
86 commits
Select commit
Hold shift + click to select a range
2cce223
Merge pull request #2 from Meniole/development
gentlementlegen 48203fe
chore: removed yarn 4
gentlementlegen ba529e1
chore: renamed wrangler.toml
gentlementlegen d3226ca
chore: default enabled true
gentlementlegen e0f9047
chore: added manifest.json
gentlementlegen dad7b2b
chore: added manifest.json
gentlementlegen 0c40f37
chore: added manifest.json
gentlementlegen e4fee10
chore: manifest.json
gentlementlegen acb228c
chore: manifest.json
gentlementlegen 565fd1c
feat: max task assignment for collaborators
a74d31b
feat: make task limits configurable
b30416e
Merge pull request #3 from Meniole/development
gentlementlegen 8017e89
Merge branch 'refs/heads/development' into meniole-main
gentlementlegen cddec1e
chore: manifest.json
gentlementlegen c36730a
Merge remote-tracking branch 'meniole/main' into meniole-main
gentlementlegen a1508fc
fix: get contributors from config
f5199d8
fix: remove union type
df7770e
fix: return lowest task limit if user role doesnt match any of those …
661fb85
chore: code cleanups
b2ec2f6
test: add test to cover functionality
14f7c33
chore: fix merge conflicts
05f43ed
fix: pass correct inputs to getUserRole
71e1c1a
test: add test
06294ca
test: cleanups
ec2e5bb
test: made requested changes
38be206
test: cleanpup test
468cd49
fix: set smallest task on error when getting user role
91984c7
chore: fix merge conflicts
86accef
fix: refactor test to use the correct format
1927aed
fix: add db issues to cover all test
a06ab85
fix: assignee issue and open pr fetching
Keyrxng e343a6d
chore: fix filter and use math.abs
Keyrxng 09c1091
chore: tests
Keyrxng c341006
Merge branch 'max-assignments' into fix/max-and-assigned
Keyrxng ca1f0c5
chore: ci
Keyrxng 944883d
Merge pull request #21 from ubq-testing/fix/max-and-assigned
jordan-ae dcddd3e
chore: rename file to match return
jordan-ae 8221f51
fix: use logger to display message
jordan-ae 0d55cb1
Update src/handlers/shared/start.ts
jordan-ae 101c1f8
chore: rename function to match return
jordan-ae 0084bcb
chore: address code reviews
jordan-ae 92ce9d4
fix: clean up queries
jordan-ae 665fd5c
Merge branch 'fork/ubq-test-jordan/max-assignments' into meniole-main
gentlementlegen c8a018d
feat: user self assign message display
gentlementlegen cffb784
Merge branch 'feat/notify-self-assign' into meniole-main
gentlementlegen c3fc7ca
Merge remote-tracking branch 'meniole/main' into meniole-main
gentlementlegen d0a00d8
chore: fixed event name
gentlementlegen 6fc8f09
chore: simplified deadline calculation
gentlementlegen 2bf7363
feat: check unassigns
1f82df8
feat: previous assignment filter
Keyrxng 7310c50
chore: make bot name match configurable
Keyrxng b69f4c3
chore: tests and configurable botUsernames
Keyrxng 9089fe3
chore: remove botUsernames item
Keyrxng 316ca9e
chore: fix test
Keyrxng f3b1e52
chore: minor fixes
Keyrxng 671be3d
chore: refactor tests
Keyrxng bedb60b
feat: max task assignment for collaborators
71324b8
feat: make task limits configurable
212f9bf
fix: get contributors from config
391bdde
fix: return lowest task limit if user role doesnt match any of those …
bf6e7da
chore: code cleanups
da1b51d
test: add test to cover functionality
83a6941
test: cleanups
9005f01
test: made requested changes
10b985a
fix: set smallest task on error when getting user role
73119b5
chore: fix filter and use math.abs
Keyrxng 0753f36
chore: rename file to match return
jordan-ae 42cdf6e
chore: rename function to match return
jordan-ae 535ec00
chore: address code reviews
jordan-ae df7ff28
fix: clean up queries
jordan-ae c148fba
chore: merging
gentlementlegen 4fe61f3
chore: merge changes
gentlementlegen dd7eb09
chore: merge changes
gentlementlegen eb37a3d
Update src/handlers/shared/start.ts
gentlementlegen 1977116
chore: merge changes
gentlementlegen d7ca019
chore: merge changes
gentlementlegen 615d16a
chore: merge changes
gentlementlegen 68890ac
chore: merge changes
gentlementlegen 22b64c4
chore: fixed test
gentlementlegen 5437e76
chore: fixed test
gentlementlegen 10650d8
chore: fixed test
gentlementlegen b8cd7af
chore: changed status code
gentlementlegen 3764eea
chore: fixed tests
gentlementlegen 0dc082b
Merge branch 'development' into feat/notify-self-assign
gentlementlegen ebf98c4
fix: skipping deadline self assign post if no deadline
gentlementlegen c79b63f
fix: the messages are displayed on catch errors
gentlementlegen File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
import { Context, Env, SupportedEventsU } from "../types"; | ||
import { userSelfAssign, userStartStop } from "./user-start-stop"; | ||
|
||
export enum HttpStatusCode { | ||
OK = 200, | ||
NOT_MODIFIED = 304, | ||
} | ||
|
||
export interface Result { | ||
status: HttpStatusCode; | ||
content?: string; | ||
reason?: string; | ||
} | ||
|
||
const callbacks: { [k in SupportedEventsU]: (context: Context, env: Env) => Result | Promise<Result> } = { | ||
"issues.assigned": userSelfAssign, | ||
"issue_comment.created": userStartStop, | ||
}; | ||
|
||
export function proxyCallbacks({ logger }: Context) { | ||
return new Proxy(callbacks, { | ||
get(target, prop: SupportedEventsU) { | ||
if (!(prop in target)) { | ||
logger.error(`${prop} is not supported, skipping.`); | ||
return async () => ({ status: "skipped", reason: "unsupported_event" }); | ||
} | ||
return target[prop].bind(target); | ||
}, | ||
}); | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
What is this file?
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.
It contains proxy calls for events.
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.
What is a "proxy call"?
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.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Proxy 😄
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 would appreciate comments in this file and I think newer contributors would too. The first time I saw it in another plugin I thought "this looks like heavy artillery"
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.
https://chatgpt.com/share/915e1875-a26a-4deb-b9fc-705e59f0212a
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.
But all this proxy does is convolute what could be a simple
if
orswitch
statement as we do not perform any mutation or action within the proxy other than calling one of two functions.rpc-handler
uses aProxy
but it does mutate and control logic so it makes sense there, here it seems very OP.I guess my point is, are we adopting this approach across plugins as standard rather than the previous method of using an
if
for threeSupportedEvents
or less and aswitch
for more than thee?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.
Sure a switch can work. Having the proxy enforces implementing all the supported events that would be the biggest difference.
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 am not against the Proxy but I think if we are adopting it as standard we should comment it clearly so that no-one else has to ask GPT what's going here
This is a good reason for it.
I cannot shake the thought: if events are arbitrary and not defined in our
SupportedEvents
theswitch
and theProxy
will not run.If plugins are built to work on any event like
user-activity-watcher
I think you said @gentlementlegen then the proxy would need to handle all events.Plugins which cannot run on any event need to be restricted to only their supported events at the plugin config level I think.
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 can make it a switch case, I just think that the more actions we should handle the more the switch case will be massive so that was to make code lighter. The proxy is needed because eventually the called key might not be in the object. In OOP it would just be a pointer to function object, which would be less fancy.