#108 Refactor CrystaTask to service interface; add test fakes #109

afshinalizadeh · 2025-12-12T15:35:39Z

Replaced ICrystaTaskRepository with ICrystaTaskService across the codebase, updating all usages and DI registrations. Introduced CrystaTaskServiceFake for in-memory testing and updated AzureBoardSyncServiceTests to use fakes. Adjusted CrystaProgramSyncModuleServiceFake to accept configuration. Added src.sln for Visual Studio project structure.

Copilot

Pull request overview

This PR refactors the CrystaTask data access layer from a repository pattern to a service interface pattern. The refactoring renames ICrystaTaskRepository to ICrystaTaskService and introduces a fake implementation (CrystaTaskServiceFake) for in-memory testing. Additionally, the CrystaProgramSyncModuleServiceFake is updated to use constructor-based dependency injection instead of the [AutoInject] attribute.

Key changes:

Renamed ICrystaTaskRepository to ICrystaTaskService across all implementations and usages
Introduced CrystaTaskServiceFake with in-memory storage for testing scenarios
Updated all test fixtures to use the new fake implementations

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
src/Core/CrystaLearn.Core/Services/Sync/AzureBoardSyncService.cs	Updated all references from `ICrystaTaskRepository` to `ICrystaTaskService`
src/Core/CrystaLearn.Core/Services/CrystaTaskServiceFake.cs	New fake implementation providing in-memory storage for testing
src/Core/CrystaLearn.Core/Services/CrystaTaskService.cs	Renamed class from `CrystaTaskRepository` to `CrystaTaskService`
src/Core/CrystaLearn.Core/Services/CrystaProgramSyncModuleServiceFake.cs	Replaced `[AutoInject]` with constructor injection and fixed formatting
src/Core/CrystaLearn.Core/Services/Contracts/ICrystaTaskService.cs	Renamed interface from `ICrystaTaskRepository` to `ICrystaTaskService`
src/Core/CrystaLearn.Core/Extensions/ApplicationBuilderExtensions.cs	Updated DI registration for the renamed service
src/Core/CrystaLearn.Core.Test/Sync/AzureBoardSyncServiceTests.cs	Updated tests to use fake services and removed commented-out notes

Copilot · 2025-12-12T15:42:58Z

src/Core/CrystaLearn.Core/Services/CrystaTaskServiceFake.cs

+    {
+        if (ids == null || ids.Count == 0) return Task.FromResult(new List<SyncItem>());
+        var found = _comments
+            .Where(c => (c.Revision != null && ids.Contains(c.Revision)) || (c.ProviderTaskId != null && ids.Contains(c.ProviderTaskId)) || (c.Id != Guid.Empty && ids.Contains(c.Id.ToString())))


The query logic does not match the real implementation. The real implementation in CrystaTaskService.cs (line 41) filters by SyncInfo.SyncId, but this fake implementation filters by Revision, ProviderTaskId, or Id. The input 'ids' parameter contains SyncId values from SyncInfo, not these other properties. This means the fake will fail to find matching comments when they should exist, causing incorrect sync behavior in tests.

Suggested change

.Where(c => (c.Revision != null && ids.Contains(c.Revision)) || (c.ProviderTaskId != null && ids.Contains(c.ProviderTaskId)) || (c.Id != Guid.Empty && ids.Contains(c.Id.ToString())))

.Where(c => c.SyncId != null && ids.Contains(c.SyncId))

Copilot · 2025-12-12T15:42:58Z

src/Core/CrystaLearn.Core/Services/CrystaTaskServiceFake.cs

+        var found = _revisions
+            .Where(r => (r.Revision != null && ids.Contains(r.Revision)) || (r.ProviderTaskId != null && ids.Contains(r.ProviderTaskId)) || (r.Id != Guid.Empty && ids.Contains(r.Id.ToString())))
+            .Select(r => new SyncItem { Id = r.Id })


The query logic does not match the real implementation. The real implementation in CrystaTaskService.cs (line 81) filters by WorkItemSyncInfo.SyncId, but this fake implementation filters by Revision, ProviderTaskId, or Id. The input 'ids' parameter contains SyncId values from WorkItemSyncInfo, not these other properties. This means the fake will fail to find matching revisions when they should exist, causing incorrect sync behavior in tests.

Copilot · 2025-12-12T15:42:58Z

src/Core/CrystaLearn.Core/Services/CrystaProgramSyncModuleServiceFake.cs

    private static List<CrystaProgramSyncModule> _modules = new();

-    [AutoInject] private IConfiguration Configuration { get; set; } = default!;
+     private IConfiguration Configuration { get; set; } = default!;


There's an extra leading space before the 'private' keyword. This should be removed to maintain consistent indentation throughout the codebase.

Suggested change

private IConfiguration Configuration { get; set; } = default!;

private IConfiguration Configuration { get; set; } = default!;

Copilot · 2025-12-12T15:42:59Z

src/Core/CrystaLearn.Core/Services/CrystaTaskServiceFake.cs

+        if (ids == null || ids.Count == 0) return Task.FromResult(new List<SyncItem>());
+        var found = _revisions
+            .Where(r => (r.Revision != null && ids.Contains(r.Revision)) || (r.ProviderTaskId != null && ids.Contains(r.ProviderTaskId)) || (r.Id != Guid.Empty && ids.Contains(r.Id.ToString())))
+            .Select(r => new SyncItem { Id = r.Id })


The returned SyncItem objects are missing the SyncInfo property. Looking at the real implementation in CrystaTaskService.cs, GetRevisionsSyncItemsAsync should populate SyncInfo from the revision's WorkItemSyncInfo property. The current implementation only sets the Id field, but the calling code in AzureBoardSyncService expects SyncInfo.SyncId and SyncInfo.ContentHash to be populated for sync comparison logic (lines 512, 519 in AzureBoardSyncService.cs). This will cause sync operations to fail or behave incorrectly when using the fake.

Suggested change

.Select(r => new SyncItem { Id = r.Id })

.Select(r => new SyncItem { Id = r.Id, SyncInfo = r.WorkItemSyncInfo })

Copilot · 2025-12-12T15:42:59Z

src/Core/CrystaLearn.Core/Services/CrystaTaskServiceFake.cs

+        if (ids == null || ids.Count == 0) return Task.FromResult(new List<SyncItem>());
+        var found = _updates
+            .Where(u => (u.Revision != null && ids.Contains(u.Revision)) || (u.ProviderTaskId != null && ids.Contains(u.ProviderTaskId)) || (u.Id != Guid.Empty && ids.Contains(u.Id.ToString())))
+            .Select(u => new SyncItem { Id = u.Id })


The returned SyncItem objects are missing the SyncInfo property. Looking at the real implementation in CrystaTaskService.cs, GetUpdatesSyncItemsAsync should populate SyncInfo from the update's SyncInfo property. The current implementation only sets the Id field, but the calling code in AzureBoardSyncService expects SyncInfo.SyncId and SyncInfo.ContentHash to be populated for sync comparison logic (lines 263, 270 in AzureBoardSyncService.cs). This will cause sync operations to fail or behave incorrectly when using the fake.

Suggested change

.Select(u => new SyncItem { Id = u.Id })

.Select(u => new SyncItem { Id = u.Id, SyncInfo = u.SyncInfo })

Copilot · 2025-12-12T15:42:59Z

src/Core/CrystaLearn.Core/Services/CrystaTaskServiceFake.cs

+        var found = _tasks
+            .Where(t => (t.ProviderTaskId != null && ids.Contains(t.ProviderTaskId)) || (t.Id != Guid.Empty && ids.Contains(t.Id.ToString())))
+            .Select(t => new SyncItem { Id = t.Id })


The query logic does not match the real implementation. The real implementation in CrystaTaskService.cs (line 22) filters by WorkItemSyncInfo.SyncId, but this fake implementation filters by ProviderTaskId or Id. The input 'ids' parameter contains SyncId values (formatted as SyncInfo.SyncId), not ProviderTaskId values. This means the fake will fail to find matching tasks when they should exist, causing incorrect sync behavior in tests.

Copilot · 2025-12-12T15:43:00Z

src/Core/CrystaLearn.Core/Services/CrystaTaskServiceFake.cs

+        var found = _updates
+            .Where(u => (u.Revision != null && ids.Contains(u.Revision)) || (u.ProviderTaskId != null && ids.Contains(u.ProviderTaskId)) || (u.Id != Guid.Empty && ids.Contains(u.Id.ToString())))
+            .Select(u => new SyncItem { Id = u.Id })


The query logic does not match the real implementation. The real implementation in CrystaTaskService.cs (line 61) filters by SyncInfo.SyncId, but this fake implementation filters by Revision, ProviderTaskId, or Id. The input 'ids' parameter contains SyncId values from SyncInfo, not these other properties. This means the fake will fail to find matching updates when they should exist, causing incorrect sync behavior in tests.

Copilot · 2025-12-12T15:43:00Z

src/Core/CrystaLearn.Core/Services/CrystaTaskServiceFake.cs

+        if (ids == null || ids.Count == 0) return Task.FromResult(new List<SyncItem>());
+        var found = _comments
+            .Where(c => (c.Revision != null && ids.Contains(c.Revision)) || (c.ProviderTaskId != null && ids.Contains(c.ProviderTaskId)) || (c.Id != Guid.Empty && ids.Contains(c.Id.ToString())))
+            .Select(c => new SyncItem { Id = c.Id })


The returned SyncItem objects are missing the SyncInfo property. Looking at the real implementation in CrystaTaskService.cs, GetCommentsSyncItemsAsync should populate SyncInfo from the comment's SyncInfo property. The current implementation only sets the Id field, but the calling code in AzureBoardSyncService expects SyncInfo.SyncId and SyncInfo.ContentHash to be populated for sync comparison logic (lines 263, 270 in AzureBoardSyncService.cs). This will cause sync operations to fail or behave incorrectly when using the fake.

Suggested change

.Select(c => new SyncItem { Id = c.Id })

.Select(c => new SyncItem { Id = c.Id, SyncInfo = c.SyncInfo })

Copilot · 2025-12-12T15:43:00Z

src/Core/CrystaLearn.Core/Services/CrystaTaskServiceFake.cs

+        if (ids == null || ids.Count == 0) return Task.FromResult(new List<SyncItem>());
+        var found = _tasks
+            .Where(t => (t.ProviderTaskId != null && ids.Contains(t.ProviderTaskId)) || (t.Id != Guid.Empty && ids.Contains(t.Id.ToString())))
+            .Select(t => new SyncItem { Id = t.Id })


The returned SyncItem objects are missing the SyncInfo property. Looking at the real implementation in CrystaTaskService.cs (line 26), GetWorkItemSyncItemsAsync should populate SyncInfo from the task's WorkItemSyncInfo property. The current implementation only sets the Id field, but the calling code in AzureBoardSyncService expects SyncInfo.SyncId and SyncInfo.ContentHash to be populated for sync comparison logic (lines 565, 572 in AzureBoardSyncService.cs). This will cause sync operations to fail or behave incorrectly when using the fake.

Suggested change

.Select(t => new SyncItem { Id = t.Id })

.Select(t => new SyncItem

{

Id = t.Id,

SyncInfo = t.WorkItemSyncInfo == null

? null

: new SyncInfo

{

SyncId = t.WorkItemSyncInfo.SyncId,

ContentHash = t.WorkItemSyncInfo.ContentHash

}

})

Copilot · 2025-12-12T15:43:01Z

src/Core/CrystaLearn.Core/Services/CrystaTaskServiceFake.cs

+        if (!string.IsNullOrEmpty(project))
+        {
+            ids = _tasks
+                .Where(t => t.CrystaProgram != null && string.Equals(t.CrystaProgram.ToString(), project, StringComparison.OrdinalIgnoreCase) && !string.IsNullOrEmpty(t.ProviderTaskId))


Default 'ToString()': CrystaProgram inherits 'ToString()' from 'Object', and so is not suitable for printing.

Suggested change

.Where(t => t.CrystaProgram != null && string.Equals(t.CrystaProgram.ToString(), project, StringComparison.OrdinalIgnoreCase) && !string.IsNullOrEmpty(t.ProviderTaskId))

.Where(t => t.CrystaProgram != null && string.Equals(t.CrystaProgram.Name, project, StringComparison.OrdinalIgnoreCase) && !string.IsNullOrEmpty(t.ProviderTaskId))

afshinalizadeh · 2025-12-12T15:58:09Z

@copilot open a new pull request to apply changes based on the comments in this thread

Copilot · 2025-12-12T15:58:19Z

@afshinalizadeh I've opened a new pull request, #110, to work on those changes. Once the pull request is ready, I'll request review from you.

…e review Co-authored-by: afshinalizadeh <4254006+afshinalizadeh@users.noreply.github.com>

Co-authored-by: afshinalizadeh <4254006+afshinalizadeh@users.noreply.github.com>

[WIP] Refactor CrystaTask to service interface and add test fakes

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 10 comments.

src/Core/CrystaLearn.Core/Services/CrystaTaskServiceFake.cs

src/Core/CrystaLearn.Core/Services/Sync/AzureBoardSyncService.cs

src/Core/CrystaLearn.Core/Services/CrystaTaskServiceFake.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

afshinalizadeh requested a review from Copilot December 12, 2025 15:35

Copilot started reviewing on behalf of afshinalizadeh December 12, 2025 15:36 View session

Copilot AI reviewed Dec 12, 2025

View reviewed changes

Initial plan

58534af

Copilot AI mentioned this pull request Dec 12, 2025

[WIP] Refactor CrystaTask to service interface and add test fakes #110

Merged

7 tasks

Copilot AI and others added 3 commits December 12, 2025 16:04

Fix CrystaTaskServiceFake query logic and SyncInfo population per cod…

83d8c8e

…e review Co-authored-by: afshinalizadeh <4254006+afshinalizadeh@users.noreply.github.com>

Simplify GetWorkItemSyncItemsAsync to directly assign SyncInfo

bc75731

Co-authored-by: afshinalizadeh <4254006+afshinalizadeh@users.noreply.github.com>

Merge pull request #110 from cs-internship/copilot/sub-pr-109

f07864a

[WIP] Refactor CrystaTask to service interface and add test fakes

afshinalizadeh requested a review from Copilot December 12, 2025 16:09

Copilot started reviewing on behalf of afshinalizadeh December 12, 2025 16:09 View session

Copilot AI reviewed Dec 12, 2025

View reviewed changes

afshinalizadeh and others added 10 commits December 12, 2025 20:01

Update src/Core/CrystaLearn.Core/Services/CrystaTaskServiceFake.cs

e74f73c

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update src/Core/CrystaLearn.Core/Services/CrystaTaskServiceFake.cs

349823d

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update src/Core/CrystaLearn.Core/Services/CrystaTaskServiceFake.cs

cf37687

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update src/Core/CrystaLearn.Core/Services/CrystaTaskServiceFake.cs

2d84f02

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update src/Core/CrystaLearn.Core/Services/CrystaTaskServiceFake.cs

9d9ef44

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update src/Core/CrystaLearn.Core/Services/CrystaTaskServiceFake.cs

d9ee54d

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update src/Core/CrystaLearn.Core/Services/CrystaTaskServiceFake.cs

1ea5ea7

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update src/Core/CrystaLearn.Core/Services/CrystaTaskServiceFake.cs

97be68d

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update src/Core/CrystaLearn.Core/Services/CrystaTaskServiceFake.cs

27dccd0

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Bugfix. Correct AI Generated Code

74438c9

afshinalizadeh merged commit 2844953 into develop Dec 12, 2025
1 check passed

	.Where(c => (c.Revision != null && ids.Contains(c.Revision)) \|\| (c.ProviderTaskId != null && ids.Contains(c.ProviderTaskId)) \|\| (c.Id != Guid.Empty && ids.Contains(c.Id.ToString())))
	.Where(c => c.SyncId != null && ids.Contains(c.SyncId))

	private IConfiguration Configuration { get; set; } = default!;
	private IConfiguration Configuration { get; set; } = default!;

	.Select(r => new SyncItem { Id = r.Id })
	.Select(r => new SyncItem { Id = r.Id, SyncInfo = r.WorkItemSyncInfo })

	.Select(u => new SyncItem { Id = u.Id })
	.Select(u => new SyncItem { Id = u.Id, SyncInfo = u.SyncInfo })

	.Select(c => new SyncItem { Id = c.Id })
	.Select(c => new SyncItem { Id = c.Id, SyncInfo = c.SyncInfo })

-            .Select(t => new SyncItem { Id = t.Id })
+            .Select(t => new SyncItem
+            {
+                Id = t.Id,
+                SyncInfo = t.WorkItemSyncInfo == null
+                    ? null
+                    : new SyncInfo
+                    {
+                        SyncId = t.WorkItemSyncInfo.SyncId,
+                        ContentHash = t.WorkItemSyncInfo.ContentHash
+                    }
+            })

	.Where(t => t.CrystaProgram != null && string.Equals(t.CrystaProgram.ToString(), project, StringComparison.OrdinalIgnoreCase) && !string.IsNullOrEmpty(t.ProviderTaskId))
	.Where(t => t.CrystaProgram != null && string.Equals(t.CrystaProgram.Name, project, StringComparison.OrdinalIgnoreCase) && !string.IsNullOrEmpty(t.ProviderTaskId))

#108 Refactor CrystaTask to service interface; add test fakes #109

#108 Refactor CrystaTask to service interface; add test fakes #109

Uh oh!

Conversation

afshinalizadeh commented Dec 12, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

afshinalizadeh commented Dec 12, 2025

Uh oh!

Copilot AI commented Dec 12, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants