Skip to content

Commit a1c9af6

Browse files
authored
Merge pull request #2143 from TechnologyEnhancedLearning/Develop/Fixes/TD-937-TD-2522-ValidateReportsRowLimit
TD-937 TD-2522 validation to prevent report filter returning too many…
2 parents a86d9d7 + 6b128c2 commit a1c9af6

File tree

12 files changed

+146
-26
lines changed

12 files changed

+146
-26
lines changed

DigitalLearningSolutions.Web.Tests/Controllers/TrackingSystem/Centre/ReportsControllerTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ public void EditFilters_post_sets_cookie_value()
7070
var result = reportsController.EditFilters(model);
7171

7272
// Then
73-
A.CallTo(() => httpResponse.Cookies.Append("ReportsFilterCookie", A<string>._, A<CookieOptions>._))
73+
A.CallTo(() => httpResponse.Cookies.Append("CourseUsageReportFilterCookie", A<string>._, A<CookieOptions>._))
7474
.MustHaveHappened();
7575
result.Should().BeRedirectToActionResult().WithActionName("Index");
7676
}

DigitalLearningSolutions.Web.Tests/ViewModels/TrackingSystem/Centre/Reports/EditFilterViewModelTests.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ public void Missing_end_date_doesnt_trigger_validation_when_EndDate_bool_is_fals
5656
StartMonth = 1,
5757
StartYear = 2021,
5858
EndDate = false,
59+
ReportInterval = Data.Enums.ReportInterval.Months,
5960
};
6061

6162
// When
@@ -75,6 +76,7 @@ public void Missing_end_date_triggers_validation_when_EndDate_bool_is_true()
7576
StartMonth = 1,
7677
StartYear = 2021,
7778
EndDate = true,
79+
ReportInterval = Data.Enums.ReportInterval.Months,
7880
};
7981
const string expectedErrorMessage = "Enter an End Date";
8082

@@ -146,6 +148,7 @@ int endYear
146148
EndMonth = endMonth,
147149
EndYear = endYear,
148150
EndDate = false,
151+
ReportInterval = Data.Enums.ReportInterval.Months,
149152
};
150153

151154
// When

