Skip to content

Commit ab72745

Browse files
committed
Address all review feedback: FieldOr queries, remove manual dates, pattern matching, test naming
1 parent 028e805 commit ab72745

7 files changed

Lines changed: 42 additions & 37 deletions

File tree

src/Exceptionless.Core/Repositories/Interfaces/ISavedViewRepository.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ namespace Exceptionless.Core.Repositories;
66

77
public interface ISavedViewRepository : IRepositoryOwnedByOrganization<SavedView>
88
{
9-
Task<FindResults<SavedView>> GetByViewAsync(string organizationId, string dashboardView, CommandOptionsDescriptor<SavedView>? options = null);
10-
Task<FindResults<SavedView>> GetByViewForUserAsync(string organizationId, string dashboardView, string userId, CommandOptionsDescriptor<SavedView>? options = null);
9+
Task<FindResults<SavedView>> GetByViewAsync(string organizationId, string viewName, CommandOptionsDescriptor<SavedView>? options = null);
10+
Task<FindResults<SavedView>> GetByViewForUserAsync(string organizationId, string viewName, string userId, CommandOptionsDescriptor<SavedView>? options = null);
1111
Task<FindResults<SavedView>> GetByOrganizationForUserAsync(string organizationId, string userId, CommandOptionsDescriptor<SavedView>? options = null);
1212
Task<long> CountByOrganizationIdAsync(string organizationId);
1313

src/Exceptionless.Core/Repositories/SavedViewRepository.cs

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,29 +12,37 @@ public SavedViewRepository(ExceptionlessElasticConfiguration configuration, AppO
1212
{
1313
}
1414

15-
public Task<FindResults<SavedView>> GetByViewAsync(string organizationId, string dashboardView, CommandOptionsDescriptor<SavedView>? options = null)
15+
public Task<FindResults<SavedView>> GetByViewAsync(string organizationId, string viewName, CommandOptionsDescriptor<SavedView>? options = null)
1616
{
1717
return FindAsync(q => q
1818
.Organization(organizationId)
19-
.FieldEquals(e => e.View, dashboardView)
19+
.FieldEquals(e => e.View, viewName)
2020
.SortExpression("name"), options);
2121
}
2222

23-
public Task<FindResults<SavedView>> GetByViewForUserAsync(string organizationId, string dashboardView, string userId, CommandOptionsDescriptor<SavedView>? options = null)
23+
public Task<FindResults<SavedView>> GetByViewForUserAsync(string organizationId, string viewName, string userId, CommandOptionsDescriptor<SavedView>? options = null)
2424
{
25+
#pragma warning disable CS8603 // FieldOr is non-null at runtime; Foundatio nullable annotation issue
2526
return FindAsync(q => q
2627
.Organization(organizationId)
27-
.FieldEquals(e => e.View, dashboardView)
28-
.FilterExpression($"(_missing_:user_id OR user_id:{userId})")
29-
.SortExpression("name"), options);
28+
.FieldEquals(e => e.View, viewName)
29+
.SortExpression("name")
30+
.FieldOr(g => g
31+
.FieldEmpty(e => e.UserId)
32+
.FieldEquals(e => e.UserId!, userId)), options);
33+
#pragma warning restore CS8603
3034
}
3135

3236
public Task<FindResults<SavedView>> GetByOrganizationForUserAsync(string organizationId, string userId, CommandOptionsDescriptor<SavedView>? options = null)
3337
{
38+
#pragma warning disable CS8603 // FieldOr is non-null at runtime; Foundatio nullable annotation issue
3439
return FindAsync(q => q
3540
.Organization(organizationId)
36-
.FilterExpression($"(_missing_:user_id OR user_id:{userId})")
37-
.SortExpression("name"), options);
41+
.SortExpression("name")
42+
.FieldOr(g => g
43+
.FieldEmpty(e => e.UserId)
44+
.FieldEquals(e => e.UserId!, userId)), options);
45+
#pragma warning restore CS8603
3846
}
3947

4048
public async Task<long> RemovePrivateByUserIdAsync(string organizationId, string userId)

src/Exceptionless.Core/Services/OrganizationService.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,7 @@ public Task<long> RemoveSavedViewsAsync(Organization organization)
190190

191191
public Task<long> RemoveUserSavedViewsAsync(string organizationId, string userId)
192192
{
193+
_logger.LogDebug("Removing private saved views for user {UserId} in organization {OrganizationId}", userId, organizationId);
193194
return _savedViewRepository.RemovePrivateByUserIdAsync(organizationId, userId);
194195
}
195196

