Add display name and priority support to endpoints #7442

adamint · 2025-02-06T21:07:59Z

Description

Adds two optional properties to Url and EndpointAnnotation - namely, DisplayName and Priority, that are used in the dashboard to determine 1) display order, and 2) display name.

Links still show up on the dashboard, just with the text replaced with the display name if set.

Additionally, adds a third Display name column to the dashboard that shows the display name of the endpoint if it is set.
(no display names)

(pgAdmin has a display name)

Also sets the endpoint display name on the CosmosDB data explorer to "Data Explorer".

Fixes #7104

Checklist

Is this feature complete?
- Yes. Ready to ship.
- No. Follow-up changes expected.
Are you including unit tests for the changes and scenario tests if relevant?
- Yes
- No
Did you add public API?
- Yes
  - If yes, did you have an API Review for it?
    - Yes
    - No
  - Did you add <remarks /> and <code /> elements on your triple slash comments?
    - Yes
    - No
- No
Does the change make any security assumptions or guarantees?
- Yes
  - If yes, have you done a threat model and had a security review?
    - Yes
    - No
- No
Does the change require an update in our Aspire docs?
- Yes
  - Is this introducing a breaking change?
    - Yes
      - Link to aspire-docs issue (please use this breaking-change template):
    - No
      - Link to aspire-docs issue (please use this doc-idea template):
- No I think?

src/Aspire.Hosting/ApplicationModel/EndpointAnnotation.cs

davidfowl · 2025-02-06T23:04:19Z

I'm trying to figure out if we should split the visual impact of the endpoint vs the endpoint definition. They should be different annotations.

adamint · 2025-02-06T23:56:19Z

I'm trying to figure out if we should split the visual impact of the endpoint vs the endpoint definition. They should be different annotations.

Why should they? If display name is a friendly name for the endpoint, that seems like an integral part of the definition itself

davidfowl · 2025-02-07T00:06:09Z

This data doesn't go into the manifest right? I am thinking that there's a core annotation that is used for networking and another part of it that is used for display in the dashboard.

Copilot reviewed 12 out of 27 changed files in this pull request and generated no comments.

Files not reviewed (15)

src/Aspire.Dashboard/Resources/ControlsStrings.Designer.cs: Language not supported
src/Aspire.Dashboard/Resources/ControlsStrings.resx: Language not supported
src/Aspire.Dashboard/Resources/xlf/ControlsStrings.cs.xlf: Language not supported
src/Aspire.Dashboard/Resources/xlf/ControlsStrings.de.xlf: Language not supported
src/Aspire.Dashboard/Resources/xlf/ControlsStrings.es.xlf: Language not supported
src/Aspire.Dashboard/Resources/xlf/ControlsStrings.fr.xlf: Language not supported
src/Aspire.Dashboard/Resources/xlf/ControlsStrings.it.xlf: Language not supported
src/Aspire.Dashboard/Resources/xlf/ControlsStrings.ja.xlf: Language not supported
src/Aspire.Dashboard/Resources/xlf/ControlsStrings.ko.xlf: Language not supported
src/Aspire.Dashboard/Resources/xlf/ControlsStrings.pl.xlf: Language not supported
src/Aspire.Dashboard/Resources/xlf/ControlsStrings.pt-BR.xlf: Language not supported
src/Aspire.Dashboard/Resources/xlf/ControlsStrings.ru.xlf: Language not supported
src/Aspire.Dashboard/Resources/xlf/ControlsStrings.tr.xlf: Language not supported
src/Aspire.Dashboard/Resources/xlf/ControlsStrings.zh-Hans.xlf: Language not supported
src/Aspire.Dashboard/Resources/xlf/ControlsStrings.zh-Hant.xlf: Language not supported

Comments suppressed due to low confidence (1)

src/Aspire.Dashboard/Model/ResourceViewModel.cs:330

The constructor parameters 'displayName' and 'priority' should be renamed to 'DisplayName' and 'Priority' to match the property names.

public UrlViewModel(string name, Uri url, bool isInternal, string? displayName = null, int? priority = null)

davidfowl · 2025-02-07T02:37:32Z

Thinking out loud, maybe it's fine to create a nested display properties on EndpointAnnotion where we store these properties. I don't think we add to the chaos that is part of these methods an maybe we treat these like properties you set with

WithEndpoint("name", e => e.DisplayName = "")

JamesNK · 2025-02-07T03:09:11Z

What do other people think about having the grid in details showing a name and display name column? It feels like we lose out on the simplicit of a name/value grid.

Instead, what about showing the display name as the link text in the value column? The visualizer would open to show the underlying address. That makes the details consistent with what is shown in the grid (which shows the link using the display name), lets endpoints grid still be name/value, and it makes the code simpler.

