-
-
Notifications
You must be signed in to change notification settings - Fork 38
feat: Add rule & collection job status indicators #1659
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
Conversation
8de9add
to
77127d1
Compare
a9bec6a
to
7f2c854
Compare
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.
Great work! The code looks solid, and I’m excited to see it in action. I do have some concerns about the changes to the rule and collection workers (not because of the code, but simply because it’s an area I don't like to touch). But we should quickly spot any issues if something doesn’t work as expected in that area.
I'll add unit tests for both of the workers, might as well start now! I'll split out the media deletion logic to new classes at the same time to make the tests easier to write. There's a feature request for one-off handling in the UI which will be easy to implement after that refactor. |
Great idea! Unit tests for these areas would be very valuable. |
264e8ed
to
1bf300d
Compare
@jorenn92 That latest commit should be good to go. I still need to perform a thorough test of the handling options. A summary of the changes: |
280aad3
to
0c1cbc1
Compare
That’s a significant refactor! It looks much cleaner now, and having tests is a great improvement. I’d like us to thoroughly test this before the final release though. Would it make sense to do a pre-release first before rolling it out to everyone? |
/release-pr |
Definitely 😎 |
Released to |
/release-pr |
Released to |
Fix test workflow Stop emitting app logs when under test Add Jest test runner extension recommendation Add Jest test runner settings
f1a93d7
to
2b8bd33
Compare
2b8bd33
to
0b179ad
Compare
/release-pr |
Released to |
/release-pr |
Released to |
🎉 This PR is included in version 2.14.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
I love this. Now I don't have to go to the logs to see if it finished. Great work. |
This PR adds rule & collection job progress above the version info, similar to *arrs but with actual bars! The run buttons now reflect the task running status and I have added an error toast if trying to run when already running (edge case if the status isn't available as the button is usually disabled).
This uses the event emitter from Nest.js to emit started, progress and finished events for both jobs. The events are sent down to the client using a new SSE endpoint and
@OnEvent
decorators. I've set this up to make adding new events and listening to them on the client super easy. One future use-case would be emitting events when media is added & removed from a collection and having the collection view be live updating.