Skip to content

FTP: unified-canvas UX, legacy designer removal, SFTP enumeration fixes [STUD-80716] - #590

Open
viogroza wants to merge 3 commits into
developfrom
fix/ftp_ux_changes
Open

FTP: unified-canvas UX, legacy designer removal, SFTP enumeration fixes [STUD-80716]#590
viogroza wants to merge 3 commits into
developfrom
fix/ftp_ux_changes

Conversation

@viogroza

@viogroza viogroza commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Adopts the unified canvas across all 8 FTP activities, removes the legacy WPF designer project, and fixes the reported SFTP enumeration crash. Follows the Cryptography pack (c21a272, STUD-80718) as the reference implementation.

Unified canvas

  • Property configuration moved out of ActivitiesMetadata.json into the ViewModels. The JSON now declares activities only ("Properties": []), matching Cryptography. WithFtpSession's Body is declared as a DesignProperty and hidden in the ViewModel, the same way JavaScopeViewModel handles its scope body.
  • One section vocabulary across both layers: Input -> Options -> Security / Proxy (scope only) -> Output last, applied to the ViewModels and the runtime [LocalizedCategory] attributes so the two cannot drift.
  • Toggle for every boolean (including ContinueOnError), Number for Port / Timeout / ProxyPort, a data source for FtpsMode, placeholders on path fields.
  • Verb-first renames: Directory Exists -> Check If Folder Exists, File Exists -> Check If File Exists, Enumerate Objects -> List Files and Folders. Old names keep working as search aliases via DisplayNameAliasKeys.
  • Three field-anchored design-time validations on WithFtpSession (missing username, no authentication method, negative timeout).

Legacy designer removal

UiPath.FTP.Activities.Design deleted (40 files) - activities render from their ViewModels now, mirroring Cryptography's feb1052. Packaging narrows to $(PortableFramework); the Windows TFM existed only to carry the Design dll. Activity search synonyms were preserved through the metadata JSON.

SFTP enumeration fix

The reported failure - SftpPathNotFoundException: No such file when enumerating ".":

  • An unreadable sub-directory no longer discards the whole enumeration. Servers can return an entry from readdir and then refuse opendir on it. That sub-directory is now skipped and its path traced, instead of failing the operation. Connection and timeout failures still propagate, and the caller's own RemotePath is not covered by the leniency.
  • Dropped the ChangeDirectory / restore dance. It was not in a finally, so a mid-walk failure left the shared session's working directory pointing into the tree - and since the scope shares one session, with ContinueOnError = True every later relative path resolved from the wrong place.
  • 16 listing tests added (11 enumeration, 5 download).

Behaviour changes reviewers should look at

  1. The enumeration leniency above - by CONTRIBUTING's wording ("an exception is no longer thrown") this is a Bucket 1 change. It is the point of the fix, but calling it out explicitly. Affects List Files and Folders and Download Files on SFTP only; Delete, Upload, Move and the Exists pair are untouched, as is plain FTP/FTPS.
  2. Download Files can now report success on a partial transfer when a sub-folder is skipped. The trace names the path; the activity outcome does not. Documented.
  3. The new design-time validations change exception type and timing for an already-broken scope - InvalidWorkflowException at workflow load instead of ArgumentNullException during execution, which an in-workflow Try/Catch can no longer intercept. No previously-working workflow is affected: each validation fires only where ExecuteAsync already threw.

Symbolic-link handling is unchanged across all 8 activities - the recursion and file-selection predicates are equivalent to the pre-branch ones. No public API change: no activity property added, removed, renamed or retyped; no [RequiredArgument] / [Obsolete] / [DefaultValue] change; no design-time IsRequired change; UiPath.FTP.Activities.API untouched.

Known limitation - documented, pinned, deliberately not fixed here

