fix(downloads): OneDrive-aware Downloads path + PDF uses html_download_button
User reported downloads "do nothing on click" in tool pages and
"acts like it downloads but no file in the folder" in the PDF
tool. Two root causes, two fixes.
**Root cause #1 — wrong Downloads folder on Windows.**
``_downloads_dir()`` returned ``Path.home() / "Downloads"``
unconditionally. On Windows machines with OneDrive enabled
(very common for business users), the real Downloads folder
is redirected to ``C:\Users\<u>\OneDrive\Downloads``. Our
helper would write to ``C:\Users\<u>\Downloads`` instead —
a folder that may not even exist until ``mkdir`` creates it —
and the user, naturally opening their actual OneDrive
Downloads, sees no file and concludes nothing happened.
Now: on Windows, ``_downloads_dir`` queries the registry key
``Software\Microsoft\Windows\CurrentVersion\Explorer\User
Shell Folders`` for FOLDERID_Downloads (GUID
``{374DE290-123F-4565-9164-39C4925E467B}``). This entry returns
the redirected path when OneDrive is active, the original
``%USERPROFILE%\Downloads`` otherwise — exactly what the user's
File Explorer reads. ``%USERPROFILE%`` expansion is applied
via ``os.path.expandvars``. Any registry hiccup falls through
to ``Path.home() / "Downloads"`` so the helper never raises.
The sanity check (path exists OR parent exists) catches the
edge case where the registry points into a deleted OneDrive
mount.
**Root cause #2 — PDF page used st.download_button.**
Every other tool uses the project's ``html_download_button``
helper (which is ``local_download_button`` under the hood —
the rename happened in b9147f3). ``st.download_button`` has a
long-standing bug where the second-or-later instance in a
script pass silently fails to fire. The PDF tool predated the
rewrite that switched everyone over and was still using the
broken native widget. ``_Logs.py`` had the same problem in two
places.
Swapped all three call sites to ``html_download_button``. They
now save to ``~/Downloads/<filename>`` (correctly resolved per
fix #1) and show the saved path + "Open Downloads folder"
button below the click, matching every other tool in the suite.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1391,15 +1391,53 @@ def _farewell_script() -> str:
|
||||
def _downloads_dir() -> "Path":
|
||||
"""Return the user's Downloads folder.
|
||||
|
||||
Defaults to ``~/Downloads``. Overrideable via the
|
||||
``DATATOOLS_DOWNLOADS_DIR`` environment variable so tests can write
|
||||
to a temp directory instead of polluting the developer's home.
|
||||
Resolution order:
|
||||
|
||||
1. ``DATATOOLS_DOWNLOADS_DIR`` env var (tests + power users).
|
||||
2. On Windows, the *real* Downloads path from the
|
||||
``User Shell Folders`` registry key. This matters because
|
||||
OneDrive can redirect Downloads to
|
||||
``C:\\Users\\<user>\\OneDrive\\Downloads`` — without the
|
||||
registry lookup we'd write files into the un-redirected
|
||||
``C:\\Users\\<user>\\Downloads`` and the user would never
|
||||
see them in the Downloads they actually open.
|
||||
3. ``Path.home() / "Downloads"`` as the final fallback.
|
||||
"""
|
||||
import os
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
override = os.environ.get("DATATOOLS_DOWNLOADS_DIR")
|
||||
if override:
|
||||
return Path(override)
|
||||
|
||||
if sys.platform == "win32":
|
||||
try:
|
||||
import winreg # noqa: PLC0415
|
||||
|
||||
with winreg.OpenKey(
|
||||
winreg.HKEY_CURRENT_USER,
|
||||
r"Software\Microsoft\Windows\CurrentVersion\Explorer"
|
||||
r"\User Shell Folders",
|
||||
) as key:
|
||||
# GUID for FOLDERID_Downloads. The User Shell
|
||||
# Folders entry returns the redirected path when
|
||||
# OneDrive is active, the original ~/Downloads
|
||||
# otherwise — exactly what we want.
|
||||
value, _ = winreg.QueryValueEx(
|
||||
key, "{374DE290-123F-4565-9164-39C4925E467B}",
|
||||
)
|
||||
expanded = os.path.expandvars(value)
|
||||
resolved = Path(expanded)
|
||||
# Sanity check: only trust the registry path if it
|
||||
# exists OR can be created (don't return a path that
|
||||
# points into a deleted/borked OneDrive mount).
|
||||
if resolved.exists() or resolved.parent.exists():
|
||||
return resolved
|
||||
except Exception:
|
||||
# Any registry hiccup — fall through to ``Path.home()``.
|
||||
pass
|
||||
|
||||
return Path.home() / "Downloads"
|
||||
|
||||
|
||||
|
||||
@@ -21,7 +21,11 @@ if str(_project_root) not in sys.path:
|
||||
sys.path.insert(0, str(_project_root))
|
||||
|
||||
from src.audit import log_event, log_page_open
|
||||
from src.gui.components import hide_streamlit_chrome, render_sticky_footer
|
||||
from src.gui.components import (
|
||||
hide_streamlit_chrome,
|
||||
html_download_button,
|
||||
render_sticky_footer,
|
||||
)
|
||||
from src.pdf_extract import (
|
||||
PdfDependencyMissing,
|
||||
diagnose_pdf_lines,
|
||||
@@ -627,12 +631,16 @@ else:
|
||||
):
|
||||
export[amt_col] = export[amt_col].map(format_amount)
|
||||
csv_bytes = export.to_csv(index=False).encode("utf-8")
|
||||
st.download_button(
|
||||
# Save server-side (consistent with the other tools) —
|
||||
# writes to the user's Downloads folder and shows the
|
||||
# exact path. Avoids the st.download_button quirk where
|
||||
# the second-or-later button in a script pass silently
|
||||
# fails to fire.
|
||||
html_download_button(
|
||||
f"Download {len(export):,} rows as CSV",
|
||||
data=csv_bytes,
|
||||
csv_bytes,
|
||||
file_name=f"transactions-{ts}.csv",
|
||||
mime="text/csv",
|
||||
type="primary",
|
||||
)
|
||||
|
||||
if not selected.empty:
|
||||
|
||||
@@ -24,7 +24,11 @@ if str(_project_root) not in sys.path:
|
||||
sys.path.insert(0, str(_project_root))
|
||||
|
||||
from src.audit import _RETENTION_DAYS, audit_log_dir, audit_log_path
|
||||
from src.gui.components import hide_streamlit_chrome, render_sticky_footer
|
||||
from src.gui.components import (
|
||||
hide_streamlit_chrome,
|
||||
html_download_button,
|
||||
render_sticky_footer,
|
||||
)
|
||||
|
||||
_ICON_PATH = str(Path(__file__).parent.parent / "assets" / "datatools_icon_256.png")
|
||||
st.set_page_config(
|
||||
@@ -54,12 +58,11 @@ if today_path.exists():
|
||||
st.caption(f"{size:,} bytes")
|
||||
try:
|
||||
data = today_path.read_bytes()
|
||||
st.download_button(
|
||||
label=f"Download {today_path.name}",
|
||||
data=data,
|
||||
html_download_button(
|
||||
f"Download {today_path.name}",
|
||||
data,
|
||||
file_name=today_path.name,
|
||||
mime="application/x-ndjson",
|
||||
key="dl_today",
|
||||
)
|
||||
except Exception as e:
|
||||
st.warning(f"Could not read today's log: {type(e).__name__}: {e}")
|
||||
@@ -88,12 +91,11 @@ else:
|
||||
f"{st_mtime:%Y-%m-%d %H:%M} · {size:,} bytes"
|
||||
)
|
||||
with cols[2]:
|
||||
st.download_button(
|
||||
label="Download",
|
||||
data=p.read_bytes(),
|
||||
html_download_button(
|
||||
"Download",
|
||||
p.read_bytes(),
|
||||
file_name=p.name,
|
||||
mime="application/x-ndjson",
|
||||
key=f"dl_{p.name}",
|
||||
)
|
||||
except Exception as e:
|
||||
st.caption(
|
||||
|
||||
Reference in New Issue
Block a user