feat: search album by name #13434
feat: search album by name #13434
Conversation
martabal
force-pushed
the
feat/search-album-by-name
branch
4 times, most recently
from
296f79e to
843eb4f
Compare
| @@ -302,4 +302,47 @@ export class AlbumRepository implements IAlbumRepository { | |||
|
|
|||
| return result.affected; | |||
| } | |||
|
|
|||
| @GenerateSql({ params: [DummyValue.UUID, DummyValue.STRING, undefined] }) | |||
| getByName(userId: string, albumName: string, shared?: boolean): Promise<AlbumEntity[]> { | |||
There was a problem hiding this comment.
This should be paginated, no? Only returning the first 1k results is optimistic 😅
There was a problem hiding this comment.
It would be nice but I don't think that really matters: the goal of this endpoint is not to get all albums.
There was a problem hiding this comment.
IMO we should try to have all of our endpoints behave in a similar manner. Search person vs search album vs search something else should all have the same method signatures.
There was a problem hiding this comment.
I strongly agree and there are people out there with >10k albums, for whom a search result may very well include >1k albums. I think it's bad just not supporting that.
There was a problem hiding this comment.
I think it's bad just not supporting that.
I agree. But that's for a future PR 😝
There was a problem hiding this comment.
I agree. But that's for a future PR 😝
Why? :P
There was a problem hiding this comment.
Why? :P
Because I agree with @jrasm91 too. Do we want the same method signatures or not ?
There was a problem hiding this comment.
Well... I added pagination for both endpoints.
There was a problem hiding this comment.
LOL. Thank you very much! :)
martabal
force-pushed
the
feat/search-album-by-name
branch
from
843eb4f to
f391adf
Compare
martabal
force-pushed
the
feat/search-album-by-name
branch
4 times, most recently
from
87fa15f to
1751e5f
Compare
martabal
force-pushed
the
feat/search-album-by-name
branch
2 times, most recently
from
1751e5f to
5889faa
Compare
|
@danieldietzler So the API for |
Yup, we probably should. It's great that we don't have mobile conflicts though |
martabal
force-pushed
the
feat/search-album-by-name
branch
3 times, most recently
from
b9c5144 to
c0a39fa
Compare
martabal
force-pushed
the
feat/search-album-by-name
branch
from
c0a39fa to
274de40
Compare
martabal
force-pushed
the
feat/search-album-by-name
branch
from
b843db3 to
db1915c
Compare
This new
searchAlbumendpoint behaves like thesearchPersonendpoint. It returns a list of albums based on the beginning of the album name you are searching for. Currently, album search based on name is done on the client side, which is not ideal when you have hundreds or thousands of albums.