Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
8d67715
Fix UpdateCollectionCommand to set RevisionDate using TimeProvider an…
r-tome Apr 2, 2026
a330cd6
Enhance BulkAddCollectionAccessCommand to include revision date in ac…
r-tome Apr 3, 2026
59a90c6
Update GroupRepository and stored procedures to bump RevisionDate for…
r-tome Apr 3, 2026
4ee23dd
Implement revision date updates for affected collections in Organizat…
r-tome Apr 3, 2026
0a7c65f
Update database migration script
r-tome Apr 3, 2026
e878ae2
Update migration script summary
r-tome Apr 3, 2026
48d9e7c
Refactor OrganizationUserReplaceTests to create collection first
r-tome Apr 3, 2026
2cc7800
Merge branch 'main' into ac/pm-22450/collection-revision-date-is-not-…
r-tome Apr 4, 2026
2aafad2
Refactor stored procedures to use Common Table Expressions (CTEs) for…
r-tome Apr 6, 2026
ab54a70
Enhance OrganizationUser_CreateManyWithCollectionsAndGroups stored pr…
r-tome Apr 6, 2026
4d80849
Refactor OrganizationUser_CreateManyWithCollectionsGroups and migrati…
r-tome Apr 6, 2026
7206101
Refactor OrganizationUserRepository to consistently use RevisionDate …
r-tome Apr 6, 2026
1f9ae7b
Refactor tests to ensure consistent handling of RevisionDate across G…
r-tome Apr 6, 2026
0e81b49
Restore BOM in Group_UpdateWithCollections and OrganizationUser_Updat…
r-tome Apr 6, 2026
1bd19cb
Refactor GroupRepository and OrganizationUserRepository to improve ha…
r-tome Apr 6, 2026
dabb672
Bump migration script date
r-tome Apr 6, 2026
4948c15
Remove internal set from RevisionDate on Group and OrganizationUser
r-tome Apr 6, 2026
7ae69c9
Refactor OrganizationUser_CreateManyWithCollectionsGroups and migrati…
r-tome Apr 6, 2026
7b39459
Merge branch 'main' into ac/pm-22450/collection-revision-date-is-not-…
r-tome Apr 7, 2026
c3cf999
Fix sprocs styling
r-tome Apr 7, 2026
7de27a9
Added early return to OrganizationUserRepository.CreateManyAsync if t…
r-tome Apr 7, 2026
a308ef2
Merge branch 'main' into ac/pm-22450/collection-revision-date-is-not-…
r-tome Apr 8, 2026
d339457
Merge branch 'main' into ac/pm-22450/collection-revision-date-is-not-…
r-tome Apr 9, 2026
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
2 changes: 1 addition & 1 deletion src/Core/AdminConsole/Entities/Group.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public class Group : ITableObject<Guid>, IExternal
[MaxLength(300)]
public string? ExternalId { get; set; }
public DateTime CreationDate { get; internal set; } = DateTime.UtcNow;
public DateTime RevisionDate { get; internal set; } = DateTime.UtcNow;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The repository serializes/deserializes the entity for the sproc call, and the internal set prevented RevisionDate from being set, causing the value to be lost. Making it public fixes that.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I also saw that only some entities have an internal set. I'm not sure why we have that inconsistency.

public DateTime RevisionDate { get; set; } = DateTime.UtcNow;
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.

🎨 SUGGESTED: The internal set β†’ set change here (and on OrganizationUser.RevisionDate) widens the access modifier. Since Core.csproj already declares InternalsVisibleTo for Core.Test, Infrastructure.IntegrationTest, and Identity.IntegrationTest, the test assemblies can already use internal set. The production callers in this PR (EF/Dapper repos) only read Group.RevisionDate and OrganizationUser.RevisionDate β€” they don't set them.

Consider keeping internal set to preserve the encapsulation that prevents accidental modification from assemblies outside Core. CreationDate on both entities still uses internal set, so this change also introduces an inconsistency between the two date properties.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That is wrong, Claude. I had to make the set public because otherwise on

