Fix partial filename from paths_prefix #591
Conversation
| @@ -440,6 +440,8 @@ def paths(self, request, versions__dandiset__pk: str, versions__version: str, ** | |||
| is_folder = folder_index >= 0 | |||
|
|
|||
| if not is_folder: | |||
| # Ensure base_path is entire filename | |||
| base_path = asset.path[asset.path.rfind('/') + 1 :] | |||
There was a problem hiding this comment.
could you please add a unittest for this case?
There was a problem hiding this comment.
Apologies for never providing an update: I attempted to add a quick test for this, but ran into logistical issues. Since at the time I wasn't active on this project, and this enhancement came from work on another repository, I had to abandon this to continue my other obligations. Maybe now that I'm back on the project I can finish this up.
There was a problem hiding this comment.
Welcome back @AlmightyYakob !
There was a problem hiding this comment.
@AlmightyYakob please update on adding a unittest -- this is the eldest (6 mo?) PR ATM. We better decide on its destiny asap
There was a problem hiding this comment.
@AlmightyYakob ping on this PR.
There was a problem hiding this comment.
Adding a unit test here would be nice, but requires a refactor of our testing setup, and doesn't provide much value, so I'll just merge this.
|
🚀 PR was released in |
I was using the asset paths code for OASIS project, and I noticed a bug: If you provide part of an asset's path in
path_prefix, the resulting path infileswill be the remainder, not the entire filename, as you'd expect.For example, given an asset with the name
a/b/c.zip, providinga/b/ctopath_prefixyields an entry infileswith.zip.This PR just fixes the name if it's a file. The behavior is still valid if it's a folder, so it doesn't touch that branch.
While that endpoint does have the following description
I don't see a reason not to handle this case 🤷♂️