src/Exceptionless.Web/ClientApp/src/routes/(app)/+layout.svelte

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,6 @@
267267
}
268268
269269
const queryParams = new URLSearchParams(queryEntries);
270-
271270
return `${baseHref}?${queryParams.toString()}`;
272271
}
273272

src/Exceptionless.Web/Controllers/SavedViewController.cs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,6 @@ protected override async Task<PermissionResult> CanUpdateAsync(SavedView origina
226226

227227
protected override async Task<SavedView> AddModelAsync(SavedView value)
228228
{
229-
value.CreatedUtc = value.UpdatedUtc = _timeProvider.GetUtcNow().UtcDateTime;
230229
value.CreatedByUserId = CurrentUser.Id;
231230
value.Version = 1;
232231

@@ -238,7 +237,6 @@ protected override async Task<SavedView> AddModelAsync(SavedView value)
238237

239238
protected override async Task<SavedView> UpdateModelAsync(SavedView original, Delta<UpdateSavedView> changes)
240239
{
241-
original.UpdatedUtc = _timeProvider.GetUtcNow().UtcDateTime;
242240
original.UpdatedByUserId = CurrentUser.Id;
243241

244242
if (changes.GetChangedPropertyNames().Contains(nameof(UpdateSavedView.IsDefault))

src/Exceptionless.Web/Models/SavedView/NewSavedView.cs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,15 +41,15 @@ public record NewSavedView : IOwnedByOrganization, IValidatableObject
4141

4242
public IEnumerable<ValidationResult> Validate(ValidationContext validationContext)
4343
{
44-
if (!String.IsNullOrEmpty(View) && !SavedView.ValidViews.Contains(View))
44+
if (View is { Length: > 0 } && !SavedView.ValidViews.Contains(View))
4545
{
4646
yield return new ValidationResult(
4747
$"View must be one of: {String.Join(", ", SavedView.ValidViews)}",
4848
[nameof(View)]
4949
);
5050
}
5151

52-
if (!String.IsNullOrEmpty(FilterDefinitions) && !IsValidJsonArray(FilterDefinitions))
52+
if (FilterDefinitions is { Length: > 0 } && !IsValidJsonArray(FilterDefinitions))
5353
{
5454
yield return new ValidationResult(
5555
"FilterDefinitions must be a valid JSON array",
@@ -89,7 +89,6 @@ private static bool IsValidJsonArray(string json)
8989
try
9090
{
9191
using var document = JsonDocument.Parse(json);
92-
9392
return document.RootElement.ValueKind == JsonValueKind.Array;
9493
}
9594
catch (JsonException)

tests/Exceptionless.Tests/Controllers/SavedViewControllerTests.cs

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -700,26 +700,26 @@ await SendRequestAsync(r => r
700700
public async Task RemoveUser_DeletesPrivateSavedViews_ButPreservesOrganizationWideViews()
701701
{
702702
// Arrange — create an organization-wide view and a private view for the test organization user
703-
var orgUser = await _userRepository.GetByEmailAddressAsync(SampleDataService.TEST_ORG_USER_EMAIL);
704-
Assert.NotNull(orgUser);
703+
var testUser = await _userRepository.GetByEmailAddressAsync(SampleDataService.TEST_ORG_USER_EMAIL);
704+
Assert.NotNull(testUser);
705705

706-
var orgWideView = await _savedViewRepository.AddAsync(new SavedView
706+
var organizationWideView = await _savedViewRepository.AddAsync(new SavedView
707707
{
708708
OrganizationId = SampleDataService.TEST_ORG_ID,
709709
Name = "Organization Wide",
710710
Filter = "status:open",
711711
View = "events",
712-
CreatedByUserId = orgUser.Id
712+
CreatedByUserId = testUser.Id
713713
});
714714

715715
var privateView = await _savedViewRepository.AddAsync(new SavedView
716716
{
717717
OrganizationId = SampleDataService.TEST_ORG_ID,
718-
UserId = orgUser.Id,
718+
UserId = testUser.Id,
719719
Name = "My Private View",
720720
Filter = "type:error",
721721
View = "events",
722-
CreatedByUserId = orgUser.Id
722+
CreatedByUserId = testUser.Id
723723
});
724724

725725
await RefreshDataAsync();
@@ -736,33 +736,33 @@ await SendRequestAsync(r => r
736736

737737
// Assert — private view is gone, organization-wide view remains
738738
Assert.Null(await _savedViewRepository.GetByIdAsync(privateView.Id));
739-
Assert.NotNull(await _savedViewRepository.GetByIdAsync(orgWideView.Id));
739+
Assert.NotNull(await _savedViewRepository.GetByIdAsync(organizationWideView.Id));
740740
}
741741

742742
[Fact]
743743
public async Task SoftDeleteOrganization_RemovesAllSavedViews()
744744
{
745745
// Arrange
746-
var orgUser = await _userRepository.GetByEmailAddressAsync(SampleDataService.TEST_USER_EMAIL);
747-
Assert.NotNull(orgUser);
746+
var testUser = await _userRepository.GetByEmailAddressAsync(SampleDataService.TEST_USER_EMAIL);
747+
Assert.NotNull(testUser);
748748

749749
await _savedViewRepository.AddAsync(new SavedView
750750
{
751751
OrganizationId = SampleDataService.TEST_ORG_ID,
752752
Name = "Organization View",
753753
Filter = "status:open",
754754
View = "events",
755-
CreatedByUserId = orgUser.Id
755+
CreatedByUserId = testUser.Id
756756
});
757757

758758
await _savedViewRepository.AddAsync(new SavedView
759759
{
760760
OrganizationId = SampleDataService.TEST_ORG_ID,
761-
UserId = orgUser.Id,
761+
UserId = testUser.Id,
762762
Name = "Private View",
763763
Filter = "type:error",
764764
View = "events",
765-
CreatedByUserId = orgUser.Id
765+
CreatedByUserId = testUser.Id
766766
});
767767

768768
await RefreshDataAsync();
@@ -774,7 +774,7 @@ await _savedViewRepository.AddAsync(new SavedView
774774
var organizationRepository = GetService<IOrganizationRepository>();
775775
var organization = await organizationRepository.GetByIdAsync(SampleDataService.TEST_ORG_ID);
776776
Assert.NotNull(organization);
777-
await _organizationService.SoftDeleteOrganizationAsync(organization, orgUser.Id);
777+
await _organizationService.SoftDeleteOrganizationAsync(organization, testUser.Id);
778778
await RefreshDataAsync();
779779

780780
// Assert
@@ -786,38 +786,38 @@ await _savedViewRepository.AddAsync(new SavedView
786786
public async Task RemoveUserSavedViews_OnlyDeletesPrivateViews()
787787
{
788788
// Arrange
789-
var orgUser = await _userRepository.GetByEmailAddressAsync(SampleDataService.TEST_ORG_USER_EMAIL);
790-
Assert.NotNull(orgUser);
789+
var testUser = await _userRepository.GetByEmailAddressAsync(SampleDataService.TEST_ORG_USER_EMAIL);
790+
Assert.NotNull(testUser);
791791

792-
var orgWide = await _savedViewRepository.AddAsync(new SavedView
792+
var organizationWide = await _savedViewRepository.AddAsync(new SavedView
793793
{
794794
OrganizationId = SampleDataService.TEST_ORG_ID,
795795
Name = "Organization Wide",
796796
Filter = "status:open",
797797
View = "events",
798-
CreatedByUserId = orgUser.Id
798+
CreatedByUserId = testUser.Id
799799
});
800800

801801
var privateView = await _savedViewRepository.AddAsync(new SavedView
802802
{
803803
OrganizationId = SampleDataService.TEST_ORG_ID,
804-
UserId = orgUser.Id,
804+
UserId = testUser.Id,
805805
Name = "Private",
806806
Filter = "type:error",
807807
View = "events",
808-
CreatedByUserId = orgUser.Id
808+
CreatedByUserId = testUser.Id
809809
});
810810

811811
await RefreshDataAsync();
812812

813813
// Act
814-
var removed = await _organizationService.RemoveUserSavedViewsAsync(SampleDataService.TEST_ORG_ID, orgUser.Id);
814+
var removed = await _organizationService.RemoveUserSavedViewsAsync(SampleDataService.TEST_ORG_ID, testUser.Id);
815815
await RefreshDataAsync();
816816

817817
// Assert
818818
Assert.Equal(1, removed);
819819
Assert.Null(await _savedViewRepository.GetByIdAsync(privateView.Id));
820-
Assert.NotNull(await _savedViewRepository.GetByIdAsync(orgWide.Id));
820+
Assert.NotNull(await _savedViewRepository.GetByIdAsync(organizationWide.Id));
821821
}
822822

823823
private async Task<ViewSavedView?> CreateSavedViewAsync(string name, string filter, string view, bool isPrivate = false, bool isDefault = false)

0 commit comments

Comments
 (0)