FtpObjectType.Link is unreachable on the SFTP path. SSH.NET's IsRegularFile is a (mode & S_IFREG) == S_IFREG test and S_IFLNK contains S_IFREG, so every symbolic link classifies as File: List Files and Folders' Link filter matches nothing over SFTP, and Check If File Exists answers True for a link while Check If Folder Exists answers False for a linked folder. FTP/FTPS report Link correctly, so the two protocols disagree.

The fix is understood - separate the dispatch classifier from the reporting one; a naive reorder regresses Download and FileExists - but it changes FtpObjectInfo.Type for shipped workflows, so it belongs in its own change with its own release note. The shipped docs now state it everywhere they previously promised Link, and SFTP_GetFtpObjectType_ClassifiesSymbolicLinkAsFile pins it so it cannot be changed by accident.

Verification

  • Activities.FTP.sln builds clean - 0 errors; the only warnings are 2 pre-existing CS0618 (FtpSslProtocols.Default, x2 TFMs) and 2 NuGet pack deprecations.
  • dotnet test Activities.FTP.sln: 59/59 in UiPath.FTP.Tests, 18/19 in UiPath.FTP.Activities.API.Tests (1 pre-existing skip).
  • Package verified: single lib/net6.0 with UiPath.FTP.dll, UiPath.FTP.Activities.dll, .API.dll/.xml and 13 satellite locales - no Design dll, no lib/net6.0-windows. Same layout Cryptography has shipped since feb1052.
  • Every metadata / .resx / C# resource key resolves; all 8 icons present; runtime [LocalizedCategory] matches ViewModel Category on every visible property.

Before merge

  • Not yet opened in Studio. This is a UX change and it has only been verified structurally. A canvas smoke test (and screenshots) is the main gap.
  • Localization sync required. 52 changed strings still carry their old translations in all 13 locales, and 18 new keys have none. Missing keys fall back to English; stale ones do not - a German user still sees "Directory Exists". Also worth excluding the deleted UiPath.FTP.Activities.Design.*.resx path on the Localization side so the orphans do not come back.
  • Version bump? Release VersionPrefix is still 3.2.0. The package now ships one fewer assembly and drops a TFM. Neither reference commit touched the version props, so precedent says leave it - flagging for a maintainer call.

Follow-up, needs a repo-owner decision

UseSftp (bool) + FtpsMode (3-value enum) are two controls for one concept: the transport is really a four-way choice, so 6 states encode 4 meanings and FtpsMode is silently ignored when UseSftp is on. Consolidating into one Protocol selector touches public properties, so it needs owner sign-off. A cheap non-breaking slice exists in the meantime: there is no rule on UseSftp, so with SFTP selected the user still sees FtpsMode, SslProtocols and AcceptAllCertificates, none of which SFTP reads.

🤖 Generated with Claude Code

…es [STUD-80716]

Unified canvas adoption across all 8 FTP activities:
- Property configuration moved from ActivitiesMetadata.json into the
  ViewModels; the metadata JSON now declares activities only, matching
  the Cryptography pack. WithFtpSession's Body is declared as a
  DesignProperty and hidden there, the same way JavaScopeViewModel
  handles its scope body.
- One section vocabulary across both layers: Input -> Options ->
  Security / Proxy (scope only) -> Output last, applied to the
  ViewModels and the runtime [LocalizedCategory] attributes.
- Toggle widget for all booleans (including ContinueOnError), Number
  widget for Port / Timeout / ProxyPort, FtpsMode data source added.
- Verb-first renames: Directory Exists -> Check If Folder Exists,
  File Exists -> Check If File Exists, Enumerate Objects -> List Files
  and Folders. Terminology, casing and placeholders normalised.
- Three field-anchored design-time validations on WithFtpSession
  (missing username, no authentication method, negative timeout). Each
  fires only where ExecuteAsync already threw, so no workflow that ran
  before now fails validation.

Legacy WPF designer removal:
- UiPath.FTP.Activities.Design deleted (40 files); activities now
  render from their ViewModels, mirroring Cryptography's feb1052.