var objWithCollections = JsonSerializer.Deserialize<GroupWithCollections>(JsonSerializer.Serialize(obj))!;
it fails to populate RevisionDate with the given value and defaults to DateTime.UtcNow. I noticed this because the tests would fail.


public void SetNewId()
{
Expand Down
2 changes: 1 addition & 1 deletion src/Core/AdminConsole/Entities/OrganizationUser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public class OrganizationUser : ITableObject<Guid>, IExternal, IOrganizationUser
/// <summary>
/// The last date the OrganizationUser entry was updated.
/// </summary>
public DateTime RevisionDate { get; internal set; } = DateTime.UtcNow;
public DateTime RevisionDate { get; set; } = DateTime.UtcNow;
/// <summary>
/// A json blob representing the <see cref="Bit.Core.Models.Data.Permissions"/> of the OrganizationUser if they
/// are a Custom user role (i.e. the <see cref="OrganizationUserType"/> is Custom). MAY be NULL if they are not
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,20 @@ public class BulkAddCollectionAccessCommand : IBulkAddCollectionAccessCommand
private readonly IOrganizationUserRepository _organizationUserRepository;
private readonly IGroupRepository _groupRepository;
private readonly IEventService _eventService;
private readonly TimeProvider _timeProvider;

public BulkAddCollectionAccessCommand(
ICollectionRepository collectionRepository,
IOrganizationUserRepository organizationUserRepository,
IGroupRepository groupRepository,
IEventService eventService)
IEventService eventService,
TimeProvider timeProvider)
{
_collectionRepository = collectionRepository;
_organizationUserRepository = organizationUserRepository;
_groupRepository = groupRepository;
_eventService = eventService;
_timeProvider = timeProvider;
}

public async Task AddAccessAsync(ICollection<Collection> collections,
Expand All @@ -34,15 +37,18 @@ public async Task AddAccessAsync(ICollection<Collection> collections,
{
await ValidateRequestAsync(collections, users, groups);

var revisionDate = _timeProvider.GetUtcNow().UtcDateTime;

await _collectionRepository.CreateOrUpdateAccessForManyAsync(
collections.First().OrganizationId,
collections.Select(c => c.Id),
users,
groups
groups,
revisionDate
);

await _eventService.LogCollectionEventsAsync(collections.Select(c =>
(c, EventType.Collection_Updated, (DateTime?)DateTime.UtcNow)));
(c, EventType.Collection_Updated, (DateTime?)revisionDate)));
}

private async Task ValidateRequestAsync(ICollection<Collection> collections, ICollection<CollectionAccessSelection> usersAccess, ICollection<CollectionAccessSelection> groupsAccess)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,18 @@ public class UpdateCollectionCommand : IUpdateCollectionCommand
private readonly IEventService _eventService;
private readonly IOrganizationRepository _organizationRepository;
private readonly ICollectionRepository _collectionRepository;
private readonly TimeProvider _timeProvider;

public UpdateCollectionCommand(
IEventService eventService,
IOrganizationRepository organizationRepository,
ICollectionRepository collectionRepository)
ICollectionRepository collectionRepository,
TimeProvider timeProvider)
{
_eventService = eventService;
_organizationRepository = organizationRepository;
_collectionRepository = collectionRepository;
_timeProvider = timeProvider;
}

public async Task<Collection> UpdateAsync(Collection collection, IEnumerable<CollectionAccessSelection> groups = null,
Expand Down Expand Up @@ -75,6 +78,7 @@ public async Task<Collection> UpdateAsync(Collection collection, IEnumerable<Col
}
}

collection.RevisionDate = _timeProvider.GetUtcNow().UtcDateTime;
await _collectionRepository.ReplaceAsync(collection, org.UseGroups ? groupsList : null, usersList);
await _eventService.LogCollectionEventAsync(collection, Enums.EventType.Collection_Updated);

Expand Down
12 changes: 11 additions & 1 deletion src/Core/Repositories/ICollectionRepository.cs
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,18 @@ public interface ICollectionRepository : IRepository<Collection, Guid>
Task UpdateUsersAsync(Guid id, IEnumerable<CollectionAccessSelection> users);
Task<ICollection<CollectionAccessSelection>> GetManyUsersByIdAsync(Guid id);
Task DeleteManyAsync(IEnumerable<Guid> collectionIds);

/// <summary>
/// Creates or updates the access for many collections.
/// </summary>
/// <param name="organizationId">The Organization ID.</param>
/// <param name="collectionIds">The Collection IDs to create or update access for.</param>
/// <param name="users">The users to grant access to.</param>
/// <param name="groups">The groups to grant access to.</param>
/// <param name="revisionDate">The revision date to use for the collections.</param>
Task CreateOrUpdateAccessForManyAsync(Guid organizationId, IEnumerable<Guid> collectionIds,
IEnumerable<CollectionAccessSelection> users, IEnumerable<CollectionAccessSelection> groups);
IEnumerable<CollectionAccessSelection> users, IEnumerable<CollectionAccessSelection> groups,
DateTime revisionDate);

/// <summary>
/// Creates default user collections for the specified organization users.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -652,6 +652,10 @@ public async Task CreateManyAsync(IEnumerable<CreateOrganizationUser> organizati
await using var connection = new SqlConnection(_marsConnectionString);

var organizationUsersList = organizationUserCollection.ToList();
if (organizationUsersList.Count == 0)
{
return;
}

await connection.ExecuteAsync(
$"[{Schema}].[OrganizationUser_CreateManyWithCollectionsAndGroups]",
Expand All @@ -672,7 +676,9 @@ await connection.ExecuteAsync(
{
GroupId = group,
OrganizationUserId = user.OrganizationUser.Id
}))
})),
// Use the same RevisionDate as the created OrganizationUsers
RevisionDate = organizationUsersList.First().OrganizationUser.RevisionDate
},
commandType: CommandType.StoredProcedure);
}
Expand Down
12 changes: 10 additions & 2 deletions src/Infrastructure.Dapper/Repositories/CollectionRepository.cs
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,8 @@ await connection.ExecuteAsync("[dbo].[Collection_DeleteByIds]",
}

public async Task CreateOrUpdateAccessForManyAsync(Guid organizationId, IEnumerable<Guid> collectionIds,
IEnumerable<CollectionAccessSelection> users, IEnumerable<CollectionAccessSelection> groups)
IEnumerable<CollectionAccessSelection> users, IEnumerable<CollectionAccessSelection> groups,
DateTime revisionDate)
{
using (var connection = new SqlConnection(ConnectionString))
{
Expand All @@ -310,7 +311,14 @@ public async Task CreateOrUpdateAccessForManyAsync(Guid organizationId, IEnumera

var results = await connection.ExecuteAsync(
$"[{Schema}].[Collection_CreateOrUpdateAccessForMany]",
new { OrganizationId = organizationId, CollectionIds = collectionIds.ToGuidIdArrayTVP(), Users = usersArray, Groups = groupsArray },
new
{
OrganizationId = organizationId,
CollectionIds = collectionIds.ToGuidIdArrayTVP(),
Users = usersArray,
Groups = groupsArray,
RevisionDate = revisionDate
},
commandType: CommandType.StoredProcedure);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ from c in dbContext.Collections
Manage = y.Manage,
});
await dbContext.CollectionGroups.AddRangeAsync(collectionGroups);
// Bump RevisionDate on all affected collections
var filteredCollectionIds = filteredCollections.Select(fc => fc.Id).ToHashSet();
foreach (var c in availableCollections.Where(a => filteredCollectionIds.Contains(a.Id)))
{
c.RevisionDate = grp.RevisionDate;
}
await dbContext.SaveChangesAsync();
}
}
Expand Down Expand Up @@ -227,6 +233,20 @@ public async Task ReplaceAsync(AdminConsoleEntities.Group group, IEnumerable<Col
dbContext.CollectionGroups.RemoveRange(
existingCollectionGroups.Where(cg => !requestedCollectionIds.Contains(cg.CollectionId)));

// Bump the revision date on all affected collections
var allAffectedCollectionIds = existingCollectionGroups.Select(cg => cg.CollectionId)
.Union(requestedCollections.Select(rc => rc.Id))
.Distinct()
.ToList();
var affectedCollections = await dbContext.Collections
.Where(c => c.OrganizationId == group.OrganizationId
&& allAffectedCollectionIds.Contains(c.Id))
.ToListAsync();
foreach (var c in affectedCollections)
{
c.RevisionDate = group.RevisionDate;
}

await dbContext.UserBumpAccountRevisionDateByOrganizationIdAsync(group.OrganizationId);
await dbContext.SaveChangesAsync();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,12 @@ from c in dbContext.Collections
Manage = y.Manage
});
await dbContext.CollectionUsers.AddRangeAsync(collectionUsers);
// Bump RevisionDate on all affected collections
var filteredCollectionIds = filteredCollections.Select(fc => fc.Id).ToHashSet();
foreach (var c in availableCollections.Where(a => filteredCollectionIds.Contains(a.Id)))
{
c.RevisionDate = organizationUser.RevisionDate;
}
await dbContext.SaveChangesAsync();
}

