Skip to content

feat: button to remove a movie from Radarr/Sonarr#182

Merged
fallenbagel merged 6 commits intoseerr-team:developfrom
dd060606:features/deleteMediaFile
Feb 18, 2023
Merged

feat: button to remove a movie from Radarr/Sonarr#182
fallenbagel merged 6 commits intoseerr-team:developfrom
dd060606:features/deleteMediaFile

Conversation

@dd060606
Copy link
Copy Markdown
Contributor

@dd060606 dd060606 commented Jul 8, 2022

Description

Added button in ManageSlideOver to remove movie and media file from Radarr/Sonarr

Screenshot (if UI-related)

image

To-Dos

  • Successful build yarn build
  • Translation keys yarn i18n:extract
  • Database migration (if required)

(Sorry for my English, I'm French)

@fallenbagel
Copy link
Copy Markdown
Collaborator

I was wondering wouldn't it be better to hide that button when default service is not configured as opposed to throwing the error to log? (This might reduce some confusion in users + another dev also suggested the same)

@dd060606
Copy link
Copy Markdown
Contributor Author

I was wondering wouldn't it be better to hide that button when default service is not configured as opposed to throwing the error to log? (This might reduce some confusion in users + another dev also suggested the same)

Yes it's a good idea. I will fix this as soon as possible.

@dd060606
Copy link
Copy Markdown
Contributor Author

I was wondering wouldn't it be better to hide that button when default service is not configured as opposed to throwing the error to log? (This might reduce some confusion in users + another dev also suggested the same)

After checking, there is already a condition to check if the service URL is not null

{hasPermission(Permission.ADMIN) &&
    data?.mediaInfo?.serviceUrl && (

And when I remove the default service, the button doesn't appear
image

Was your question about that?

@fallenbagel
Copy link
Copy Markdown
Collaborator

Was your question about that?

Yes it was. I wonder why the button appeared on mine. I'll test again tonight and if all's good will merge

@fallenbagel
Copy link
Copy Markdown
Collaborator

@dd060606 Hi. Sorry took a while to test but from my testing when I have the default service not selected i can still see the button

image

image

@dd060606
Copy link
Copy Markdown
Contributor Author

@dd060606 Hi. Sorry took a while to test but from my testing when I have the default service not selected i can still see the button

image

image

Ok I will fix this as soon as possible 👍

@fallenbagel fallenbagel added the merge conflict Cannot merge due to merge conflicts label Sep 7, 2022
@fallenbagel
Copy link
Copy Markdown
Collaborator

@dd060606 Could you resolve the merge conflict? Once done could deploy this for testing

@dd060606
Copy link
Copy Markdown
Contributor Author

@fallenbagel ok I will fix this as soon as possible 👍

@dd060606
Copy link
Copy Markdown
Contributor Author

@fallenbagel I solved the merge conflict

@fallenbagel fallenbagel removed the merge conflict Cannot merge due to merge conflicts label Sep 14, 2022
@fallenbagel
Copy link
Copy Markdown
Collaborator

Hi could you fix the lint issues?

@dd060606
Copy link
Copy Markdown
Contributor Author

Hi could you fix the lint issues?

Yes I will fix that 👍

@dd060606
Copy link
Copy Markdown
Contributor Author

@fallenbagel I think I solved the conflict.

@fallenbagel
Copy link
Copy Markdown
Collaborator

fallenbagel commented Sep 28, 2022

Thanks

@fallenbagel
Copy link
Copy Markdown
Collaborator

I've tested this a lot, and I have not had any issues. I think we're good to merge soon!

@rowra44
Copy link
Copy Markdown

rowra44 commented Nov 4, 2022

Any updates on the state of this long on-going / due merge? It'd be very welcome!

Also, in my opinion deleting media should be a single process too, like "delete from Radarr" => deletes from Radarr, including the files and clears all data from within Jellyseer too. Is clearing from Jellyseer included in this function?

@dd060606
Copy link
Copy Markdown
Contributor Author

dd060606 commented Nov 4, 2022

Any updates on the state of this long on-going / due merge? It'd be very welcome!

Also, in my opinion deleting media should be a single process too, like "delete from Radarr" => deletes from Radarr, including the files and clears all data from within Jellyseer too. Is clearing from Jellyseer included in this function?

The "delete from Radarr" button already removes all data on the movie including all files and clears all data from Jellyseerr

@fallenbagel
Copy link
Copy Markdown
Collaborator

Any updates on the state of this long on-going / due merge? It'd be very welcome!

I'm currently deploying this as preview-pr182 for testing purposes. It should be built in about 1-2hrs depending on how fast github actions does it

@fallenbagel fallenbagel added the preview PRs deployed for testing with tag `:preview-prxx` label Nov 17, 2022
@Gauvino
Copy link
Copy Markdown
Contributor

Gauvino commented Jan 9, 2023

when merging to master ? @fallenbagel

@fallenbagel fallenbagel added the merge conflict Cannot merge due to merge conflicts label Feb 9, 2023
@fallenbagel
Copy link
Copy Markdown
Collaborator

fallenbagel commented Feb 9, 2023

when merging to master ? @fallenbagel

I am looking to merging this to master along with OIDC this week. I'll merge this one as soon as the merge conflicts are taken care of

@dd060606 sorry for the loooong delay on this PR as this is kind of a big feature but I am ready to merge ASAP c:

@dd060606
Copy link
Copy Markdown
Contributor Author

dd060606 commented Feb 9, 2023

when merging to master ? @fallenbagel

I am looking to merging this to master along with OIDC this week. I'll merge this one as soon as the merge conflicts are taken care of

@dd060606 sorry for the loooong delay on this PR as this is kind of a big feature but I am ready to merge ASAP c:

@fallenbagel I solved the merge conflict 👍

@fallenbagel
Copy link
Copy Markdown
Collaborator

fallenbagel commented Feb 12, 2023

@dd060606 could you run prettier on the manageSlideOver file?

@dd060606
Copy link
Copy Markdown
Contributor Author

@dd060606 could you run prettier on the manageSlideOver file?

@fallenbagel I think I solved the code style issues

@fallenbagel fallenbagel merged commit ddbc377 into seerr-team:develop Feb 18, 2023
@fallenbagel
Copy link
Copy Markdown
Collaborator

@dd060606 Is the button always supposed to show or only when media is available? Just making sure if it was the intended behavior or not (a question on discord by a user)

@dd060606
Copy link
Copy Markdown
Contributor Author

@dd060606 Is the button always supposed to show or only when media is available? Just making sure if it was the intended behavior or not (a question on discord by a user)

The button is supposed to show when the movie is detected in Radarr or Sonarr but it doesn't check if the movie is available or not (like the "open in Radarr" button)

@fallenbagel
Copy link
Copy Markdown
Collaborator

fallenbagel commented Feb 27, 2023

The button is supposed to show when the movie is detected in Radarr or Sonarr but it doesn't check if the movie is available or not (like the "open in Radarr" button)

According to them, it is always showing despite the media being absent on *arr. I haven't tested it myself but currently I am updating to test it. Would edit this comment after testing

EDIT: It's working as expected! Thank you for the PR!

@fallenbagel
Copy link
Copy Markdown
Collaborator

🎉 This PR is included in version 1.5.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@fallenbagel fallenbagel removed merge conflict Cannot merge due to merge conflicts preview PRs deployed for testing with tag `:preview-prxx` labels Nov 7, 2023
@fallenbagel
Copy link
Copy Markdown
Collaborator

@all-contributors please add @dd060606 for code

@allcontributors
Copy link
Copy Markdown
Contributor

@fallenbagel

I've put up a pull request to add @dd060606! 🎉

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.

4 participants