- Activity search synonyms preserved via DisplayNameAliasKeys in the
  metadata JSON.
- Packaging narrowed to $(PortableFramework); the Windows TFM existed
  only to carry the Design dll. InternalsVisibleTo grants and the
  stale XmlnsDefinition removed.

SFTP enumeration fix (UiPath.FTP/SftpSession.cs) -- the reported
"SftpPathNotFoundException: No such file" when enumerating ".":
- An unreadable sub-directory (no-such-file, permission denied, bare
  SSH_FX_FAILURE) is now skipped and traced instead of discarding the
  whole enumeration; connection and timeout failures still propagate.
  The caller's own RemotePath is not covered by this leniency. This is
  the actual fix, applied to all four walks, sync and async.
- Dropped the ChangeDirectory / restore dance; it was not in a finally,
  so a mid-walk failure left the shared session's working directory
  pointing into the tree.
- No symbolic-link guard: measured against SSH.NET 2024.1.0, S_IFDIR
  and S_IFLNK are distinct values of the mode word's type field, so a
  link is never reported as a directory and never enters the recursion
  set. An earlier revision added an !IsSymbolicLink clause on the
  strength of a mock that set both flags; it could not fire and has
  been removed, with the reasoning recorded so it is not reinstated.
- 16 listing tests (11 enumeration, 5 download). FakeItem now derives
  its type flags from one kind so a fake cannot describe an entry no
  server can send -- notably IsRegularFile is true for a link, which
  SSH.NET reports and the old fake denied.

Package documentation (docs/ ships in the nupkg as content/docs):
- List Files and Folders and Download Files now document the recursive
  walk: links are not descended into, an unreadable sub-folder is
  skipped and traced rather than fatal, connection failures still fail,
  and a bad RemotePath still fails. Download Files also states that it
  reports success on a partial transfer.
- The same notes added to the coded-workflow API reference for
  EnumerateObjects and DownloadFiles.
- The Link limitation below is now stated wherever the docs previously
  promised it: the Filter enum table, FtpObjectInfo.Type, the
  FtpObjectType reference in overview.md and coded-api.md, the
  filter-by-type example, and the Check If File/Folder Exists pair
  (a link answers True as a file and False as a folder).

Known limitation, documented in the shipped docs, pinned by tests, and
deliberately not fixed here: FtpObjectType.Link is unreachable on the SFTP path, because
SSH.NET's IsRegularFile is a (mode & S_IFREG) == S_IFREG test and
S_IFLNK contains S_IFREG. List Files and Folders therefore reports
links as File and its Link filter matches nothing, where FTP/FTPS
reports them correctly. Correcting it changes FtpObjectInfo.Type for
shipped workflows, so it belongs in its own change with its own
release note.

Follow-ups: localization sync for the renamed strings and the three
synonym keys; the UseSftp + FtpsMode protocol-selector consolidation
needs a repo-owner decision (breaking contract change).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

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.

Pull request overview

This PR modernizes the FTP activity pack by moving to unified-canvas ViewModel-driven UX, removing the legacy WPF designer assembly/TFM, and hardening SFTP recursive enumeration to avoid failing the whole operation when a listed subdirectory cannot be opened.

Changes:

  • Refactors FTP activity ViewModels to fully configure unified-canvas property UI (ordering, sections/categories, widgets, placeholders, and visibility).
  • Removes the legacy UiPath.FTP.Activities.Design project and updates packaging/solutions to ship only the portable TFM output.
  • Updates SFTP listing walks to avoid ChangeDirectory side-effects and to skip unreadable subdirectories during recursion (with trace warnings), plus adjusts link classification behavior docs.

Reviewed changes

Copilot reviewed 70 out of 73 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
Activities/FTP/UiPath.FTP/SftpSession.cs Refactors SFTP recursive listing/walk logic, adds async listing seam, and introduces “skip unreadable subdirectory” behavior with tracing.
Activities/FTP/UiPath.FTP/Extensions.cs Documents and enforces SFTP symlink classification behavior (links reported as files), removes unreachable IsSymbolicLink branch.
Activities/FTP/UiPath.FTP.Activities/WithFtpSession.cs Aligns runtime property categories with unified-canvas section vocabulary and adds design-time validations.
Activities/FTP/UiPath.FTP.Activities/UiPath.FTP.Activities.csproj Removes InternalsVisibleTo for the deleted legacy designer assembly.
Activities/FTP/UiPath.FTP.Activities/Properties/AssemblyInfo.cs Removes XmlnsDefinition mapping for the deleted legacy designer namespace.
Activities/FTP/UiPath.FTP.Activities/NetCore/ViewModels/WithFtpSessionViewModel.cs Moves scope UI/property configuration into ViewModel (sections, widgets, data sources, visibility, menu actions).
Activities/FTP/UiPath.FTP.Activities/NetCore/ViewModels/UploadFilesViewModel.cs ViewModel-driven unified-canvas property configuration (labels/tooltips/placeholders/widgets/sections).
Activities/FTP/UiPath.FTP.Activities/NetCore/ViewModels/MoveItemViewModel.cs ViewModel-driven unified-canvas property configuration and ContinueOnError wiring.
Activities/FTP/UiPath.FTP.Activities/NetCore/ViewModels/FileExistsViewModel.cs ViewModel-driven unified-canvas property configuration including Output section placement.
Activities/FTP/UiPath.FTP.Activities/NetCore/ViewModels/EnumerateObjectsViewModel.cs ViewModel-driven unified-canvas property configuration and Output section placement.
Activities/FTP/UiPath.FTP.Activities/NetCore/ViewModels/DownloadFilesViewModel.cs ViewModel-driven unified-canvas property configuration and ContinueOnError wiring.
Activities/FTP/UiPath.FTP.Activities/NetCore/ViewModels/DirectoryExistsViewModel.cs ViewModel-driven unified-canvas property configuration including Output section placement.
Activities/FTP/UiPath.FTP.Activities/NetCore/ViewModels/DeleteViewModel.cs ViewModel-driven unified-canvas property configuration and ContinueOnError wiring.
Activities/FTP/UiPath.FTP.Activities/NetCore/ViewModels/BaseFtpViewModel.cs Centralizes ContinueOnError UI configuration for consistent unified-canvas behavior across activities.
Activities/FTP/UiPath.FTP.Activities/MoveItem.cs Aligns runtime ContinueOnError category/strings with unified-canvas Options section vocabulary.
Activities/FTP/UiPath.FTP.Activities/DownloadFiles.cs Fixes runtime category for LocalPath to be Input (not Output).
Activities/FTP/UiPath.FTP.Activities.Packaging/UiPath.FTP.Activities.Packaging.csproj Drops Windows TFM and removes packaging references to the legacy Design assembly.
Activities/FTP/UiPath.FTP.Activities.Packaging/docs/overview.md Updates activity names/descriptions and documents SFTP “Link never produced” behavior.
Activities/FTP/UiPath.FTP.Activities.Packaging/docs/coded-api.md Documents recursive-walk behavior changes and SFTP symlink/type limitations for coded API consumers.
Activities/FTP/UiPath.FTP.Activities.Packaging/docs/activities/WithFtpSession.md Updates property table text, placeholders, and wording to match unified-canvas UX.
Activities/FTP/UiPath.FTP.Activities.Packaging/docs/activities/UploadFiles.md Updates naming/wording and adds placeholders consistent with unified-canvas UX.
Activities/FTP/UiPath.FTP.Activities.Packaging/docs/activities/MoveItem.md Updates naming/wording and adds placeholders consistent with unified-canvas UX.
Activities/FTP/UiPath.FTP.Activities.Packaging/docs/activities/FileExists.md Renames doc title/display name and adds SFTP symlink caveat.
Activities/FTP/UiPath.FTP.Activities.Packaging/docs/activities/EnumerateObjects.md Renames doc title/display name and documents recursive-walk + SFTP symlink limitations.
Activities/FTP/UiPath.FTP.Activities.Packaging/docs/activities/DownloadFiles.md Adds recursive-walk behavior notes and updates wording/placeholders.
Activities/FTP/UiPath.FTP.Activities.Packaging/docs/activities/DirectoryExists.md Renames doc title/display name and adds SFTP symlink caveat.
Activities/FTP/UiPath.FTP.Activities.Packaging/docs/activities/Delete.md Updates wording and adds placeholders consistent with unified-canvas UX.
Activities/FTP/UiPath.FTP.Activities.Design/WithFtpSessionDesigner.xaml.cs Deleted as part of legacy designer project removal.
Activities/FTP/UiPath.FTP.Activities.Design/WithFtpSessionDesigner.xaml Deleted as part of legacy designer project removal.
Activities/FTP/UiPath.FTP.Activities.Design/UploadFilesDesigner.xaml.cs Deleted as part of legacy designer project removal.
Activities/FTP/UiPath.FTP.Activities.Design/UploadFilesDesigner.xaml Deleted as part of legacy designer project removal.
Activities/FTP/UiPath.FTP.Activities.Design/UiPath.FTP.Activities.Design.csproj Deleted (legacy WPF designer project removed).
Activities/FTP/UiPath.FTP.Activities.Design/ReleaseNotes.txt Included in legacy designer removal set (no functional code impact).
Activities/FTP/UiPath.FTP.Activities.Design/Properties/UiPath.FTP.Activities.Design.zh-TW.resx Deleted legacy designer localization resource.
Activities/FTP/UiPath.FTP.Activities.Design/Properties/UiPath.FTP.Activities.Design.zh-CN.resx Deleted legacy designer localization resource.
Activities/FTP/UiPath.FTP.Activities.Design/Properties/UiPath.FTP.Activities.Design.tr.resx Deleted legacy designer localization resource.
Activities/FTP/UiPath.FTP.Activities.Design/Properties/UiPath.FTP.Activities.Design.ru.resx Deleted legacy designer localization resource.
Activities/FTP/UiPath.FTP.Activities.Design/Properties/UiPath.FTP.Activities.Design.ro.resx Deleted legacy designer localization resource.
Activities/FTP/UiPath.FTP.Activities.Design/Properties/UiPath.FTP.Activities.Design.resx Deleted legacy designer localization resource.
Activities/FTP/UiPath.FTP.Activities.Design/Properties/UiPath.FTP.Activities.Design.pt.resx Deleted legacy designer localization resource.
Activities/FTP/UiPath.FTP.Activities.Design/Properties/UiPath.FTP.Activities.Design.pt-BR.resx Deleted legacy designer localization resource.
Activities/FTP/UiPath.FTP.Activities.Design/Properties/UiPath.FTP.Activities.Design.ko.resx Deleted legacy designer localization resource.
Activities/FTP/UiPath.FTP.Activities.Design/Properties/UiPath.FTP.Activities.Design.ja.resx Deleted legacy designer localization resource.
Activities/FTP/UiPath.FTP.Activities.Design/Properties/UiPath.FTP.Activities.Design.fr.resx Deleted legacy designer localization resource.
Activities/FTP/UiPath.FTP.Activities.Design/Properties/UiPath.FTP.Activities.Design.es.resx Deleted legacy designer localization resource.
Activities/FTP/UiPath.FTP.Activities.Design/Properties/UiPath.FTP.Activities.Design.es-MX.resx Deleted legacy designer localization resource.
Activities/FTP/UiPath.FTP.Activities.Design/Properties/UiPath.FTP.Activities.Design.de.resx Deleted legacy designer localization resource.
Activities/FTP/UiPath.FTP.Activities.Design/Properties/UiPath.FTP.Activities.Design.Designer.cs Deleted legacy designer generated resource accessor.
Activities/FTP/UiPath.FTP.Activities.Design/Properties/SharedResources.cs Deleted legacy designer resource bridge type.
Activities/FTP/UiPath.FTP.Activities.Design/Properties/Resources.cs Deleted legacy designer resource wrapper.
Activities/FTP/UiPath.FTP.Activities.Design/Properties/AssemblyInfo.cs Deleted legacy designer assembly metadata.
Activities/FTP/UiPath.FTP.Activities.Design/MoveItemsDesigner.xaml.cs Deleted as part of legacy designer project removal.
Activities/FTP/UiPath.FTP.Activities.Design/MoveItemsDesigner.xaml Deleted as part of legacy designer project removal.
Activities/FTP/UiPath.FTP.Activities.Design/FileExistsDesigner.xaml.cs Deleted as part of legacy designer project removal.
Activities/FTP/UiPath.FTP.Activities.Design/FileExistsDesigner.xaml Deleted as part of legacy designer project removal.
Activities/FTP/UiPath.FTP.Activities.Design/EnumerateObjectsDesigner.xaml.cs Deleted as part of legacy designer project removal.
Activities/FTP/UiPath.FTP.Activities.Design/EnumerateObjectsDesigner.xaml Deleted as part of legacy designer project removal.
Activities/FTP/UiPath.FTP.Activities.Design/DownloadFilesDesigner.xaml.cs Deleted as part of legacy designer project removal.
Activities/FTP/UiPath.FTP.Activities.Design/DownloadFilesDesigner.xaml Deleted as part of legacy designer project removal.
Activities/FTP/UiPath.FTP.Activities.Design/DirectoryExistsDesigner.xaml.cs Deleted as part of legacy designer project removal.
Activities/FTP/UiPath.FTP.Activities.Design/DirectoryExistsDesigner.xaml Deleted as part of legacy designer project removal.
Activities/FTP/UiPath.FTP.Activities.Design/DesignerMetadata.cs Deleted legacy designer metadata/registration.
Activities/FTP/UiPath.FTP.Activities.Design/DeleteDesigner.xaml.cs Deleted as part of legacy designer project removal.
Activities/FTP/UiPath.FTP.Activities.Design/DeleteDesigner.xaml Deleted as part of legacy designer project removal.
Activities/FTP/UiPath.FTP.Activities.Design/ApiRegistrationBase.cs Deleted legacy synonym API registration base.
Activities/FTP/UiPath.FTP.Activities.Design/ActivitySynonymApiRegistration.cs Deleted legacy activity synonym registration logic (now preserved via metadata).
Activities/Credentials/UiPath.Credentials.Activities/UiPath.Credentials.Activities.csproj Removes an incorrect/obsolete InternalsVisibleTo to the deleted FTP designer assembly.
Activities/Community.Activities.sln Removes the legacy FTP designer project from the top-level solution.
Activities/Activities.FTP.sln Removes the legacy FTP designer project from the FTP pack solution.
Files not reviewed (2)
  • Activities/FTP/UiPath.FTP.Activities.Design/Properties/UiPath.FTP.Activities.Design.Designer.cs: Generated file
  • Activities/FTP/UiPath.FTP.Activities/Properties/UiPath.FTP.Activities.Designer.cs: Generated file

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@AlexMarinescuUiPath AlexMarinescuUiPath left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Requesting changes for the following issues:

  1. Copilot already identified that the conditional visibility handlers are registered but never applied during InitializeModel (Activities/FTP/UiPath.FTP.Activities/NetCore/ViewModels/WithFtpSessionViewModel.cs:158-189, existing comment: #590 (comment)). Restored/default password, FTPS, and proxy modes can therefore render the wrong fields until a controlling value changes.

  2. The new validation at Activities/FTP/UiPath.FTP.Activities/WithFtpSession.cs:188-209 does not reject anonymous SFTP. The inline comment explains the runtime failure path.

  3. The unified-canvas migration adds reactive rule and dependency wiring at Activities/FTP/UiPath.FTP.Activities/NetCore/ViewModels/WithFtpSessionViewModel.cs:366-385, but there are no FTP ViewModel tests for initial state, restored values, or mode transitions. This gap already allowed issue 1 through, and Sonar reports only 12.5% coverage on new code against the 80% gate. Please add focused ViewModel and validation tests with the fixes.

The runtime FTP suite passes locally (154 passed, 2 real-server tests skipped), so these requests are focused on design-time correctness and coverage.

Comment thread Activities/FTP/UiPath.FTP.Activities/WithFtpSession.cs Outdated
…st coverage [STUD-80716]

Closes gaps left by the unified-canvas migration:

- ActivitiesMetadata.json: DisplayNameAliasKeys were added for Delete,
  MoveItem and WithFtpSession (none of which changed name) but not for
  the three activities the migration actually renamed -- Directory
  Exists, File Exists, Enumerate Objects. Added the missing aliases so
  the activity panel still finds them under their old names.

- WithFtpSession.CacheMetadata: UseSftp + UseAnonymousLogin passed
  design-time validation but always failed at OpenAsync --
  SftpSession has no anonymous-login path (unlike FtpSession, which
  skips setting credentials for FluentFTP's anonymous default) and
  ExecuteAsync never populates Username when UseAnonymousLogin is on.
  This is a long-standing (2018) runtime defect, not something this
  migration introduced; what the migration added was the CacheMetadata
  validation surface that had a natural gap for it. Added the missing
  validation error and its resource string.

- WithFtpSessionViewModel's reactive rule/dependency wiring
  (InitializeRules/ManualRegisterDependencies + the five
  *_Changed_Action handlers) and InitializeModel's Configure*/menu-action
  wiring had no tests. Its DesignProperty/DesignInArgument properties
  also had no field initializers, unlike every Cryptography ViewModel's
  `= new DesignProperty<T>()` convention -- harmless in production
  (Studio always populates them via InitializeAsync first) but it left
  the ViewModel untestable. Added the initializers (also to
  BaseFtpViewModel.ContinueOnError, shared by all 8 FTP ViewModels) and
  widened the five action methods from private to internal, mirroring
  the internal FormatChanged_Action precedent in Cryptography, so tests
  can call them directly for mode-transition coverage.

- UiPath.FTP.Tests.csproj re-includes System.Activities.Metadata
  (PrivateAssets="All" repo-wide in Directory.build.targets) so
  DesignProperty mutations don't throw FileNotFoundException in tests;
  verified this does not leak into the shipped nupkg. That fix also
  unblocks the SDK's own InitializeAsync pipeline in tests, so the
  initial-state and restored-value tests drive InitializeModel,
  InitializeRules/ManualRegisterDependencies and ExecuteAll for real
  instead of only simulating the five rules by calling them directly.

- New tests: WithFtpSessionValidationTests (CacheMetadata coverage,
  previously at zero for this activity) and WithFtpSessionViewModelTests
  (initial state, restored values, mode transitions). Measured with
  coverlet: WithFtpSessionViewModel.cs 12.5% -> 88.7% line / 71.9% branch,
  BaseFtpViewModel.cs -> 100%/100%, WithFtpSession.cs -> 85.9% line /
  51.9% branch.

FTP test suite: 63 -> 75 tests, passing on net6.0 and net6.0-windows.
@viogroza
viogroza force-pushed the fix/ftp_ux_changes branch from ef8a4ea to eff7128 Compare August 6, 2026 16:17
…overage [STUD-80716]

- WithFtpSessionViewModel: dropped the newly-added Category assignment on
  the scope's principal connection fields (Host/Username/Password/
  SecurePassword) and gave Body an explicit OrderIndex of 0. Pre-migration,
  these fields were never assigned a Category on this scope activity;
  adding one broke the canvas layout (Username/Password rendering detached
  from the "Use FTP connection" container instead of stacked inside it).
- BaseFtpViewModel.ContinueOnError: widget reverted from Toggle to
  NullableBoolean, matching the pre-migration rendering.
- Sonar cleanups: SftpSession.Dispose() now calls GC.SuppressFinalize and
  is implicit (public void Dispose()) rather than an explicit
  IDisposable.Dispose(), so it pairs cleanly with Dispose(bool). Cognitive
  complexity reduced (guard clauses + extracted helpers, no behavior
  change) on SftpSession's constructor, Upload/UploadAsync,
  Download/DownloadAsync, and Move, and on WithFtpSession's ExecuteAsync
  and CacheMetadata. GetLocalListing/Descend/DescendAsync marked static
  (no instance state accessed).
- Added missing field initializers to Delete/DirectoryExists/FileExists/
  MoveItem/DownloadFiles/UploadFiles/EnumerateObjects ViewModels (the same
  gap fixed for WithFtpSessionViewModel in eff7128) and added ViewModel
  tests for all seven, covering InitializeModel's property configuration
  end to end. Every FTP ViewModel now has test coverage.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

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.

Pull request overview

Copilot reviewed 80 out of 83 changed files in this pull request and generated no new comments.

Files not reviewed (2)
  • Activities/FTP/UiPath.FTP.Activities.Design/Properties/UiPath.FTP.Activities.Design.Designer.cs: Generated file
  • Activities/FTP/UiPath.FTP.Activities/Properties/UiPath.FTP.Activities.Designer.cs: Generated file
Suppressed comments (3)

Activities/FTP/UiPath.FTP.Activities/NetCore/ViewModels/BaseFtpViewModel.cs:40

  • ContinueOnError is configured with ViewModelWidgetType.NullableBoolean, but the PR description explicitly calls out “Toggle for every boolean (including ContinueOnError)”. This also differs from the Cryptography pack’s unified-canvas ViewModels, which use a Toggle widget for ContinueOnError. Consider switching this widget to Toggle so the UX matches the stated intent and other boolean options in this pack.
    Activities/FTP/UiPath.FTP.Tests/UiPath.FTP.Tests.csproj:34
  • PrivateAssets="" here is misleading: Activities/Directory.build.targets applies PackageReference Update="System.Activities.Metadata" ... PrivateAssets="All" repo-wide, so this metadata will be overridden during evaluation. Since this is a test project (not packed) and you mainly need the direct reference to get runtime assets into the output folder, consider removing the empty attribute to avoid implying it changes the repo-wide setting.
    Activities/FTP/UiPath.FTP.Tests/ActivitiesTests/DeleteViewModelTests.cs:43
  • This test pins ContinueOnError’s widget type to NullableBoolean. If ContinueOnError is intended to be a Toggle per the unified-canvas UX goal, this assertion should be updated; otherwise the test will block changing the widget to match the PR description.

@sonarqubecloud

sonarqubecloud Bot commented Aug 7, 2026

Copy link
Copy Markdown

@AlexMarinescuUiPath AlexMarinescuUiPath left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Re-reviewed commits eff7128 and 0c039be against the previously reviewed head.

The requested changes are addressed:

  • Copilot's initial-visibility finding is now covered by real InitializeAsync() tests for default and restored password, FTPS, and proxy states.
  • Anonymous SFTP is rejected in CacheMetadata, including when other credentials are present, with focused validation tests.
  • All eight migrated ViewModels now have targeted configuration coverage, and the Sonar quality gate is green.

I also checked the Sonar-driven SftpSession and WithFtpSession refactors for authentication, exception, cancellation, overwrite, scheduling, and disposal drift; no new blocking issue found. Local result: 230 passed, 2 real-server tests skipped, 0 failed across both target frameworks.

Residual UX risk: the PR still notes that it has not been smoke-tested in Studio. The ViewModel tests validate state and rule wiring, but not host renderer/layout parity, so that manual check should still happen before merge.

@viogroza

viogroza commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator Author

basic testing was done in studio

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants