10000 feat: Add rule & collection job status indicators by benscobie · Pull Request #1659 · jorenn92/Maintainerr · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

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

Merged
merged 8 commits into from
Apr 6, 2025
Merged

Conversation

benscobie
Copy link
Collaborator
@benscobie benscobie commented Mar 26, 2025

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).

rules_progress

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.

@benscobie benscobie force-pushed the feat-job-detail branch 4 times, most recently from 8de9add to 77127d1 Compare March 26, 2025 10:47
@benscobie benscobie marked this pull request as ready for review March 26, 2025 10:47
@benscobie benscobie requested review from ydkmlt84 and jorenn92 March 26, 2025 10:52
@benscobie benscobie force-pushed the feat-job-detail branch 2 times, most recently from a9bec6a to 7f2c854 Compare March 26, 2025 22:20
Copy link
Owner
@jorenn92 jorenn92 left a 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.

@benscobie
Copy link
Collaborator Author
benscobie commented Mar 28, 2025

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).

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.

@jorenn92
Copy link
Owner

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).

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.

@benscobie benscobie force-pushed the feat-job-detail branch 2 times, most recently from 264e8ed to 1bf300d Compare March 30, 2025 23:44
@benscobie
Copy link
Collaborator Author
benscobie commented Mar 30, 2025

@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:
[new] CollectionHandler - Responsible for handling a single collection media item. Tests added for this.
[new] RadarrActionHandler & SonarrActionHandler - Responsible for handling a single collection media item for a specific service. Tests added for this.
[new] MediaIdFinder - Contains the TVDBId lookup code. I noticed there's a few other ID lookup type functions elsewhere so I may need to tidy. I haven't added tests for this.
[changed] CollectionWorkerService - This class is now very simple. It looks up collections & media to handle and uses CollectionHandler for the handling, and then triggers availability sync. Tests added for this.

@benscobie benscobie force-pushed the feat-job-detail branch 2 times, most recently from 280aad3 to 0c1cbc1 Compare March 31, 2025 15:36
@jorenn92
Copy link
Owner
jorenn92 commented Apr 1, 2025

@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: [new] CollectionHandler - Responsible for handling a single collection media item. Tests added for this. [new] RadarrActionHandler & SonarrActionHandler - Responsible for handling a single collection media item for a specific service. Tests added for this. [new] MediaIdFinder - Contains the TVDBId lookup code. I noticed there's a few other ID lookup type functions elsewhere so I may need to tidy. I haven't added tests for this. [changed] CollectionWorkerService - This class is now very simple. It looks up collections & media to handle and uses CollectionHandler for the handling, and then triggers availability sync. Tests added for this.

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?

@benscobie
Copy link
Collaborator Author

/release-pr

@benscobie
Copy link
Collaborator Author

Would it make sense to do a pre-release first before rolling it out to everyone?

Definitely 😎

Copy link
Contributor
github-actions bot commented Apr 1, 2025

Released to jorenn92/maintainerr:pr-1659 🚀

@benscobie
Copy link
Collaborator Author

/release-pr

Copy link
Contributor
github-actions bot commented Apr 1, 2025

Released to jorenn92/maintainerr:pr-1659 🚀

Fix test workflow
Stop emitting app logs when under test
Add Jest test runner extension recommendation
Add Jest test runner settings
@benscobie
Copy link
Collaborator Author

/release-pr

Copy link
Contributor
github-actions bot commented Apr 5, 2025

Released to jorenn92/maintainerr:pr-1659 🚀

@benscobie
Copy link
Collaborator Author

/release-pr

Copy link
Contributor
github-actions bot commented Apr 6, 2025

Released to jorenn92/maintainerr:pr-1659 🚀

@benscobie benscobie merged commit aebf3ae into main Apr 6, 2025
7 checks passed
8000
@benscobie benscobie deleted the feat-job-detail branch April 6, 2025 19:06
@jorenn92
Copy link
Owner

🎉 This PR is included in version 2.14.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@danthom1704
Copy link

I love this. Now I don't have to go to the logs to see if it finished. Great work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0