-
Notifications
You must be signed in to change notification settings - Fork 11
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
auto-updating config via websocket | setting: automatically include limit in query for tags/services #189
base: main
Are you sure you want to change the base?
Conversation
We try to make as much configurable during runtime without restarting pkappa2. So some config page and rest endpoints to change the config values would be good. You don't have to add the UI part yet to configure it. The current way the I think it's useful to add it to the other tags too. When clicking on a tag like We'll have to test if this is annoying or intuitive to use during a CTF. |
@peace-maker came up with a solution, now on result-view creation it gets checked if setting option is is enabled, if yes + query is empty the limit gets auto applied. Also if any link is clicked in the sidebar the router sends an additional queryparam that indicates exactly that, then the limit also gets auto applied. Opening links with already defined queries in url just are just getting placed as is. Only remaining issue: searching with empty query not possible atm, limit always get applied every time now. |
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.
Nice, thank you for working on this! The empty query could be allowed by not interacting with an manually entered empty query. So after clearing the searchbox and pressing enter, you wouldn't insert the time
query part.
I think adding another websocket event to notify all clients of the changed config would be useful instead of polling, but that can be done later.
A note on the Settings page that changes are global and affect all users would hopefully avoid confusion on people trying to disable the autofill for themselves.
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 haven't tried running the code yet, so this is still just visual inspection. Looks good overall.
I had the idea of turning the config option into an arbitrary suffix to add to the query, so you can change the currently hardcoded "ltime:-1h:"
. I'm not sure it'd help usability though and this can be switched to that option when a different query was desired during a CTF. Just for future reference.
There are multiple linter warnings still yelling about.
2c17232
to
57a7529
Compare
frontend glue code load config on component load get config from server config flag
yarn format
57a7529
to
8318fd8
Compare
Fixed conditional vue-if
Removed obsolete code SetClientConfig now returns nothing linting <3
linting <3333
dont return config after updating
8318fd8
to
14a0fe3
Compare
Config option still missingWebsocket connection to update setting for all connected users on the fly
Introduces config endpoint that lets us configure settings on the fly and send it over instantly to all connected clients
@peace-maker not sure if https://www.youtube.com/watch?v=cDA3_5982h8 moment, but I tried.
#181