Skip to content

#88 Umami.Net: impossible to specify unique_id / DistinctId / Id#90

Merged
scottgal merged 3 commits into
scottgal:mainfrom
Shaddix:main
May 14, 2026
Merged

#88 Umami.Net: impossible to specify unique_id / DistinctId / Id#90
scottgal merged 3 commits into
scottgal:mainfrom
Shaddix:main

Conversation

@Shaddix
Copy link
Copy Markdown
Contributor

@Shaddix Shaddix commented Mar 13, 2026

#88

Copy link
Copy Markdown

Copilot AI left a comment

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 addresses issue #88 by adding support for passing an Umami distinct/unique identifier through the .NET client so users can be consistently identified across sessions.

Changes:

  • Adds a new distinctId parameter to Identify/IdentifyAndDecode flows and forwards it into UmamiPayload.
  • Introduces a new payload property (Id) and ensures PayloadService propagates it during payload population.
  • Updates a background-sender test call site to use a named argument due to the new parameter position.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
Umami.Net/UmamiClient.cs Adds distinctId support to identify APIs and forwards it into the payload.
Umami.Net/UmamiBackgroundSender.cs Adds distinctId support for background identify/session identify enqueueing.
Umami.Net/PayloadService.cs Propagates the new payload identifier property when populating payloads.
Umami.Net/Models/UmamiPayload.cs Adds a new payload property (Id) intended to represent the distinct/unique identifier.
Umami.Net.Test/UmamiBackgroundSender_Tests.cs Adjusts a call site to account for the updated method signature.

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

Comment on lines 59 to 66
if (payload.SessionId != null)
newPayload.SessionId = payload.SessionId;

if (payload.Id != null)
newPayload.Id = payload.Id;

newPayload.UserAgent = payload.UserAgent ?? DefaultUserAgent;

Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

PayloadService.PopulateFromPayload now propagates payload.Id, but there’s no test coverage validating that this propagation works end-to-end (i.e., a provided distinct/unique id survives population and is sent). Consider adding a test that sets UmamiPayload.Id (or the renamed property) and verifies it is present in the final request JSON.

Copilot uses AI. Check for mistakes.
Comment thread Umami.Net/UmamiClient.cs
Comment on lines 190 to 192
public async Task<HttpResponseMessage> Identify(string? email = null, string? username = null,
string? sessionId = null, string? userId = null, UmamiEventData? eventData = null)
string? sessionId = null, string? userId = null, string? distinctId = null, UmamiEventData? eventData = null)
{
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

Adding the new optional parameter distinctId before eventData is a source/binary breaking change for any callers using positional arguments (e.g., Identify(..., eventData) will no longer compile). To maintain backward compatibility, keep the existing parameter order and add an overload (or move distinctId to the end) so existing positional call sites remain valid.

Copilot uses AI. Check for mistakes.
Comment thread Umami.Net/UmamiClient.cs Outdated
Comment on lines +176 to +177
string? distinctId = null,
UmamiEventData? eventData = null)
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

IdentifyAndDecode(string sessionId, ...) now inserts distinctId before eventData, which breaks positional callers that previously passed eventData as the next argument. Consider preserving the original signature and introducing a new overload that includes distinctId, or move distinctId to the end to avoid a breaking API change.

Suggested change
string? distinctId = null,
UmamiEventData? eventData = null)
UmamiEventData? eventData = null,
string? distinctId = null)

Copilot uses AI. Check for mistakes.
Comment thread Umami.Net/UmamiBackgroundSender.cs Outdated
Comment on lines 57 to 58
string? sessionId = null, string? userId = null, string? distinctId = null, UmamiEventData? eventData = null,
bool useDefaultUserAgent = false)
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

The new distinctId optional parameter is inserted before eventData, which is a breaking change for callers that pass eventData positionally (it will now try to bind to distinctId). Prefer keeping the existing parameter order and adding an overload (or placing distinctId at the end) to avoid breaking consumers.

Suggested change
string? sessionId = null, string? userId = null, string? distinctId = null, UmamiEventData? eventData = null,
bool useDefaultUserAgent = false)
string? sessionId = null, string? userId = null, UmamiEventData? eventData = null,
bool useDefaultUserAgent = false, string? distinctId = null)

Copilot uses AI. Check for mistakes.
Comment thread Umami.Net/UmamiBackgroundSender.cs Outdated
Comment on lines 82 to 84
public async Task IdentifySession(string sessionId, string? distinctId = null, UmamiEventData? eventData = null,
bool useDefaultUserAgent = false)
{
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

IdentifySession adds distinctId as the second parameter, which breaks existing positional calls like IdentifySession(sessionId, eventData) (the second argument no longer matches). To avoid a breaking change, keep the original signature and add an overload accepting distinctId, or move distinctId to the end after eventData/useDefaultUserAgent.

Suggested change
public async Task IdentifySession(string sessionId, string? distinctId = null, UmamiEventData? eventData = null,
bool useDefaultUserAgent = false)
{
public Task IdentifySession(string sessionId, UmamiEventData? eventData = null,
bool useDefaultUserAgent = false)
{
// Preserve backward-compatible signature and forward to the extended overload
return IdentifySession(sessionId, null, eventData, useDefaultUserAgent);
}
public async Task IdentifySession(string sessionId, string? distinctId, UmamiEventData? eventData = null,
bool useDefaultUserAgent = false)
{

Copilot uses AI. Check for mistakes.
Comment on lines 20 to 23
public string? SessionId { get; set; }

public string? Id { get; set; }

Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

UmamiPayload.Id is very generic and doesn’t communicate that it represents Umami’s distinct/unique identifier (issue #88). Consider renaming this to something explicit like DistinctId/UniqueId and mapping it to the expected JSON field name via [JsonPropertyName("id")] (or the exact key Umami expects), so the public API is self-describing without relying on the global naming policy.

Copilot uses AI. Check for mistakes.
Comment thread Umami.Net/UmamiClient.cs
Comment on lines 194 to 199
var payload = new UmamiPayload
{
SessionId = sessionId,
Id = distinctId,
Data = emailData
};
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

New behavior: distinctId is now forwarded into the payload (Id = distinctId), but there are no assertions in the test suite that this value is actually serialized/sent when provided. Add/extend tests (client and/or background sender) to pass a non-null distinctId and assert it appears in the outgoing payload, so regressions are caught.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Owner

@scottgal scottgal left a comment

Choose a reason for hiding this comment

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

Thanks for this PR @Shaddix — the feature is needed and the implementation is clean. A few items to address before merging:

Required Changes

  1. Add [JsonPropertyName("id")] to UmamiPayload.Id (UmamiPayload.cs)
    While LowerCaseNamingPolicy produces the correct "id" today, other properties (e.g. IpAddress[JsonPropertyName("ip")]) use explicit attributes. Please add one for consistency and safety:

    [JsonPropertyName("id")]
    public string? Id { get; set; }
  2. Add distinctId to UmamiClient.IdentifySession (UmamiClient.cs:201)
    UmamiBackgroundSender.IdentifySession has the new distinctId parameter, but UmamiClient.IdentifySession does not — this is an inconsistency in the public API surface.

Suggestions (optional)

  • Add a test that verifies distinctId flows through to the serialized payload's id field
  • Consider length validation — Umami docs state the id field has a 50-character limit
  • The Track/TrackPageView methods don't expose distinctId as a convenience parameter (users can set Id on a raw UmamiPayload). This is fine for now but worth considering in a follow-up.

Overall this is a well-scoped, clean change — just needs the consistency fixes above. 👍

@Shaddix
Copy link
Copy Markdown
Contributor Author

Shaddix commented Apr 2, 2026

everything is done here, could you please check @scottgal ?

@Shaddix
Copy link
Copy Markdown
Contributor Author

Shaddix commented Apr 17, 2026

It'd be great if you approve or comment on what needs to be changed :)

@Aero9999
Copy link
Copy Markdown

+1 would be great to get this merged in.

@scottgal scottgal merged commit fabea63 into scottgal:main May 14, 2026
@scottgal
Copy link
Copy Markdown
Owner

Thanks all, merged. I need to do a bigger pass / (if anone fancies it) on the new playback features too. I don't currently have time to maintain this.

@scottgal
Copy link
Copy Markdown
Owner

Will be in Umami.net 1.0.1

@Aero9999
Copy link
Copy Markdown

Thanks Scott. I've just started using Umami so alas my knowledge of replays isnt there yet!

looking forward to 1.0.1 👍

@Aero9999
Copy link
Copy Markdown

(looks like the publish fell over though)

@scottgal
Copy link
Copy Markdown
Owner

Just fixed it, will be around shortly :)

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.

4 participants