DigitalLearningSolutions.Web/Controllers/SuperAdmin/PlatformReports/IndependentSelfAssessmentReport.cs

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,13 @@ public partial class PlatformReportsController
1010
[Route("SelfAssessments/Independent")]
1111
public IActionResult IndependentSelfAssessmentsReport()
1212
{
13-
var filterData = Request.Cookies.RetrieveFilterDataFromCookie("SuperAdminDSATReportsFilterCookie", null);
14-
Response.Cookies.SetReportsFilterCookie("SuperAdminDSATReportsFilterCookie", filterData, clockUtility.UtcNow);
13+
//Removing an old cookie if it exists because it may contain problematic options (filters that return too many rows):
14+
if (HttpContext.Request.Cookies.ContainsKey("SuperAdminDSATReportsFilterCookie"))
15+
{
16+
HttpContext.Response.Cookies.Delete("SuperAdminDSATReportsFilterCookie");
17+
}
18+
var filterData = Request.Cookies.RetrieveFilterDataFromCookie("SuperAdminIndependentSAReportsFilterCookie", null);
19+
Response.Cookies.SetReportsFilterCookie("SuperAdminIndependentSAReportsFilterCookie", filterData, clockUtility.UtcNow);
1520
var activity = platformReportsService.GetSelfAssessmentActivity(filterData, false);
1621
var (regionName, centreTypeName, centreName, jobGroupName, brandName, categoryName, selfAssessmentName) = reportFilterService.GetSelfAssessmentFilterNames(filterData);
1722
var selfAssessmentReportFilterModel = new SelfAssessmentReportFilterModel(
@@ -43,7 +48,7 @@ public IActionResult IndependentSelfAssessmentsReport()
4348
[Route("SelfAssessments/Independent/EditFilters")]
4449
public IActionResult IndependentSelfAssessmentsEditFilters()
4550
{
46-
var filterData = Request.Cookies.RetrieveFilterDataFromCookie("SuperAdminDSATReportsFilterCookie", null);
51+
var filterData = Request.Cookies.RetrieveFilterDataFromCookie("SuperAdminIndependentSAReportsFilterCookie", null);
4752
var filterOptions = GetDropdownValues(false);
4853
var dataStartDate = platformReportsService.GetSelfAssessmentActivityStartDate(false);
4954
var model = new SelfAssessmentsEditFiltersViewModel(
@@ -84,7 +89,7 @@ public IActionResult IndependentSelfAssessmentsEditFilters(SelfAssessmentsEditFi
8489
model.FilterType,
8590
model.ReportInterval
8691
);
87-
Response.Cookies.SetReportsFilterCookie("SuperAdminDSATReportsFilterCookie", filterData, clockUtility.UtcNow);
92+
Response.Cookies.SetReportsFilterCookie("SuperAdminIndependentSAReportsFilterCookie", filterData, clockUtility.UtcNow);
8893

8994
return RedirectToAction("IndependentSelfAssessmentsReport");
9095
}

DigitalLearningSolutions.Web/Controllers/SuperAdmin/PlatformReports/PlatformReportsController.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ public IActionResult Index()
5454
[Route("SelfAssessments/{selfAssessmentType}/Data")]
5555
public IEnumerable<SelfAssessmentActivityDataRowModel> GetGraphData(string selfAssessmentType)
5656
{
57-
var cookieName = selfAssessmentType == "Independent" ? "SuperAdminDSATReportsFilterCookie" : "SuperAdminReportsFilterCookie";
57+
var cookieName = selfAssessmentType == "Independent" ? "SuperAdminIndependentSAReportsFilterCookie" : "SuperAdminSupervisedSAReportsFilterCookie";
5858
var filterData = Request.Cookies.RetrieveFilterDataFromCookie(cookieName, null);
5959
var activity = platformReportsService.GetSelfAssessmentActivity(filterData!, selfAssessmentType == "Independent" ? false : true);
6060
return activity.Select(

DigitalLearningSolutions.Web/Controllers/SuperAdmin/PlatformReports/SupervisedSelfAssessmentReport.cs

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,21 @@
44
using DigitalLearningSolutions.Web.Helpers;
55
using DigitalLearningSolutions.Web.ViewModels.Common;
66
using DigitalLearningSolutions.Web.ViewModels.SuperAdmin.PlatformReports;
7+
using DocumentFormat.OpenXml.InkML;
78
using Microsoft.AspNetCore.Mvc;
89

910
public partial class PlatformReportsController
1011
{
1112
[Route("SelfAssessments/Supervised")]
1213
public IActionResult SupervisedSelfAssessmentsReport()
1314
{
14-
var filterData = Request.Cookies.RetrieveFilterDataFromCookie("SuperAdminReportsFilterCookie", null);
15-
Response.Cookies.SetReportsFilterCookie("SuperAdminReportsFilterCookie", filterData, clockUtility.UtcNow);
15+
//Removing an old cookie if it exists because it may contain problematic options (filters that return too many rows):
16+
if (HttpContext.Request.Cookies.ContainsKey("SuperAdminReportsFilterCookie"))
17+
{
18+
HttpContext.Response.Cookies.Delete("SuperAdminReportsFilterCookie");
19+
}
20+
var filterData = Request.Cookies.RetrieveFilterDataFromCookie("SuperAdminSupervisedSAReportsFilterCookie", null);
21+
Response.Cookies.SetReportsFilterCookie("SuperAdminSupervisedSAReportsFilterCookie", filterData, clockUtility.UtcNow);
1622
var activity = platformReportsService.GetSelfAssessmentActivity(filterData, true);
1723
var (regionName, centreTypeName, centreName, jobGroupName, brandName, categoryName, selfAssessmentName) = reportFilterService.GetSelfAssessmentFilterNames(filterData);
1824
var selfAssessmentReportFilterModel = new SelfAssessmentReportFilterModel(
@@ -46,7 +52,7 @@ public IActionResult SupervisedSelfAssessmentsReport()
4652
[Route("SelfAssessments/Supervised/EditFilters")]
4753
public IActionResult SupervisedSelfAssessmentsEditFilters()
4854
{
49-
var filterData = Request.Cookies.RetrieveFilterDataFromCookie("SuperAdminReportsFilterCookie", null);
55+
var filterData = Request.Cookies.RetrieveFilterDataFromCookie("SuperAdminSupervisedSAReportsFilterCookie", null);
5056
var filterOptions = GetDropdownValues(true);
5157
var dataStartDate = platformReportsService.GetSelfAssessmentActivityStartDate(true);
5258
var model = new SelfAssessmentsEditFiltersViewModel(
@@ -88,7 +94,7 @@ public IActionResult SupervisedSelfAssessmentsEditFilters(SelfAssessmentsEditFil
8894
model.FilterType,
8995
model.ReportInterval
9096
);
91-
Response.Cookies.SetReportsFilterCookie("SuperAdminReportsFilterCookie", filterData, clockUtility.UtcNow);
97+
Response.Cookies.SetReportsFilterCookie("SuperAdminSupervisedSAReportsFilterCookie", filterData, clockUtility.UtcNow);
9298
return RedirectToAction("SupervisedSelfAssessmentsReport");
9399
}
94100
}

DigitalLearningSolutions.Web/Controllers/TrackingSystem/Centre/Reports/ReportsController.cs

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,14 @@ public IActionResult Index()
4646
{
4747
var centreId = User.GetCentreIdKnownNotNull();
4848
var categoryIdFilter = User.GetAdminCategoryId();
49+
//Removing an old cookie if it exists because it may contain problematic options (filters that return too many rows):
50+
if (HttpContext.Request.Cookies.ContainsKey("ReportsFilterCookie"))
51+
{
52+
HttpContext.Response.Cookies.Delete("ReportsFilterCookie");
53+
}
54+
var filterData = Request.Cookies.RetrieveFilterDataFromCookie("CourseUsageReportFilterCookie", categoryIdFilter);
4955

50-
var filterData = Request.Cookies.RetrieveFilterDataFromCookie("ReportsFilterCookie", categoryIdFilter);
51-
52-
Response.Cookies.SetReportsFilterCookie("ReportsFilterCookie", filterData, clockUtility.UtcNow);
56+
Response.Cookies.SetReportsFilterCookie("CourseUsageReportFilterCookie", filterData, clockUtility.UtcNow);
5357

5458
var activity = activityService.GetFilteredActivity(centreId, filterData);
5559

@@ -84,7 +88,7 @@ public IEnumerable<ActivityDataRowModel> GetGraphData()
8488
var centreId = User.GetCentreIdKnownNotNull();
8589
var categoryIdFilter = User.GetAdminCategoryId();
8690

87-
var filterData = Request.Cookies.RetrieveFilterDataFromCookie("ReportsFilterCookie", categoryIdFilter);
91+
var filterData = Request.Cookies.RetrieveFilterDataFromCookie("CourseUsageReportFilterCookie", categoryIdFilter);
8892

8993
var activity = activityService.GetFilteredActivity(centreId, filterData!);
9094
return activity.Select(
@@ -98,7 +102,7 @@ public IActionResult EditFilters()
98102
{
99103
var centreId = User.GetCentreIdKnownNotNull();
100104
var categoryIdFilter = User.GetAdminCategoryId();
101-
var filterData = Request.Cookies.RetrieveFilterDataFromCookie("ReportsFilterCookie", categoryIdFilter);
105+
var filterData = Request.Cookies.RetrieveFilterDataFromCookie("CourseUsageReportFilterCookie", categoryIdFilter);
102106

103107
var filterOptions = GetDropdownValues(centreId, categoryIdFilter);
104108

@@ -144,7 +148,7 @@ public IActionResult EditFilters(EditFiltersViewModel model)
144148
model.ReportInterval
145149
);
146150

147-
Response.Cookies.SetReportsFilterCookie("ReportsFilterCookie", filterData, clockUtility.UtcNow);
151+
Response.Cookies.SetReportsFilterCookie("CourseUsageReportFilterCookie", filterData, clockUtility.UtcNow);
148152

149153
return RedirectToAction("Index");
150154
}

DigitalLearningSolutions.Web/Helpers/CommonValidationErrorMessages.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,5 +34,6 @@ public static class CommonValidationErrorMessages
3434
public const string PrimaryEmailInUseDuringDelegateRegistration =
3535
"A user with this email address is already registered";
3636
public const string CentreNameAlreadyExist = "The centre name you have entered already exists, please enter a different centre name";
37+
public const string ReportFilterReturnsTooManyRows = "The report frequency is too high for the date range. Choose a lower report frequency (or shorten the date range)";
3738
}
3839
}
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
using DigitalLearningSolutions.Data.Enums;
2+
using System;
3+
4+
namespace DigitalLearningSolutions.Web.Helpers
5+
{
6+
public static class ReportValidationHelper
7+
{
8+
public static bool IsPeriodCompatibleWithDateRange(ReportInterval reportInterval, DateTime startDate, DateTime? endDate)
9+
{
10+
if (endDate == null)
11+
{
12+
endDate = DateTime.Now;
13+
}
14+
15+
int rowLimit = 250;
16+
int differenceInDays = (int)(endDate.Value - startDate).TotalDays;
17+
18+
switch (reportInterval)
19+
{
20+
case ReportInterval.Days:
21+
return (differenceInDays) <= rowLimit;
22+
case ReportInterval.Weeks:
23+
return (differenceInDays / 7) <= rowLimit;
24+
case ReportInterval.Months:
25+
return (differenceInDays / 30) <= rowLimit;
26+
case ReportInterval.Quarters:
27+
return (differenceInDays / 90) <= rowLimit;
28+
case ReportInterval.Years:
29+
return (differenceInDays / 365) <= rowLimit;
30+
default:
31+
throw new ArgumentException("Invalid report interval");
32+
}
33+
}
34+
}
35+
}

DigitalLearningSolutions.Web/ViewModels/SuperAdmin/PlatformReports/CourseUsageEditFiltersViewModel.cs

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,8 @@ public IEnumerable<ValidationResult> Validate(ValidationContext validationContex
9292
ValidateEndDate(validationResults);
9393
}
9494

95+
ValidatePeriodIsCompatibleWithDateRange(validationResults);
96+
9597
return validationResults;
9698
}
9799

@@ -150,11 +152,11 @@ private void ValidateStartDateIsAfterDataStart(List<ValidationResult> startDateV
150152
{
151153
var startDate = GetValidatedStartDate();
152154

153-
if (startDate < DataStart)
155+
if (startDate.AddDays(1) < DataStart)
154156
{
155157
startDateValidationResults.Add(
156158
new ValidationResult(
157-
"Enter a start date after the start of data for this centre",
159+
"Enter a start date after the start of data in the platform",
158160
new[]
159161
{
160162
nameof(StartDay),
@@ -217,5 +219,25 @@ private void ValidateEndDateIsAfterStartDate(List<ValidationResult> endDateValid
217219
);
218220
}
219221
}
222+
private void ValidatePeriodIsCompatibleWithDateRange(List<ValidationResult> periodValidationResults)
223+
{
224+
if (!periodValidationResults.Any())
225+
{
226+
var startDate = GetValidatedStartDate();
227+
var endDate = GetValidatedEndDate();
228+
if (!ReportValidationHelper.IsPeriodCompatibleWithDateRange(ReportInterval, startDate, endDate))
229+
{
230+
periodValidationResults.Add(
231+
new ValidationResult(
232+
CommonValidationErrorMessages.ReportFilterReturnsTooManyRows,
233+
new[]
234+
{
235+
nameof(ReportInterval),
236+
}
237+
)
238+
);
239+
}
240+
}
241+
}
220242
}
221243
}
Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,8 @@ public IEnumerable<ValidationResult> Validate(ValidationContext validationContex
9292
ValidateEndDate(validationResults);
9393
}
9494

95+
ValidatePeriodIsCompatibleWithDateRange(validationResults);
96+
9597
return validationResults;
9698
}
9799

@@ -148,11 +150,11 @@ private void ValidateStartDateIsAfterDataStart(List<ValidationResult> startDateV
148150
{
149151
var startDate = GetValidatedStartDate();
150152

151-
if (startDate < DataStart)
153+
if (startDate.AddDays(1) < DataStart)
152154
{
153155
startDateValidationResults.Add(
154156
new ValidationResult(
155-
"Enter a start date after the start of data for this centre",
157+
"Enter a start date after the start of data in the platform",
156158
new[]
157159
{
158160
nameof(StartDay),
@@ -215,5 +217,26 @@ private void ValidateEndDateIsAfterStartDate(List<ValidationResult> endDateValid
215217
);
216218
}
217219
}
220+
private void ValidatePeriodIsCompatibleWithDateRange(List<ValidationResult> periodValidationResults)
221+
{
222+
if (!periodValidationResults.Any())
223+
{
224+
var startDate = GetValidatedStartDate();
225+
var endDate = GetValidatedEndDate();
226+
227+
if (!ReportValidationHelper.IsPeriodCompatibleWithDateRange(ReportInterval, startDate, endDate))
228+
{
229+
periodValidationResults.Add(
230+
new ValidationResult(
231+
CommonValidationErrorMessages.ReportFilterReturnsTooManyRows,
232+
new[]
233+
{
234+
nameof(ReportInterval),
235+
}
236+
)
237+
);
238+
}
239+
}
240+
}
218241
}
219242
}

0 commit comments

Comments
 (0)