Code review fixes

sillsdev · Jan 7, 2025 · 5cc027e · 5cc027e
1 parent d38424d
commit 5cc027e
Show file tree

Hide file tree

Showing 7 changed files with 96 additions and 15 deletions.
diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/event-metrics/event-metrics-log.component.spec.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/event-metrics/event-metrics-log.component.spec.ts
@@ -132,6 +132,25 @@ describe('EventMetricsLogComponent', () => {
     expect(env.nextButton.nativeElement.disabled).toBeTrue();
     expect(env.rows.length).toEqual(2);
   }));
+
+  it('should show custom event type calculations', fakeAsync(() => {
+    const env = new TestEnvironment();
+    // This list is to ensure complete test coverage of the custom cases in EventMetricsLogComponent.getEventType()
+    let eventMetrics: Partial<EventMetric>[] = [
+      { eventType: 'SetIsValidAsync', payload: { isValid: true } },
+      { eventType: 'SetIsValidAsync', payload: { isValid: false } },
+      { eventType: 'SetDraftAppliedAsync', payload: { draftApplied: true } },
+      { eventType: 'SetDraftAppliedAsync', payload: { draftApplied: false } },
+      { eventType: 'SetPreTranslateAsync', payload: { preTranslate: true } },
+      { eventType: 'SetPreTranslateAsync', payload: { preTranslate: false } }
+    ];
+
+    env.populateEventMetrics(eventMetrics);
+    env.wait();
+    env.wait();
+
+    expect(env.rows.length).toEqual(eventMetrics.length);
+  }));
 });
 
 class TestEnvironment {
@@ -179,14 +198,15 @@ class TestEnvironment {
     this.wait();
   }
 
-  populateEventMetrics(): void {
+  populateEventMetrics(eventMetrics: Partial<EventMetric>[] | undefined = undefined): void {
+    eventMetrics ??= [
+      { scope: EventScope.Sync, timeStamp: new Date().toISOString(), eventType: 'SyncAsync' } as EventMetric,
+      { scope: EventScope.Settings, timeStamp: new Date().toISOString(), eventType: 'UnknownType' } as EventMetric
+    ];
     when(mockedProjectService.onlineEventMetrics(anything(), anything(), anything())).thenReturn(
       Promise.resolve({
-        results: [
-          { scope: EventScope.Sync, timeStamp: new Date().toISOString(), eventType: 'SyncAsync' } as EventMetric,
-          { scope: EventScope.Settings, timeStamp: new Date().toISOString(), eventType: 'UnknownType' } as EventMetric
-        ],
-        unpagedCount: 4
+        results: eventMetrics,
+        unpagedCount: eventMetrics.length * 2
       } as QueryResults<EventMetric>)
     );
   }

diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/event-metrics/event-metrics-log.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/event-metrics/event-metrics-log.component.ts
@@ -37,7 +37,7 @@ export class EventMetricsLogComponent extends DataLoadingComponent implements On
   rows: Row[] = [];
 
   private pageIndex$ = new BehaviorSubject<number>(0);
-  private pageSize$ = new BehaviorSubject<number>(50);
+  private pageSize$ = new BehaviorSubject<number>(10);
   length: number = 0;
 
   get pageIndex(): number {
@@ -125,7 +125,7 @@ export class EventMetricsLogComponent extends DataLoadingComponent implements On
     for (const eventMetric of this.eventMetrics) {
       rows.push({
         dialogData: eventMetric,
-        eventType: this.getEventType(eventMetric.eventType),
+        eventType: this.getEventType(eventMetric),
         scope: eventMetric.scope,
         successful: eventMetric.exception == null,
         timeStamp: this.i18n.formatDate(new Date(eventMetric.timeStamp), { showTimeZone: true }),
@@ -135,7 +135,7 @@ export class EventMetricsLogComponent extends DataLoadingComponent implements On
     this.rows = rows;
   }
 
-  private getEventType(eventType: string): string {
+  private getEventType(eventMetric: EventMetric): string {
     // These values are the functions that have the LogEventMetric attribute, where:
     //  - The case is the name of the method
     //  - The return value is a user friendly description of what the method does
@@ -153,6 +153,22 @@ export class EventMetricsLogComponent extends DataLoadingComponent implements On
       StartPreTranslationBuildAsync: 'Start draft generation',
       SyncAsync: 'Start synchronization with Paratext'
     };
-    return eventTypeMap[eventType] || eventType;
+
+    // Allow specific cases based on payload values
+    if (eventMetric.eventType === 'SetIsValidAsync' && eventMetric.payload['isValid'] === true) {
+      return 'Marked chapter as valid';
+    } else if (eventMetric.eventType === 'SetIsValidAsync' && eventMetric.payload['isValid'] === false) {
+      return 'Marked chapter as invalid';
+    } else if (eventMetric.eventType === 'SetDraftAppliedAsync' && eventMetric.payload['draftApplied'] === true) {
+      return 'Marked chapter as having draft applied';
+    } else if (eventMetric.eventType === 'SetDraftAppliedAsync' && eventMetric.payload['draftApplied'] === false) {
+      return 'Marked chapter as not having a draft applied';
+    } else if (eventMetric.eventType === 'SetPreTranslateAsync' && eventMetric.payload['preTranslate'] === true) {
+      return 'Set drafting as enabled for the project';
+    } else if (eventMetric.eventType === 'SetPreTranslateAsync' && eventMetric.payload['preTranslate'] === false) {
+      return 'Set drafting as disabled for the project';
+    } else {
+      return eventTypeMap[eventMetric.eventType] || eventMetric.eventType;
+    }
   }
 }
diff --git a/src/SIL.XForge.Scripture/Services/ISFProjectService.cs b/src/SIL.XForge.Scripture/Services/ISFProjectService.cs
@@ -81,7 +81,7 @@ string audioUrl
     Task SetDraftAppliedAsync(string userId, string projectId, int book, int chapter, bool draftApplied);
 
     [LogEventMetric(EventScope.Drafting)]
-    Task SetIsValidAsync(string userId, string projectId, int book, int chapter, bool draftApplied);
+    Task SetIsValidAsync(string userId, string projectId, int book, int chapter, bool isValid);
     Task SetRoleProjectPermissionsAsync(string curUserId, string projectId, string role, string[] permissions);
     Task SetUserProjectPermissionsAsync(string curUserId, string projectId, string userId, string[] permissions);
 }
diff --git a/src/SIL.XForge/DataAccess/BsonValueConverter.cs b/src/SIL.XForge/DataAccess/BsonValueConverter.cs
@@ -23,6 +23,7 @@ public override BsonValue Read(ref Utf8JsonReader reader, Type typeToConvert, Js
             {
                 bsonArray.Add(Read(ref reader, typeof(BsonValue), options));
             }
+
             return bsonArray;
         }
 

diff --git a/src/SIL.XForge/DataAccess/MemoryRepository.cs b/src/SIL.XForge/DataAccess/MemoryRepository.cs
@@ -233,7 +233,7 @@ JsonSerializer serializer
                 var bsonArray = new BsonArray();
                 while (reader.Read() && reader.TokenType != JsonToken.EndArray)
                 {
-                    bsonArray.Add(BsonValue.Create(reader.Value));
+                    bsonArray.Add(ReadJson(reader, typeof(BsonValue), null, false, serializer));
                 }
 
                 return bsonArray;

diff --git a/src/SIL.XForge/Services/EventMetricService.cs b/src/SIL.XForge/Services/EventMetricService.cs
@@ -91,8 +91,6 @@ private static BsonValue GetBsonValue(object? objectValue) =>
             decimal value => new BsonDecimal128(value),
             DateTime value => new BsonDateTime(value),
             null => BsonNull.Value,
-            _ => BsonValue.Create(
-                JsonConvert.DeserializeObject<Dictionary<string, object>>(JsonConvert.SerializeObject(objectValue))
-            ),
+            _ => BsonDocument.Parse(JsonConvert.SerializeObject(objectValue)),
         };
 }
diff --git a/test/SIL.XForge.Tests/Services/EventMetricServiceTests.cs b/test/SIL.XForge.Tests/Services/EventMetricServiceTests.cs
@@ -302,6 +302,41 @@ await env.Service.SaveEventMetricAsync(
         Assert.AreEqual(BsonNull.Value, eventMetric.Result);
     }
 
+    [Test]
+    public async Task SaveEventMetricAsync_ArraysOfObjects()
+    {
+        var env = new TestEnvironment();
+        var objectValue = new TestClass { Records = [new TestRecord { ProjectId = Project01, UserId = User01 }] };
+        Dictionary<string, object> argumentsWithNames = new Dictionary<string, object> { { "myObject", objectValue } };
+        string expectedJson = JsonConvert.SerializeObject(objectValue);
+        Dictionary<string, BsonValue> expectedPayload = new Dictionary<string, BsonValue>
+        {
+            { "myObject", BsonDocument.Parse(expectedJson) },
+        };
+
+        // SUT
+        await env.Service.SaveEventMetricAsync(
+            Project01,
+            User01,
+            EventType01,
+            EventScope01,
+            argumentsWithNames,
+            result: null,
+            exception: null
+        );
+
+        // Verify the saved event metric
+        EventMetric eventMetric = env.EventMetrics.Query().OrderByDescending(e => e.TimeStamp).First();
+        Assert.AreEqual(EventScope01, eventMetric.Scope);
+        Assert.AreEqual(Project01, eventMetric.ProjectId);
+        Assert.AreEqual(User01, eventMetric.UserId);
+        Assert.AreEqual(EventType01, eventMetric.EventType);
+        Assert.IsTrue(env.PayloadEqualityComparer.Equals(expectedPayload, eventMetric.Payload));
+        string actualJson = eventMetric.Payload["myObject"].ToJson().Replace(" ", string.Empty);
+        Assert.AreEqual(expectedJson, actualJson);
+        Assert.AreEqual(BsonNull.Value, eventMetric.Result);
+    }
+
     private class TestEnvironment
     {
         public TestEnvironment()
@@ -357,4 +392,15 @@ public TestEnvironment()
         public IRepository<EventMetric> EventMetrics { get; }
         public IEventMetricService Service { get; }
     }
+
+    public class TestClass
+    {
+        public HashSet<TestRecord> Records = [];
+    }
+
+    public record TestRecord
+    {
+        public string ProjectId { get; set; } = string.Empty;
+        public string UserId { get; set; } = string.Empty;
+    }
 }
-Original file line number
+Diff line change
@@ Expand Up @@
                 {
                     bsonArray.Add(Read(ref reader, typeof(BsonValue), options));
                 }
                 return bsonArray;
             }
@@ Expand Down @@