Skip to content
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

ensure WebDAVProvider.rootPath is a folder #336

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sdumetz
Copy link
Contributor

@sdumetz sdumetz commented Dec 18, 2024

Setting up the default WebDAV server, if you create a folder (cube in my example) with a dummy scene with it and navigate to http://localhost:8000/voyager-story-dev.html?document=/cube/scene.svx.json WITHOUT setting the root parameter, the scene kinda works but for the asset tree not matching properly.

Easiest way to show this is click Create in the Article Task. Nothing is shown until you click somewhere to deselect said article.

I traced it back to WebDAVProvider.rootPath, being normally set to /cube/ but defaults to the value of ExplorerApplication.props.document, here /cube/scene.svx.json.

I checked usage of this path and it looks like it's only used in WebDAVProvider.parseResponse

Not sure if expecting rootPath to always end with a trailing "/" is a valid option though? It doesn't seem to work otherwise anyway? if I set root=/cube the scene won't load

@gjcope
Copy link
Collaborator

gjcope commented Jan 17, 2025

Sounds reasonable and seems to work fine. Merged with https://github.com/Smithsonian/dpo-voyager/tree/rc-48

Seems like this could be extended to the root url as well as it is not forgiving about the trailing slash. I don't think it's an issue to expect a well formed path to a directory, but it's also nice to make it mistake-proof where possible.

@sdumetz
Copy link
Contributor Author

sdumetz commented Jan 22, 2025

Seems like this could be extended to the root url as well as it is not forgiving about the trailing slash. I don't think it's an issue to expect a well formed path to a directory, but it's also nice to make it mistake-proof where possible.

As someone who made this mistake more than once, I wholeheartedly agree. It won't break anything for me but I don't know if anyone might be relying on the current behavior?

@gjcope
Copy link
Collaborator

gjcope commented Jan 22, 2025

I don't think sticking a slash on the end when it's not there is too intrusive. The only scenario I can think of is if someone is doing url routing to their backend (which I know has happened) and it messes with their tokenizing or something.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants