Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,13 @@ public record ProposalDto(
List<ProposalOperationDto> Operations
)
{
/// <summary>
/// The username of the actor who approved or rejected this proposal, when the actor still
/// resolves to a user. The id remains available separately for technical correlation, but the
/// review surface must not present that opaque id as the actor's name (#2195).
/// </summary>
public string? DecidedByUserName { get; init; }

public ProposalPresentationDto Presentation { get; init; } = ProposalPresentationDto.Empty;

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ namespace Taskdeck.Application.Interfaces;

public interface IUserRepository : IRepository<User>
{
Task<IReadOnlyDictionary<Guid, string>> GetUsernamesByIdsAsync(
IEnumerable<Guid> ids,
CancellationToken cancellationToken = default);
Task<User?> GetByUsernameAsync(string username, CancellationToken cancellationToken = default);
Task<User?> GetByEmailAsync(string email, CancellationToken cancellationToken = default);
Task<bool> ExistsAsync(string username, string email, CancellationToken cancellationToken = default);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -430,11 +430,13 @@ public async Task<Result<IEnumerable<ProposalDto>>> GetProposalsAsync(ProposalFi
// revision" — exactly the null the single-proposal read produces — so the builder maps the
// proposal's original operations for those items.
var effectiveRevisions = await GetEffectiveRevisionsAsync(page, cancellationToken);
var decidedByUserNames = await ResolveDecidedByUserNamesAsync(page, cancellationToken);

var dtos = page
.Select(proposal => BuildEffectiveProposalDto(
proposal,
effectiveRevisions.TryGetValue(proposal.Id, out var revision) ? revision : null))
effectiveRevisions.TryGetValue(proposal.Id, out var revision) ? revision : null,
GetDecidedByUserName(proposal, decidedByUserNames)))
.ToList();

return Result.Success<IEnumerable<ProposalDto>>(dtos);
Expand Down Expand Up @@ -532,7 +534,11 @@ public async Task<Result<ProposalDto>> ApproveProposalAsync(Guid id, Guid decide
// Echo the effective (pinned) operations Apply will run, not the stale originals (#1424).
// latestRevision IS the pinned revision (its id was stored as ApprovedRevisionId), so map
// the DTO from it directly rather than re-reading it via GetEffectiveRevisionAsync.
return Result.Success(BuildEffectiveProposalDto(proposal, latestRevision));
var decidedByUserNames = await ResolveDecidedByUserNamesAsync(new[] { proposal }, cancellationToken);
return Result.Success(BuildEffectiveProposalDto(
proposal,
latestRevision,
GetDecidedByUserName(proposal, decidedByUserNames)));
}
catch (DomainException ex)
{
Expand Down Expand Up @@ -1076,7 +1082,11 @@ private async Task<Result<ProposalDto>> BuildEffectiveProposalDtoAsync(
CancellationToken cancellationToken)
{
var effectiveRevision = await GetEffectiveRevisionAsync(proposal, cancellationToken);
return Result.Success(BuildEffectiveProposalDto(proposal, effectiveRevision));
var decidedByUserNames = await ResolveDecidedByUserNamesAsync(new[] { proposal }, cancellationToken);
return Result.Success(BuildEffectiveProposalDto(
proposal,
effectiveRevision,
GetDecidedByUserName(proposal, decidedByUserNames)));
}

/// <summary>
Expand All @@ -1103,9 +1113,10 @@ private async Task<Result<ProposalDto>> BuildEffectiveProposalDtoAsync(
/// </summary>
private static ProposalDto BuildEffectiveProposalDto(
AutomationProposal proposal,
ProposalRevision? effectiveRevision)
ProposalRevision? effectiveRevision,
string? decidedByUserName = null)
{
var dto = MapToDto(proposal) with
var dto = MapToDto(proposal, decidedByUserName) with
{
LatestRevisionId = proposal.Status == ProposalStatus.PendingReview
? effectiveRevision?.Id
Expand Down Expand Up @@ -1216,7 +1227,8 @@ public async Task<Result<ProposalDto>> MarkAsAppliedAsync(Guid id, CancellationT
if (!notifyResult.IsSuccess)
return Result.Failure<ProposalDto>(notifyResult.ErrorCode, notifyResult.ErrorMessage);

return Result.Success(MapToDto(proposal));
var decidedByUserNames = await ResolveDecidedByUserNamesAsync(new[] { proposal }, cancellationToken);
return Result.Success(MapToDto(proposal, GetDecidedByUserName(proposal, decidedByUserNames)));
}
catch (DomainException ex)
{
Expand All @@ -1243,7 +1255,8 @@ public async Task<Result<ProposalDto>> MarkAsFailedAsync(Guid id, string failure
if (!notifyResult.IsSuccess)
return Result.Failure<ProposalDto>(notifyResult.ErrorCode, notifyResult.ErrorMessage);

return Result.Success(MapToDto(proposal));
var decidedByUserNames = await ResolveDecidedByUserNamesAsync(new[] { proposal }, cancellationToken);
return Result.Success(MapToDto(proposal, GetDecidedByUserName(proposal, decidedByUserNames)));
}
catch (DomainException ex)
{
Expand Down Expand Up @@ -1603,7 +1616,36 @@ private Task<Result> GuardProposalDecisionWriteAsync(
CancellationToken cancellationToken) =>
_policyEngine.GuardProposalDecisionWritesAsync(new[] { boardId }, cancellationToken);

private static ProposalDto MapToDto(AutomationProposal proposal)
private async Task<IReadOnlyDictionary<Guid, string>> ResolveDecidedByUserNamesAsync(
IEnumerable<AutomationProposal> proposals,
CancellationToken cancellationToken)
{
var ids = proposals
.Select(proposal => proposal.DecidedByUserId)
.Where(id => id.HasValue && id.Value != Guid.Empty)
.Select(id => id!.Value)
.Distinct()
.ToList();

if (ids.Count == 0 || _unitOfWork.Users is null)
return new Dictionary<Guid, string>();

// Actor names are response enrichment. A missing user (for example, a legacy row whose
// actor was removed) stays explicitly unavailable rather than leaking the opaque id.
var names = await _unitOfWork.Users.GetUsernamesByIdsAsync(ids, cancellationToken);
return names ?? new Dictionary<Guid, string>();
}

private static string? GetDecidedByUserName(
AutomationProposal proposal,
IReadOnlyDictionary<Guid, string> names)
{
return proposal.DecidedByUserId is Guid id && names.TryGetValue(id, out var name)
? name
: null;
}

private static ProposalDto MapToDto(AutomationProposal proposal, string? decidedByUserName = null)
{
var operationDtos = proposal.Operations.Select(MapOperationToDto).ToList();

Expand Down Expand Up @@ -1635,6 +1677,7 @@ private static ProposalDto MapToDto(AutomationProposal proposal)
Presentation = BuildPresentation(proposal.Summary, proposal.RiskLevel, proposal.SourceType, operationDtos),
IsExpired = proposal.IsExpired,
DeferredUntil = proposal.DeferredUntil,
DecidedByUserName = decidedByUserName,
ApprovedRevisionId = proposal.ApprovedRevisionId
};
}
Expand Down
18 changes: 18 additions & 0 deletions backend/src/Taskdeck.Infrastructure/Repositories/UserRepository.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,24 @@ public UserRepository(TaskdeckDbContext context) : base(context)
{
}

public async Task<IReadOnlyDictionary<Guid, string>> GetUsernamesByIdsAsync(
IEnumerable<Guid> ids,
CancellationToken cancellationToken = default)
{
var uniqueIds = ids
.Where(id => id != Guid.Empty)
.Distinct()
.ToList();

if (uniqueIds.Count == 0)
return new Dictionary<Guid, string>();

return await _context.Users
.AsNoTracking()
.Where(user => uniqueIds.Contains(user.Id))
.ToDictionaryAsync(user => user.Id, user => user.Username, cancellationToken);
}

public async Task<User?> GetByUsernameAsync(string username, CancellationToken cancellationToken = default)
{
return await _context.Users
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,10 @@ public StubUserRepository(User? userToReturn)
=> throw new NotImplementedException();
public Task<User?> GetByEmailAsync(string email, CancellationToken cancellationToken = default)
=> throw new NotImplementedException();
public Task<IReadOnlyDictionary<Guid, string>> GetUsernamesByIdsAsync(
IEnumerable<Guid> ids,
CancellationToken cancellationToken = default)
=> throw new NotImplementedException();
public Task<bool> ExistsAsync(string username, string email, CancellationToken cancellationToken = default)
=> throw new NotImplementedException();
public Task<IEnumerable<User>> GetAllAsync(CancellationToken cancellationToken = default)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,8 @@ public async Task ApproveProposal_ShouldUpdateStatus()
approvedProposal.Should().NotBeNull();
approvedProposal!.Status.Should().Be(ProposalStatus.Approved);
approvedProposal.DecidedByUserId.Should().Be(userId);
approvedProposal.DecidedByUserName.Should().StartWith("automation-approve_");
approvedProposal.DecidedByUserName.Should().NotBe(approvedProposal.DecidedByUserId.ToString());
approvedProposal.DecidedAt.Should().NotBeNull();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -669,6 +669,11 @@ public async Task ApproveProposalAsync_ShouldReturnSuccess_WhenPending()

_proposalRepoMock.Setup(r => r.GetByIdAsync(proposalId, default))
.ReturnsAsync(proposal);
_userRepoMock
.Setup(r => r.GetUsernamesByIdsAsync(
It.Is<IEnumerable<Guid>>(ids => ids.Single() == deciderId),
It.IsAny<CancellationToken>()))
.ReturnsAsync(new Dictionary<Guid, string> { [deciderId] = "Ada" });

// Act
var result = await _service.ApproveProposalAsync(proposalId, deciderId);
Expand All @@ -677,6 +682,7 @@ public async Task ApproveProposalAsync_ShouldReturnSuccess_WhenPending()
result.IsSuccess.Should().BeTrue();
result.Value.Status.Should().Be(ProposalStatus.Approved);
result.Value.DecidedByUserId.Should().Be(deciderId);
result.Value.DecidedByUserName.Should().Be("Ada");
result.Value.DecidedAt.Should().NotBeNull();
_unitOfWorkMock.Verify(u => u.SaveChangesAsync(default), Times.Once);
_notificationServiceMock.Verify(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ const props = defineProps<{

const { t, locale } = useI18n()

const uuidPattern = /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i

const dateLocale = computed(() => {
const active = locale.value
const preferred =
Expand All @@ -23,8 +21,8 @@ const dateLocale = computed(() => {
})

const decisionActor = computed(() => {
const actor = props.proposal.decidedByUserId?.trim() ?? ''
return uuidPattern.test(actor) ? actor : t('review.appliedRecord.value.notRecorded')
const actorName = props.proposal.decidedByUserName?.trim() ?? ''
return actorName || t('review.appliedRecord.value.notRecorded')
})

function formatTimestamp(value: string | null): string {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ function makeProposal(overrides: Partial<Proposal> = {}): Proposal {
expiresAt: '2026-08-25T08:00:00.000Z',
decidedAt: '2026-08-24T09:00:00.000Z',
decidedByUserId: '31f21efa-8ce7-4e85-8c18-0eefac9edcb7',
decidedByUserName: 'Ada',
appliedAt: '2026-08-24T09:30:00.000Z',
failureReason: null,
correlationId: 'correlation-1',
Expand Down Expand Up @@ -66,7 +67,7 @@ describe('ReviewAppliedDecisionRecord', () => {
expect(wrapper.get('[data-testid="applied-record-outcome"]').text()).toBe('Applied')
expect(wrapper.get('[data-testid="applied-record-decision"]').text()).toBe('Approved')
expect(wrapper.get('[data-testid="applied-record-decision-actor"]').text()).toBe(
'31f21efa-8ce7-4e85-8c18-0eefac9edcb7',
'Ada',
)
expect(wrapper.get('[data-testid="applied-record-decision-time"]').text()).toContain('2026')
expect(wrapper.get('[data-testid="applied-record-applied-time"]').text()).toContain('2026')
Expand Down Expand Up @@ -103,6 +104,7 @@ describe('ReviewAppliedDecisionRecord', () => {
proposal: makeProposal({
decidedAt: 'not-a-date',
decidedByUserId: 'legacy-user',
decidedByUserName: null,
appliedAt: null,
operations: [],
presentation: undefined,
Expand All @@ -115,4 +117,17 @@ describe('ReviewAppliedDecisionRecord', () => {
expect(wrapper.get('[data-testid="applied-record-applied-time"]').text()).toBe('Not recorded')
expect(wrapper.get('[data-testid="applied-record-operations-empty"]').text()).toBe('Not recorded')
})

it('does not expose the actor id when the friendly name is unavailable', () => {
const wrapper = mount(ReviewAppliedDecisionRecord, {
props: {
proposal: makeProposal({ decidedByUserName: null }),
},
})

expect(wrapper.get('[data-testid="applied-record-decision-actor"]').text()).toBe('Not recorded')
expect(wrapper.get('[data-testid="applied-record-decision-actor"]').text()).not.toContain(
'31f21efa-8ce7-4e85-8c18-0eefac9edcb7',
)
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -1309,6 +1309,7 @@ describe('PaperReviewView', () => {
summary: 'Newer applied work',
decidedAt: new Date(Date.now() - 35 * 60_000).toISOString(),
decidedByUserId: '31f21efa-8ce7-4e85-8c18-0eefac9edcb7',
decidedByUserName: 'Ada',
appliedAt: newerAppliedAt,
presentation: {
plainSummary: 'Newer applied work',
Expand Down Expand Up @@ -1341,7 +1342,7 @@ describe('PaperReviewView', () => {
'Create card "Exact applied work".',
)
expect(wrapper.get('[data-testid="applied-record-decision-actor"]').text()).toBe(
'31f21efa-8ce7-4e85-8c18-0eefac9edcb7',
'Ada',
)
expect(wrapper.find('[data-testid="decision-apply"]').exists()).toBe(false)
expect(wrapper.find('[data-testid="decision-reject"]').exists()).toBe(false)
Expand Down
2 changes: 2 additions & 0 deletions frontend/taskdeck-web/src/types/automation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ export interface Proposal {
expiresAt: string
decidedAt: string | null
decidedByUserId: string | null
/** Username resolved by the API for the historical decision actor; absent on legacy payloads. */
decidedByUserName?: string | null
appliedAt: string | null
failureReason: string | null
correlationId: string
Expand Down
Loading