On the other hand, it makes it more difficult to see the address port in details at a glance, which people might want.

Thoughts?

davidfowl · 2025-02-07T03:12:41Z

Not a fan of the 2 columns

adamint · 2025-02-07T12:48:51Z

What do other people think about having the grid in details showing a name and display name column? It feels like we lose out on the simplicit of a name/value grid.

Instead, what about showing the display name as the link text in the value column? The visualizer would open to show the underlying address. That makes the details consistent with what is shown in the grid (which shows the link using the display name), lets endpoints grid still be name/value, and it makes the code simpler.

On the other hand, it makes it more difficult to see the address port in details at a glance, which people might want.

Thoughts?

Yeah, there's a tension between showing all the data and adding more visual complexity.

On the other hand, it makes it more difficult to see the address port in details at a glance, which people might want.

was the reason I initially added the third column - I'd prefer to be able to see the endpoint URL without needing to hover over the value, but don't have a strong opinion here

adamint · 2025-02-07T13:35:23Z

Thinking out loud, maybe it's fine to create a nested display properties on EndpointAnnotion where we store these properties. I don't think we add to the chaos that is part of these methods an maybe we treat these like properties you set with
WithEndpoint("name", e => e.DisplayName = "")

Yeah I like that idea. Done

src/Aspire.Dashboard/Model/ResourceEndpointHelpers.cs

src/Aspire.Hosting/Dashboard/proto/resource_service.proto

JamesNK · 2025-02-07T15:28:24Z

Add a display name to some endpoints in TestShop. I'll pull the branch and try it out tomorrow.

src/Aspire.Hosting/ApplicationModel/EndpointAnnotation.cs

davidfowl · 2025-02-07T17:56:21Z

src/Aspire.Hosting/ApplicationModel/EndpointReference.cs

+    /// <summary>
+    /// Gets the display properties for this endpoint.
+    /// </summary>
+    public EndpointDisplayProperties DisplayProperties => EndpointAnnotation.DisplayProperties;


Who uses this?

ResourceSnapshotBuilder, #239, #246, #253

src/Aspire.Hosting.Azure.CosmosDB/AzureCosmosDBExtensions.cs

src/Aspire.Hosting/ResourceBuilderExtensions.cs

src/Aspire.Hosting/ApplicationModel/CustomResourceSnapshot.cs

davidfowl

Address the breaking change and this is good to go!

davidfowl · 2025-02-07T22:12:52Z

playground/TestShop/TestShop.AppHost/ResourceBuilderExtensions.cs

