mirror of
https://github.com/navidrome/navidrome.git
synced 2026-10-08 10:27:08 +02:00
fix(jellyfin): honor IsPublic when creating a playlist (#6204)
POST /Playlists dropped the client's IsPublic flag, so every playlist was created private. JellyBox Player's create-playlist form defaults its "public" checkbox to true, so JellyBox users could never create a public playlist. Upstream's PlaylistsController passes IsPublic into PlaylistCreationRequest. core/playlists.Create has no visibility parameter and widening it would ripple into the Subsonic and native APIs, so createPlaylist follows the same pattern updatePlaylist already uses: after Create succeeds, a non-nil IsPublic is applied with a follow-up Update. The field is a pointer so an absent one keeps today's default instead of forcing private. If that second write fails the handler surfaces the error through playlistError rather than returning the id: answering 200 for a playlist that is not as visible as the client asked is the same silent drop this fixes.
This commit is contained in:
parent
a3f41fb422
commit
39028f65c8
4 changed files with 67 additions and 6 deletions
|
|
@ -222,8 +222,14 @@ func createPlaylistAs(user model.User, name string, encodedIds ...string) string
|
|||
}
|
||||
body, err := json.Marshal(map[string]any{"Name": name, "Ids": encodedIds})
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
return createPlaylistBodyAs(user, string(body))
|
||||
}
|
||||
|
||||
// createPlaylistBodyAs posts a raw create body, for tests that need fields the helpers above don't
|
||||
// build, and returns the new playlist's decoded id.
|
||||
func createPlaylistBodyAs(user model.User, body string) string {
|
||||
var res map[string]string
|
||||
parseInto(postAs(user, "/Playlists", string(body)), &res)
|
||||
parseInto(postAs(user, "/Playlists", body), &res)
|
||||
Expect(res["Id"]).ToNot(BeEmpty())
|
||||
id, ok := dto.DecodeID(res["Id"])
|
||||
Expect(ok).To(BeTrue())
|
||||
|
|
|
|||
|
|
@ -21,12 +21,19 @@ var _ = Describe("Playlists", func() {
|
|||
}
|
||||
order := func(plID string) []string { return names(playlistItems(plID).Items) }
|
||||
|
||||
createWith := func(body string) string { return createPlaylistBodyAs(adminUser, body) }
|
||||
openAccess := func(plID string) bool {
|
||||
var info dto.PlaylistInfo
|
||||
parseInto(get("/Playlists/"+enc(plID)), &info)
|
||||
return info.OpenAccess
|
||||
}
|
||||
|
||||
Describe("create", func() {
|
||||
It("creates an empty playlist", func() {
|
||||
plID := createPlaylist("Empty", nil)
|
||||
var info dto.PlaylistInfo
|
||||
parseInto(get("/Playlists/"+enc(plID)), &info)
|
||||
Expect(info.OpenAccess).To(BeFalse())
|
||||
Expect(info.OpenAccess).To(BeFalse(), "a playlist created without IsPublic stays private")
|
||||
Expect(info.Shares).To(BeEmpty())
|
||||
Expect(info.ItemIds).To(BeEmpty())
|
||||
})
|
||||
|
|
@ -48,6 +55,14 @@ var _ = Describe("Playlists", func() {
|
|||
Expect(playlistItems(plID).TotalRecordCount).To(Equal(3)) // Abbey Road (2) + Help! (1)
|
||||
})
|
||||
|
||||
It("creates a public playlist when the client sends IsPublic true", func() {
|
||||
Expect(openAccess(createWith(`{"Name":"Public","Ids":[],"IsPublic":true}`))).To(BeTrue())
|
||||
})
|
||||
|
||||
It("creates a private playlist when the client sends IsPublic false", func() {
|
||||
Expect(openAccess(createWith(`{"Name":"Private","Ids":[],"IsPublic":false}`))).To(BeFalse())
|
||||
})
|
||||
|
||||
// dto.DecodeIDs is all-or-nothing: a malformed entry must 404 the whole request, not get
|
||||
// dropped while the well-formed entries are still used to create a playlist.
|
||||
It("404s when one of the Ids is malformed, without creating a playlist", func() {
|
||||
|
|
@ -355,10 +370,7 @@ var _ = Describe("Playlists", func() {
|
|||
It("makes a playlist public", func() {
|
||||
plID := createPlaylist("Make Public", nil)
|
||||
Expect(post("/Playlists/"+enc(plID), `{"Name":"Make Public","IsPublic":true}`).Code).To(Equal(http.StatusNoContent))
|
||||
|
||||
var info dto.PlaylistInfo
|
||||
parseInto(get("/Playlists/"+enc(plID)), &info)
|
||||
Expect(info.OpenAccess).To(BeTrue())
|
||||
Expect(openAccess(plID)).To(BeTrue())
|
||||
// Now visible to other users.
|
||||
Expect(queryResult(getAs(regularUser, "/Items?IncludeItemTypes=Playlist&Recursive=true")).TotalRecordCount).To(Equal(1))
|
||||
})
|
||||
|
|
|
|||
|
|
@ -48,6 +48,7 @@ type createPlaylistRequest struct {
|
|||
Name string `json:"Name"`
|
||||
Ids []string `json:"Ids"`
|
||||
MediaType string `json:"MediaType"`
|
||||
IsPublic *bool `json:"IsPublic"`
|
||||
}
|
||||
|
||||
// createPlaylist always creates a new playlist (playlistId "" tells core/playlists.Create not to
|
||||
|
|
@ -69,6 +70,13 @@ func (api *Router) createPlaylist(w http.ResponseWriter, r *http.Request) {
|
|||
api.internalError(w, r, err)
|
||||
return
|
||||
}
|
||||
// Create takes no visibility, so a requested one costs a second write.
|
||||
if body.IsPublic != nil {
|
||||
if err := api.playlists.Update(r.Context(), id, nil, nil, body.IsPublic, nil, nil); err != nil {
|
||||
api.playlistError(w, r, err)
|
||||
return
|
||||
}
|
||||
}
|
||||
api.ok(w, r, map[string]string{"Id": dto.EncodeID(id)})
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -55,6 +55,16 @@ type fakePlaylists struct {
|
|||
|
||||
deletePlaylistID string
|
||||
deleteErr error
|
||||
|
||||
updatePlaylistID string
|
||||
updatePublic *bool
|
||||
updateErr error
|
||||
}
|
||||
|
||||
func (f *fakePlaylists) Update(_ context.Context, playlistID string, _ *string, _ *string, public *bool, _ []string, _ []int) error {
|
||||
f.updatePlaylistID = playlistID
|
||||
f.updatePublic = public
|
||||
return f.updateErr
|
||||
}
|
||||
|
||||
func (f *fakePlaylists) Delete(_ context.Context, id string) error {
|
||||
|
|
@ -184,6 +194,31 @@ var _ = Describe("Playlists", func() {
|
|||
invoke(api.createPlaylist, w, r)
|
||||
Expect(w.Code).To(Equal(http.StatusInternalServerError))
|
||||
})
|
||||
|
||||
createReq := func(body string) *http.Request {
|
||||
return httptest.NewRequest("POST", "/Playlists", strings.NewReader(body)).
|
||||
WithContext(GinkgoT().Context())
|
||||
}
|
||||
|
||||
DescribeTable("visibility",
|
||||
func(body string, wantPublic *bool, wantUpdatedID string) {
|
||||
w := httptest.NewRecorder()
|
||||
invoke(api.createPlaylist, w, createReq(body))
|
||||
Expect(w.Code).To(Equal(http.StatusOK))
|
||||
Expect(fp.updatePublic).To(Equal(wantPublic))
|
||||
Expect(fp.updatePlaylistID).To(Equal(wantUpdatedID))
|
||||
},
|
||||
Entry("applies IsPublic true", `{"Name":"Mix","IsPublic":true}`, new(true), testID("pl-new")),
|
||||
Entry("applies an explicit IsPublic false", `{"Name":"Mix","IsPublic":false}`, new(false), testID("pl-new")),
|
||||
Entry("leaves visibility alone when IsPublic is omitted", `{"Name":"Mix"}`, nil, ""),
|
||||
)
|
||||
|
||||
It("returns 500 when the visibility update fails", func() {
|
||||
fp.updateErr = errors.New("boom")
|
||||
w := httptest.NewRecorder()
|
||||
invoke(api.createPlaylist, w, createReq(`{"Name":"Mix","IsPublic":true}`))
|
||||
Expect(w.Code).To(Equal(http.StatusInternalServerError))
|
||||
})
|
||||
})
|
||||
|
||||
Describe("getPlaylistItems", func() {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue