Support derived activity descriptor customization#7422
Conversation
Agent-Logs-Url: https://github.com/elsa-workflows/elsa-core/sessions/060f49c8-ae65-4ac1-a40b-f45c49e99e17 Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com>
Agent-Logs-Url: https://github.com/elsa-workflows/elsa-core/sessions/060f49c8-ae65-4ac1-a40b-f45c49e99e17 Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com>
Greptile SummaryThis PR fixes descriptor inheritance for derived activity classes by switching all Confidence Score: 5/5Safe to merge; no P0/P1 issues found — only minor consistency gaps in inherited attribute handling. All findings are P2. The core logic of GetPublicInstanceProperties and the inherit: false changes is correct and well-covered by new tests. The two remaining inconsistencies (BrowsableAttribute class-level and GetCustomAttributes(true)) are edge-case scenarios unlikely to affect most users. src/modules/Elsa.Workflows.Core/Services/ActivityDescriber.cs — lines 64 and 67 still use inherited attribute lookups inconsistently with the rest of the PR.
|
| Filename | Overview |
|---|---|
| src/modules/Elsa.Workflows.Core/Helpers/ActivityTypeNameHelper.cs | Adds inherit: false to both GetCustomAttribute<ActivityAttribute>() calls so derived classes without their own [Activity] get their own namespace/type name; also adds a missing newline at EOF. |
| src/modules/Elsa.Workflows.Core/Services/ActivityDescriber.cs | Switches all descriptor-attribute lookups to inherit: false and introduces GetPublicInstanceProperties that sorts by inheritance distance to prefer the most-derived property declaration; BrowsableAttribute (class-level, line 64) and GetCustomAttributes(true) (line 67) still inherit from base types inconsistently. |
| test/unit/Elsa.Workflows.Core.UnitTests/Services/ActivityDescriberTests.cs | New test file with four focused tests covering: derived type identity, constructor identity, property deduplication (input/output/embedded ports), customized flow-node outcomes, and non-inheritance of base flow ports. Good coverage of the core scenarios. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[DescribeActivityAsync] --> B{inherit:false lookup\nActivityAttribute}
B -->|found on concrete type| C[Use declared namespace / type / version / display / category / description / kind]
B -->|not found| D[Derive from type name & CLR namespace]
A --> E[GetPublicInstanceProperties]
E --> F[GetProperties Public+Instance]
F --> G[OrderBy InheritanceDistance\nclosest to concrete first]
G --> H[DistinctBy Name\nderived property wins]
H --> I[Filter: Input properties]
H --> J[Filter: Output properties]
H --> K[Filter: Embedded port properties\nIActivity assignable]
B2{inherit:false lookup\nFlowNodeAttribute} --> L[Flow ports from outcomes]
B2 -->|not found| M[No flow ports]
A --> B2
I --> N[ActivityDescriptor]
J --> N
K --> N
L --> N
M --> N
Prompt To Fix All With AI
This is a comment left during a code review.
Path: src/modules/Elsa.Workflows.Core/Services/ActivityDescriber.cs
Line: 64
Comment:
**`BrowsableAttribute` class-level still inherits from base types**
Line 64 still calls `GetCustomAttribute<BrowsableAttribute>()` with the default `inherit: true`, while every other descriptor attribute in this method (`ActivityAttribute`, `DisplayNameAttribute`, `CategoryAttribute`, `DescriptionAttribute`, `FlowNodeAttribute`) was explicitly changed to `inherit: false`. A derived class without its own `[Browsable]` annotation will inherit the base class's visibility setting — for example, if the base activity is `[Browsable(false)]`, the derived type becomes invisibly non-browsable even when the author intends it to be a distinct, visible activity.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: src/modules/Elsa.Workflows.Core/Services/ActivityDescriber.cs
Line: 67
Comment:
**`GetCustomAttributes(true)` still collects inherited `OutputAttribute`**
`GetCustomAttributes(true)` on line 67 retrieves all attributes from the type hierarchy. The `OutputAttribute` extracted from this list on line 68 controls the `IsSerializable` flag of the default output descriptor. A derived class without its own `[Output(...)]` class-level annotation silently inherits the base class's `IsSerializable` value, which is inconsistent with the non-inherited treatment applied to every other descriptor attribute in this PR.
```suggestion
var attributes = activityType.GetCustomAttributes(false).Cast<Attribute>().ToList();
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (2): Last reviewed commit: "fix: use inherit: false for FlowNodeAttr..." | Re-trigger Greptile
|
@claude This is a comment left during a code review. Comment:
Can you resolve this? |
|
Claude finished @sfmskywalker's task in 58s —— View job Fixing
|
…iber Aligns FlowNodeAttribute lookup with all other descriptor attribute lookups so derived activities without their own [FlowNode] get no inherited flow ports, matching the clean-slate behaviour of Activity-, DisplayName-, Category-, and DescriptionAttribute. Co-authored-by: Sipke Schoorstra <sfmskywalker@users.noreply.github.com>
Purpose
Allow custom activities derived from existing activities to define their own descriptor identity and customize inputs, outputs, ports, and outcomes without inheriting conflicting base metadata.
Scope
Select one primary concern:
Description
Problem
Derived activity classes inherited base activity metadata during descriptor/type-name generation. This caused custom activity types copied or derived from built-ins to collide with the base activity identity and prevented custom descriptor aspects from being reliably surfaced.
Solution
Concrete activity identity
ActivityTypeNameHelpernow readsActivityAttributeonly when declared on the concrete type.[Activity]gets its own namespace/type name instead of inheriting the base activity identity.Derived descriptor metadata
ActivityDescribernow uses non-inherited activity/display/category/description attributes.Derived property customization
Example:
MyHttpRequestis now described as its own activity type, with its derived input metadata preferred.Verification
Steps:
ActivityDescriberTests.dotnet test test/unit/Elsa.Workflows.Core.UnitTests/Elsa.Workflows.Core.UnitTests.csproj /p:RestoreIgnoreFailedSources=true.Expected outcome:
Derived activity descriptors no longer inherit base activity identity unless explicitly declared, and derived inputs/outputs/ports/outcomes are reflected correctly.
Screenshots / Recordings (if applicable)
N/A
Commit Convention
We recommend using conventional commit prefixes:
fix:– Bug fixes (behavior change)feat:– New featuresrefactor:– Code changes without behavior changedocs:– Documentation updateschore:– Maintenance, tooling, or dependency updatestest:– Test additions or modificationsClear commit messages make reviews easier and history more meaningful.
Checklist