Expand Down Expand Up @@ -651,6 +657,21 @@ join c in dbContext.Collections on cu.CollectionId equals c.Id
// Remove all existing ones that are no longer requested
var requestedCollectionIds = requestedCollections.Select(c => c.Id).ToList();
dbContext.CollectionUsers.RemoveRange(existingCollectionUsers.Where(cu => !requestedCollectionIds.Contains(cu.CollectionId)));

// Bump the revision date on all affected collections
var allAffectedCollectionIds = existingCollectionUsers.Select(cu => cu.CollectionId)
.Union(requestedCollections.Select(rc => rc.Id))
.Distinct()
.ToList();
var affectedCollections = await dbContext.Collections
.Where(c => c.OrganizationId == obj.OrganizationId
&& allAffectedCollectionIds.Contains(c.Id))
.ToListAsync();
foreach (var c in affectedCollections)
{
c.RevisionDate = obj.RevisionDate;
}

await dbContext.SaveChangesAsync();
}
}
Expand Down Expand Up @@ -923,6 +944,11 @@ on ou.UserId equals u.Id

public async Task CreateManyAsync(IEnumerable<CreateOrganizationUser> organizationUserCollection)
{
if (!organizationUserCollection.Any())
{
return;
}

using var scope = ServiceScopeFactory.CreateScope();

await using var dbContext = GetDatabaseContext(scope);
Expand All @@ -942,6 +968,27 @@ public async Task CreateManyAsync(IEnumerable<CreateOrganizationUser> organizati
OrganizationUserId = user.OrganizationUser.Id
}));

// Bump RevisionDate on all affected collections
var affectedCollectionIds = organizationUserCollection
.SelectMany(x => x.Collections)
.Select(c => c.Id)
.Distinct()
.ToList();
if (affectedCollectionIds.Count > 0)
{
var organizationId = organizationUserCollection.First().OrganizationUser.OrganizationId;
var affectedCollections = await dbContext.Collections
.Where(c => c.OrganizationId == organizationId
&& affectedCollectionIds.Contains(c.Id))
.ToListAsync();
// Use the same RevisionDate as the created OrganizationUsers
var revisionDate = organizationUserCollection.First().OrganizationUser.RevisionDate;
foreach (var c in affectedCollections)
{
c.RevisionDate = revisionDate;
}
}

await dbContext.SaveChangesAsync();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -630,7 +630,8 @@ public async Task DeleteManyAsync(IEnumerable<Guid> collectionIds)
}

