Skip to content

Add personalised mounting instructions for research drive - #81

Merged
akea013 merged 2 commits into
masterfrom
feat/personalised-drive-instructions
Oct 8, 2026
Merged

akea013 merged 2 commits into
masterfrom
feat/personalised-drive-instructions

Conversation

@devslashnoel

Copy link
Copy Markdown
Member

Currently, the changes apply to Windows and Mac. Do we want it for Linux too?

Comment thread assets/doc/personalised-instructions.js Fixed
…te scripting'

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@cszhe
cszhe requested a balanced review from Copilot October 8, 2026 10:20

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The personalised paths match the documented backend prefixes and preserve existing instructions when parameters are absent.

0 open findings

What changed in this PR

Adds URL-driven personalised Research Drive paths to the Windows and macOS connection guides.

Changes:

  • Builds legacy or VAST drive paths from query parameters.
  • Shows personalised paths while retaining default guidance as fallback.
  • Applies personalisation to Windows and macOS pages; Linux remains unchanged.
File Description
assets/​doc/​personalised-instructions.js Generates OS- and backend-specific drive paths.
_doc/​map-drive-windows.md Adds personalised Windows path elements and script initialization.
_doc/​map-drive-mac.md Adds personalised macOS path elements and script initialization.

🧠 Review effort: Balanced


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

@UoA-eResearch UoA-eResearch deleted a comment from claude Bot Oct 8, 2026
@UoA-eResearch UoA-eResearch deleted a comment from claude Bot Oct 8, 2026
@cszhe

cszhe commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator

@claude please review the PR.

@claude

claude Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Claude finished @cszhe's task in 20s —— View job


Review of PR #81: personalised Research Drive mounting instructions

I read the final state of assets/doc/personalised-instructions.js, _doc/map-drive-windows.md and _doc/map-drive-mac.md. I didn't build the site or run the page in a browser. git diff origin/master...HEAD failed with "no merge base" on this shallow checkout, so I reviewed the files as they are on the branch.

Verdict: looks good to merge, with a few optional suggestions.

Security

  • The CodeQL XSS finding is fixed. The script now uses textContent (personalised-instructions.js:22) instead of innerHTML, so a crafted ?name=<img onerror=…> renders as plain text. The style.display toggles take no user input.
  • name is only shown as text, never used as a link or navigation target. A crafted link can still show a misleading path, for example ?name=anything. That is low risk because the user only reads the path. If you want to tighten it, restrict the value with something like /^[\w .\-]+$/ and fall back to the default instructions when it doesn't match.

Correctness

  • The path prefixes look right:
    • Windows legacy: \\files.auckland.ac.nz\research\<name>
    • Windows VAST: \\research.drive.auckland.ac.nz\<name>
    • Mac: smb://… with / dividers
  • If name is absent, the default instructions stay visible, so the pages work as before.
  • The scripts are loaded after the elements they touch, so the getElementById calls find them.

Minor suggestions

  1. Simplify the backend check (line 14): params.get("backend") !== null && params.get("backend") === "vast" can be params.get("backend") === "vast".
  2. Missing semicolon on the VAST basePath assignment (line 15). It works through automatic semicolon insertion but is inconsistent with the surrounding lines.
  3. Whitespace in name: use params.get("name")?.trim() so ?name=%20 doesn't trigger personalisation with an empty path.
  4. Missing elements: if the script is later reused on a page without these elements (for example Linux), getElementById(...) returns null and .style throws. A small guard avoids that.
  5. No .catch or fallback for old browsers: URLSearchParams is fine for all current browsers, so I'd leave it.

Your question: Linux?

It's worth doing only if Linux users mount the drive by typing a path. If the Linux guide has a mount or smb:// step, the same script can be reused: pass "linux" and it already produces smb:// paths. If the Linux instructions are command-line (mount -t cifs //host/share), the format differs (//host/share), so the script would need a third branch. I'd merge Windows and Mac now and do Linux as a follow-up once the Linux guide's path format is settled.

@UoA-eResearch UoA-eResearch deleted a comment from claude Bot Oct 8, 2026
@akea013
akea013 merged commit b06257d into master Oct 8, 2026
4 checks passed
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.

5 participants