SF-3895 Allow Serval admins to view settings, users, sync; add sync log - #4038
SF-3895 Allow Serval admins to view settings, users, sync; add sync log#4038Nateowami wants to merge 1 commit into
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #4038 +/- ##
==========================================
+ Coverage 81.08% 81.14% +0.05%
==========================================
Files 661 665 +4
Lines 43061 43200 +139
Branches 7036 7059 +23
==========================================
+ Hits 34917 35053 +136
+ Misses 6987 6980 -7
- Partials 1157 1167 +10 ☔ View full report in Codecov by Harness. |
|
✅ No screenshot differences — all stories are identical. View the diff page at: https://pr-4038--sf-screenshot-diffs.netlify.app |
9dea622 to
16427b3
Compare
16427b3 to
0f5a6e2
Compare
0f5a6e2 to
f297de4
Compare
f297de4 to
da9d755
Compare
da9d755 to
660f601
Compare
660f601 to
982e7bd
Compare
RaymondLuong3
left a comment
There was a problem hiding this comment.
Nice work! I like the sync log. I think it may help to diagnose issues now that serval admins and system admins can quickly see the issue in the UI. I understand the decision to hide error details from the user since it would not be helpful most of the time to them. Just a few comments on this.
@RaymondLuong3 reviewed 47 files and all commit messages, and made 3 comments.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on Nateowami).
src/SIL.XForge.Scripture/ClientApp/src/app/sync/sync-log/sync-log.component.spec.ts line 88 at r1 (raw file):
env.clickButton(env.showMoreButton); env.wait(); verify(mockedProjectService.onlineSyncMetrics(env.mockProjectId, 0, 15)).once();
Nit: Can this number of calculated rather than a magic number?
Code quote:
env.wait();
verify(mockedProjectService.onlineSyncMetrics(env.mockProjectId, 0, 15)).once();src/SIL.XForge.Scripture/Services/SFProjectService.cs line 1116 at r1 (raw file):
/// <exception cref="FormatException">The page index or page size is invalid.</exception> private static void ValidatePagingParameters(int pageIndex, int pageSize)
C# methods should start with an uppercase.
Code quote:
private static void ValidatePagingParameters(int pageIndex, int pageSize)982e7bd to
82cff89
Compare
Nateowami
left a comment
There was a problem hiding this comment.
The main reason not to show the raw exception is that there are usually security implications to providing stack traces and raw error messages. But yes, it's also not very useful for users.
@Nateowami made 3 comments and resolved 1 discussion.
Reviewable status: 46 of 47 files reviewed, 1 unresolved discussion (waiting on RaymondLuong3).
src/SIL.XForge.Scripture/ClientApp/src/app/sync/sync-log/sync-log.component.spec.ts line 88 at r1 (raw file):
Previously, RaymondLuong3 (Raymond Luong) wrote…
Nit: Can this number of calculated rather than a magic number?
Done.
src/SIL.XForge.Scripture/Services/SFProjectService.cs line 1116 at r1 (raw file):
Previously, RaymondLuong3 (Raymond Luong) wrote…
C# methods should start with an uppercase.
It's uppercase. Or else I don't know what you're referring to.
RaymondLuong3
left a comment
There was a problem hiding this comment.
@RaymondLuong3 reviewed 1 file and all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on Nateowami).
src/SIL.XForge.Scripture/Services/SFProjectService.cs line 1116 at r1 (raw file):
Previously, Nateowami wrote…
It's uppercase. Or else I don't know what you're referring to.
Aha, my eyes are just deceiving me.
This change is