+
+public static class ResourceBuilderExtensions
+{
+    public static IResourceBuilder<T> WithModifiedEndpoints<T>(this IResourceBuilder<T> builder, Action<EndpointAnnotation> callback) where T : IResourceWithEndpoints


Do you need this?

JamesNK · 2025-02-07T23:32:08Z

playground/TestShop/TestShop.AppHost/Program.cs

+       .WithModifiedEndpoints(c =>
+       {
+           c.DisplayProperties.DisplayName = "TestShop frontend access";
+       })


This seems like a bit of a sledge hammer to give names to endpoints from the launch profile.

@davidfowl Do you want to be able to give names to individual endpoints from the launch profile? Is there an existing pattern for enriching EndpointAnnotation instances created from the launch profile with extra values?

I think so yes.

src/Aspire.Dashboard/Components/Controls/ResourceDetails.razor

src/Aspire.Dashboard/ResourceService/Partials.cs

src/Aspire.Dashboard/Model/ResourceViewModel.cs

davidfowl · 2025-02-08T16:17:53Z

I'd also want to use this in the BicepProvisioner to set the display for the deployment url.

adamint · 2025-02-10T20:09:04Z

BicepProvisioner

What do you want to be displayed?

davidfowl · 2025-02-10T23:30:32Z

#7442 (comment)

Don't block on this.

Adam Ratzman added 4 commits February 6, 2025 15:13

Add display name and priority to Urls

d510d16

Make sure to plumb new properties through gRPC

ed426a4

Add third display name column for endpoints when there is a display name

d5a5876

add Data Explorer display name to cosmos db, make properties settable

8cc202c

adamint requested review from davidfowl, JamesNK, drewnoakes, eerhardt and karolz-ms February 6, 2025 21:07

clean up DisplayedEndpoint type

cf8f358

drewnoakes reviewed Feb 6, 2025

View reviewed changes

src/Aspire.Hosting/ApplicationModel/EndpointAnnotation.cs Outdated Show resolved Hide resolved

danmoseley requested a review from Copilot February 7, 2025 02:10

Copilot AI reviewed Feb 7, 2025

View reviewed changes

Adam Ratzman added 2 commits February 7, 2025 07:49

Merge branch 'main' into dev/adamint/add-display-name-endpoint-support

61720f9

Use display properties object instead on EndpointAnnotation

5d46e88

adamint requested a review from drewnoakes February 7, 2025 13:35

JamesNK reviewed Feb 7, 2025

View reviewed changes

src/Aspire.Dashboard/Model/ResourceEndpointHelpers.cs Outdated Show resolved Hide resolved

src/Aspire.Hosting/Dashboard/proto/resource_service.proto Outdated Show resolved Hide resolved

davidfowl reviewed Feb 7, 2025

View reviewed changes

src/Aspire.Hosting/ApplicationModel/EndpointAnnotation.cs Outdated Show resolved Hide resolved

davidfowl reviewed Feb 7, 2025

View reviewed changes

eerhardt reviewed Feb 7, 2025

View reviewed changes

src/Aspire.Hosting.Azure.CosmosDB/AzureCosmosDBExtensions.cs Outdated Show resolved Hide resolved

davidfowl reviewed Feb 7, 2025

View reviewed changes

src/Aspire.Hosting/ResourceBuilderExtensions.cs Outdated Show resolved Hide resolved

Adam Ratzman added 2 commits February 7, 2025 16:37

Add unit test, remove breaking api changes

a9ac084

Customize testshop frontend access endpoint

a9f3a83

davidfowl reviewed Feb 7, 2025

View reviewed changes

src/Aspire.Hosting/ApplicationModel/CustomResourceSnapshot.cs Outdated Show resolved Hide resolved

davidfowl approved these changes Feb 7, 2025

View reviewed changes

davidfowl reviewed Feb 7, 2025

View reviewed changes

JamesNK reviewed Feb 7, 2025

View reviewed changes

src/Aspire.Dashboard/Components/Controls/ResourceDetails.razor Outdated Show resolved Hide resolved

JamesNK reviewed Feb 7, 2025

View reviewed changes

src/Aspire.Dashboard/Components/Controls/ResourceDetails.razor Outdated Show resolved Hide resolved

JamesNK reviewed Feb 7, 2025

View reviewed changes

src/Aspire.Dashboard/Components/Controls/ResourceDetails.razor Outdated Show resolved Hide resolved

JamesNK reviewed Feb 7, 2025

View reviewed changes

src/Aspire.Dashboard/ResourceService/Partials.cs Outdated Show resolved Hide resolved

JamesNK reviewed Feb 7, 2025

View reviewed changes

src/Aspire.Dashboard/Model/ResourceViewModel.cs Outdated Show resolved Hide resolved

Merge branch 'main' into dev/adamint/add-display-name-endpoint-support

dca882f

Make display properties not nullable, remove breaking change

51f030c

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add display name and priority support to endpoints #7442

Add display name and priority support to endpoints #7442

adamint commented Feb 6, 2025

davidfowl commented Feb 6, 2025

adamint commented Feb 6, 2025

davidfowl commented Feb 7, 2025

davidfowl commented Feb 7, 2025

JamesNK commented Feb 7, 2025

davidfowl commented Feb 7, 2025

adamint commented Feb 7, 2025

adamint commented Feb 7, 2025

JamesNK commented Feb 7, 2025

davidfowl Feb 7, 2025

adamint Feb 7, 2025

davidfowl left a comment

davidfowl Feb 7, 2025

JamesNK Feb 7, 2025 •

edited

Loading

davidfowl Feb 7, 2025

davidfowl commented Feb 8, 2025

adamint commented Feb 10, 2025

davidfowl commented Feb 10, 2025

Add display name and priority support to endpoints #7442

Are you sure you want to change the base?

Add display name and priority support to endpoints #7442

Conversation

adamint commented Feb 6, 2025

Description

Checklist

davidfowl commented Feb 6, 2025

adamint commented Feb 6, 2025

davidfowl commented Feb 7, 2025

Choose a reason for hiding this comment

davidfowl commented Feb 7, 2025

JamesNK commented Feb 7, 2025

davidfowl commented Feb 7, 2025

adamint commented Feb 7, 2025

adamint commented Feb 7, 2025

JamesNK commented Feb 7, 2025

davidfowl Feb 7, 2025

Choose a reason for hiding this comment

adamint Feb 7, 2025

Choose a reason for hiding this comment

davidfowl left a comment

Choose a reason for hiding this comment

davidfowl Feb 7, 2025

Choose a reason for hiding this comment

JamesNK Feb 7, 2025 • edited Loading

Choose a reason for hiding this comment

davidfowl Feb 7, 2025

Choose a reason for hiding this comment

davidfowl commented Feb 8, 2025

adamint commented Feb 10, 2025

davidfowl commented Feb 10, 2025

JamesNK Feb 7, 2025 •

edited

Loading