WEBUI-1916: Drive action buttons handle failure [LTS-2023]#3065
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves Nuxeo Drive integrations in Web UI by adding a Drive “direct download” selection action and updating Drive action buttons to better handle protocol invocation failures, along with new i18n strings and unit tests.
Changes:
- Add a new
nuxeo-drive-download-buttoncomponent + icon and register it as a results selection action. - Improve Drive action behavior by detecting when
nxdrive://URLs aren’t handled and showing the install dialog / error toast. - Extend i18n messages and add unit tests for the new download button logic.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| i18n/messages.json | Fix typo (“occurred”) and add i18n strings for Drive download action + errors. |
| elements/nuxeo-app.js | Adds a direct import of the Drive addon into the core app. |
| addons/nuxeo-drive/test/nuxeo-drive-download-button.test.js | New unit tests for the download button behavior and URL building. |
| addons/nuxeo-drive/nuxeo-drive.html | Adds a new RESULTS_SELECTION_ACTIONS slot contribution for Drive download. |
| addons/nuxeo-drive/index.js | Registers the new download button element and injects addon HTML. |
| addons/nuxeo-drive/elements/nuxeo-drive-upload-button.js | Adds protocol-handler failure detection and improved toast error reporting. |
| addons/nuxeo-drive/elements/nuxeo-drive-icons.js | Adds the Drive download icon to the iconset. |
| addons/nuxeo-drive/elements/nuxeo-drive-edit-button.js | Adds protocol-handler failure detection when invoking Drive edit. |
| addons/nuxeo-drive/elements/nuxeo-drive-download-button.js | New element implementing Drive direct-download action, guardrails, and error handling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
782a136 to
7fa2e2c
Compare
38b6a39 to
f86fbc4
Compare
52570e2 to
b360683
Compare
6df44c5 to
a43daa4
Compare
47ca79b to
f0b8645
Compare
|
@copilot resolve the merge conflicts in this pull request |
Merge conflicts have been resolved in commit
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
f455896 to
6e0d473
Compare
- Left-align title and description, matching standard Nuxeo dialog layout - Move Open with Nuxeo Drive button to footer (Close left, Open right) - Style Close button with outline border matching Cancel in other dialogs - Reduce dialog padding for compact layout - Use red color for failure message text - Add _failureVisible flag so error message persists after blur events - Align desktop packages table headers with content - Improve text contrast for a11y compliance
…inline-block install link
…show install option - Remove all blur/focus/timeout detection logic and browser/OS sniffing - Use simple window.location.href for nxdrive:// protocol - Always show install link; auto-expand when no Drive token found - Show hint text after Open click with download packages - Disable Open button after click; reset on dialog reopen - Remove driveButton.dialog.couldNotOpen i18n key - Add driveButton.dialog.install.hint i18n key
…% coverage - Add addGoErrorSuites: token-found branch (_hasToken=true, _installExpanded stays false) - Add addOpenDriveSuite: covers _openDrive (_opened, _installExpanded, window.location.href) - Add addToggleInstallSuite: covers _toggleInstall (_installExpanded, preventDefault) - Wire new shared suites into edit, upload and download button test files - Add driveEditURL construction tests (valid URL shape, filename encoding) - Add _download token-found and catch branch tests for download button"
- Replace _showInstall with _installExpanded (correct property name) - Token-fetch failure tests check _installExpanded instead of toast (catch branch sets _installExpanded, not showError) - Remove addLaunchDriveSuite from edit/upload/download buttons (those elements have no _navigateTo / _launchDrive methods) - Guard window.location.href in addOpenDriveSuite setup to prevent nxdrive:// assignments triggering Karma page reload - Fix inline _openDrive suite in download-button test accordingly
Each drive button's _openDrive() now delegates window.location.href assignment to a dedicated _navigate(url) method. This makes the navigation stubbable in unit tests, eliminating the Karma 'full page reload' CI failure caused by Chrome 148 handling nxdrive:// URLs as real navigation. Updated addOpenDriveSuite in test helpers and the download-button inline _openDrive suite to stub _navigate and assert the correct URL is passed.
- Add _navigate() test suite inside addOpenDriveSuite that exercises the real window.location.href assignment path via a property getter spy - Add _compressFromOriginalUrl server-too-long error path test in download-button (crafted 300-char server segment triggers the throw) - Replace stub-based server-too-long test in upload-button with one that stubs TextEncoder.prototype.encode to force the > 255 guard, ensuring lines 242-244 of nuxeo-drive-upload-button.js are hit - Exclude addons/*/test/**/*.js from sonar coverage (test helpers are not production code; previously nuxeo-drive-test-helpers.js dragged coverage on new code down to 34.4%)
…ers.test.js The .test.js suffix matches sonar.coverage.exclusions (**/*.test.js) so Sonar no longer counts the helper file as uncovered production code, fixing the 34.4% coverage on new code reported in the PR. Also remove redundant /* global */ comment — the test environment already provides sinon, suite, setup, teardown, test and expect as built-in globals; the comment caused no-redeclare ESLint errors when the .test.js eslint config applied its test environment.
The nuxeo-drive-test-helpers.js file was renamed to nuxeo-drive-test-helpers.test.js, so it now matches the existing **/*.test.js exclusion pattern. The explicit addons/*/test/**/*.js rule added in the previous commit is no longer needed.
…e duplication Create nuxeo-drive-utils.js with: - fetchTokenAndToggleDialog(): replaces the identical 18-line token-fetch + dialog.toggle() block duplicated across edit, download and upload buttons - base64UrlSafeEncode(): replaces the identical _base64UrlSafeEncode() method duplicated across download and upload buttons Update all three element files to import from the shared utils module. Update nuxeo-drive-upload-button.test.js to call base64UrlSafeEncode() directly instead of via element._base64UrlSafeEncode() (method removed). All 1965 tests continue to pass.
- Optional chain: tokens && tokens.length -> tokens?.length (L41) - String.fromCodePoint over String.fromCharCode (L62) - replaceAll over regex replace for + and / substitutions (L64, x2)
Replace /=+$/ regex with indexOf + slice to strip Base64 padding, eliminating the security hotspot without changing behaviour.
- Replace unreliable protocol detection with immediate nxdrive:// navigation - Show lightweight fallback dialog with expandable install links - Remove token fetch, sessionStorage suppression, and detection hacks - Simplify all three buttons (edit, upload, download) to consistent UX - Add i18n keys for new dialog heading and hint text
Rewrite test files to match the refactored Drive action buttons: - Remove references to removed APIs ($.token, _hasToken, _opened, _navigate, _openDrive) - Replace old suite factories with addGoSuite, addShowErrorSuite, addToggleInstallSuite - Keep URL compression, guard condition, and availability tests
Replace globalThis.location.href with anchor element click in navigateAndShowFallback to prevent full page reloads in browsers that don't recognize the nxdrive:// custom protocol. Anchor clicks with unknown schemes are silently ignored by the browser. Move the HTMLAnchorElement.prototype.click stub into test helpers so all drive test files benefit from it.
Add tests for uncovered branches: - edit button: _isAvailable returning true, blob without appLinks property - upload button: _isAvailable with null/undefined doc, _go catch branch - download button: _download catch branch with/without userMessage
…igate - Add defensive check for element._navigate existence before stubbing - Prevents flaky tests caused by shared globalThis.location state - Ensures test isolation: each element instance has its own navigation stub - Gracefully handles elements without _navigate method - Addresses Copilot review comment about test order dependency
9e55b20 to
1408f1e
Compare
|



https://hyland.atlassian.net/browse/WEBUI-1916