-
Notifications
You must be signed in to change notification settings - Fork 19
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
Add movie editor endpoints #69
Conversation
@@ -40,7 +40,7 @@ func (test *TestMockData) GetMockServer(t *testing.T) *httptest.Server { | |||
assert.EqualValues(t, test.ExpectedPath, req.URL.String()) | |||
writer.WriteHeader(test.ResponseStatus) | |||
|
|||
assert.EqualValues(t, req.Method, test.ExpectedMethod) | |||
assert.EqualValues(t, test.ExpectedMethod, req.Method) |
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.
This was backward.
@@ -0,0 +1,23 @@ | |||
package starr |
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.
These may be useful down the road too.
Looks great! Unmonitoring works, but DeleteFiles and AddImportExclusion doesn't work on my end. |
Did you mean We'll have to address |
|
I meant
So it bulk deletes all movies and their files. For the
and sent it via:
url is xxx/api/v3/movie/editor?apikey=xxx |
I suspect those things do not actually work in radarr. Do you know how to do these actions in the UI? So I can reproduce and see what the difference is. |
oh, I see what the problem is. |
Okay, I gave delete the same inputs as edit, so you can pass in those fields. Good to know how it works, thanks! I also renamed the struct to
|
Awesome it works now :D |
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.
just a thought about the usage of pointers
@@ -0,0 +1,45 @@ | |||
package radarr_test |
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.
@davidnewhall this really rock! thanks 🚀
radarr/movieeditor.go
Outdated
type BulkEdit struct { | ||
MovieIDs []int64 `json:"movieIds"` | ||
Monitored *bool `json:"monitored,omitempty"` | ||
QualityProfileID *int64 `json:"qualityProfileId,omitempty"` | ||
MinimumAvailability *string `json:"minimumAvailability,omitempty"` // tba | ||
RootFolderPath *string `json:"rootFolderPath,omitempty"` // path | ||
Tags []int `json:"tags,omitempty"` // [0] | ||
ApplyTags *string `json:"applyTags,omitempty"` // add | ||
MoveFiles *bool `json:"moveFiles,omitempty"` | ||
DeleteFiles *bool `json:"deleteFiles,omitempty"` // delete only | ||
AddImportExclusion *bool `json:"addImportExclusion,omitempty"` // delete only |
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.
are they pointer to let them leverage the omitempty
tag and use the default 0 value as well?
when they are not present which is the default behaviour? if it is the same as the 0 value you can use values instead of pointers and remove the omitempty
tag.
I haven't tried this, so they're just some thoughts
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.
These members are purposely pointers, because the default value is different than the "missing" value.
This is a bulk-change endpoint. When you pick monitored
, for instance, there are three choices. monitored
, not monitored
and no change
. We achieve "no change" by omitting the key entirely. If we set it to true
, then all movie IDs provided are monitored. If we set it to false
then they are all unmonitored. If we do not want to change the monitored status, but rather, change something else, then we have to omit monitored
.
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.
got it, thanks for the example
radarr/movieeditor.go
Outdated
MinimumAvailability *string `json:"minimumAvailability,omitempty"` // tba | ||
RootFolderPath *string `json:"rootFolderPath,omitempty"` // path | ||
Tags []int `json:"tags,omitempty"` // [0] | ||
ApplyTags *string `json:"applyTags,omitempty"` // add |
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.
ApplyTags
could probably be enum constants, but I do not know all the values.
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.
add, remove, replace
ref https://radarr.video/docs/api/#/MovieEditor/put_api_v3_movie_editor
radarr/movieeditor.go
Outdated
MovieIDs []int64 `json:"movieIds"` | ||
Monitored *bool `json:"monitored,omitempty"` | ||
QualityProfileID *int64 `json:"qualityProfileId,omitempty"` | ||
MinimumAvailability *string `json:"minimumAvailability,omitempty"` // tba |
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.
MinimumAvailability
could probably be enum constants, but I do not know all the values.
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.
tba, announced, inCinemas, released, deleted
ref https://radarr.video/docs/api/#/MovieEditor/put_api_v3_movie_editor
This is a modal prompt when you delete movies. Same prompt when deleting movie (not via editor) delete files |
discover tab has this option to select and exclude Otherwise when deleting a movie or movies the exclusion list is a modal confirmation checkbox |
I think I'm gonna turn those inputs into enums. We can probably re-use them in a few places. |
How's that look @Fuochi ? |
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 don't fully get the Ptr() functions added... but I'm sure I'm missing a use case
// Ptr returns a pointer to a minimum availability. Useful for a BulkEdit struct. | ||
func (a Availability) Ptr() *Availability { | ||
return &a | ||
} |
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 don't get the use case of this, can't you just use the &
operator? why is a method needed?
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.
You cannot make pointers to constants, so the user would have to do:
avail := radarr.AvailabilityInCinemas
Then pass in &avail
. Having the Ptr
method allows passing a value in 1 line, as shown in the test.
// Availability is an enum used as MinimumAvailability in a few places throughout Radarr. | ||
type Availability string | ||
|
||
// Availability / MinimumAvailability constants. | ||
// https://radarr.video/docs/api/#/MovieEditor/put_api_v3_movie_editor | ||
const ( | ||
AvailabilityToBeAnnounced Availability = "tba" | ||
AvailabilityAnnounced Availability = "announced" | ||
AvailabilityInCinemas Availability = "inCinemas" | ||
AvailabilityReleased Availability = "released" | ||
AvailabilityDeleted Availability = "deleted" | ||
) |
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.
this is nice for a user perspective since it provides users with valid values. However, users can still use any string they want... so I'd say it doesn't make much difference for me, if you like this approach you can continue to use it!
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's all about users. This, if nothing else, gives them an easy way to see what values are appropriate.
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.
There is a difference in status and availability. tba and deleted will not work as a minimum availability in radarr. "announced", "released" & "inCinemas"
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.
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.
Practical use would be the 3 nit mentioned
- deleted is a status from tmdb
- tba is legacy carryover
neither really has any practical use
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.
All good. I just made them constants. Users don't have to use them. :)
You can add movies to the exclusion list on the List Exclusions page @NCRoxas ... There is a + at the bottom of the list. |
Adds movie editor endpoint.
Adds movie delete and bulk delete endpoints.
Closes #68. @NCRoxas, can you test this?