public async Task CreateOrUpdateAccessForManyAsync(Guid organizationId, IEnumerable<Guid> collectionIds,
IEnumerable<CollectionAccessSelection> users, IEnumerable<CollectionAccessSelection> groups)
IEnumerable<CollectionAccessSelection> users, IEnumerable<CollectionAccessSelection> groups,
DateTime revisionDate)
{
using (var scope = ServiceScopeFactory.CreateScope())
{
Expand Down Expand Up @@ -715,6 +716,16 @@ public async Task CreateOrUpdateAccessForManyAsync(Guid organizationId, IEnumera
}
// Need to save the new collection users/groups before running the bump revision code
await dbContext.SaveChangesAsync();

// Bump the revision date on all affected collections
var collections = await dbContext.Collections
.Where(c => collectionIdsList.Contains(c.Id))
.ToListAsync();
foreach (var c in collections)
{
c.RevisionDate = revisionDate;
}

await dbContext.UserBumpAccountRevisionDateByCollectionIdsAsync(collectionIdsList, organizationId);
await dbContext.SaveChangesAsync();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ CREATE PROCEDURE [dbo].[Collection_CreateOrUpdateAccessForMany]
@OrganizationId UNIQUEIDENTIFIER,
@CollectionIds AS [dbo].[GuidIdArray] READONLY,
@Groups AS [dbo].[CollectionAccessSelectionType] READONLY,
@Users AS [dbo].[CollectionAccessSelectionType] READONLY
@Users AS [dbo].[CollectionAccessSelectionType] READONLY,
@RevisionDate DATETIME2(7) = NULL
AS
BEGIN
SET NOCOUNT ON
Expand Down Expand Up @@ -109,5 +110,18 @@ BEGIN
[Source].[Manage]
);

IF @RevisionDate IS NOT NULL
BEGIN
-- Bump the revision date on all affected collections
UPDATE
C
SET
C.[RevisionDate] = @RevisionDate
FROM
[dbo].[Collection] C
INNER JOIN
@CollectionIds CI ON C.[Id] = CI.[Id]
END

EXEC [dbo].[User_BumpAccountRevisionDateByCollectionIds] @CollectionIds, @OrganizationId
END
11 changes: 11 additions & 0 deletions src/Sql/dbo/Stored Procedures/Group_CreateWithCollections.sql
Original file line number Diff line number Diff line change
Expand Up @@ -39,5 +39,16 @@ BEGIN
WHERE
[Id] IN (SELECT [Id] FROM [AvailableCollectionsCTE])

-- Bump RevisionDate on all affected collections
UPDATE
C
SET
C.[RevisionDate] = @RevisionDate
FROM
[dbo].[Collection] C
WHERE
C.[OrganizationId] = @OrganizationId
AND C.[Id] IN (SELECT [Id] FROM @Collections)

EXEC [dbo].[User_BumpAccountRevisionDateByOrganizationId] @OrganizationId
END
26 changes: 26 additions & 0 deletions src/Sql/dbo/Stored Procedures/Group_UpdateWithCollections.sql
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,32 @@ BEGIN

EXEC [dbo].[Group_Update] @Id, @OrganizationId, @Name, @ExternalId, @CreationDate, @RevisionDate

-- Bump RevisionDate on all affected collections
;WITH [AffectedCollectionsCTE] AS (
SELECT
[Id]
FROM
@Collections

UNION

SELECT
CG.[CollectionId]
FROM
[dbo].[CollectionGroup] CG
WHERE
CG.[GroupId] = @Id
)
UPDATE
C
SET
C.[RevisionDate] = @RevisionDate
FROM
[dbo].[Collection] C
WHERE
C.[OrganizationId] = @OrganizationId
AND C.[Id] IN (SELECT [Id] FROM [AffectedCollectionsCTE])

;WITH [AvailableCollectionsCTE] AS(
SELECT
Id
Expand Down
Loading
Loading