NXDRIVE-3117: Direct transfer Web UI icon doesn't work for a target that has spaces in its name#6346
Open
gitofanindya wants to merge 10 commits into
Conversation
…hat has spaces in its name
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideFixes parsing of Direct Transfer protocol URLs whose target path contains spaces by decoding URL-encoded characters, and updates the 7.0.0 changelog entry to reference NXDRIVE-3117 and describe the issue accurately. Flow diagram for Direct Transfer protocol URL parsing with URL decodingflowchart TD
A[Direct_Transfer_icon_clicked_in_Web_UI] --> B[Browser_opens_nxdrive_protocol_URL]
B --> C[parse_protocol_url_receives_url_string]
C --> D[Detect_Direct_Transfer_command_and_extract_remote_path_with_regex]
D --> E[Decode_URL_encoded_characters_in_remote_path_using_unquote]
E --> F[Return_parsed_data_with_command_and_decoded_remote_path]
F --> G[NX_Drive_uses_remote_path_even_if_it_contains_spaces]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider limiting
unquoteto the path component only (e.g., viaurlparse) to avoid unintentionally decoding query strings or fragments if those are ever appended to the protocol URL. - It may be worth adding a brief comment or assertion about the expected form of
url_string(e.g., whether it can already be decoded or contain%2520), so future maintainers understand why a singleunquoteis considered safe here.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider limiting `unquote` to the path component only (e.g., via `urlparse`) to avoid unintentionally decoding query strings or fragments if those are ever appended to the protocol URL.
- It may be worth adding a brief comment or assertion about the expected form of `url_string` (e.g., whether it can already be decoded or contain `%2520`), so future maintainers understand why a single `unquote` is considered safe here.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6346 +/- ##
==========================================
+ Coverage 85.88% 85.90% +0.01%
==========================================
Files 96 96
Lines 16517 16519 +2
==========================================
+ Hits 14185 14190 +5
+ Misses 2332 2329 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes Direct Transfer protocol URL handling for Web UI targets whose names include URL-encoded characters (notably spaces), so the correct remote folder can be preselected in the Direct Transfer dialog.
Changes:
- Decode the extracted Web UI
remote_pathinparse_protocol_url()usingurllib.parse.unquote(). - Update the 7.0.0 changelog entry to reference NXDRIVE-3117 with the correct description.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
nxdrive/utils.py |
Decodes URL-encoded characters in Web UI direct-transfer remote_path before returning it. |
docs/changes/7.0.0.md |
Adds the NXDRIVE-3117 entry under Direct Transfer. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+925
to
+927
| # Decode URL-encoded characters (e.g., %20 -> space) | ||
| # Note: literal %20 in folder names would be encoded as %2520, so this is safe | ||
| remote_path = unquote(remote_path) |
Comment on lines
925
to
928
| # Decode URL-encoded characters (e.g., %20 -> space) | ||
| # Note: literal %20 in folder names would be encoded as %2520, so this is safe | ||
| remote_path = unquote(remote_path) | ||
| return { |
gitofanindya
force-pushed
the
wip-NXDRIVE-3117-direct-transfer-web-ui-icon-doesnt-work-for-a-target-that-has-spaces-in-its-name
branch
from
bea2e91 to
5cf5373
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes the nxdrive://direct-transfer/... Web UI protocol handler so remote target paths containing URL-encoded characters (notably spaces encoded as %20) are properly decoded before being used by the Direct Transfer UI, and documents the fix in the 7.0.0 changelog.
Changes:
- Decode the extracted Web UI
remote_pathviaurllib.parse.unquote()when handlingdirect-transferprotocol URLs. - Update the 7.0.0 changelog entry to reference NXDRIVE-3117 and describe the issue/fix.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| nxdrive/utils.py | Decodes URL-encoded characters in remote_path for Web UI direct-transfer protocol URLs. |
| docs/changes/7.0.0.md | Documents the NXDRIVE-3117 Direct Transfer Web UI fix in the 7.0.0 changelog. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…k-for-a-target-that-has-spaces-in-its-name' of https://github.com/nuxeo/nuxeo-drive into wip-NXDRIVE-3117-direct-transfer-web-ui-icon-doesnt-work-for-a-target-that-has-spaces-in-its-name
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes Windows handling of nxdrive://direct-transfer/... URLs originating from the Web UI by decoding URL-encoded characters in the extracted remote path, ensuring targets with spaces (and other encoded chars) resolve correctly. Adds focused unit tests and updates the 7.0.0 changelog entry.
Changes:
- Decode
remote_pathviaurllib.parse.unquote()for direct-transfer Web UI protocol URLs on Windows. - Add unit tests covering
%20decoding,%2520preserving literal%20, and non-Windows no-decoding behavior. - Update
docs/changes/7.0.0.mdto reference NXDRIVE-3117.
Reviewed changes
Copilot reviewed 4 out of 15 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
nxdrive/utils.py |
Decode Web UI direct-transfer remote_path on Windows to support spaces/URL-encoded characters. |
tests/unit/test_utils.py |
Add OS-specific tests validating decoding behavior for direct-transfer Web UI URLs. |
docs/changes/7.0.0.md |
Document NXDRIVE-3117 in the 7.0.0 changelog. |
nxdrive/updater/__init__.py |
Docstring formatting cleanup. |
nxdrive/osi/darwin/pyNotificationCenter.py |
Docstring formatting cleanup. |
nxdrive/logging_config.py |
Docstring formatting cleanup. |
nxdrive/gui/folders_loader.py |
Minor unicode escape formatting change in an error label. |
nxdrive/gui/application.py |
Docstring formatting cleanup. |
nxdrive/console.py |
Docstring formatting cleanup. |
nxdrive/commandline.py |
Docstring formatting cleanup. |
nxdrive/client/local/windows.py |
Docstring formatting cleanup. |
nxdrive/client/local/linux.py |
Docstring formatting cleanup. |
nxdrive/client/local/darwin.py |
Docstring formatting cleanup. |
nxdrive/client/local/base.py |
Docstring formatting cleanup. |
nxdrive/client/local/__init__.py |
Docstring formatting cleanup. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes NXDRIVE-3117 by ensuring nxdrive://direct-transfer/... Web UI protocol URLs correctly resolve remote targets whose paths contain URL-encoded characters (notably spaces) on Windows, and records the change in the 7.0.0 changelog. The PR also includes a handful of formatting/cleanup changes (docstrings and test data literals) that don’t alter behavior.
Changes:
- Decode URL-encoded remote paths for direct-transfer Web UI protocol URLs on Windows (
%20→ space;%2520→ literal%20). - Add unit tests covering Windows decoding behavior and non-Windows no-decoding behavior.
- Update the 7.0.0 changelog entry for NXDRIVE-3117; minor formatting cleanups across tests/docs.
Reviewed changes
Copilot reviewed 9 out of 23 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
nxdrive/utils.py |
Decode remote_path via urllib.parse.unquote() for Web UI direct-transfer URLs on Windows. |
tests/unit/test_utils.py |
Adds unit tests validating decoding behavior (Windows) and preservation (non-Windows). |
docs/changes/7.0.0.md |
Documents the NXDRIVE-3117 fix in the 7.0.0 changelog. |
nxdrive/gui/folders_loader.py |
No functional change; normalizes Unicode escape casing in an error placeholder label. |
tests/integration/windows/utils.py |
Refactors a debug print() string concatenation for readability (no behavior change). |
tests/functional/test_remote_watcher.py |
Replaces eval()-built structures with explicit dict literals (safer/clearer test data). |
tests/functional/test_remote_client.py |
Cleans up token string literals (line continuation removal; no behavior change). |
tools/scripts/slow_thread_detection.py |
Docstring formatting cleanup. |
tests/unit/gui/test_folders_treeview.py |
Removes an extraneous blank line. |
tests/unit/gui/__init__.py |
Docstring formatting cleanup. |
tests/old_functional/test_local_changes_when_offline.py |
Fixes/normalizes module docstring quoting/formatting. |
tests/old_functional/common.py |
Docstring formatting cleanup. |
nxdrive/updater/__init__.py |
Docstring formatting cleanup. |
nxdrive/osi/darwin/pyNotificationCenter.py |
Docstring formatting cleanup. |
nxdrive/logging_config.py |
Docstring formatting cleanup. |
nxdrive/gui/application.py |
Docstring formatting cleanup (no protocol-handling logic changes in this diff). |
nxdrive/console.py |
Docstring formatting cleanup. |
nxdrive/commandline.py |
Docstring formatting cleanup. |
nxdrive/client/local/windows.py |
Docstring formatting cleanup. |
nxdrive/client/local/linux.py |
Docstring formatting cleanup. |
nxdrive/client/local/darwin.py |
Docstring formatting cleanup. |
nxdrive/client/local/base.py |
Docstring formatting cleanup. |
nxdrive/client/local/__init__.py |
Docstring formatting cleanup. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes the Windows direct-transfer “Web UI icon” flow by decoding URL-encoded remote paths (e.g., %20 → space) when parsing nxdrive://direct-transfer/... URLs, and adds regression tests plus changelog entry. The remainder of the PR is largely formatting/cleanup (docstrings, test fixtures, and removing eval() in tests) that supports maintainability and test safety.
Changes:
- Decode URL-encoded characters for direct-transfer Web UI protocol URLs on Windows.
- Add unit tests covering decoding behavior (including
%2520→ literal%20) and non-Windows behavior. - Update 7.0.0 changelog and apply various small formatting/cleanup adjustments (including replacing
eval()with dict literals in tests).
Reviewed changes
Copilot reviewed 12 out of 26 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
nxdrive/utils.py |
Decode direct-transfer Web UI remote paths on Windows via unquote(). |
tests/unit/test_utils.py |
Add targeted tests for Windows decoding and non-Windows no-decoding. |
docs/changes/7.0.0.md |
Document NXDRIVE-3117 fix in the 7.0.0 changelog. |
tests/functional/test_remote_watcher.py |
Replace eval()-based test data with explicit dict literals for safer tests. |
tests/integration/windows/utils.py |
Normalize multi-line debug print() formatting. |
tests/unit/test_pytest_random.py |
Formatting-only updates to embedded test source strings. |
tests/unit/test_commandline.py |
Formatting-only updates to multi-line strings written to INI files. |
tests/unit/gui/test_folders_treeview.py |
Remove stray blank line (formatting cleanup). |
tests/unit/gui/__init__.py |
Docstring formatting cleanup. |
tests/old_functional/common.py |
Docstring formatting cleanup. |
tests/old_functional/test_local_changes_when_offline.py |
Fix malformed opening docstring delimiter. |
tests/functional/test_remote_client.py |
Formatting-only updates to token strings (remove line continuations). |
nxdrive/gui/folders_loader.py |
Minor unicode escape formatting change (no behavioral impact). |
nxdrive/gui/application.py |
Docstring formatting cleanup. |
nxdrive/commandline.py |
Docstring formatting cleanup. |
nxdrive/console.py |
Docstring formatting cleanup. |
nxdrive/logging_config.py |
Docstring formatting cleanup. |
nxdrive/updater/__init__.py |
Docstring formatting cleanup. |
nxdrive/osi/darwin/pyNotificationCenter.py |
Docstring formatting cleanup. |
nxdrive/client/local/__init__.py |
Docstring formatting cleanup. |
nxdrive/client/local/base.py |
Docstring formatting cleanup. |
nxdrive/client/local/darwin.py |
Docstring formatting cleanup. |
nxdrive/client/local/linux.py |
Docstring formatting cleanup. |
nxdrive/client/local/windows.py |
Docstring formatting cleanup. |
tools/scripts/slow_thread_detection.py |
Docstring formatting cleanup. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes Direct Transfer Web UI protocol handling so that URL-encoded remote paths (notably spaces) are decoded on Windows, ensuring the target folder is correctly selected/used when launched from the browser.
Changes:
- Decode URL-encoded characters in
nxdrive://direct-transfer/.../nuxeo/...remote paths on Windows. - Add unit tests covering decoding behavior (Windows) and non-decoding behavior (non-Windows) for Web UI direct-transfer URLs.
- Update the 7.0.0 changelog entry for NXDRIVE-3117 (plus assorted formatting/cleanup changes in tools/tests).
Reviewed changes
Copilot reviewed 12 out of 26 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tools/scripts/slow_thread_detection.py | Docstring formatting cleanup. |
| tools/changelog.py | Reformat multi-line print() example string construction. |
| tests/unit/test_utils.py | Add unit tests validating Windows-only decoding for Web UI direct-transfer URLs. |
| tests/unit/test_pytest_random.py | Reformat makepyfile() multi-line strings for readability. |
| tests/unit/test_commandline.py | Reformat INI file multi-line string writes. |
| tests/unit/gui/test_folders_treeview.py | Remove stray blank line. |
| tests/unit/gui/init.py | Docstring formatting cleanup. |
| tests/old_functional/test_local_changes_when_offline.py | Fix malformed module docstring quoting. |
| tests/old_functional/common.py | Docstring formatting cleanup. |
| tests/integration/windows/utils.py | Replace line-continuation f-string with concatenated f-strings (readability). |
| tests/functional/test_remote_watcher.py | Replace eval()-built structures with explicit dict literals (safer/clearer). |
| tests/functional/test_remote_client.py | Reformat long token strings (no semantic change). |
| nxdrive/utils.py | Implement Windows-only unquote() of Web UI direct-transfer remote_path. |
| nxdrive/updater/init.py | Docstring formatting cleanup. |
| nxdrive/osi/darwin/pyNotificationCenter.py | Docstring formatting cleanup. |
| nxdrive/logging_config.py | Docstring formatting cleanup. |
| nxdrive/gui/folders_loader.py | Unicode escape hex-case change (no semantic impact). |
| nxdrive/gui/application.py | Docstring formatting cleanup. |
| nxdrive/console.py | Docstring formatting cleanup. |
| nxdrive/commandline.py | Docstring formatting cleanup. |
| nxdrive/client/local/windows.py | Docstring formatting cleanup. |
| nxdrive/client/local/linux.py | Docstring formatting cleanup. |
| nxdrive/client/local/darwin.py | Docstring formatting cleanup. |
| nxdrive/client/local/base.py | Docstring formatting cleanup. |
| nxdrive/client/local/init.py | Docstring formatting cleanup. |
| docs/changes/7.0.0.md | Add changelog entry for NXDRIVE-3117 Direct Transfer Web UI fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
S-Ghosh-Dev2
approved these changes
Apr 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…hat has spaces in its name
Summary by Sourcery
Decode direct transfer URLs to correctly handle remote paths containing URL-encoded characters and document the associated change.
Bug Fixes:
Documentation: