diff --git a/VueApp/src/CTS/pages/ManageMilestones.vue b/VueApp/src/CTS/pages/ManageMilestones.vue index 3f72c44fd..fda3feba8 100644 --- a/VueApp/src/CTS/pages/ManageMilestones.vue +++ b/VueApp/src/CTS/pages/ManageMilestones.vue @@ -22,8 +22,9 @@ async function load() { ]) //filter competencies to just those without a milestone already defined + const milestoneCompetencyIds = new Set(milestones.value.map((m) => m.competencyId)) competencies.value = competencies.value.filter( - (c) => !milestones.value.some((m) => m.competencyId === c.competencyId), + (c) => c.competencyId === null || !milestoneCompetencyIds.has(c.competencyId), ) loaded.value = true diff --git a/VueApp/src/Effort/pages/ScheduledCliWeeks.vue b/VueApp/src/Effort/pages/ScheduledCliWeeks.vue index c733b31c9..42506558c 100644 --- a/VueApp/src/Effort/pages/ScheduledCliWeeks.vue +++ b/VueApp/src/Effort/pages/ScheduledCliWeeks.vue @@ -133,8 +133,9 @@ const tableRows = computed(() => { if (!report.value) return [] return report.value.instructors.map((inst) => { const row: CliWeeksRow = { mothraId: inst.mothraId, instructor: inst.instructor } + const termsByName = new Map(inst.terms.map((t) => [t.termName, t])) for (const termName of report.value!.termNames) { - const term = inst.terms.find((t) => t.termName === termName) + const term = termsByName.get(termName) if (term) { row[termName] = Object.entries(term.weeksByService) .filter(([, weeks]) => weeks > 0) diff --git a/VueApp/src/Students/pages/StudentClassYear.vue b/VueApp/src/Students/pages/StudentClassYear.vue index 4b6582db6..0465fff0b 100644 --- a/VueApp/src/Students/pages/StudentClassYear.vue +++ b/VueApp/src/Students/pages/StudentClassYear.vue @@ -72,8 +72,9 @@ async function getStudents() { //get problems u = apiUrl + "students/dvm/classYearReport?classYear=" + classYear.value.value const problems = await get(u) + const studentsByPersonId = new Map(allStudents.value.map((std) => [std.personId, std])) problems.result.forEach((p: any) => { - var s = allStudents.value.find((std) => std.personId === p.personId) + const s = studentsByPersonId.get(p.personId) if (s !== undefined) { s.problems = p.problems } diff --git a/test/CMS/CMSLinkCollectionLinksTest.cs b/test/CMS/CMSLinkCollectionLinksTest.cs new file mode 100644 index 000000000..8674a3260 --- /dev/null +++ b/test/CMS/CMSLinkCollectionLinksTest.cs @@ -0,0 +1,162 @@ +using Areas.CMS.Models; +using Areas.CMS.Models.DTOs; +using Microsoft.AspNetCore.Mvc; +using MockQueryable.NSubstitute; +using NSubstitute; +using Viper.Areas.CMS.Controllers; +using Viper.Classes.SQLContext; + +namespace Viper.test.CMS +{ + /// + /// Covers the grouped-links ordering in GetLinks: links are grouped by + /// tag value, de-duplicated, and links whose only tag value in the category is + /// null are appended (not dropped) after the grouped links. + /// + public class CMSLinkCollectionLinksTest + { + private const int CollectionId = 1; + private const int AudienceCategoryId = 10; + private const int OtherCategoryId = 99; + + private readonly VIPERContext _mockContext; + private readonly CMSLinkCollectionLinks _controller; + + private static readonly LinkCollectionTagCategory _audience = new() + { + LinkCollectionTagCategoryId = AudienceCategoryId, + LinkCollectionId = CollectionId, + LinkCollectionTagCategoryName = "Audience", + SortOrder = 1, + }; + private static readonly LinkCollectionTagCategory _other = new() + { + LinkCollectionTagCategoryId = OtherCategoryId, + LinkCollectionId = CollectionId, + LinkCollectionTagCategoryName = "Other", + SortOrder = 2, + }; + + public CMSLinkCollectionLinksTest() + { + _mockContext = Substitute.For(); + _controller = new CMSLinkCollectionLinks(_mockContext); + } + + private static Link MakeLink(int id, int sortOrder, LinkCollectionTagCategory category, string? tagValue) + { + var link = new Link + { + LinkId = id, + LinkCollectionId = CollectionId, + Url = $"/link/{id}", + Title = $"Link {id}", + SortOrder = sortOrder, + }; + link.LinkTags.Add(new LinkTag + { + LinkTagId = id * 10, + LinkId = id, + LinkCollectionTagCategoryId = category.LinkCollectionTagCategoryId, + LinkCollectionTagCategory = category, + SortOrder = 1, + Value = tagValue, + Link = link, + }); + return link; + } + + private void Setup(List links) + { + // BuildMockDbSet() makes its own NSubstitute calls, so materialize every + // mock set before any .Returns() or NSubstitute loses track of the last call. + var collections = new List { new() { LinkCollectionId = CollectionId } }.BuildMockDbSet(); + var categories = new List { _audience, _other }.BuildMockDbSet(); + var linkSet = links.BuildMockDbSet(); + + _mockContext.LinkCollections.Returns(collections); + _mockContext.LinkCollectionTagCategories.Returns(categories); + _mockContext.Links.Returns(linkSet); + } + + private static Link WithTag(Link link, LinkCollectionTagCategory category, string? value) + { + link.LinkTags.Add(new LinkTag + { + LinkTagId = link.LinkId * 10 + link.LinkTags.Count, + LinkId = link.LinkId, + LinkCollectionTagCategoryId = category.LinkCollectionTagCategoryId, + LinkCollectionTagCategory = category, + SortOrder = link.LinkTags.Count + 1, + Value = value, + Link = link, + }); + return link; + } + + private static List OkLinks(ActionResult> result) + { + var ok = Assert.IsType(result.Result); + return Assert.IsAssignableFrom>(ok.Value).ToList(); + } + + [Fact] + public async Task GetLinks_GroupedByCategory_OrdersByTagValueThenAppendsNullValuedLinks() + { + Setup(new List + { + MakeLink(1, 1, _audience, "Students"), + MakeLink(2, 2, _audience, "Faculty"), + MakeLink(3, 3, _audience, "Students"), // shares a value with link 1 + MakeLink(4, 4, _audience, null), // uncategorized -> appended last + MakeLink(5, 5, _other, "Other"), // different category -> excluded + }); + + var dtos = OkLinks(await _controller.GetLinks(CollectionId, "Audience")); + + // Students group (1, 3 in sort order), then Faculty (2), then null-valued (4). + // Link 5 (other category) is not part of this grouping. + Assert.Equal(new[] { 1, 3, 2, 4 }, dtos.Select(d => d.LinkId).ToArray()); + } + + [Fact] + public async Task GetLinks_GroupedByCategory_LinkWithNullAndNonNullValue_AppearsOnceInGroup() + { + Setup(new List + { + WithTag(MakeLink(1, 1, _audience, "Students"), _audience, null), // both a value and a null tag + MakeLink(2, 2, _audience, null), // only null -> uncategorized + }); + + var dtos = OkLinks(await _controller.GetLinks(CollectionId, "Audience")); + + // Link 1 is grouped under "Students" and not re-appended as uncategorized. + Assert.Equal(new[] { 1, 2 }, dtos.Select(d => d.LinkId).ToArray()); + } + + [Fact] + public async Task GetLinks_NoGrouping_ReturnsAllLinksInSortOrder() + { + Setup(new List + { + MakeLink(1, 1, _audience, "Students"), + MakeLink(2, 2, _audience, "Faculty"), + MakeLink(5, 5, _other, "Other"), + }); + + var dtos = OkLinks(await _controller.GetLinks(CollectionId)); + + Assert.Equal(new[] { 1, 2, 5 }, dtos.Select(d => d.LinkId).ToArray()); + } + + [Fact] + public async Task GetLinks_UnknownCollection_ReturnsNotFound() + { + Setup(new List { MakeLink(1, 1, _audience, "Students") }); + + var result = await _controller.GetLinks(999); + + Assert.IsType(result.Result); + } + } +} diff --git a/test/CTS/CompetencyControllerTest.cs b/test/CTS/CompetencyControllerTest.cs new file mode 100644 index 000000000..d7888590b --- /dev/null +++ b/test/CTS/CompetencyControllerTest.cs @@ -0,0 +1,79 @@ +using MockQueryable.NSubstitute; +using NSubstitute; +using Viper.Areas.CTS.Controllers; +using Viper.Classes.SQLContext; +using Viper.Models.CTS; + +namespace Viper.test.CTS +{ + public class CompetencyControllerTest + { + private readonly VIPERContext _mockContext; + private readonly CompetencyController _controller; + private static readonly Domain _domain = new() { DomainId = 1, Name = "Domain", Order = 1 }; + + public CompetencyControllerTest() + { + _mockContext = Substitute.For(); + _controller = new CompetencyController(_mockContext); + } + + private void SetupCompetencies(params Competency[] comps) + { + // BuildMockDbSet() makes its own NSubstitute calls, so materialize it + // before .Returns() or NSubstitute loses track of the last call. + var mockDbSet = comps.ToList().BuildMockDbSet(); + _mockContext.Competencies.Returns(mockDbSet); + } + + private static Competency Comp(int id, string number, int? parentId, int order) + => new() + { + CompetencyId = id, + Number = number, + Name = $"Competency {number}", + DomainId = _domain.DomainId, + Domain = _domain, + ParentId = parentId, + Order = order, + }; + + [Fact] + public async Task GetCompetencyHierarchy_NestsChildrenUnderParents() + { + SetupCompetencies( + Comp(1, "1", null, 1), // root + Comp(2, "1.1", 1, 2), // child of 1 + Comp(3, "1.1.1", 2, 3), // grandchild (child of 2) + Comp(4, "2", null, 4)); // second root + + var result = await _controller.GetCompetencyHierarchy(); + + var roots = result.Value!; + Assert.Equal(new[] { 1, 4 }, roots.Select(r => r.CompetencyId).ToArray()); + + var root1 = roots.Single(c => c.CompetencyId == 1); + var child = Assert.Single(root1.Children); + Assert.Equal(2, child.CompetencyId); + var grandchild = Assert.Single(child.Children); + Assert.Equal(3, grandchild.CompetencyId); + + Assert.Empty(roots.Single(c => c.CompetencyId == 4).Children); + } + + [Fact] + public async Task GetCompetencyHierarchy_OrphanWithMissingParent_IsDroppedNotDuplicated() + { + SetupCompetencies( + Comp(1, "1", null, 1), // root + Comp(2, "9.9", 999, 2)); // parent 999 is not in the set + + var result = await _controller.GetCompetencyHierarchy(); + + var roots = result.Value!; + var root = Assert.Single(roots); + Assert.Equal(1, root.CompetencyId); + Assert.Empty(root.Children); + } + } +} diff --git a/web/Areas/CMS/Controllers/CMSLinkCollectionLinks.cs b/web/Areas/CMS/Controllers/CMSLinkCollectionLinks.cs index dde148b4c..f4ec728e9 100644 --- a/web/Areas/CMS/Controllers/CMSLinkCollectionLinks.cs +++ b/web/Areas/CMS/Controllers/CMSLinkCollectionLinks.cs @@ -21,12 +21,13 @@ public CMSLinkCollectionLinks(VIPERContext context) [HttpGet("{collectionId}/links")] public async Task>> GetLinks(int collectionId, string? groupByTagCategory = null) { - if (await _context.LinkCollections.FirstOrDefaultAsync(lc => lc.LinkCollectionId == collectionId) == null) + if (await _context.LinkCollections.AsNoTracking().FirstOrDefaultAsync(lc => lc.LinkCollectionId == collectionId) == null) { return NotFound(); } var links = await _context.Links + .AsNoTracking() .Include(l => l.LinkTags.OrderBy(lt => lt.LinkCollectionTagCategory.SortOrder).ThenBy(lt => lt.SortOrder).ThenBy(lt => lt.Value)) .ThenInclude(lt => lt.LinkCollectionTagCategory) .Where(l => l.LinkCollectionId == collectionId) @@ -50,11 +51,12 @@ public async Task>> GetLinks(int collectionId, Value = lt.Value, CategoryName = lt.LinkCollectionTagCategory.LinkCollectionTagCategoryName }).OrderBy(lt => lt.Value).ToList() - }); + }).ToList(); if (!string.IsNullOrEmpty(groupByTagCategory)) { var tagCategories = await _context.LinkCollectionTagCategories + .AsNoTracking() .Where(tc => tc.LinkCollectionId == collectionId) .Where(tc => tc.LinkCollectionTagCategoryName.ToLower() == groupByTagCategory.ToLower()) .FirstOrDefaultAsync(); @@ -63,21 +65,30 @@ public async Task>> GetLinks(int collectionId, return BadRequest("Invalid tag category to group by."); } - var allValues = await _context.LinkTags - .Where(lt => lt.LinkCollectionTagCategoryId == tagCategories.LinkCollectionTagCategoryId) - .Select(lt => lt.Value) - .Distinct() - .ToListAsync(); - - var orderedGroupedResult = new List(); - var added = new HashSet(); - foreach (var r in allValues - .SelectMany(v => result - .Where(r => r.LinkTags.Any(lt => lt.Value == v && lt.LinkCollectionTagCategoryId == tagCategories.LinkCollectionTagCategoryId) && !added.Contains(r.LinkId)))) - { - added.Add(r.LinkId); - orderedGroupedResult.Add(r); - } + var categoryId = tagCategories.LinkCollectionTagCategoryId; + + // Group links by tag value, taking the value order from the already-ordered + // in-memory result. LINQ GroupBy keeps first-occurrence key order, so the response + // is deterministic (unlike a SELECT DISTINCT with no ORDER BY) and avoids an extra + // round-trip. A link with several values lands in each group, so DistinctBy below + // keeps it at its first position. + var groupedLinks = result + .SelectMany(r => r.LinkTags + .Where(lt => lt.LinkCollectionTagCategoryId == categoryId && lt.Value != null) + .Select(lt => new { Value = lt.Value!, Link = r })) + .GroupBy(x => x.Value) + .SelectMany(g => g.Select(x => x.Link)); + + // A link with a tag in this category but no non-null value counts as uncategorized, + // so append those links to keep the value grouping from dropping them. + var uncategorizedLinks = result.Where(r => + r.LinkTags.Any(lt => lt.LinkCollectionTagCategoryId == categoryId && lt.Value == null) + && !r.LinkTags.Any(lt => lt.LinkCollectionTagCategoryId == categoryId && lt.Value != null)); + + var orderedGroupedResult = groupedLinks + .Concat(uncategorizedLinks) + .DistinctBy(r => r.LinkId) + .ToList(); return Ok(orderedGroupedResult); } diff --git a/web/Areas/CTS/Controllers/CompetencyController.cs b/web/Areas/CTS/Controllers/CompetencyController.cs index 027b5716d..b0561b7d3 100644 --- a/web/Areas/CTS/Controllers/CompetencyController.cs +++ b/web/Areas/CTS/Controllers/CompetencyController.cs @@ -62,25 +62,23 @@ public async Task>> GetChildren(int competencyI public async Task>> GetCompetencyHierarchy() { var comps = await context.Competencies + .AsNoTracking() .Include(c => c.Domain) .OrderBy(c => c.Domain.Order) .ThenBy(c => c.Order) .ToListAsync(); var compHierarchy = new List(); var allCompDtos = CtsMapper.ToCompetencyHierarchyDtos(comps); + var compsById = allCompDtos.ToDictionary(c => c.CompetencyId); foreach (var comp in allCompDtos) { if (comp.ParentId == null) { compHierarchy.Add(comp); } - else + else if (compsById.TryGetValue(comp.ParentId.Value, out var parent)) { - var parent = allCompDtos.FirstOrDefault(c => c.CompetencyId == comp.ParentId); - if (parent != null) - { - parent.Children.Add(comp); - } + parent.Children.Add(comp); } } return compHierarchy; diff --git a/web/Classes/SQLContext/VIPERContext.cs b/web/Classes/SQLContext/VIPERContext.cs index a633c70f1..18c437af5 100644 --- a/web/Classes/SQLContext/VIPERContext.cs +++ b/web/Classes/SQLContext/VIPERContext.cs @@ -111,10 +111,10 @@ public VIPERContext(DbContextOptions options) public virtual DbSet WorkflowStageVersions { get; set; } - public DbSet LinkCollections { get; set; } - public DbSet LinkCollectionTagCategories { get; set; } - public DbSet Links { get; set; } - public DbSet LinkTags { get; set; } + public virtual DbSet LinkCollections { get; set; } + public virtual DbSet LinkCollectionTagCategories { get; set; } + public virtual DbSet Links { get; set; } + public virtual DbSet LinkTags { get; set; } /* users */ public virtual DbSet People { get; set; }